From dafc696053c91439e7dce36b460f5047fd897bee Mon Sep 17 00:00:00 2001 From: Joe Julian Date: Wed, 1 Apr 2026 17:12:28 -0700 Subject: [PATCH] Improve autofill status feedback --- apiaudit/audit.go | 3 + apiaudit/audit_test.go | 19 +++ autofillcache/cache.go | 41 ++++-- autofillcache/cache_test.go | 43 +++++++ main.go | 240 ++++++++++++++++++++++++++++++++++++ main_test.go | 106 ++++++++++++++++ 6 files changed, 441 insertions(+), 11 deletions(-) diff --git a/apiaudit/audit.go b/apiaudit/audit.go index 07ee80c..f64b1e4 100644 --- a/apiaudit/audit.go +++ b/apiaudit/audit.go @@ -16,6 +16,9 @@ const ( EventApprovalDenied EventType = "approval_denied" EventApprovalCanceled EventType = "approval_canceled" EventApprovalTimedOut EventType = "approval_timed_out" + EventAutofillFound EventType = "autofill_found" + EventAutofillAmbiguous EventType = "autofill_ambiguous" + EventAutofillBlocked EventType = "autofill_blocked" EventAuthRejected EventType = "auth_rejected" ) diff --git a/apiaudit/audit_test.go b/apiaudit/audit_test.go index fc8d664..c42f978 100644 --- a/apiaudit/audit_test.go +++ b/apiaudit/audit_test.go @@ -47,3 +47,22 @@ func TestLogPreservesRecordedMetadata(t *testing.T) { t.Fatalf("Events()[0] = %#v, want preserved metadata", events[0]) } } + +func TestLogStoresAutofillEventTypes(t *testing.T) { + t.Parallel() + + log := New(5) + log.Record(Event{ + Type: EventAutofillAmbiguous, + TokenName: "Browser Extension", + Message: "multiple matches for example.com", + }) + + events := log.Events() + if len(events) != 1 { + t.Fatalf("len(Events()) = %d, want 1", len(events)) + } + if events[0].Type != EventAutofillAmbiguous { + t.Fatalf("Events()[0].Type = %q, want %q", events[0].Type, EventAutofillAmbiguous) + } +} diff --git a/autofillcache/cache.go b/autofillcache/cache.go index a3f2531..e267526 100644 --- a/autofillcache/cache.go +++ b/autofillcache/cache.go @@ -28,10 +28,29 @@ type File struct { Entries []Entry `json:"entries"` } +type MatchStatus string + +const ( + MatchStatusNone MatchStatus = "" + MatchStatusFound MatchStatus = "found" + MatchStatusAmbiguous MatchStatus = "ambiguous" + MatchStatusMissing MatchStatus = "missing" +) + +type MatchResult struct { + Status MatchStatus `json:"status"` + Entry Entry `json:"entry,omitempty"` +} + func Match(cache File, webURL string) (Entry, bool) { + result := Resolve(cache, webURL) + return result.Entry, result.Status == MatchStatusFound +} + +func Resolve(cache File, webURL string) MatchResult { target := normalizeURL(webURL) if target.host == "" { - return Entry{}, false + return MatchResult{Status: MatchStatusMissing} } exactHost := make([]Entry, 0) @@ -46,8 +65,8 @@ func Match(cache File, webURL string) (Entry, bool) { } } - if matched, ok := chooseEntry(target, exactHost); ok { - return matched, true + if result := chooseEntry(target, exactHost); result.Status != MatchStatusMissing { + return result } return chooseEntry(target, parentHost) } @@ -152,12 +171,12 @@ func cleanPath(path string) string { return path } -func chooseEntry(target normalizedTarget, entries []Entry) (Entry, bool) { +func chooseEntry(target normalizedTarget, entries []Entry) MatchResult { switch len(entries) { case 0: - return Entry{}, false + return MatchResult{Status: MatchStatusMissing} case 1: - return entries[0], true + return MatchResult{Status: MatchStatusFound, Entry: entries[0]} } exact := make([]Entry, 0) @@ -181,18 +200,18 @@ func chooseEntry(target normalizedTarget, entries []Entry) (Entry, bool) { } } if len(exact) == 1 { - return exact[0], true + return MatchResult{Status: MatchStatusFound, Entry: exact[0]} } if len(exact) > 1 { - return Entry{}, false + return MatchResult{Status: MatchStatusAmbiguous} } if len(bestPrefix) == 1 { - return bestPrefix[0], true + return MatchResult{Status: MatchStatusFound, Entry: bestPrefix[0]} } if len(bestPrefix) == 0 { - return Entry{}, false + return MatchResult{Status: MatchStatusMissing} } - return Entry{}, false + return MatchResult{Status: MatchStatusAmbiguous} } func collectTargets(item vault.Entry) []string { diff --git a/autofillcache/cache_test.go b/autofillcache/cache_test.go index f9762d5..f47050c 100644 --- a/autofillcache/cache_test.go +++ b/autofillcache/cache_test.go @@ -156,6 +156,49 @@ func TestMatchRejectsAmbiguousSharedHost(t *testing.T) { } } +func TestResolveReportsFoundAmbiguousAndMissingStatuses(t *testing.T) { + t.Parallel() + + cache := File{ + Entries: []Entry{ + { + ID: "one", + Title: "Admin Login", + Username: "admin", + Password: "secret1", + URL: "https://example.com/admin", + Host: "example.com", + }, + { + ID: "two", + Title: "Shared Login A", + Username: "shared-a", + Password: "secret2", + URL: "https://shared.example.com", + Host: "shared.example.com", + }, + { + ID: "three", + Title: "Shared Login B", + Username: "shared-b", + Password: "secret3", + URL: "https://shared.example.com", + Host: "shared.example.com", + }, + }, + } + + if got := Resolve(cache, "https://example.com/admin/login"); got.Status != MatchStatusFound || got.Entry.ID != "one" { + t.Fatalf("Resolve(found) = %#v, want found entry one", got) + } + if got := Resolve(cache, "https://shared.example.com"); got.Status != MatchStatusAmbiguous { + t.Fatalf("Resolve(ambiguous) = %#v, want ambiguous", got) + } + if got := Resolve(cache, "https://nowhere.invalid"); got.Status != MatchStatusMissing { + t.Fatalf("Resolve(missing) = %#v, want missing", got) + } +} + func TestMatchChoosesLongestPathPrefix(t *testing.T) { t.Parallel() diff --git a/main.go b/main.go index ba58008..ebdf2bd 100644 --- a/main.go +++ b/main.go @@ -49,6 +49,7 @@ const ( const ( maxAttachmentBytes = 10 << 20 statusBannerDuration = 2600 * time.Millisecond + autofillStatusTTL = 12 * time.Second ) type bannerKind string @@ -67,6 +68,23 @@ type uiBanner struct { Dismissable bool } +type autofillStatusKind string + +const ( + autofillStatusNone autofillStatusKind = "" + autofillStatusFound autofillStatusKind = "found" + autofillStatusAmbiguous autofillStatusKind = "ambiguous" + autofillStatusBlocked autofillStatusKind = "blocked" + autofillStatusAwaitingApproval autofillStatusKind = "awaiting_approval" +) + +type uiAutofillStatus struct { + Kind autofillStatusKind + Title string + Message string + Detail string +} + type uiSurface struct { Title string Message string @@ -1809,6 +1827,152 @@ func (u *ui) statusToastSurface() uiBanner { } } +func (u *ui) autofillStatusSurface() uiAutofillStatus { + if request, ok := u.pendingAutofillApproval(); ok { + detail := approvalResourceText(request) + if strings.TrimSpace(detail) == "" { + detail = "Review the request to allow or deny this fill attempt." + } + return uiAutofillStatus{ + Kind: autofillStatusAwaitingApproval, + Title: "Autofill approval needed", + Message: formatAutofillRequester(request.ClientName, request.TokenName) + " is waiting to fill credentials.", + Detail: detail, + } + } + if u.auditLog == nil { + return uiAutofillStatus{} + } + for _, event := range u.auditLog.Events() { + if status, ok := autofillStatusFromAuditEvent(event, u.now()); ok { + return status + } + } + return uiAutofillStatus{} +} + +func (u *ui) pendingAutofillApproval() (apiapproval.Request, bool) { + for _, request := range u.state.PendingApprovals() { + if isAutofillOperation(request.Operation) { + return request, true + } + } + return apiapproval.Request{}, false +} + +func autofillStatusFromAuditEvent(event apiaudit.Event, now time.Time) (uiAutofillStatus, bool) { + if !event.At.IsZero() && !now.Before(event.At) && now.Sub(event.At) > autofillStatusTTL { + return uiAutofillStatus{}, false + } + + requester := formatAutofillRequester(event.ClientName, event.TokenName) + switch event.Type { + case apiaudit.EventAutofillFound: + return uiAutofillStatus{ + Kind: autofillStatusFound, + Title: "Autofill match ready", + Message: defaultAutofillMessage(event.Message, requester+" found a credential to fill."), + Detail: autofillEventDetail(event), + }, true + case apiaudit.EventAutofillAmbiguous: + return uiAutofillStatus{ + Kind: autofillStatusAmbiguous, + Title: "Autofill needs a narrower match", + Message: defaultAutofillMessage(event.Message, requester+" found more than one matching credential."), + Detail: autofillEventDetail(event), + }, true + case apiaudit.EventAutofillBlocked: + return uiAutofillStatus{ + Kind: autofillStatusBlocked, + Title: "Autofill is blocked", + Message: defaultAutofillMessage(event.Message, requester+" could not fill this target."), + Detail: autofillEventDetail(event), + }, true + case apiaudit.EventApprovalAllowed: + if !isAutofillOperation(event.Operation) { + return uiAutofillStatus{}, false + } + return uiAutofillStatus{ + Kind: autofillStatusFound, + Title: "Autofill approved", + Message: defaultAutofillMessage(event.Message, requester+" can fill this target now."), + Detail: autofillEventDetail(event), + }, true + case apiaudit.EventApprovalDenied, apiaudit.EventApprovalCanceled, apiaudit.EventApprovalTimedOut: + if !isAutofillOperation(event.Operation) { + return uiAutofillStatus{}, false + } + return uiAutofillStatus{ + Kind: autofillStatusBlocked, + Title: "Autofill was not allowed", + Message: defaultAutofillMessage(event.Message, autofillBlockedMessage(event.Type, requester)), + Detail: autofillEventDetail(event), + }, true + default: + return uiAutofillStatus{}, false + } +} + +func autofillEventDetail(event apiaudit.Event) string { + return strings.TrimSpace(resourceDetailText(event.Resource)) +} + +func resourceDetailText(resource apitokens.Resource) string { + switch resource.Kind { + case apitokens.ResourceEntry: + if entryID := strings.TrimSpace(resource.EntryID); entryID != "" { + return "Entry ID: " + entryID + } + case apitokens.ResourceGroup: + if len(resource.Path) > 0 { + return "Group: " + strings.Join(resource.Path, " / ") + } + } + return "" +} + +func formatAutofillRequester(clientName, tokenName string) string { + switch { + case strings.TrimSpace(clientName) != "" && strings.TrimSpace(tokenName) != "": + return strings.TrimSpace(clientName) + " (" + strings.TrimSpace(tokenName) + ")" + case strings.TrimSpace(clientName) != "": + return strings.TrimSpace(clientName) + case strings.TrimSpace(tokenName) != "": + return strings.TrimSpace(tokenName) + default: + return "A trusted client" + } +} + +func defaultAutofillMessage(value, fallback string) string { + if strings.TrimSpace(value) != "" { + return strings.TrimSpace(value) + } + return fallback +} + +func autofillBlockedMessage(eventType apiaudit.EventType, requester string) string { + switch eventType { + case apiaudit.EventApprovalDenied: + return requester + " was denied for this fill request." + case apiaudit.EventApprovalCanceled: + return requester + " canceled this fill request." + case apiaudit.EventApprovalTimedOut: + return requester + " timed out while waiting for approval." + default: + return requester + " could not fill this target." + } +} + +func isAutofillOperation(operation apitokens.Operation) bool { + switch operation { + case apitokens.OperationReadEntry, apitokens.OperationCopyUsername, apitokens.OperationCopyPassword, apitokens.OperationCopyURL: + return true + default: + return false + } +} + func (u *ui) loadingDetailMessage() string { if !u.shouldShowLifecycleSetup() { return "" @@ -2429,6 +2593,19 @@ func (u *ui) layout(gtx layout.Context) layout.Dimensions { layout.Rigid(layout.Spacer{Height: unit.Dp(12)}.Layout), ) }), + layout.Rigid(func(gtx layout.Context) layout.Dimensions { + if u.bannerSurface().Kind != bannerNone { + return layout.Dimensions{} + } + if u.autofillStatusSurface().Kind == autofillStatusNone { + return layout.Dimensions{} + } + return layout.Flex{Axis: layout.Vertical}.Layout(gtx, + layout.Rigid(layout.Spacer{Height: unit.Dp(10)}.Layout), + layout.Rigid(u.autofillStatusCard), + layout.Rigid(layout.Spacer{Height: unit.Dp(10)}.Layout), + ) + }), layout.Flexed(1, func(gtx layout.Context) layout.Dimensions { if u.shouldShowLifecycleSetup() { return u.lifecycleScreen(gtx) @@ -3667,6 +3844,69 @@ func (u *ui) statusToast(gtx layout.Context) layout.Dimensions { }) } +func (u *ui) autofillStatusCard(gtx layout.Context) layout.Dimensions { + status := u.autofillStatusSurface() + if status.Kind == autofillStatusNone { + return layout.Dimensions{} + } + + bg := color.NRGBA{R: 233, G: 241, B: 237, A: 255} + accent := accentColor + switch status.Kind { + case autofillStatusAmbiguous: + bg = color.NRGBA{R: 245, G: 239, B: 223, A: 255} + accent = color.NRGBA{R: 117, G: 88, B: 24, A: 255} + case autofillStatusBlocked: + bg = color.NRGBA{R: 247, G: 232, B: 228, A: 255} + accent = color.NRGBA{R: 125, G: 40, B: 30, A: 255} + case autofillStatusAwaitingApproval: + bg = color.NRGBA{R: 229, G: 236, B: 244, A: 255} + accent = color.NRGBA{R: 30, G: 76, B: 128, A: 255} + } + + return layout.Background{}.Layout(gtx, fill(bg), func(gtx layout.Context) layout.Dimensions { + return layout.UniformInset(unit.Dp(12)).Layout(gtx, func(gtx layout.Context) layout.Dimensions { + return layout.Flex{Axis: layout.Horizontal, Alignment: layout.Start}.Layout(gtx, + layout.Rigid(func(gtx layout.Context) layout.Dimensions { + return layout.Inset{Right: unit.Dp(12), Top: unit.Dp(2)}.Layout(gtx, func(gtx layout.Context) layout.Dimensions { + label := material.Label(u.theme, unit.Sp(12), "Autofill") + label.Color = accent + label.Font.Weight = 600 + return label.Layout(gtx) + }) + }), + layout.Flexed(1, func(gtx layout.Context) layout.Dimensions { + return layout.Flex{Axis: layout.Vertical}.Layout(gtx, + layout.Rigid(func(gtx layout.Context) layout.Dimensions { + label := material.Label(u.theme, unit.Sp(14), status.Title) + label.Color = accent + label.Font.Weight = 600 + return label.Layout(gtx) + }), + layout.Rigid(func(gtx layout.Context) layout.Dimensions { + return layout.Inset{Top: unit.Dp(2)}.Layout(gtx, func(gtx layout.Context) layout.Dimensions { + label := material.Label(u.theme, unit.Sp(12), status.Message) + label.Color = color.NRGBA{R: 52, G: 50, B: 46, A: 255} + return label.Layout(gtx) + }) + }), + layout.Rigid(func(gtx layout.Context) layout.Dimensions { + if strings.TrimSpace(status.Detail) == "" { + return layout.Dimensions{} + } + return layout.Inset{Top: unit.Dp(2)}.Layout(gtx, func(gtx layout.Context) layout.Dimensions { + label := material.Label(u.theme, unit.Sp(11), status.Detail) + label.Color = mutedColor + return label.Layout(gtx) + }) + }), + ) + }), + ) + }) + }) +} + func (u *ui) historyPanel(gtx layout.Context) layout.Dimensions { history := u.visibleHistory() u.ensureHistoryClickables() diff --git a/main_test.go b/main_test.go index e33da3b..1cd904a 100644 --- a/main_test.go +++ b/main_test.go @@ -2563,6 +2563,112 @@ func TestUIStatusToastExpiresAfterTimeout(t *testing.T) { } } +func TestUIAutofillStatusSurfaceUsesPendingApproval(t *testing.T) { + t.Parallel() + + u := newUIWithModel("desktop", vault.Model{}) + u.state.Approvals = &mainStubApprovalManager{ + pending: []apiapproval.Request{ + { + ID: "approval-1", + TokenName: "Browser Extension", + ClientName: "Firefox", + Operation: apitokens.OperationCopyPassword, + Resource: apitokens.Resource{Kind: apitokens.ResourceEntry, EntryID: "entry-1"}, + }, + }, + } + + got := u.autofillStatusSurface() + if got.Kind != autofillStatusAwaitingApproval { + t.Fatalf("autofillStatusSurface().Kind = %q, want %q", got.Kind, autofillStatusAwaitingApproval) + } + if got.Title != "Autofill approval needed" { + t.Fatalf("autofillStatusSurface().Title = %q, want %q", got.Title, "Autofill approval needed") + } + if !strings.Contains(got.Message, "Firefox (Browser Extension)") { + t.Fatalf("autofillStatusSurface().Message = %q, want requester details", got.Message) + } + if got.Detail != "Entry entry-1" { + t.Fatalf("autofillStatusSurface().Detail = %q, want %q", got.Detail, "Entry entry-1") + } +} + +func TestUIAutofillStatusSurfaceUsesAuditEventsForFoundAmbiguousAndBlocked(t *testing.T) { + t.Parallel() + + now := time.Date(2026, time.March, 31, 12, 0, 0, 0, time.UTC) + u := newUIWithModel("desktop", vault.Model{}) + u.now = func() time.Time { return now } + u.auditLog = apiaudit.New(10) + + u.auditLog.Record(apiaudit.Event{ + Type: apiaudit.EventAutofillFound, + At: now, + TokenName: "Browser Extension", + Message: "Vault Console is ready to fill.", + Resource: apitokens.Resource{Kind: apitokens.ResourceEntry, EntryID: "vault-console"}, + }) + if got := u.autofillStatusSurface(); got.Kind != autofillStatusFound || got.Title != "Autofill match ready" { + t.Fatalf("autofillStatusSurface(found) = %#v, want found status", got) + } + + u.auditLog = apiaudit.New(10) + u.auditLog.Record(apiaudit.Event{ + Type: apiaudit.EventAutofillAmbiguous, + At: now, + TokenName: "Browser Extension", + Message: "Multiple entries match example.com.", + }) + if got := u.autofillStatusSurface(); got.Kind != autofillStatusAmbiguous || got.Title != "Autofill needs a narrower match" { + t.Fatalf("autofillStatusSurface(ambiguous) = %#v, want ambiguous status", got) + } + + u.auditLog = apiaudit.New(10) + u.auditLog.Record(apiaudit.Event{ + Type: apiaudit.EventApprovalDenied, + At: now, + TokenName: "Browser Extension", + ClientName: "Firefox", + Operation: apitokens.OperationCopyPassword, + Resource: apitokens.Resource{Kind: apitokens.ResourceEntry, EntryID: "vault-console"}, + }) + if got := u.autofillStatusSurface(); got.Kind != autofillStatusBlocked || got.Title != "Autofill was not allowed" { + t.Fatalf("autofillStatusSurface(blocked) = %#v, want blocked status", got) + } +} + +func TestUIAutofillStatusSurfaceIgnoresExpiredAndNonAutofillAuditEvents(t *testing.T) { + t.Parallel() + + now := time.Date(2026, time.March, 31, 12, 0, 0, 0, time.UTC) + u := newUIWithModel("desktop", vault.Model{}) + u.now = func() time.Time { return now } + u.auditLog = apiaudit.New(10) + + u.auditLog.Record(apiaudit.Event{ + Type: apiaudit.EventAutofillFound, + At: now.Add(-autofillStatusTTL - time.Second), + TokenName: "Browser Extension", + Message: "stale event", + }) + if got := u.autofillStatusSurface(); got.Kind != autofillStatusNone { + t.Fatalf("autofillStatusSurface(stale) = %#v, want none", got) + } + + u.auditLog = apiaudit.New(10) + u.auditLog.Record(apiaudit.Event{ + Type: apiaudit.EventApprovalAllowed, + At: now, + TokenName: "CLI", + Operation: apitokens.OperationListEntries, + Message: "not autofill", + }) + if got := u.autofillStatusSurface(); got.Kind != autofillStatusNone { + t.Fatalf("autofillStatusSurface(non-autofill) = %#v, want none", got) + } +} + func TestUIRunActionNormalizesRemoteSaveConflictsForDisplay(t *testing.T) { t.Parallel()