Skip to content

Enable redirect-based OAuth flow#127

Open
thepatrickchin wants to merge 10 commits into
NVIDIA:mainfrom
thepatrickchin:feat/redirect-auth
Open

Enable redirect-based OAuth flow#127
thepatrickchin wants to merge 10 commits into
NVIDIA:mainfrom
thepatrickchin:feat/redirect-auth

Conversation

@thepatrickchin

Copy link
Copy Markdown
Member

Description

This is a complementary PR for NVIDIA/NeMo-Agent-Toolkit#1835
Closes NVIDIA/NeMo-Agent-Toolkit#1834

By Submitting this PR I confirm:

  • I am familiar with the Contributing Guidelines.
  • We require that all contributors "sign-off" on their commits. This certifies that the contribution is your original work, or you have rights to submit it under the same license, or a compatible license.
    • Any contribution which contains commits that are not Signed-Off will not be accepted.
  • When the PR is ready for review, new or existing tests cover these changes.
  • When the PR is ready for review, the documentation is up to date with these changes.

@thepatrickchin thepatrickchin requested a review from a team as a code owner April 2, 2026 06:09
@copy-pr-bot

copy-pr-bot Bot commented Apr 2, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

Copy link
Copy Markdown

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions Bot added the Stale Activity is stale; may be automatically closed without update label May 25, 2026
@thepatrickchin thepatrickchin force-pushed the feat/redirect-auth branch 2 times, most recently from a4e5644 to f853e4b Compare May 28, 2026 10:11
@github-actions github-actions Bot removed the Stale Activity is stale; may be automatically closed without update label May 28, 2026
@thepatrickchin thepatrickchin force-pushed the feat/redirect-auth branch 2 times, most recently from 76d54dc to 161b487 Compare June 22, 2026 20:52
Signed-off-by: Patrick Chin <8509935+thepatrickchin@users.noreply.github.com>
Signed-off-by: Patrick Chin <8509935+thepatrickchin@users.noreply.github.com>
Signed-off-by: Patrick Chin <8509935+thepatrickchin@users.noreply.github.com>
Signed-off-by: Patrick Chin <8509935+thepatrickchin@users.noreply.github.com>
Signed-off-by: Patrick Chin <8509935+thepatrickchin@users.noreply.github.com>
@thepatrickchin thepatrickchin force-pushed the feat/redirect-auth branch 2 times, most recently from d986ad4 to 46c06d4 Compare June 23, 2026 02:13
thepatrickchin and others added 5 commits June 22, 2026 22:15
The resume effect deleted 2 messages (user + assumed assistant placeholder)
before resubmitting. No assistant placeholder is appended at send time; the
assistant bubble is only created when a response/intermediate message arrives.
For preflight auth the consent arrives before any agent execution, so deleting
2 removed the user message AND the previous turn's assistant reply.

Delete the assistant bubble only when one actually exists, mirroring the
cancellation branch's logic.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Patrick Chin <8509935+thepatrickchin@users.noreply.github.com>
window.open returns null when noopener/noreferrer is set, so the popup handle
was always null and the popup.close() on auth completion never ran. Drop those
flags so the handle is usable and the popup can be closed programmatically.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Patrick Chin <8509935+thepatrickchin@users.noreply.github.com>
The popup message listener acted on AUTH_CANCELLED from any window. The backend
posts that message from the NAT server's /auth/redirect callback with
targetOrigin '*', and oauthUrl points at the IdP authorize endpoint, so neither
an origin check nor the oauthUrl origin identifies the legitimate sender.
Gate on event.source === popup instead: the source window identity is the
correct trust boundary and is stable across the popup's cross-origin hops.
Relies on the popup handle being non-null (noopener/noreferrer removed).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Patrick Chin <8509935+thepatrickchin@users.noreply.github.com>
…n error

When pre-flight authentication is cancelled or fails, the backend sends a bare
Error payload ({code, message, details}) with no type/conversation_id. The
strict message validator rejected it and surfaced a raw "Invalid WebSocket
message structure" toast, while the loading spinner stayed stuck.

Detect the user_auth_error payload before validation, clear the in-flight
loading/streaming state, and show the backend's auth message in a toast.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Patrick Chin <8509935+thepatrickchin@users.noreply.github.com>
With use_redirect_auth + preflight_auth, cancelling the OAuth login caused an
infinite loop: cancel → reload → WS reconnects → preflight → redirect → cancel.

The fix uses a sessionStorage flag (oauth_redirect_cancelled) to suppress
auto-reconnect after a cancelled redirect, breaking the loop. Four coordinated
changes:

1. oauth_redirect_initiated is stamped before window.location.href so the flag
   survives the page reload even when preflight fires before any user message
   exists (no pending message is saved in that case, so the resume effect returns
   early and cannot set the guard itself).
2. A new early effect reads oauth_redirect_initiated on reload and sets
   oauth_redirect_cancelled if oauth_auth_completed is absent from the URL. It
   runs before the WS toggle effect so the guard is in place before
   connectWebSocket is called.
3. The WS toggle effect checks oauth_redirect_cancelled before connecting,
   preventing a new preflight from starting.
4. openOAuthConsentUrl also checks the flag as defense-in-depth.
5. handleSend clears the flag so the user can retry by sending a new message.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Patrick Chin <8509935+thepatrickchin@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request New feature or request non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add option to enable redirect-based OAuth flow

2 participants