Ask for the keychain once, not on every focus - #97
Merged
Conversation
Fiber is signed ad-hoc, so a new build is a new code identity and every keychain item's access list still names the old one. For one launch after each update, reading a credential is a macOS authorization dialog. That is the intended cost; it should be one dialog per credential. It was not. A failed loader run writes no cache, so `loadedAt` never moved, the collection stayed stale, and the focus trigger re-ran it — and the authorization dialog is itself a focus event, handing the window back the moment it is dismissed. Allow, refocus, re-run, prompt, for as long as the API keeps refusing the credential. A failure now counts as an attempt, so the TTL means the same thing for both outcomes: don't ask this API again for another `ttlSeconds`. Refresh and Sign in again call `refresh` directly and are untouched — those are the user asking. The second prompt arrived with the credentials file in 0.15.0. Saving a section reconciled that file, reconciling it means unsealing it, and unsealing it reads the sealing key from the keychain. Fiber saves a section on any edit, debounced, so renaming a request or typing in a URL came through here — and a prompt has to belong to something the user did that needs it. It is now reconciled only when sharing itself moves: the switch, or which credential the collection uses, which is what it was always for.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the prompt loop that made the app unusable after updating to 0.16.0.
What happens
Fiber is signed ad-hoc (
"signingIdentity": "-"), so a new build is a new code identity and everydev.fiber.appkeychain item's access list still names the old one. For one launch after each update, reading a credential is a macOS authorization dialog. That is the intended cost, and the code says so in several places — it should be one dialog per credential.It was not, for two reasons.
A failed loader run is retried on every focus.
refresh()only writes a cache on success, soloadedAtnever moves, the collection stays stale for ever, andrefreshStale()— wired to<svelte:window onfocus>— runs it again. The authorization dialog is itself a focus event: it takes focus, and dismissing it hands focus straight back to the window that starts the next run, which reads the keychain, which raises the next dialog. Allow, refocus, prompt, forever, for as long as the API keeps refusing the credential.A failure now counts as an attempt, so the TTL means the same thing for both outcomes: don't ask this API again for another
ttlSeconds. Refresh and Sign in again callrefresh()directly and are untouched — those are the user asking.Saving a section reached for the keychain.
sync_section_sharinglanded with the credentials file in 0.15.0 and runs on everysave_section. Reconciling that file means unsealing it, and unsealing it reads the sealing key from the keychain — so renaming a request or typing in a URL could raise a password prompt. A prompt has to belong to something the user did that needs it. It is now reconciled only when sharing itself moves: the switch, or which credential the collection uses, which is what it was always for.Tests
a loader that failed is not re-run on every focus— three focus events after a 403, one run.an_ordinary_edit_is_not_a_change_to_sharing— a rename is not a sharing change; toggling the switch or swapping the reference is.Local: 135 Rust tests,
cargo check --all-targetswith theguifeature,cargo fmt --check, clippy-D warnings,pnpm check, and all 339 e2e tests.Not fixed here
The prompt itself. The real answer is a stable Developer ID signature, which
release.ymlalready has scaffolded and commented out pending theAPPLE_*secrets — with it, the access lists survive an update and this whole class of prompt goes away. Happy to do that next.