-
Notifications
You must be signed in to change notification settings - Fork 20
feat(mac): skip OAuth on localhost — adopt bootstrap PAT #777
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -385,8 +385,9 @@ final class AppState: ObservableObject { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // MARK: - Connect (URL-driven sign-in) -------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// The connection card's primary action. Auto-starts the embedded server | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// when the URL is the exact one our runner manages; otherwise just OAuths | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// against the typed URL. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// when the URL is the exact one our runner manages, then sidesteps OAuth | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// by lifting the bootstrap PAT `lobu run` writes for us. For everything | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// else, falls through to the normal OAuth device flow. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func connect() async { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let raw = customServerDraft.trimmingCharacters(in: .whitespacesAndNewlines) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let urlString = raw.isEmpty ? cloudURL : raw | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -403,14 +404,107 @@ final class AppState: ObservableObject { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await startLocalLobu() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| guard localLobuStatus.isRunning else { return } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // serverMode = .local ONLY when this URL is the runner we manage. Other | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // loopback URLs (someone else's localhost dev server, custom ports) get | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // .remote so we don't auto-spawn our runner on next launch. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // serverMode = .local ONLY when this URL is the runner we manage. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Other loopback URLs (someone else's dev server, custom ports) get | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // .remote so next launch doesn't auto-spawn our runner. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| serverMode = autoStart ? .local : .remote | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // No-auth path for the managed runner: the server's bootstrap minted | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // a long-lived PAT + a user + an org for us; reading that file and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // synthesising credentials is the entire "sign-in". If the file | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // isn't there yet (race) or unreadable, fall back to OAuth so we're | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // never strictly worse than before. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if autoStart, await adoptBootstrapCredentialsIfAvailable(baseURL: urlString) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| startAutoPollIfSignedIn() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setBaseURL(urlString) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await signIn() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Try to read `lobu run`'s bootstrap PAT, verify it actually works | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// against the server we're connecting to, and only then adopt it as | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// the menu bar's identity. Verification prevents two failure modes | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// pi flagged in PR #777 review: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// 1. A stale / unrelated / malicious process squatting on :8787 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// receiving our PAT. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// 2. The file exists but DB was reset / PAT revoked / bootstrap | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// skipped — UI would transition to "signed in" and break later. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// We deliberately do NOT use `/userinfo` for verification: that | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// endpoint only accepts OAuth tokens, not PATs, so it 403s every | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// time. Instead we hit a real org-scoped REST endpoint (the unread | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// notifications count — cheapest auth-required call) against the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// `baseURL` we already control. No server-supplied URLs are | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// followed, so a malicious metadata response can't redirect the PAT. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Identity is synthesised from the same constants `ensureBootstrapPat` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// uses in `packages/server/src/start-local.ts` (BOOTSTRAP_USER_*, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// BOOTSTRAP_ORG_*). Keep these in lock-step if you change either. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private func adoptBootstrapCredentialsIfAvailable(baseURL: String) async -> Bool { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| guard let pat = await waitForBootstrapPAT() else { return false } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let verifyClient = WorkerClient(baseURL: baseURL, accessToken: pat) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| do { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ = try await verifyClient.getUnreadCount(orgSlug: AppState.bootstrapOrgSlug) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NSLog("[Lobu] bootstrap adopt: verify call rejected the PAT — not our server, or PAT is stale: \(error.localizedDescription)") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let info = OAuthUserInfo( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sub: AppState.bootstrapUserId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| email: AppState.bootstrapUserEmail, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: AppState.bootstrapUserName, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| picture: nil, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| organization_slug: AppState.bootstrapOrgSlug, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| organizations: [.init(slug: AppState.bootstrapOrgSlug, name: AppState.bootstrapOrgName)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let creds = OAuthCredentials( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| baseURL: baseURL, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clientID: "menubar-local", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| clientSecret: nil, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| accessToken: pat, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| refreshToken: nil, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| expiresAt: nil, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| userInfo: info | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| do { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try credentialStore.save(creds) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NSLog("[Lobu] bootstrap adopt: keychain save failed: \(error.localizedDescription)") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| credentials = creds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setBaseURL(baseURL) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setStatus("") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+465
to
+482
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Normalize the bootstrap
Suggested fix- let creds = OAuthCredentials(
- baseURL: baseURL,
+ let normalizedBaseURL = baseURL.trimmedTrailingSlash()
+ let creds = OAuthCredentials(
+ baseURL: normalizedBaseURL,
clientID: "menubar-local",
clientSecret: nil,
accessToken: pat,
refreshToken: nil,
expiresAt: nil,
userInfo: info
)
@@
credentials = creds
- setBaseURL(baseURL)
+ setBaseURL(normalizedBaseURL)
setStatus("")
return true📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Contract with `ensureBootstrapPat` in packages/server/src/start-local.ts. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Changing either side without the other will make the menu bar's display | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // disagree with reality. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static let bootstrapUserId = "bootstrap-user" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static let bootstrapUserName = "Local Developer" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static let bootstrapUserEmail = "dev@lobu.local" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static let bootstrapOrgSlug = "dev" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private static let bootstrapOrgName = "Local Dev" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Poll for the bootstrap PAT file `lobu run` writes after binding. The | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// HTTP listener comes up before `ensureBootstrapPat` finishes, so the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// runner being "reachable" doesn't guarantee the file exists yet. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private func waitForBootstrapPAT(timeout: TimeInterval = 10) async -> String? { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let path = LocalLobuRunner.bootstrapPATPath | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let deadline = Date().addingTimeInterval(timeout) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| while Date() < deadline { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if let raw = try? String(contentsOf: path, encoding: .utf8) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let token = raw.trimmingCharacters(in: .whitespacesAndNewlines) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if !token.isEmpty { return token } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try? await Task.sleep(nanoseconds: 250_000_000) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return nil | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// True iff this URL targets the embedded server the menu bar manages. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Requires an exact scheme + host + effective-port match against | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// `LocalLobuRunner.baseURL`. Treats `localhost`, `127.0.0.1`, `::1`, and | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When port 8787 is already serving a Lobu process that this app did not spawn,
LocalLobuRunner.start()returns early before settingLOBU_DATA_DIR, but this branch still reads~/lobu/data/bootstrap-pat.txtand treats it as valid for that server. If the user's manually-startedlocalhost:8787uses~/.lobu/dataor any other data dir while an old~/lobu/data/bootstrap-pat.txtexists, the app persists a bearer token from the wrong database and skips OAuth, so subsequent API/sync calls get 401 until credentials are cleared. Only adopt the bootstrap PAT when it is known to belong to the server on that port, or validate it against the server before returning.Useful? React with 👍 / 👎.