Skip to content

Send the configured signInOptions when signIn() is called without options - #158

Open
brionmario wants to merge 1 commit into
thunder-id:mainfrom
brionmario:playground-react-resource
Open

brionmario wants to merge 1 commit into
thunder-id:mainfrom
brionmario:playground-react-resource

Conversation

@brionmario

Copy link
Copy Markdown
Member

Purpose

signInOptions is documented as the extra parameters for the authorize request, but a redirect sign-in only sent the options passed to that call. A bare signIn() dropped them.

That matters for the RFC 8707 resource indicator. A ThunderID server binds every token to one resource server, so on a server with no default resource server, sign-in fails with invalid_target: No resource parameter supplied and no default resource server is configured unless resource is sent. Setting it on the provider (signInOptions={{resource}}) did nothing for an app that calls signIn() itself. ProtectedRoute and SignInButton only worked because they pass the context's signInOptions explicitly.

The React playground showed this. It has no way to send resource and calls signIn() without options, so it could not sign in to a stock server.

Approach

  • @thunderid/browser: when signIn() starts a redirect sign-in with no options, or an empty options object, it now sends the configured signInOptions. The same applies to the silent sign-in tried before the redirect, when session.silentSignIn.beforeRedirect is on. Options passed to the call still take their place, so ProtectedRoute, SignInButton and any caller passing its own options behave as before. The empty-object case matters because the provider's callback handling calls signIn({callOnlyOnRedirect: true}), which is {} once that flag is removed. React, Vue, Next.js and Nuxt get this through @thunderid/browser.
  • React playground: new VITE_THUNDERID_RESOURCE, passed as signInOptions.resource and shown in the Overview configuration, with .env.example and README entries.

Testing

  • New ThunderIDBrowserClient.signInOptions.test.ts: configured options are sent on a bare signIn() and on signIn({}), the call's options take their place, and nothing extra is sent when none are configured. Two of its tests fail without the fix.
  • @thunderid/browser (141 tests) and @thunderid/react (727 tests) pass. Type-check passes, and the change adds no lint errors to ThunderIDBrowserClient.ts.
  • @thunderid/vue tests are flaky in local browser mode on main too (failing file counts change between runs). The Vue redirect sign-in test (authorization-details.test.ts) passes in isolation on this branch.
  • Manual: React playground against a local ThunderID Data Plane with no default resource server, VITE_THUNDERID_RESOURCE=https://localhost:8090/mcp. signIn() completes, the access token carries scope: openid profile email system, and useGrantedPermissions() returns the same, with hierarchical can() checks (system grants system:user:view). Without the variable, sign-in fails with invalid_target, as before.

Related Issues

  • N/A

Related PRs

  • N/A

No sample is affected: the only sample setting signInOptions (b2b/manage) signs in through ProtectedRoute and SignInButton, which already pass them.

Cross-SDK parity

  • iOS: already adds the configured signInOptions to every authorize request.
  • Android: ThunderIDConfig.signInOptions is declared but never used; buildSignInUrl only adds prompt, login_hint and fidp from the call. Flutter on Android inherits this. Needs a port.

Checklist

  • Followed the contribution guidelines.
  • Manual test round performed and verified.
  • Documentation provided. (Add links if there are any)
  • Tests provided. (Add links if there are any)
    • Unit Tests
    • Integration Tests
  • Breaking changes. (Fill if applicable)
    • Breaking changes section filled.
    • breaking change label added.
  • Cross-SDK parity. Exactly one of parity/prs-raised or parity/prs-not-needed added.
    • If parity/prs-raised, the port links are posted as a reply on the parity check's comment.

Security checks

  • Followed secure coding standards.
  • Confirmed that this PR doesn't commit any keys, passwords, tokens, usernames, or other secrets.

…ions

The signInOptions configuration is documented as the extra parameters for the authorize request,
but a redirect sign-in only sent the options passed to that call. A bare signIn() therefore
dropped them, so an RFC 8707 resource indicator set there never reached a server that has no
default resource server, and sign-in failed with invalid_target. ProtectedRoute and SignInButton
worked around it by passing the context's signInOptions themselves.

A redirect sign-in called without options, or with an empty options object, now sends the
configured signInOptions, including on the silent sign-in tried before the redirect. Options
passed to the call still take their place.

The React playground gains VITE_THUNDERID_RESOURCE, passed as signInOptions.resource and shown
in the Overview configuration, so it can sign in to a server without a default resource server.

Signed-off-by: Brion <info@brionmario.com>
@coderabbitai

coderabbitai Bot commented Nov 6, 2026

Copy link
Copy Markdown

Important

  • Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

🔀 Cross-SDK feature parity

Does this change need to ship in the other ThunderID SDKs too? This check stays red until one of these labels answers that:

Label What it asserts
parity/prs-raised The ports exist in every other SDK this change reaches, or are recorded as not applying there.
parity/prs-not-needed Nothing to port: a fix, refactor, docs, CI, dependency bump, release, or a change specific to this platform.

Ports (DO NOT EDIT)

  • thunder-id/ios-sdks, iOS
  • thunder-id/android-sdks, Android
  • thunder-id/flutter-sdks, Flutter

No sibling SDKs accounted for on this thread yet.

Important

Reply to this comment with this block, filled in. The boxes above tick
themselves when you post it.

- thunder-id/ios-sdks: <paste the port link here>
- thunder-id/android-sdks: <paste the port link here>
- thunder-id/flutter-sdks: <paste the port link here>

A link can be a pull request or a tracked issue, written in full or as thunder-id/ios-sdks#123.
Where the change does not reach an SDK, replace the placeholder with N/A and a short
reason, as in - thunder-id/ios-sdks: N/A, the capability has no equivalent there.

The contract is in the SDK development specification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant