Add CLI OAuth device login#1590
Open
ChiragAgg5k wants to merge 24 commits into
Open
Conversation
Contributor
Greptile Summary
Confidence Score: 5/5No blocking issues were identified in the reviewed changes. The changes are broad but focused on CLI authentication behavior, with tests and generated output updated alongside the implementation.
 T-Rex LogsWhat T-Rex did
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Extract the OAuth device-login logic that landed in commands/generic.ts into a cohesive lib/auth/ layer, preserving all behavior: - lib/auth/oauth.ts: OAuth2 client factory, device-token polling, token refresh, revoke, and id_token decoding (getValidAccessToken moved here from sdks.ts; the 3x-duplicated Oauth2 client construction is unified behind createOauth2). - lib/auth/session.ts: typed session accessor, classification + current- session helpers, deleteServerSession, and a single logoutSessions() that replaces the three near-identical cleanup loops. - lib/auth/login.ts: login orchestration and flows (loginCommand, loginWithEmailPassword, loginWithOAuthDevice, switchToAccount, completeMfaLogin, getCurrentAccount). - utils.ts: endpoint classifiers placed next to isCloudHostname. - generic.ts slimmed from ~1060 to ~330 lines (command definitions only). Repointed sdks.ts/deployment.ts imports and registered the new auth files in CLI.php getFiles(). Device-login output uses process.stdout.write (matching the existing spinner idiom); dropped thin wrapper helpers.
The questionsListFactors choices loader built its console SDK with requiresAuth: false. Since the OAuth login change made sdkForConsole attach the session cookie only when requiresAuth is true, the MFA factor list was fetched as a guest and failed with 401 — blocking self-hosted email/password logins that reach MFA without --mfa from picking a factor. Use an authenticated client so the partial MFA session cookie is sent.
Add a logic-layer auth section to the CLI e2e script that asserts the pure/near-pure functions introduced by the OAuth device-login work: endpoint classification (cloud/regional/localhost/dev-override), id_token decoding, authorization-pending detection, device-token polling (success/retry/error/timeout via a fake oauth2), cached access-token reuse, session classifiers, planSessionLogout grouping, and restoreCurrentSessionFallback. Driven by AUTH_LOGIC_RESPONSES in Base.php and the three CLIBun test classes; no new runner or mock-server changes.
Add lib/flags.ts, a central registry of CLI feature flags read from env
vars via isFlagEnabled("<name>") so future flags are a one-line addition.
Route the OAuth device-login gate (APPWRITE_CLI_OAUTH_LOGIN) and the
localhost-as-Cloud dev override (APPWRITE_CLI_DEV_CLOUD_LOGIN) through it,
replacing the ad-hoc per-flag helpers in utils.ts.
OAuth device login stays off by default: Cloud endpoints use email/password
login (and the legacy-cookie migration nudges stay silent) until the flag is
enabled. Covered by the e2e auth-logic assertions.
pollForDeviceToken now honors slow_down by increasing the polling interval by 5s per RFC 8628 (it previously retried at the original interval), and treats an empty/unrecognized error body during polling as a transient pending response instead of aborting the device flow with a blank AppwriteException. Genuine terminal errors still propagate. Adds e2e coverage for the slow_down backoff and empty-body retry paths.
The CLI generator renders lib/constants.ts from constants.ts.twig (the only entry registered in CLI.php getFiles); the plain constants.ts was a stale leftover from an earlier refactor, missing CONFIG_RESOURCE_KEYS, HOMEBREW_FORMULA, UPDATE_CHECK_INTERVAL_MS, and TOP_LEVEL_RESOURCE_ARRAY_KEYS. It is never read during generation, so the generated SDK was unaffected, but building the template dir directly imported the stale file and failed. Removing it leaves a single source of truth. Generated output is unchanged.
…elf-signed legacy revoke - pollForDeviceToken now defaults to a 5s interval when the device authorization response omits one (RFC 8628 §3.5); previously an undefined interval produced NaN and busy-polled the token endpoint until expiry. - getCurrentAccount/switchToAccount no longer persist the normalized console endpoint back into the session. sdkForConsole already normalizes when building the console client, so persisting it overwrote a regional Cloud endpoint and could route later project calls to the generic Cloud host. - deleteServerSession builds the legacy client with the target session's own selfSigned setting (added to SessionData) instead of the current session's, so revoking a self-signed legacy session during OAuth migration no longer fails TLS verification and strand the session. Adds an e2e assertion for the default polling interval.
Comment on lines
+138
to
+141
| globalConfig.setCurrentSession(sessionId); | ||
| const serverDeleted = await deleteServerSession(sessionId); | ||
| if (serverDeleted) { | ||
| globalConfig.removeSession(sessionId); |
Contributor
There was a problem hiding this comment.
logoutSessions sets the global current session to each session it processes, but the multi-account logout path does not restore the original current session afterward. When a user logs out a non-current account and that revocation succeeds, this removes the selected session while leaving currentSession pointing at the deleted id. Later commands can then report no active session even though the original current session still exists. Preserve the caller’s current session after this helper, or revoke using the target session data without changing global current state.
Artifacts
Repro: focused logoutSessions harness
- Contains supporting evidence from the run (text/javascript; charset=utf-8).
Stack trace captured during the T-Rex run
- Keeps the raw stack trace available without making the summary code-heavy.
The previous default-interval guard treated interval <= 0 as omitted, so a deviceAuth with interval 0 (used to poll without delay) was forced to the 5s default and consumed the whole expiry window before the next attempt, breaking the retry/slow_down/empty-body poll paths in CI. Only non-finite (omitted) intervals now fall back to 5s; an explicit 0 is honored. Strengthens the default-interval test to assert the fallback delay actually applies.
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
1 participant
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.
Summary
This PR adds the new browser-based OAuth2 device login flow to the Appwrite CLI while preserving existing self-hosted email/password login and legacy cookie sessions. It also hardens session switching, logout, reset, token refresh, deployment log streaming, and local Cloud development behavior around the new auth model.
What Changed
Browser-based CLI login
/accountbefore reporting login success.slow_downresponses by increasing the polling interval.login --newfrom showing the legacy-session warning for the command that is explicitly migrating to the new flow.Backward compatibility and self-hosted login
APPWRITE_CLI_DEV_CLOUD_LOGIN=1treatslocalhost/loopback endpoints as Cloud for OAuth device login testing.Session switching and auth precedence
login --switchso selecting an existing account actually switches instead of starting a new OAuth login.Logout, reset, and server-side cleanup
client --resetcan fully clear local CLI configuration without requiring server revocation.Deployment and realtime behavior
localhost, the CLI fetches the project region in memory and formats console links likeproject-fra-<projectId>without writing region data intoappwrite.config.json.Generated CLI output
@appwrite.io/consolepackage that includes OAuth2 device authorization, token refresh, and revoke support.Testing
Commands run during this PR:
./scripts/update-lockfiles.sh cliphp example.php clicomposer lint-twigcomposer refactor:checkvendor/bin/phpunit --testsuite Unitnpm installinexamples/clinpm run build:typesinexamples/clinpm run build:runtimeinexamples/clinpm run buildinexamples/clinpm run mac-arm64inexamples/clinpx eslint lib/commands/generic.ts lib/commands/init.ts lib/sdks.ts lib/types.ts lib/commands/utils/deployment.tsinexamples/cliManual/local verification performed:
./build/appwrite-cli-darwin-arm64 login --new --endpoint "http://localhost/v1"against a local Cloud server withAPPWRITE_CLI_DEV_CLOUD_LOGIN=1.login --newno longer shows the legacy cookie-session warning for that command.project list-policies --limit 1succeeds with bearer auth against local Cloud.http://localhost/console/project-fra-...for regional local Cloud projects.