-
Notifications
You must be signed in to change notification settings - Fork 11
fix(install): verify claim status before setting a default homepage #252
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 |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ import { formatWorkOSCommand } from '../utils/command-invocation.js'; | |
| import ui from '../utils/ui.js'; | ||
| import { getActiveEnvironment, isUnclaimedEnvironment } from './config-store.js'; | ||
| import { getCallbackPath } from './port-detection.js'; | ||
| import { createClaimNonce } from './unclaimed-env-api.js'; | ||
|
|
||
| const WORKOS_API_BASE = 'https://api.workos.com'; | ||
|
|
||
|
|
@@ -147,6 +148,25 @@ export async function setHomepageUrl( | |
| return { success: true, alreadyExists: false }; | ||
| } | ||
|
|
||
| /** Best-effort live claim check; this is not atomic with the later homepage write. */ | ||
| export async function isUnclaimedEnvironmentKey(apiKey: string, clientId?: string): Promise<boolean> { | ||
| try { | ||
| const environment = getActiveEnvironment(); | ||
| if ( | ||
| !environment || | ||
| !isUnclaimedEnvironment(environment) || | ||
| environment.apiKey !== apiKey || | ||
| (clientId !== undefined && environment.clientId !== clientId) | ||
| ) | ||
| return false; | ||
| const claim = await createClaimNonce(environment.clientId, environment.claimToken); | ||
| return !claim.alreadyClaimed; | ||
| } catch { | ||
| // Unavailable keyring or claim status: unknown, so leave the homepage alone. | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Where the credentials being used came from, so the rows below say *where* the | ||
| * writes landed and not just what was written. | ||
|
|
@@ -161,17 +181,6 @@ export async function setHomepageUrl( | |
| * store entirely, and naming an untouched environment is exactly the confusion | ||
| * this row exists to prevent. | ||
| */ | ||
| /** Whether `apiKey` is the key of the stored, still-unclaimed environment. */ | ||
| export function isUnclaimedEnvironmentKey(apiKey: string): boolean { | ||
| try { | ||
| const environment = getActiveEnvironment(); | ||
| return !!environment && isUnclaimedEnvironment(environment) && environment.apiKey === apiKey; | ||
| } catch { | ||
| // Keyring unavailable: unknown, so treat it as claimed. | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| function describeCredentialProvenance(apiKey: string): string { | ||
| let activeEnv: EnvironmentConfig | null = null; | ||
| try { | ||
|
|
@@ -245,11 +254,11 @@ export async function autoConfigureWorkOSEnvironment( | |
|
|
||
| // The homepage is one value per environment, and the REST API can set it | ||
| // but not read it (the GET answers 404), so this step can't tell whether | ||
| // someone already chose one. Write it only when nothing can be overwritten: | ||
| // the user asked for it (--homepage-url), or the environment is unclaimed, | ||
| // so nobody has had its dashboard. With a login, the later dashboard step | ||
| // reads the current value and fills an empty one. | ||
| const writeHomepage = Boolean(options.homepageUrl) || isUnclaimedEnvironmentKey(apiKey); | ||
| // someone already chose one. Write only an explicit homepage or a default | ||
| // for an environment the server still reports as unclaimed. Local claim | ||
| // status can be stale. With a login, the later dashboard step reads the | ||
| // current value and fills an empty one. | ||
| const writeHomepage = Boolean(options.homepageUrl) || (await isUnclaimedEnvironmentKey(apiKey)); | ||
|
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.
Knowledge Base Used: Application installation workflows Prompt To Fix With AIThis is a comment left during a code review.
Path: src/lib/workos-management.ts
Line: 261
Comment:
**Claim check delays other settings** This path waits for the nonce request before starting redirect and CORS registration or showing setup progress. If the request stalls, its 30-second timeout delays both settings, although claim status is needed only for the homepage decision. Start the additive settings independently of the check.
**Knowledge Base Used:** [Application installation workflows](https://app.greptile.com/workos/-/custom-context/knowledge-base/workos/cli/-/docs/application-installation.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly. |
||
|
|
||
| ui.log.step('Configuring WorkOS dashboard settings...'); | ||
|
|
||
|
|
||
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.
WORKOS_API_URLis set, this nonce request goes to that host, while the homepage PUT still goes tohttps://api.workos.com. A failed check can leave the homepage unset even when the write endpoint is available, and a successful check does not confirm claim status at the host receiving the write. Use the same API host for both requests.Knowledge Base Used: Authentication and configuration lifecycle
Prompt To Fix With AI