wso2 login against ThunderID: bind access to a named resource by kanushka · Pull Request #46 · wso2/wso2-cli

kanushka · 2026-08-06T15:02:50Z

Closes #41.

What this is

wso2 login now works against ThunderID, the way #17 made it work against Asgardeo and Identity Server 7.3.0. The slice began as a spike, and the spike changed the design — so the measurement is here alongside the code it forced.

What the measurement found

Measured against ThunderID v1.0.0-beta at https://localhost:8490, 2026-08-06. Full verdicts are in the research document §3.2 and on #41.

The headline risk did not fire: Thunder has an API-resource-and-scopes model, and it maps cleanly. A resource server's identifier is the audience, its resources are the scopes.

What did break was an assumption. Thunder decides an access token's audience per request, from an RFC 8707 resource indicator it requires at authorization and accepts exactly one of. The broker's scoped refresh cannot even establish a session there — the refusal happens at authorization, before a session exists. So this slice is production code, not configuration.

Two consequences worth reading before the diff:

The audience guarantee gets stronger. On Asgardeo aud is the client ID and cannot distinguish products (Decide what the broker claims when a deployment cannot bind a token to one product #37). On Thunder it is the resource server identifier and nothing else.
One session reaches one product. Only one indicator is accepted per authorization — "Only a single resource parameter is supported". Thunder identities are therefore restricted to one product, refused when the document is read rather than at the end of a browser sign-in. Lifting that needs per-product sessions: Per-product login sessions: one identity's session cannot serve several products on Thunder #43. It was already recorded as a required gap in the compatibility research §1.5; this is the first hard evidence.

What changed

internal/contexts — an identity may name its provider and, optionally, its narrowing. Both optional; absent both means the derivation that ships today, so every existing document keeps working and there is no migration. A provider states a default, an explicit derivation overrides it. Two refusals are decided at read time: a resource-bound identity with more than one product, and one whose product names no audience.
internal/auth + internal/app — the resource indicator is carried on the authorization request and on the client-credentials grant, where the identity says the deployment reads one, and nowhere else. The refresh grant needed nothing: the binding is inherited, measured. invalid_target is read as its own refusal, because it is the one failure here caused by the context document rather than by the deployment.
internal/auth/fakeissuer — grows the deployment behaviour rather than the tests growing a second fake.
Docs — a Thunder walkthrough as its own guide, reached by one line from the login guide; Thunder's verdict cells beside Asgardeo's and Identity Server's.
test/smoke — a Thunder deployment block, the ported experiments behind make empirical-thunder, and make smoke-ci.

Testing

Zero new seams. Three existing ones: Broker.Acquire against the fake issuer, smoke.Config in the default gate, and the tagged live runs. No exported strategy type was added so a test could reach it.

Proven live, not only against the fake:

THUNDER REFRESH NARROWING AND AUDIENCE BINDING: honored
  deployment: https://localhost:8490
--- PASS: TestThunderEmpirical (215.95s)

That run drove the real shell end to end — oauthflow.Login sent the indicator, the session stored, and Broker.Acquire granted access narrowed to one permission and bound to the resource, with the broker's own verification proving it.

make test vet lint acceptance all pass.

Things a reviewer should push on

A default resource server changes the answer. Setting one makes the indicator optional and binds every token to that default whichever product asked — reintroducing exactly the Decide what the broker claims when a deployment cannot bind a token to one product #37 weakness on the one product that can avoid it. The walkthrough says not to set one. Recorded on wso2 login against Thunder (ThunderID): measure the derivation strategy, then build only what it forces #41 as a correction to my own earlier, too-strong claim.
The first live CI run exists but has not been run green against Thunder. Thunder models machine identities as agents, and I did not find how to grant an m2m client permissions; the run skips without configuration. The path is proven against the fake issuer as it is for every other product. Worth deciding whether that gap blocks this slice.
The walkthrough is a separate file, which is asymmetric with Asgardeo and Identity Server being inline. That was a coordination compromise, deliberately taken, and is filed as Split the per-product login walkthroughs into their own guides #45.
Single sign-on is unmeasured. A second authorization minutes after a completed sign-in presented the sign-in form again on a default deployment. That is recorded narrowly and should not be read as "Thunder does not do SSO".

Follow-ups filed

Per-product login sessions: one identity's session cannot serve several products on Thunder #43 — per-product sessions
Split the per-product login walkthroughs into their own guides #45 — split the per-product walkthroughs symmetrically

https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod

Asgardeo and Identity Server decide an access token's audience from the application's registration, so one login session serves every product an identity declares. ThunderID decides it per request, from an RFC 8707 resource indicator it requires at authorization and accepts only one of. Measured at v1.0.0-beta: without one the authorization request is refused outright. An identity may now name the provider behind its issuer, and may name the derivation explicitly when a deployment does not match what its provider ordinarily requires. Both are optional, both are names of behaviour rather than locations of secrets, and an identity that declares neither derives exactly as every identity did before — so every document that works today keeps working and no migration is needed. Naming the provider is what the person writing the document knows. What that product requires of a token request is what the shell knows, and the pair exists so the second does not have to be written by hand. An explicit derivation wins over the one a provider implies, because a Thunder deployment whose resource server is not registered yet is a real state and the document has to be able to say so. Two refusals are decided when the document is read rather than at the end of a browser sign-in the user cannot act on: an identity that derives by resource and declares more than one product cannot be served by one session at all, and a product that names no audience leaves nothing to bind access to. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod

A deployment that decides the audience per request will not issue access at all without being told which protected resource it is for. ThunderID at v1.0.0-beta answers invalid_target — on the authorization request and on the client-credentials grant alike. The indicator is sent where the identity says the deployment reads one, and nowhere else, so no deployment already working starts receiving a parameter it never agreed to interpret. It belongs to the login rather than to the grant that follows because these deployments decide the audience once, at authorization: a session established without it cannot be bound afterwards, and one established with it reaches that resource and no other. The refresh grant needs nothing — the binding is inherited, measured. The client-credentials grant has no earlier authorization to inherit from, so it carries the indicator itself, taking it from the module's own request. invalid_target is read as its own refusal rather than falling through to the answer for a deployment nobody can classify. It is the one failure here caused by the context document rather than by the deployment: the identity did not say what kind of deployment this is, and the fix is a line in a document rather than a change to a registration. The stable code list does not grow — it stays auth.narrowing_unavailable, which is where a caller is left — but the guidance says the thing the reader can act on. The fake issuer grows the deployment behaviour rather than the tests growing a second fake: a request carrying an indicator is bound to it, and one that omits it against a deployment requiring one is refused exactly as the measured deployment refuses it. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod

A live run against Thunder differs from the two deployments already described in two ways, and both follow from Thunder deciding an access token's audience per request: the audience is the resource server's identifier and must be an absolute URI, and the run has to say which identity provider it is describing or the login is refused with invalid_target before a session exists. The provider is validated when the environment is read rather than left to the document, so a deployment described with a name this shell does not read is reported before a run reaches a browser and wastes a person's attention. The non-interactive run needs a client secret. Its variable is named in code, beside the secure-store reference that is already fixed there, and deliberately not in the deployment description: that file is one people copy and keep, and naming the variable in it invites the value to be pasted beside the name. The run reads the variable from the process environment exactly as the shell reads the variable a context names. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod

The walkthrough is its own guide rather than a third numbered section of the login guide. Thunder is pre-1.0 and will change faster than the other two products, and churn is cheapest in a file nothing cross-references; the login guide was also already long enough that a third inline walkthrough would have pushed it past thirteen hundred lines. The asymmetry is a compromise and is filed as its own issue rather than left as a wart. Every step in it was run against the deployment it describes, including the console navigation: the resource hierarchy panel, the OAuth2 configuration under Advanced Settings, and the four loopback callbacks on the General tab. The version it was written against is named, because a console layout is the fastest thing in an alpha product to go stale and a reader needs to know what the steps described. The research document gains Thunder's verdict cells beside Asgardeo's and Identity Server's, each with the date and the deployment that produced it. It also gains a row the other two never raise — where the audience is decided — because that is the difference everything else follows from. Two findings are recorded that a reader would otherwise measure differently and conclude the other was wrong: a default resource server makes the resource indicator optional and binds every token to the same audience, and a resource handle cannot contain the delimiter that joins permission names. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod

The experiments are the ported half of the spike: the throwaway that answered the design question is gone, and what remains is the part that backs a recorded verdict. They ask Thunder the two questions sections 3 and 3.1 ask of Asgardeo and Identity Server, so its column is comparable rather than a separate story, plus the one those products never raise — whether the audience can be chosen at all. They refuse to run against a deployment that is not a Thunder one, because a resource experiment against a product that takes no resource indicator would print a verdict about nothing. The CI run is the first live coverage of the non-interactive path against any deployment. Until now that source was proven only against the in-process fake issuer, on every product, so the guarantee it makes to a module rested on a fixture. It asks for one permission out of the configured set rather than for all of them, because a run that asks for everything cannot tell narrowing from a deployment that ignored the request — the defect wso2#35 fixed in the browser run, written out of this one from the start. It needs no browser and no human, so it can run unattended, and it is the one live target that can. Its client secret comes from the environment and from no file, and the document it installs holds a secret's variable and no secure-store reference, which is what the schema requires of a kind that never logs in. That document is built in the untagged half of the package so this package's own tests prove the shell will read it before a live run depends on it. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod

The resource-indicator experiment classified every error as the deployment refusing a login that named no resource. A deployment the shell cannot reach at all produces an error too, so an unreachable host was recorded as the strongest possible finding about a question that was never asked. This is not hypothetical. The first run of this experiment against a rebuilt container reported "required" when what had actually happened was that the container had regenerated its certificate and discovery failed. The verdict would have gone into the research document as evidence. auth.discovery_failed now reads as inconclusive and says so, matching how the Asgardeo experiments already treat the same code. A verdict that cannot tell a refusal from an unreachable host is not evidence, and the whole reason these experiments are committed rather than run by hand is that a recorded finding has to be reproducible by someone who was not there. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod

coderabbitai · 2026-08-06T15:03:01Z

Warning

Review limit reached

@kanushka, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 34 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 01cd6e8b-1e3a-4f17-8c72-aa501521a7e7

📥 Commits

Reviewing files that changed from the base of the PR and between cc5b77a and 1567098.

📒 Files selected for processing (9)

docs/guides/login-thunder.md
internal/auth/fakeissuer/fakeissuer.go
internal/auth/narrowing.go
internal/auth/resource_test.go
internal/auth/source_browser_test.go
internal/auth/source_clientcred.go
internal/auth/tokenrequest.go
internal/contexts/derivation_test.go
internal/contexts/identity.go

📝 Walkthrough

Summary

Added ThunderID provider support to wso2 login.
Added provider and narrowing configuration with backward-compatible defaults.
Added RFC 8707 resource indicators for authorization and client-credentials flows.
Added resource and audience validation with typed invalid_target handling.
Added ThunderID smoke tests, empirical tests, CI support, and Make targets.
Added ThunderID registration, configuration, troubleshooting, and research documentation.
Verified behavior with automated tests and a live ThunderID run.

Walkthrough

The change adds ThunderID provider and derivation support with RFC 8707 resource indicators. Interactive and client-credentials flows send derived product resources and preserve audience binding. Validation enforces one product and an absolute URI audience for token-resource identities. Smoke configuration now supports ThunderID and unattended CI credentials. New empirical tests measure Thunder behavior. Guides and research notes document deployment, registration, configuration, and observed results.

Sequence Diagram(s)

sequenceDiagram
  participant Context
  participant Login
  participant OAuthIssuer
  participant AuthenticationBroker
  Context->>Login: resolve ThunderID product resource
  Login->>OAuthIssuer: authorize with resource indicator
  OAuthIssuer-->>Login: return authorization code
  Login->>OAuthIssuer: exchange code with resource indicator
  OAuthIssuer-->>AuthenticationBroker: issue audience-bound token
  AuthenticationBroker-->>Context: validate scope, audience, and expiry

Suggested reviewers: hevayo

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR addresses the measured resource-binding requirements, but the required live non-interactive client-credentials run remains unproven [`#41`].	Run and record a successful uncached Thunder client-credentials test with environment-sourced secrets, or document an approved exception.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the primary ThunderID resource-binding change.
Description check	✅ Passed	The description directly explains the ThunderID implementation, measurements, tests, documentation, and known follow-ups.
Out of Scope Changes check	✅ Passed	The code, tests, smoke support, research, and documentation changes all support the ThunderID objectives in issue `#41`.
Docstring Coverage	✅ Passed	Docstring coverage is 90.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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

_{Comment @coderabbitai help to get the list of available commands.}

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds ThunderID support for resource-indicator-based audience binding and introduces CI/non-interactive smoke coverage, alongside updated docs and research notes.

Changes:

Extend context schema to support provider-implied derivation and explicit narrowing, with validation for resource-bound derivations.
Send RFC 8707 resource indicator during browser login (authorize + code exchange) and client-credentials token minting when required.
Add Thunder-specific smoke/config tests, CI smoke test target, and documentation for Thunder registration and empirical findings.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
test/smoke/thunder_empirical_test.go	Adds empirical smoke experiments specific to Thunder’s resource-indicator behavior.
test/smoke/thunder_config_test.go	Validates smoke config/document behavior for Thunder provider and CI document shape.
test/smoke/env.example	Documents Thunder-specific env configuration block.
test/smoke/config.go	Adds smoke config knobs for provider + CI client/secret variable and emits appropriate documents.
test/smoke/ci_smoke_test.go	Adds live smoke test for non-interactive (client-credentials) acquisition path.
test/smoke/RUNNING.md	Documents new smoke variables and explains secret handling for CI runs.
internal/contexts/identity.go	Introduces provider/derivation concepts and validation for resource-bound deployments.
internal/contexts/derivation_test.go	Adds coverage for provider/derivation semantics and Thunder-specific validation rules.
internal/contexts/contexts_test.go	Updates JSON-member allowlist to account for new auth fields.
internal/auth/tokenrequest.go	Adds detection for “missing resource indicator” (`invalid_target`) refusals.
internal/auth/source_clientcred.go	Sends `resource` on client-credentials grant when derivation requires it; improves refusal messaging.
internal/auth/resource_test.go	Verifies inline identity binds to resource when provider implies resource derivation.
internal/auth/oauthflow/resource_test.go	Verifies browser login includes resource indicator and binds session audience accordingly.
internal/auth/oauthflow/login.go	Adds `Login.Resource` and passes it through authorize and token exchange.
internal/auth/narrowing.go	Adds recovery guidance for missing resource indicator scenarios.
internal/auth/fakeissuer/fakeissuer.go	Extends fake issuer to model resource-indicator-required deployments + audience minting from resource.
internal/app/login_resource_test.go	Adds app-level tests ensuring login includes resource indicator only when needed.
internal/app/login.go	Plumbs product audience into login as OAuth resource indicator based on derivation.
docs/research/asgardeo-redirect-uri-and-scope-narrowing.md	Records Thunder empirical results and implications for the CLI derivation approach.
docs/guides/login.md	Links Thunder registration as an alternative path.
docs/guides/login-thunder.md	Adds full Thunder registration/troubleshooting guide.
Makefile	Adds `smoke-ci` and `empirical-thunder` targets and updates help text.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…te URI Review found that a resource-bound identity was validated only for a non-empty audience, so a document naming the bare identifier the other two products use loaded successfully and then failed at the deployment with invalid_target. That is the failure this validation exists to prevent: the walkthrough already warns that a bare name here costs a browser sign-in to discover, and the schema was letting it through anyway. The rule is RFC 8707 section 2's — an absolute URI carrying no fragment — and stops there. Requiring a particular scheme, or a host, would refuse identifiers the specification permits; a URN names a resource server perfectly well, and this shell never dereferences the value. What the check cannot claim is written down beside it. "localhost:8490/x" is an absolute URI whose scheme happens to be "localhost", almost certainly a missing "https://", and it is accepted — refusing it would mean holding an opinion about plausibility that nothing here is entitled to hold. It fails at the deployment, which knows which resource servers it registered. The provider list is now exported and read by the live-run harness rather than copied into it. A copy would drift the moment a product is added: the harness would refuse a provider the shell accepts, and report a correct description as a deployment that could not be run. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod

coderabbitai

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/auth/fakeissuer/fakeissuer.go (1)
391-405: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Validate the resource during code exchange.

exchangeCode does not read the submitted resource parameter. Removing the token-exchange indicator from oauthflow.Login would still pass these resource-binding tests. When RequireResource is set, require the submitted resource to match grant.resource and add missing-resource and mismatched-resource tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/auth/fakeissuer/fakeissuer.go` around lines 391 - 405, Update
Issuer.exchangeCode to read the submitted resource parameter and validate it
against grant.resource when RequireResource is enabled. Reject missing or
mismatched resources before minting the access token, while preserving existing
behavior when resource validation is not required; add tests covering both
missing and mismatched-resource exchanges.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/guides/login-thunder.md`:
- Around line 79-104: The Thunder guide uses port 8090 in follow-up discovery,
issuer, and console examples despite the alternate mapping to 8490. Update the
affected commands and URLs to consistently use the selected host port, and
explicitly instruct users to substitute their chosen host port when it differs
from 8090.
- Around line 115-129: Add the text language identifier to the fenced Markdown
blocks containing the certificate error output and URL list, including the
corresponding block around the referenced later section, to satisfy MD040
without changing their contents.

In `@internal/auth/fakeissuer/fakeissuer.go`:
- Line 405: Update the refresh-token state created alongside mintAccessTokenFor
in the fake issuer flow to store grant.resource in addition to scopes, then
update the refresh handling to pass that stored resource when minting the
refreshed access token. Preserve the established audience across refresh grants
while leaving initial token issuance unchanged.

In `@internal/auth/tokenrequest.go`:
- Around line 83-92: Keep issuerRefusal.requiresResourceIndicator in
internal/auth/tokenrequest.go as a generic invalid_target classification without
implying the resource was omitted. In internal/auth/source_clientcred.go, choose
missing-indicator recovery only when the client-credential derivation did not
send a resource; otherwise use the generic invalid-target handling. Update the
narrowing configuration guidance in internal/auth/narrowing.go to state both
valid paths: a supported provider or explicit narrowing: "token-resource".

In `@internal/contexts/identity.go`:
- Around line 230-264: Update Identity.validateDerivation to require exactly one
product for DerivationTokenResource identities, rejecting both zero and multiple
products before iterating audiences. Add a decode test covering a token-resource
identity document with no products and assert that validation returns a
malformed-identity error.

---

Outside diff comments:
In `@internal/auth/fakeissuer/fakeissuer.go`:
- Around line 391-405: Update Issuer.exchangeCode to read the submitted resource
parameter and validate it against grant.resource when RequireResource is
enabled. Reject missing or mismatched resources before minting the access token,
while preserving existing behavior when resource validation is not required; add
tests covering both missing and mismatched-resource exchanges.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b823593e-b30d-458b-952e-2e0b220a9668

📥 Commits

Reviewing files that changed from the base of the PR and between ea083c2 and cc5b77a.

📒 Files selected for processing (22)

Makefile
docs/guides/login-thunder.md
docs/guides/login.md
docs/research/asgardeo-redirect-uri-and-scope-narrowing.md
internal/app/login.go
internal/app/login_resource_test.go
internal/auth/fakeissuer/fakeissuer.go
internal/auth/narrowing.go
internal/auth/oauthflow/login.go
internal/auth/oauthflow/resource_test.go
internal/auth/resource_test.go
internal/auth/source_clientcred.go
internal/auth/tokenrequest.go
internal/contexts/contexts_test.go
internal/contexts/derivation_test.go
internal/contexts/identity.go
test/smoke/RUNNING.md
test/smoke/ci_smoke_test.go
test/smoke/config.go
test/smoke/env.example
test/smoke/thunder_config_test.go
test/smoke/thunder_empirical_test.go

…d_targets apart Three findings from review, all valid. The fake issuer bound only the initial access token to its resource. Refresh state carried the granted permissions and not the resource, so a renewal fell back to the registration's audience — and the whole reason the refresh grant was left untouched is that a deployment which binds by resource carries the binding forward. The fixture disagreed with the deployment on exactly the point the design rests on, and no test could have noticed: the audience it fell back to happened to equal the resource in every fixture. Giving the registration a different audience is what makes the new test able to fail at all. invalid_target covered two opposite causes and always reported one of them. A request that named no resource and a request that named one the deployment does not know arrive as the same OAuth error, and the shell told both to go and name an identity provider — advice the second has already followed. Which happened is a fact about the request, so the classification says only that the target was rejected and the caller, which made the request, decides what to say. A resource-bound identity with no products passed validation, and login then sent no indicator. The rule is exactly one product, not at most one: an identity with none has nothing to bind to. Claude-Session: https://claude.ai/code/session_01YDkmmpLxjac7VhBvHyoJod

Copilot

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (2)

internal/auth/resource_test.go:86

This comment has a doubled phrase: "one that did name a resource named a resource the deployment does not recognise." The words "name a resource named a resource" repeat and make the sentence hard to parse. It should read something like "one that named a resource the deployment does not recognise."

// deployment does not accept, and one that did name a resource named a resource

internal/auth/source_clientcred.go:108

This doc comment is now stale after the change: it still says "Three answers are worth telling apart, because they send the user to three different places," but the function now switches over five cases and adds two more destinations (a resource server the deployment does not know, and a deployment that must be told which resource). Additionally, the new sent parameter description is appended directly onto the "three different places" paragraph without a blank line, so it reads as part of that sentence. Consider updating the count and separating the parameter documentation.

// Three answers are worth telling apart, because they send the user to three
// different places: a registration the deployment owns, a secret the job owns,
// and an issuer this shell cannot speak for.
// sent is the resource indicator this request carried, empty when it carried
// none. It is what tells the two halves of invalid_target apart.

kanushka added 6 commits August 6, 2026 19:48

kanushka requested a review from hevayo as a code owner August 6, 2026 15:02

kanushka requested a lite review from Copilot August 6, 2026 16:03

Copilot AI reviewed Aug 6, 2026

View reviewed changes

Comment thread internal/contexts/identity.go

Comment thread internal/app/login_resource_test.go Outdated

Comment thread test/smoke/config.go Outdated

coderabbitai Bot reviewed Aug 7, 2026

View reviewed changes

Comment thread docs/guides/login-thunder.md

Comment thread docs/guides/login-thunder.md Outdated

Comment thread internal/auth/fakeissuer/fakeissuer.go

Comment thread internal/auth/tokenrequest.go Outdated

Comment thread internal/contexts/identity.go

kanushka requested a balanced review from Copilot August 7, 2026 05:57

Copilot started reviewing on behalf of kanushka August 7, 2026 05:57 View session

Copilot AI reviewed Aug 7, 2026

View reviewed changes

kanushka requested review from axewilledge, kaje94 and sachiniSam August 7, 2026 07:25

axewilledge approved these changes Aug 7, 2026

View reviewed changes

Navigation Menu

Uh oh!

wso2 login against ThunderID: bind access to a named resource - #46

wso2 login against ThunderID: bind access to a named resource#46
kanushka wants to merge 8 commits into
wso2:mainfrom
kanushka:feature/thunder-login

kanushka commented Aug 6, 2026

Uh oh!

coderabbitai Bot commented Aug 6, 2026 •

edited

Loading

Review limit reached

Summary

Walkthrough

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

kanushka commented Aug 6, 2026

What this is

What the measurement found

What changed

Testing

Things a reviewer should push on

Follow-ups filed

Uh oh!

coderabbitai Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Summary

Walkthrough

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai Bot commented Aug 6, 2026 •

edited

Loading