[CLI] Auth token per host - #2813
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughCLI authentication now resolves validated API and dashboard URLs, stores credentials per backend with legacy migration support, and routes login, logout, context, legacy requests, and create-instant-app through shared authentication helpers. ChangesBackend-scoped authentication
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant Config
participant Auth
participant Filesystem
CLI->>Config: resolve configured API URL
Config-->>CLI: return validated backend URI
CLI->>Auth: readAuthToken(backend URI)
Auth->>Filesystem: read scoped credential
Filesystem-->>Auth: return token or legacy credential
Auth-->>CLI: return authentication token
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
View Vercel preview at instant-www-js-auth-token-by-host-jsv.vercel.app. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
client/packages/cli/__tests__/config.test.ts (1)
26-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for
getBaseUrl, defaults, and invalid-URL validation.Only
getDashUrl's config-file and env-var precedence paths are tested. GivengetBaseUrlmirrors this logic, consider adding equivalent tests for it, plus cases for the dev/prod default fallback and theBadArgsErrorpath whenapiURI/dashURIis not a valid HTTP(S) URL.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/packages/cli/__tests__/config.test.ts` around lines 26 - 47, Expand the dashboard URL configuration tests around getDashUrl and the corresponding getBaseUrl tests to cover config-file values, INSTANT_CLI_* environment-variable precedence, and development/production default fallbacks. Add validation cases asserting BadArgsError when apiURI or dashURI is not a valid HTTP(S) URL, while preserving the existing valid URL behavior.client/packages/create-instant-app/src/utils/fetch.ts (1)
6-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider validating
INSTANT_CLI_DASH_URIlike the CLI's URL resolvers.This mirrors the pre-existing unvalidated pattern for
instantBackendOrigin, but the broader PR introduces HTTP(S) validation for resolved URLs elsewhere (e.g. the CLI's auth module rejects non-http(s)URIs). A malformedINSTANT_CLI_DASH_URIhere would silently flow intoopenInBrowseratlogin.ts:319with no early, clear error.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/packages/create-instant-app/src/utils/fetch.ts` around lines 6 - 8, Validate INSTANT_CLI_DASH_URI when resolving instantDashOrigin, requiring a valid HTTP(S) URL before it can reach openInBrowser. Reuse the CLI’s existing URL-validation/resolution helper and preserve the localhost/production fallback when the environment variable is unset.client/packages/cli/src/auth/index.ts (1)
22-46: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMinor:
normalizeApiURIis re-parsed twice per call.
readAuthToken,writeAuthToken, andremoveAuthTokeneach call bothgetAuthConfigFilePathandgetLegacyAuthConfigFilePath, each of which independently re-runsnormalizeApiURI(apiURI)on the same input. Negligible cost here, but could be consolidated by normalizing once and passing the normalized string down.♻️ Sketch
-function getAuthConfigFilePath(apiURI: string) { - const normalizedApiURI = normalizeApiURI(apiURI); +function getAuthConfigFilePath(normalizedApiURI: string) { const backendKey = createHash('sha256') .update(normalizedApiURI) .digest('hex'); const { config: configDir } = envPaths('instantdb-prod'); return join(configDir, 'auth', backendKey); } -function getLegacyAuthConfigFilePath(apiURI: string) { - const normalizedApiURI = normalizeApiURI(apiURI); +function getLegacyAuthConfigFilePath(normalizedApiURI: string) { ... }Then call
normalizeApiURI(apiURI)once at the top of each exported function and pass the result down.Also applies to: 59-105
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/packages/cli/src/auth/index.ts` around lines 22 - 46, Normalize the API URI once at the start of each exported readAuthToken, writeAuthToken, and removeAuthToken function, then pass that normalized value to getAuthConfigFilePath and getLegacyAuthConfigFilePath. Update both helper signatures to accept the normalized URI without calling normalizeApiURI again, preserving their existing path-selection behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/packages/cli/src/lib/config.ts`:
- Around line 14-68: Consolidate getBaseUrl and getDashUrl into one parametrized
helper that selects the environment key, config property, fallback URL, and
validation message for each URL type. Validate both environment-sourced and
instant.config.ts values with HttpUrl before returning them, mapping failures to
the appropriate BadArgsError. Keep the existing development and production
fallbacks unchanged while having both public functions delegate to the shared
helper.
---
Nitpick comments:
In `@client/packages/cli/__tests__/config.test.ts`:
- Around line 26-47: Expand the dashboard URL configuration tests around
getDashUrl and the corresponding getBaseUrl tests to cover config-file values,
INSTANT_CLI_* environment-variable precedence, and development/production
default fallbacks. Add validation cases asserting BadArgsError when apiURI or
dashURI is not a valid HTTP(S) URL, while preserving the existing valid URL
behavior.
In `@client/packages/cli/src/auth/index.ts`:
- Around line 22-46: Normalize the API URI once at the start of each exported
readAuthToken, writeAuthToken, and removeAuthToken function, then pass that
normalized value to getAuthConfigFilePath and getLegacyAuthConfigFilePath.
Update both helper signatures to accept the normalized URI without calling
normalizeApiURI again, preserving their existing path-selection behavior.
In `@client/packages/create-instant-app/src/utils/fetch.ts`:
- Around line 6-8: Validate INSTANT_CLI_DASH_URI when resolving
instantDashOrigin, requiring a valid HTTP(S) URL before it can reach
openInBrowser. Reuse the CLI’s existing URL-validation/resolution helper and
preserve the localhost/production fallback when the environment variable is
unset.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 113a0197-7704-4de5-8f73-ee99fd0afbce
⛔ Files ignored due to path filters (1)
client/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
client/packages/cli/__tests__/auth.test.tsclient/packages/cli/__tests__/config.test.tsclient/packages/cli/package.jsonclient/packages/cli/src/auth/index.tsclient/packages/cli/src/commands/login.tsclient/packages/cli/src/commands/logout.tsclient/packages/cli/src/context/authToken.tsclient/packages/cli/src/lib/config.tsclient/packages/cli/src/lib/http.tsclient/packages/cli/src/lib/login.tsclient/packages/cli/src/old.jsclient/packages/cli/src/util/getAuthPaths.tsclient/packages/create-instant-app/package.jsonclient/packages/create-instant-app/src/login.tsclient/packages/create-instant-app/src/utils/fetch.test.tsclient/packages/create-instant-app/src/utils/fetch.ts
💤 Files with no reviewable changes (2)
- client/packages/create-instant-app/package.json
- client/packages/cli/src/util/getAuthPaths.ts
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
2 participants
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.
Previously, the CLI stored one auth token. Switching from Instant Cloud to a self-hosted backend could send the Cloud token to the self-hosted API.
With this PR tokens are now stored per backend and shared by instant-cli and create-instant-app. Logging out only removes the token for the current backend.
Existing cloud/localhost tokens will migrate automatically and remain compatible with older CLI versions.