feat(egress): client-side Dynamic Client Registration for Atlassian Rovo authv2 - #1519
feat(egress): client-side Dynamic Client Registration for Atlassian Rovo authv2#1519go-faustino wants to merge 2 commits into
Conversation
5b1391d to
c0f2078
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
173f181 to
cc71053
Compare
|
Hi @go-faustino — we pulled this PR and tested the full DCR + public-PKCE flow end-to-end against Atlassian's live Rovo authv2 AS. It mostly works; we made one fix on top and hit a wall on the Atlassian side we couldn't get past. Sharing everything so you can take the last mile (you presumably have a working authv2 org). What works (verified live):
One fix we'd suggest — Atlassian authv2 DCR returns a confidential client. Config gotchas we hit (not code):
The wall — need your input. After the domain was allowlisted and scopes were valid, the authorize leg still redirects to: …for a freshly-minted DCR client — even though an earlier, identical-shape request (different client) had reached the consent screen minutes before. No callback ever reached the gateway; the failure is entirely Atlassian-side of the authorize step. We suspect DCR-client churn/propagation (we created ~6 clients while debugging), an org entitlement nuance, or an authorize detail authv2 needs beyond the standard set. Questions:
Minor robustness bug (unfixed, your call): Full patch (our commit on top of this PR head —
|
cc71053 to
68a40e0
Compare
…d DCR consent Incorporates review feedback from @aarora79 (agentic-community#1519), who tested the flow end-to-end against Atlassian's live authv2 AS. Atlassian's authv2 DCR endpoint returns a CONFIDENTIAL client (a client_secret) even when we register token_endpoint_auth_method=none. The previous code forced client_secret="" whenever cfg.public_client on the token/refresh legs, dropping the secret Atlassian issued and expects. Decide the token-leg auth by whether a secret is actually held, not by the declared public_client intent: - oauth_engine._build_token_request: key the public-client branch off the ABSENCE of a secret (`if not client_secret`) rather than cfg.public_client. PKCE is always sent, so this works whether the DCR client ends up public or confidential. - service._token_leg_secret: send the stored secret if one was persisted, else stay public. Used by both exchange and refresh. - service.build_consent_url: a requires_dcr provider whose client_id has not been registered yet now raises a clear EgressAuthError instead of a KeyError; the initiate + connect routes translate it to a clean 400 ("re-save egress config to register") instead of a 500. Tests updated/added for secret-presence token requests, _token_leg_secret, and the unregistered-DCR 400 on both consent routes.
|
Thanks @aarora79 — this is an incredibly useful review, especially validating the discovery/DCR/authorize legs against the live AS. I've pushed Adopted from your patchToken-leg auth keyed off secret presence (your option (a)). You're right — Atlassian's authv2 DCR hands back a
I kept Robustness bug (the unguarded Scope gotcha — confirmed, independentlySame finding on our side: Your questions1. Did I get an end-to-end token mint on my org, and what did the final working authorize look like? Straight answer: I have not reproduced a clean interactive mint through the gateway's own callback either — I hit the identical 2. What does authv2 need after DCR? Ranked by how well it fits our shared symptom (identical-shape request reaches consent for one client, fails for the next):
NetWith your token-leg fix + valid scopes + the domain allowlist, the only thing between this and a working vend is that intermittent authorize-side |
…d DCR consent Incorporates review feedback from @aarora79 (agentic-community#1519), who tested the flow end-to-end against Atlassian's live authv2 AS. Atlassian's authv2 DCR endpoint returns a CONFIDENTIAL client (a client_secret) even when we register token_endpoint_auth_method=none. The previous code forced client_secret="" whenever cfg.public_client on the token/refresh legs, dropping the secret Atlassian issued and expects. Decide the token-leg auth by whether a secret is actually held, not by the declared public_client intent: - oauth_engine._build_token_request: key the public-client branch off the ABSENCE of a secret (`if not client_secret`) rather than cfg.public_client. PKCE is always sent, so this works whether the DCR client ends up public or confidential. - service._token_leg_secret: send the stored secret if one was persisted, else stay public. Used by both exchange and refresh. - service.build_consent_url: a requires_dcr provider whose client_id has not been registered yet now raises a clear EgressAuthError instead of a KeyError; the initiate + connect routes translate it to a clean 400 ("re-save egress config to register") instead of a 500. Tests updated/added for secret-presence token requests, _token_leg_secret, and the unregistered-DCR 400 on both consent routes.
85b5c8e to
5c97e5c
Compare
…d DCR consent Incorporates review feedback from @aarora79 (agentic-community#1519), who tested the flow end-to-end against Atlassian's live authv2 AS. Atlassian's authv2 DCR endpoint returns a CONFIDENTIAL client (a client_secret) even when we register token_endpoint_auth_method=none. The previous code forced client_secret="" whenever cfg.public_client on the token/refresh legs, dropping the secret Atlassian issued and expects. Decide the token-leg auth by whether a secret is actually held, not by the declared public_client intent: - oauth_engine._build_token_request: key the public-client branch off the ABSENCE of a secret (`if not client_secret`) rather than cfg.public_client. PKCE is always sent, so this works whether the DCR client ends up public or confidential. - service._token_leg_secret: send the stored secret if one was persisted, else stay public. Used by both exchange and refresh. - service.build_consent_url: a requires_dcr provider whose client_id has not been registered yet now raises a clear EgressAuthError instead of a KeyError; the initiate + connect routes translate it to a clean 400 ("re-save egress config to register") instead of a 500. Tests updated/added for secret-presence token requests, _token_leg_secret, and the unregistered-DCR 400 on both consent routes.
|
Hi @go-faustino, will test this next week, so with this do you have Atlassian-Rovo server working? |
|
Hi @aarora79 — thanks, appreciate you taking another pass. I'll be away until September so won't be able to respond to findings in real time, but to set expectations honestly: as of my last comment, no, I haven't gotten a clean end-to-end token mint through the gateway's own callback. The DCR/PKCE/discovery/authorize-URL shape is validated correct (yours and mine both reach consent), but we're both hitting the same invalid_request wall at /authorize for freshly-minted DCR clients — I think it's Atlassian-side (propagation/churn or entitlement), not something in this code, but I couldn't confirm. If you get a chance to test: the two things that'd actually move this forward are (1) whether your DCR 201 echoes back redirect_uris, and (2) whether a single non-churned client succeeds after a short delay (~30-60s) rather than registering repeatedly while debugging. If you get a clean mint, that'd basically close this out. I'll pick this back up in September — feel free to push commits or just leave findings on the thread in the meantime. |
5c97e5c to
9a0d3b0
Compare
…d DCR consent Incorporates review feedback from @aarora79 (agentic-community#1519), who tested the flow end-to-end against Atlassian's live authv2 AS. Atlassian's authv2 DCR endpoint returns a CONFIDENTIAL client (a client_secret) even when we register token_endpoint_auth_method=none. The previous code forced client_secret="" whenever cfg.public_client on the token/refresh legs, dropping the secret Atlassian issued and expects. Decide the token-leg auth by whether a secret is actually held, not by the declared public_client intent: - oauth_engine._build_token_request: key the public-client branch off the ABSENCE of a secret (`if not client_secret`) rather than cfg.public_client. PKCE is always sent, so this works whether the DCR client ends up public or confidential. - service._token_leg_secret: send the stored secret if one was persisted, else stay public. Used by both exchange and refresh. - service.build_consent_url: a requires_dcr provider whose client_id has not been registered yet now raises a clear EgressAuthError instead of a KeyError; the initiate + connect routes translate it to a clean 400 ("re-save egress config to register") instead of a 500. Tests updated/added for secret-presence token requests, _token_leg_secret, and the unregistered-DCR 400 on both consent routes.
…d DCR consent Incorporates review feedback from @aarora79 (agentic-community#1519), who tested the flow end-to-end against Atlassian's live authv2 AS. Atlassian's authv2 DCR endpoint returns a CONFIDENTIAL client (a client_secret) even when we register token_endpoint_auth_method=none. The previous code forced client_secret="" whenever cfg.public_client on the token/refresh legs, dropping the secret Atlassian issued and expects. Decide the token-leg auth by whether a secret is actually held, not by the declared public_client intent: - oauth_engine._build_token_request: key the public-client branch off the ABSENCE of a secret (`if not client_secret`) rather than cfg.public_client. PKCE is always sent, so this works whether the DCR client ends up public or confidential. - service._token_leg_secret: send the stored secret if one was persisted, else stay public. Used by both exchange and refresh. - service.build_consent_url: a requires_dcr provider whose client_id has not been registered yet now raises a clear EgressAuthError instead of a KeyError; the initiate + connect routes translate it to a clean 400 ("re-save egress config to register") instead of a 500. Tests updated/added for secret-presence token requests, _token_leg_secret, and the unregistered-DCR 400 on both consent routes.
9a0d3b0 to
d0adb61
Compare
…d DCR consent Incorporates review feedback from @aarora79 (agentic-community#1519), who tested the flow end-to-end against Atlassian's live authv2 AS. Atlassian's authv2 DCR endpoint returns a CONFIDENTIAL client (a client_secret) even when we register token_endpoint_auth_method=none. The previous code forced client_secret="" whenever cfg.public_client on the token/refresh legs, dropping the secret Atlassian issued and expects. Decide the token-leg auth by whether a secret is actually held, not by the declared public_client intent: - oauth_engine._build_token_request: key the public-client branch off the ABSENCE of a secret (`if not client_secret`) rather than cfg.public_client. PKCE is always sent, so this works whether the DCR client ends up public or confidential. - service._token_leg_secret: send the stored secret if one was persisted, else stay public. Used by both exchange and refresh. - service.build_consent_url: a requires_dcr provider whose client_id has not been registered yet now raises a clear EgressAuthError instead of a KeyError; the initiate + connect routes translate it to a clean 400 ("re-save egress config to register") instead of a 500. Tests updated/added for secret-presence token requests, _token_leg_secret, and the unregistered-DCR 400 on both consent routes.
2845106 to
291b4dc
Compare
…d DCR consent Incorporates review feedback from @aarora79 (agentic-community#1519), who tested the flow end-to-end against Atlassian's live authv2 AS. Atlassian's authv2 DCR endpoint returns a CONFIDENTIAL client (a client_secret) even when we register token_endpoint_auth_method=none. The previous code forced client_secret="" whenever cfg.public_client on the token/refresh legs, dropping the secret Atlassian issued and expects. Decide the token-leg auth by whether a secret is actually held, not by the declared public_client intent: - oauth_engine._build_token_request: key the public-client branch off the ABSENCE of a secret (`if not client_secret`) rather than cfg.public_client. PKCE is always sent, so this works whether the DCR client ends up public or confidential. - service._token_leg_secret: send the stored secret if one was persisted, else stay public. Used by both exchange and refresh. - service.build_consent_url: a requires_dcr provider whose client_id has not been registered yet now raises a clear EgressAuthError instead of a KeyError; the initiate + connect routes translate it to a clean 400 ("re-save egress config to register") instead of a 500. Tests updated/added for secret-presence token requests, _token_leg_secret, and the unregistered-DCR 400 on both consent routes.
|
+1 |
1 similar comment
|
+1 |
291b4dc to
e4c4321
Compare
go-faustino
changed the title
|
Hi @aarora79 — back as promised. End-to-end mint works. Root cause is one missing scope: Rovo authv2 rejects any authorize request that omits it, after the user submits consent, with Same DCR client, same redirect, one variable: without Your question 1: the DCR Token exchange succeeds as a public client — PKCE only, no That reverses the token-leg fix you contributed. Atlassian's DCR issues a secret even when Rebased onto current What remains is DCR discovery + registration, the Happy to split the scope work into a follow-up if you would rather land DCR on its own. Thanks for the live testing that narrowed this to the post-consent step. |
…6587]
From 2026-05-27 Atlassian Identity only issues MCP-audience tokens to RFC 7591
DCR clients; a static classic-3LO app authenticates but is rejected at the Rovo
MCP tool layer because its token carries the wrong audience.
This commit lets the gateway register its own OAuth client at config time and
reuse that client_id for all subsequent consent/token flows for that server.
Schema changes (OAuthProviderConfig):
- requires_dcr (bool): provider mandates Dynamic Client Registration
- registration_url (str|None): pinned RFC 7591 endpoint; falls back to discovery
- protected_resource_metadata_url (str|None): RFC 9728 document for AS discovery
- dcr_client_name (str): client_name sent on the registration request
- default_scopes (list): used when the server config carries no scope list
- required_scopes (list): unioned into every request regardless of config
Engine additions (oauth_engine.py):
- _get_json / _post_dcr: SSRF-safe discovery and registration requests via
CREDENTIALED_OAUTH_PROFILE (same guard as the token endpoint)
- _discover_registration_url: walks RFC 9728 -> RFC 8414 append form
({as}/.well-known/oauth-authorization-server) to find registration_endpoint
- register_dcr_client: RFC 7591 POST; token_endpoint_auth_method derived from
cfg.token_endpoint_auth_style (NONE -> "none", else "client_secret_post")
- fetch_protected_resource_metadata: returns the whole PRM so one fetch serves
both scope validation and the registration-endpoint walk
- validate_scopes_against_prm: reports scopes absent from scopes_supported;
reports none when the PRM does not advertise the array at all
Route changes (configure_egress_auth):
- For requires_dcr providers: run DCR once, persist client_id; reuse on re-saves.
Confidential DCR clients that return no secret are rejected with HTTP 502.
NONE-style (public PKCE) clients with no secret are accepted, and any secret
the AS volunteers is not persisted -- the token leg never reads it.
- Resolve scopes before use: apply default_scopes when none are configured, then
union in required_scopes. DCR registers the resolved list, not the raw input.
- Validate scopes against the resource's scopes_supported before registering, so
a bad config cannot orphan a DCR client at the AS. Unsupported scopes give a
400 naming them. An unreachable PRM is fatal only when a registration is about
to happen; on the reuse path validation is skipped with a warning so re-saving
an existing server does not depend on the provider's metadata endpoint.
- TokenEndpointAuthStyle imported at module level so the check is explicit.
Facade + public routes (initiate_consent):
- Wrap build_consent_url in try/except EgressAuthError -> HTTP 400 so a
requires_dcr provider whose client_id was never registered gives a clear
error instead of a KeyError/500.
Service (build_consent_url):
- Raise EgressAuthError with an actionable message when a requires_dcr provider
has no client_id. Scoped to requires_dcr so operators of plainly
misconfigured non-DCR providers are not told to trigger a registration that
would never run.
Provider recipe (atlassian):
- requires_dcr=True, token_endpoint_auth_style=NONE, protected_resource_metadata_url
pointing at Rovo MCP protected-resource metadata, no classic audience, no
RFC 8707 resource.
- required_scopes=["read:account"]. Verified live by a controlled sweep: authv2
rejects any authorize request without read:account, and does so only AFTER the
user submits consent, with an opaque invalid_request / "Incorrect request
parameters" that names no scope. Nothing advertises the requirement (RFC 9728
has no required-scopes field), so it is not discoverable from metadata. It is
mandated rather than merely defaulted because a default is only a fallback: an
operator-supplied list omitting it would fail identically.
- default_scopes: the 5-scope set verified end to end (token minted PKCE-only,
and real data tools -- getVisibleJiraProjects, getJiraIssue, atlassianUserInfo
-- all returning 200). Confluence scopes are not defaulted; add per-server.
- The consent screen's three permission groups (Read / Write / Search) do not
affect the outcome in either direction; only read:account does.
- Token leg confirmed public: the exchange succeeds with PKCE and no
client_secret. Atlassian's DCR returns a secret (client_secret_expires_at: 0)
even when token_endpoint_auth_method is echoed as "none"; it is deliberately
unused and unpersisted. To flip to confidential if that ever changes: set
token_endpoint_auth_style to POST_BODY here and the route persists the
DCR-returned secret, with the confidential guard rejecting a secret-less
response automatically. The engine needs no change either way.
…ches [PE1-6587]
TestDynamicClientRegistration (test_oauth_engine.py):
- Discovery: RFC 9728 PRM -> RFC 8414 AS metadata walk; pinned registration_url
short-circuits discovery; missing AS list / registration_endpoint raise errors.
- Registration: NONE-style sends token_endpoint_auth_method=none and returns no
secret; confidential client returns secret; missing client_id raises.
- End-to-end: register_dcr_client through the real guarded client (mock httpx).
- A pre-fetched PRM is reused rather than re-fetched.
TestDcrTransport (test_oauth_engine.py):
- _get_json happy path, non-JSON, SSRF guard failure, HTTP error wrapping.
- _post_dcr happy path, error payload, non-JSON, SSRF guard failure, HTTP error.
TestScopeValidation (test_oauth_engine.py):
- validate_scopes_against_prm: all-supported, some-unsupported, empty input,
and a PRM with no scopes_supported (validation skipped, not failed closed).
- fetch_protected_resource_metadata happy path and unconfigured-URL error.
TestAtlassianAuthorizeUrl (test_oauth_engine.py):
- Confirm no audience or RFC 8707 resource in the Atlassian authorize URL.
TestConfigureEgressDcr (test_configure_egress_url_validation.py):
- NONE-style client: DCR runs, client_id persisted, no secret required.
- Skips DCR when client_id already present (config re-save idempotency).
- DCR engine error -> HTTP 502 with detail.
- Reuse existing client_id + rotate supplied secret (confidential provider).
- NONE-style drops an operator-supplied secret, and discards one volunteered by
the AS -- Atlassian returns a secret even for token_endpoint_auth_method=none.
- Confidential DCR client returning no secret -> HTTP 502.
- DCR receives the RESOLVED scope list (defaults applied, required unioned in),
since registering the raw operator input would mint a client whose grant can
never complete consent.
TestConfigureEgressDefaultScopes / TestConfigureScopeValidation:
- default_scopes applied only when the operator supplies none; explicit scopes
win; providers without defaults are unaffected.
- required_scopes appended to an explicit list that omits them -- the case a
default cannot cover -- without duplicating an already-correct entry, and not
applied to providers that declare none.
- Unsupported scope -> 400 naming it; valid set passes; validation skipped when
the PRM advertises no scopes_supported.
- Rejection happens before DCR (asserted on the registration path, with no
client_id supplied, so the branch is actually entered).
- Unreachable PRM: fatal 502 when registering, degrades to unvalidated on the
reuse path so an edit does not depend on the provider's metadata endpoint.
TestConsentAndCallback (test_service.py):
- requires_dcr provider with no client_id raises EgressAuthError on
build_consent_url ("not registered yet").
TestBuildConsentUrl (test_public_routes.py, test_facade_routes.py):
- EgressAuthError on consent entry points -> HTTP 400.
TestAtlassianProvider (test_providers.py):
- requires_dcr + NONE style + PRM URL pinned; no classic audience param.
- default_scopes is the verified 5-scope set; required_scopes is ["read:account"].
- Non-atlassian providers declare neither default_scopes nor required_scopes.
- test_builtin_providers_are_confidential updated: non-DCR providers stay
confidential; DCR providers (atlassian) may use NONE style.
e4c4321 to
6e3e8a2
Compare
5 participants
What this does
Adds RFC 7591 client-side Dynamic Client Registration to the egress OAuth engine, and wires the
atlassianprovider recipe to Atlassian's Rovo MCPauthv2Authorization Server.From 2026-05-27 Atlassian Identity only issues MCP-audience tokens to DCR-registered clients. A static classic-3LO app still authenticates, but its token carries the wrong audience and is rejected at the Rovo MCP tool layer. So the gateway has to register its own OAuth client rather than use operator-supplied credentials.
Verified end to end against live Atlassian
DCR-registered public client → authorize → consent → code → token → MCP session → real data-tool calls:
client_secret),HTTP 200, refresh token presentaudbinds to the DCRclient_idplus the user's site/workspace ARIs — not toapi.atlassian.com, and noresource/audienceparameter participatesMCP initialize→200,tools/list→200getVisibleJiraProjects,getJiraIssue,atlassianUserInfo,getAccessibleAtlassianResourcesall200/isError: false, with the site reporting granularread:jira-work/write:jira-workscopesThe blocker, for the record
This PR sat conflicting for six weeks because the authorize request kept failing with
invalid_request/ "Incorrect request parameters". The cause turned out to be a single missing scope:read:account.Rovo authv2 rejects any authorize request that omits it — and rejects it after the user submits consent, naming no scope. Nothing advertises the requirement: RFC 9728 has no required-scopes field, so
read:accountlooks exactly as optional as the other 21 entries inscopes_supported. Isolated by a controlled sweep, same client and redirect, one variable at a time:read:accountread:accountread:accountread:accountread:account)read:accountfully determines the outcome. The consent screen's Read/Write/Search permission groups have no effect in either direction.Ruled out with evidence, not inference: DCR client propagation/churn,
redirect_urisnot being persisted (the DCR201does echo them),prompt=consent, RFC 8707resource, classicaudience,statelength, org entitlement, callback-domain allowlisting, and site/ARI resolution.Scope of the change
Rebased onto current
main. That rebase showed most of the original PR had become redundant: upstream now hasTokenEndpointAuthStyle.NONE, whose docstring already anticipates "a client minted by an MCP resource server's Dynamic Client Registration endpoint", andservice._client_secretalready returnsNonefor that style while fail-closing for confidential ones.So the original public-client apparatus — the
public_clientfield, the_build_token_requestchange, and_token_leg_secret— is deleted and rebuilt on upstream'sNONEstyle. This PR no longer touches_build_token_request, and the fail-closed guard plus its two tests are byte-identical tomain.What remains is only what upstream lacks:
requires_dcr,registration_url,protected_resource_metadata_url,dcr_client_name,default_scopes,required_scopesonOAuthProviderConfigregister_dcr_client()with registration-endpoint discovery walking RFC 9728 → RFC 8414 (the Rovo AS is path-scoped and itsregistration_endpointis tenant-scoped, so pinning is not viable)CREDENTIALED_OAUTH_PROFILEguarded client as the token endpointclient_idso a re-save does not churn a registration;502on failure400instead of aKeyError/500at both consent entry points when arequires_dcrprovider has noclient_idyetrequired_scopes, unioned into every request regardless of operator config — deliberately not just a default, since a default only applies when the operator supplies nothing and an explicit list omittingread:accountwould fail identicallyscopes_supported, reusing the PRM the discovery walk already fetches (no extra round-trip) and running before registration so a bad config cannot orphan a DCR client at the ASatlassianrecipe:requires_dcr+NONEstyle + PRM discovery, noaudience, noresourceA NONE-style client does not persist the
client_secretAtlassian returns anyway (it arrives withclient_secret_expires_at: 0even whentoken_endpoint_auth_methodis echoed as"none"): the token leg never reads it, so storing it is needless exposure.Notes
read:jira-userand the classicread:confluence-content.all/write:confluence-contentare absent from the authv2 resource'sscopes_supported.audto Jira ARIs; operators wanting Confluence opt in per server.pushed_authorization_request_endpointandclient_id_metadata_document_supported: true, so PAR and CIMD are available for later — relevant to feat: Brokered MCP Server with CIMD Authentication #1687.Tests
334 passedintests/unit/egress_auth/.ruff checkandruff format --checkclean.