feat(core,experience): abort failed direct sign-in back to the client with access_denied - #9425
Open
feitianbubu wants to merge 1 commit into
Open
feat(core,experience): abort failed direct sign-in back to the client with access_denied#9425feitianbubu wants to merge 1 commit into
feitianbubu wants to merge 1 commit into
Conversation
feitianbubu
requested review from
charIeszhao,
gao-sun and
wangsijie
as code owners
August 12, 2026 02:11
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the direct sign-in UX by ensuring that failures during a direct_sign_in-initiated OIDC interaction are aborted back to the client app (as a standard OAuth access_denied), instead of falling back to the hosted universal sign-in page. It does this by adding a Core abort endpoint and teaching the Experience callbacks to detect direct sign-in entry and exit appropriately, while also deferring callback side-effects during browser prerendering.
Changes:
- Core: add
POST /api/experience/abortto finish an interaction withaccess_denied(optionalerror_description) and return theredirectToURL. - Experience: persist a “direct sign-in entry” marker in
sessionStorage, centralize fallback logic inuseNavigateToSignIn, and defer callback consumption whiledocument.prerenderingis true. - Tests + docs: add unit tests for the new behavior, and update OpenAPI + changeset.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/experience/src/Providers/UserInteractionContextProvider/index.tsx | Clears the new direct sign-in session marker when resetting user interaction context. |
| packages/experience/src/pages/SocialSignInWebCallback/use-social-sign-in-listener.ts | Uses centralized fallback hook + prerender gating; forwards error codes to abort when applicable. |
| packages/experience/src/pages/SocialSignInWebCallback/use-single-sign-on-listener.ts | Mirrors social callback changes for SSO: centralized fallback + prerender gating + error-code forwarding. |
| packages/experience/src/pages/SocialSignInWebCallback/social-callback.test.tsx | Adds coverage for aborting back to client on direct sign-in, and fallback behavior when abort fails. |
| packages/experience/src/pages/DirectSignIn/index.tsx | Writes the direct sign-in marker to sessionStorage before invoking social/SSO connectors. |
| packages/experience/src/pages/DirectSignIn/index.test.tsx | Adds assertions that the marker is set for valid targets and absent for fallback cases. |
| packages/experience/src/hooks/use-submit-interaction-error-handler.ts | Plumbs error codes through the “email blocked” callback contract. |
| packages/experience/src/hooks/use-social-register.ts | Switches fallback navigation to useNavigateToSignIn and updates email-blocked callback typing. |
| packages/experience/src/hooks/use-session-storages.ts | Introduces StorageKeys.DirectSignIn with guard validation. |
| packages/experience/src/hooks/use-prerendering.ts | New hook wrapping document.prerendering via useSyncExternalStore for safe callback deferral. |
| packages/experience/src/hooks/use-navigate-to-sign-in.ts | New centralized “terminal fallback” that aborts to client for direct sign-in sessions, else navigates to hosted sign-in. |
| packages/experience/src/hooks/use-email-blocked-error-handler.ts | Passes the error code into the confirm callback to support abort reasons. |
| packages/experience/src/apis/experience/interaction.ts | Adds abortInteraction(reason?) API wrapper calling the new endpoint. |
| packages/experience/src/apis/experience/index.ts | Re-exports the new abort API function. |
| packages/experience/src/apis/experience/const.ts | Adds the /api/experience/abort route constant. |
| packages/core/src/routes/experience/middleware/koa-experience-interaction.ts | Whitelists the new abort endpoint to bypass interaction initialization requirements. |
| packages/core/src/routes/experience/index.ts | Implements POST /experience/abort (mounted under /api) using assignInteractionResults with access_denied. |
| packages/core/src/routes/experience/index.test.ts | Adds route tests covering redirect URL, optional description, and charset validation. |
| packages/core/src/routes/experience/experience.openapi.json | Documents the new abort endpoint in Experience OpenAPI. |
| packages/core/src/routes/experience/const.ts | Adds the abort route constant to the Experience routes map. |
| .changeset/brave-pandas-return.md | Adds a minor changeset for @logto/core and @logto/experience describing the behavior change. |
Suppressed comments (1)
packages/core/src/routes/experience/experience.openapi.json:146
- The 400 response description is too narrow: this endpoint also returns 400 for request-body validation failures (e.g.
reasonoutside the allowed RFC 6749 charset), not only for missing/invalid interaction sessions.
"400": {
"description": "The interaction session is not found or is in an invalid state."
}
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+131
to
+135
| "properties": { | ||
| "reason": { | ||
| "description": "Optional human-readable reason, forwarded to the client as the `error_description` parameter." | ||
| } | ||
| } |
simeng-li
reviewed
Aug 12, 2026
| // Mark the session as entered through direct sign-in, so error handlers finish the | ||
| // interaction back at the client instead of falling back to the hosted sign-in page. | ||
| // The value records the entry point for debugging; its presence alone drives behavior. | ||
| set(StorageKeys.DirectSignIn, `${method}:${social.target}`); |
Contributor
There was a problem hiding this comment.
The marker can outlive the sign-in attempt it belongs to. It is only consumed in useNavigateToSignIn, but several error paths set it and then exit through other doors:
- right here: if the authorization-URL request fails (
invokeSocialSignIn→handleError→ toast) or the user declines the Manual terms modal, the marker stays while the user is stuck on the loading page; - the Google One Tap branch below redirects before any listener runs — with a missing/misconfigured
experienceSettings.googleOneTap.connectorIdit ends inSocialSignInWebCallback's connector-not-found<Navigate>to/sign-in, which does not consume the marker either; - the related-user path continues in hosted UI (
/social/link/:connectorId) with the marker still set.
Since sessionStorage survives navigations, a later authorization in the same tab without direct_sign_in will then abort that fresh hosted interaction on its first callback error and bounce the user back to the client app, which is wrong for that session.
Can we clear the marker whenever it can no longer be consumed — e.g. on hosted sign-in page mount, or at least on the failure paths above? Whatever cleanup we settle on should also cover the enterprise SSO branch below (invokeSso failures leak the marker the same way) so social and SSO behave consistently.
A more robust alternative that removes this class of bugs entirely: the interaction already carries direct_sign_in in its OIDC params (ExtraParamsKey.DirectSignIn is stored with the authorization request), so "entered via direct sign-in" could be derived per interaction on the server side instead of relying on a tab-scoped client marker.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/experience/src/pages/DirectSignIn/index.tsx:61
- The marker is consulted only by callback fallback handling. If
getSsoAuthorizationUrlfails before redirecting,useSingleSignOnonly displays the error and returns, so this page remains on its loading spinner and leaves the marker stale;useSocialhas the same behavior. This does not cover the connector-initiation failures described by the PR and can also make a later hosted flow in this tab abort unexpectedly. Route launcher failures through the abort fallback (and clear the marker if aborting cannot proceed).
set(StorageKeys.DirectSignIn, `${method}:${sso.id}`);
packages/experience/src/pages/DirectSignIn/index.tsx:43
- This provenance is stored only in
sessionStorage, but both social and SSO launchers explicitly maintain alocalStorageredirect fallback because external/in-app-browser redirects can losesessionStorage(use-social.ts:86-95,use-single-sign-on.ts:72-81). In that supported recovery path,validateAndRestore()restores the verification successfully while this marker is absent, so a later callback error still sends a direct-sign-in user to the hosted page. Include direct-sign-in provenance in the state-keyed redirect fallback bundle and restore it with the other callback context.
This issue also appears on line 61 of the same file.
// Mark the session as entered through direct sign-in, so error handlers finish the
// interaction back at the client instead of falling back to the hosted sign-in page.
// The value records the entry point for debugging; its presence alone drives behavior.
set(StorageKeys.DirectSignIn, `${method}:${social.target}`);
packages/experience/src/hooks/use-prerendering.ts:33
- The new activation-sensitive behavior has no test. Please cover an initial
document.prerendering === true, verify callback work is not started, dispatchprerenderingchangeafter setting it to false, and verify processing starts exactly once. This guards the one-time authorization data that this hook was introduced to protect.
const usePrerendering = () => useSyncExternalStore(subscribe, isPrerendering);
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| reason: z | ||
| .string() | ||
| .max(256) | ||
| .regex(/^[ !#-[\]-~]*$/) |
… with access_denied
charIeszhao
force-pushed
the
feat/direct-sign-in-abort
branch
from
August 21, 2026 05:20
da2cc21 to
2d1e109
Compare
COMPARE TO
|
| Name | Diff |
|---|---|
| .changeset/brave-pandas-return.md | 📈 +790 Bytes |
| packages/core/src/routes/experience/const.ts | 📈 +28 Bytes |
| packages/core/src/routes/experience/experience.openapi.json | 📈 +1.44 KB |
| packages/core/src/routes/experience/index.ts | 📈 +1.02 KB |
| packages/core/src/routes/experience/middleware/koa-experience-interaction.ts | 📈 +188 Bytes |
| packages/experience/src/Providers/UserInteractionContextProvider/index.tsx | 📈 +38 Bytes |
| packages/experience/src/apis/experience/const.ts | 📈 +28 Bytes |
| packages/experience/src/apis/experience/index.ts | 📈 +20 Bytes |
| packages/experience/src/apis/experience/interaction.ts | 📈 +366 Bytes |
| packages/experience/src/hooks/use-email-blocked-error-handler.ts | 📈 +27 Bytes |
| packages/experience/src/hooks/use-navigate-to-sign-in.ts | 📈 +1.93 KB |
| packages/experience/src/hooks/use-prerendering.ts | 📈 +1.21 KB |
| packages/experience/src/hooks/use-session-storages.ts | 📈 +77 Bytes |
| packages/experience/src/hooks/use-social-register.ts | 📈 +59 Bytes |
| packages/experience/src/hooks/use-submit-interaction-error-handler.ts | 📈 +17 Bytes |
| packages/experience/src/pages/DirectSignIn/index.test.tsx | 📈 +700 Bytes |
| packages/experience/src/pages/DirectSignIn/index.tsx | 📈 +541 Bytes |
| packages/experience/src/pages/SocialSignInWebCallback/social-callback.test.tsx | 📈 +2.61 KB |
| packages/experience/src/pages/SocialSignInWebCallback/use-single-sign-on-listener.ts | 📈 +160 Bytes |
| packages/experience/src/pages/SocialSignInWebCallback/use-social-sign-in-listener.ts | 📈 +320 Bytes |
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
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
When a sign-in initiated with the
direct_sign_inauthentication parameter fails (denied at the identity provider, connector error, invalid session, etc.), the hosted experience currently falls back to the universal sign-in page. For applications that use direct sign-in precisely because they own the sign-in UI, this is a dead end: the user lands on a page showing sign-in methods the application never offered, and the application is never told the sign-in failed.With this PR, such interactions are finished with a standard OAuth
access_deniederror and the user agent is redirected back to the client'sredirect_uri, so the application that owns the sign-in UI handles the failure itself. Sign-ins that entered through the hosted pages keep the existing fallback behavior.Before: client sends
/authorize?...&direct_sign_in=social:google→ user cancels at Google → user is stranded on the hosted/sign-inpage.After: the user agent returns to
https://client.example.com/callback?error=access_denied&error_description=...&state=...and the app can react (show its own error, offer a retry, etc.).Changes
Core
POST /api/experience/abort— finishes the current interaction viaassignInteractionResultswitherror: 'access_denied'and an optionalerror_descriptiontaken from the request'sreasonfield, and responds with theredirectToURL that resumes the OIDC flow.reasonis validated against the character set RFC 6749 §4.1.2.1 allows forerror_description(max 256 chars).koaExperienceInteractionso it works regardless of the interaction storage state — by the time an error surfaces, the interaction may not have been initialized or may be half-consumed.access_denied) rather than direct-sign-in specific; the experience app decides when aborting is the right exit.Experience
DirectSignInmarks the session insessionStorage(StorageKeys.DirectSignIn) when it invokes a social or SSO connector, so the entry point survives the round trip to the identity provider.useNavigateToSignInhook centralizes the terminal "give up and go to the sign-in page" fallback used by the social and SSO callback listeners (previously two copy-pasted inline callbacks):reason. The marker is consumed on use; if aborting fails (e.g. the interaction has expired), it falls back to the hosted sign-in page as before.usePrerenderinghook (useSyncExternalStoreoverdocument.prerendering): social/SSO callback handling is now deferred while the page is being prerendered (e.g. Chrome address-bar preloading). A prerendered page may never be shown, but its network requests do reach the server — so consuming the one-time authorization code/state (and now potentially aborting the whole interaction) must wait for activation.Breaking changes
None. The new endpoint is additive, hosted-page flows are unchanged, and a changeset is included (
minorfor@logto/coreand@logto/experience).Testing
access_denied(with and withouterror_description), and rejects areasonoutside the RFC 6749 charset.packages/coreroute tests and the fullpackages/experiencesuite pass locally.Happy to adjust naming, the endpoint shape, or make the behavior opt-in per application based on your feedback.