Stop reporting a successful sign-in as a Google rejection - #2
Merged
Conversation
A user signing in to a hosted agent lands back on a BLANK login form with no
explanation. Two separate defects stack up to produce that.
1. THE CALLBACK CALLS EVERY FAILURE `oauth_failed`.
The page logs you in only if access_token, token_type, role and user_id all
arrive as query params, and sends `error=oauth_failed` otherwise. But the
no-token case is not a rejection: `error` is null there — the provider
refused nothing. The credential is simply absent, which is what a node does
when it completes the sign-in and keeps the session rather than returning it
to the browser. Observed on scout (CIRISAgent 2.9.24 / ciris-server 0.5.177):
oauth sign-in resolved to a local identity provider=google role=Observer
oauth callback completed — session parked nonce_bound=false
Identity resolved, role assigned, and the user is told Google turned them
away. So they retry, and it happens again, because retrying was never going
to help. That case now emits `no_session`.
2. NOTHING ON /login EVER READ `?error=`.
The page has an `error` state, but it is for failures raised on the page
itself. The code in the URL was rendered nowhere, so all of the above arrived
as a silent redirect to an empty form.
/login now reads it and says what happened: for `no_session`, that the
sign-in completed, that the account was NOT rejected, and where to sign in
instead. Unknown codes render the code rather than being swallowed.
Read from window.location in an effect, not useSearchParams(): this is a
client component and useSearchParams() would force a Suspense boundary for
static rendering. /login is still prerendered static after this change.
3. The linking path redirects to /account, which renders `description || error`
directly into a toast — so it would have shown the user the literal string
"no_session". It now carries a human description.
This does not fix the underlying gap: a browser still cannot complete a sign-in
against a managed node, because app_nonce is desktop-only and there is no web
path that mints one. That is CIRISServer#439. This change only stops the UI
lying about what went wrong while that is sorted out.
Verified: tsc --noEmit clean, next build clean, /login still static.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…ged mode from a hostname
The prior commit made the callback SAY the right thing when no session came
back. This makes a session come back.
REDEEM THE CODE. The node no longer echoes a bearer in the redirect — a live
24h credential in a URL lands in browser history, in the Referer of every
subsequent request, and in every proxy log on the path. It parks the session and
hands this page a single-use code (`?ciris_code=`), which we now POST to
/v1/auth/oauth/exchange and receive in a response BODY (CIRISServer#439).
The legacy query-param branch is KEPT, not replaced: a node that has not adopted
the exchange still signs users in, and this page has to work against both while
the fleet rolls forward. One set of resolved values feeds everything downstream,
because the page previously read the query params directly — a session obtained
by exchange would have been saved to the AuthStore and then treated as absent
three lines later.
A refused exchange logs the node's `reason_id`. An expired code needs a retry
and a refused identity does not, and discarding that distinction is how this
whole class of bug started.
MANAGED MODE IS NOT A HOSTNAME. `hostname === 'agents.ciris.ai'` meant exactly
one deployment could be managed; every other hosted node — scout among them —
fell through to standalone and built its API base wrong.
The literal is KEPT as the default rather than deleted. agents.ciris.ai serves
the GUI at `/` with the API at `/api/{agent}`, and that root-path case has no
other signal to read — removing it outright would silently move a working
deployment into standalone mode. What changes is that it is no longer the only
way to be managed: a deployment declares itself with
NEXT_PUBLIC_DEPLOYMENT_MODE, a path-prefixed gateway is managed whatever host it
answers on, and the node states the same fact authoritatively on
GET /v1/auth/oauth/providers (`managed` / `callback_base` / `web_signin`).
detectDeploymentMode runs before any base URL exists, which is why it reads
config and URL rather than asking.
tsc --noEmit clean.
Refs CIRISServer#439
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013D4Ykkvdav5QfMMVXVf35h
|
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.
A user signing in to a hosted agent lands back on a blank login form with no explanation. Two defects stack up to produce that.
1. The callback calls every failure
oauth_failedoauth/[agent]/[provider]/callbacklogs you in only ifaccess_token,token_type,roleanduser_idall arrive as query params, and emitserror=oauth_failedotherwise. The no-token case is not a rejection —erroris null there, the provider refused nothing. The credential is simply absent, which is what a node does when it completes the sign-in and keeps the session rather than returning it to the browser.Observed on scout (CIRISAgent 2.9.24 / ciris-server 0.5.177):
Identity resolved, role assigned — and the user is told Google turned them away. So they retry, and it happens again, because retrying was never going to help. That case now emits
no_session.2. Nothing on
/loginever read?error=/loginhas anerrorstate, but it is for failures raised on the page itself. The code in the URL was rendered nowhere, so every one of these arrived as a silent redirect to an empty form.It now reads the code and says what happened — for
no_session: that sign-in completed, that the account was not rejected, and where to sign in instead. Unknown codes render the code rather than being swallowed.Read from
window.locationin an effect rather thanuseSearchParams(), which would force a Suspense boundary for static rendering./loginis still prerendered static after this change.3. The linking path would have shown a raw code
/accountrendersdescription || errorstraight into a toast, so it would have displayed the literal stringno_session. It now carries a human description.Scope
This does not fix the underlying gap — a browser still cannot complete a sign-in against a managed node, because
app_nonceis desktop-only and no web path mints one. That is CIRISServer#439. This only stops the UI lying about what went wrong while that is sorted out.Verification
tsc --noEmitcleannext buildclean/loginstill listed as○ (Static)in the build output