fix: differentiate auth errors for missing token vs no access#204
fix: differentiate auth errors for missing token vs no access#204jomifepe wants to merge 3 commits into
Conversation
The CLI treated all 401/403 responses as "Not logged in", which was misleading when a user was authenticated but lacked repository access. Route auth failures through a shared formatter that considers token presence and error type. Co-authored-by: Cursor <cursoragent@cursor.com>
levimykel
left a comment
levimykel
left a comment
There was a problem hiding this comment.
Approved with a small question/comment.
| if (error instanceof UnauthorizedRequestError || options.envToken) { | ||
| if (options.envToken) { | ||
| return "PRISMIC_TOKEN is invalid or expired. Unset it to log in with a browser, or replace it with a valid token."; | ||
| } | ||
| return "Your session is invalid or expired. Run `prismic login` to sign in again."; | ||
| } | ||
| return "You do not have access to this repository. Check the repository name or log in with an account that has access."; |
There was a problem hiding this comment.
❓ When PRISMIC_TOKEN is set, a 403 returns “PRISMIC_TOKEN is invalid or expired” — but 403 usually means the token is valid and just lacks access to this repo, so the L130 “no access” message never fires for env-token users. Does our API return 401 for bad/expired tokens and 403 only for no-access? If so, 403 should fall through to the access-denied message even when an env token is present. If the backend returns 403 for both, current behavior is fine — just want to confirm.
There was a problem hiding this comment.
💡 Depending on what you want here, this could be simplified to something like this, yeah?
if (!options.hasToken) return "Not logged in…";
if (options.envToken) return "PRISMIC_TOKEN is invalid or expired…";
if (error instanceof UnauthorizedRequestError) return "Your session is invalid or expired…";
return "You do not have access to this repository…";
There was a problem hiding this comment.
Yeah, I agree that the options.envToken would be better only checked inside the condition. Fixed that ✅
Regarding the messages, even though I like the shorter messages, I think it's good to give the user a suggestion, to provide a "way out" of the error state.
The CLI treated all 401/403 responses as "Not logged in", which was misleading when a user was authenticated but lacked repository access. The HTTP status alone can't separate "invalid token" from "no access": the user service returns 403 for an invalid token, while repository endpoints return 401 for a bad token and 403 for no access. So when PRISMIC_TOKEN is set, name both possible causes rather than guessing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jomifepe
force-pushed
the
jp/cli-unauthenticated
branch
from
dfa02f7 to
dd0d815
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit dd0d815. Configure here.
| return "Your session is invalid or expired. Run `prismic login` to sign in again."; | ||
| } | ||
|
|
||
| return "You do not have access to this repository. Check the repository name or log in with an account that has access."; |
There was a problem hiding this comment.
User service 403 wrong message
Medium Severity
For a stored session token (no PRISMIC_TOKEN), formatAuthErrorMessage treats every ForbiddenRequestError as missing repository access. The comment in the same function notes the user service returns 403 for invalid tokens, so profile calls like whoami can show the repository message when the session is actually invalid or expired.
Reviewed by Cursor Bugbot for commit dd0d815. Configure here.
| name = "UnauthorizedRequestError"; | ||
| } | ||
|
|
||
| export function formatAuthErrorMessage( |
There was a problem hiding this comment.
src/lib/request.ts is (mostly) domain-agnostic. All of lib is supposed to be generic without Prismic knowledge to keep the functions flexible and reusable. (Sorry, I know this isn't documented anywhere. Maybe we should add some docs.)
Could we move formatAuthErrorMessage somewhere else? Because it's only used once in the code base, and we handle auth errors globally, maybe we can inline this code in src/index.ts. WDYT?
| return new Response(null, { status, statusText: status === 401 ? "Unauthorized" : "Forbidden" }); | ||
| } | ||
|
|
||
| describe("formatAuthErrorMessage", () => { |
There was a problem hiding this comment.
In general, we've favored integration/e2e tests over unit tests. Is there a way to test the changes through a command rather than checking the implementation?
We can make an exception if testing these edge cases is difficult.
Co-authored-by: Codex <noreply@openai.com>
3 participants
Summary
formatAuthErrorMessage()in the shared request layer so the global handler can distinguish not logged in, invalid/expired token, and repository access deniedTest plan
npm run unit -- test/auth-errors.test.ts test/whoami.test.tsnpm run testMade with Cursor