feat(oauth2): implement Identity Assertion JWT (ID-JAG) issuance (DEP #4600)#4611
feat(oauth2): implement Identity Assertion JWT (ID-JAG) issuance (DEP #4600)#4611kanywst wants to merge 10 commits intodexidp:masterfrom
Conversation
135a54a to
bdf0a6e
Compare
87e1745 to
f1de572
Compare
5b1e4e1 to
63b015e
Compare
63b015e to
5bb1ebf
Compare
|
Hi @nabokihms @AlwxSin, DEP #4600 has been merged, and this is the implementation PR. Everything described in the DEP is covered here. Would appreciate a review when you get a chance. Thanks! |
|
|
||
| // extractJWTSubAndAud extracts the "sub" and "aud" claims from a JWT without | ||
| // verifying the signature. The aud claim may be a string or []string. | ||
| func extractJWTSubAndAud(token string) (sub string, aud []string, err error) { |
There was a problem hiding this comment.
I'm concerned that extractJWTSubAndAud doesn't verify signature.
subject_tokenis an ID token issued by this same server and should be verified against the server's own signing keys. Otherwise an attacker can impersonate any user- Also, this func doesn't check whether the subject token has expired. An expierd ID token can be exchanged for a fresh ID-JAG indefinitely
There was a problem hiding this comment.
Thanks.
Fixed in cc4bd0f. This verifies signature, expiry, issuer, and audience.
| } | ||
|
|
||
| if matched == nil { | ||
| return PolicyResult{ |
There was a problem hiding this comment.
The function returns both a non-nill error and sets Denied: true, which is confusing. I'm not sure which returned variable I should look at.
In Go convention, errors should signal unexpected failures, not business-logic denials.
There was a problem hiding this comment.
Fixed in c07a63a. Denials now return Denied: true with nil error
There was a problem hiding this comment.
Now no code path returns a non-nil error. Can we we change signature to func evaluateIDJAGPolicy(policies []TokenExchangePolicy, clientID, audience string, scopes []string) PolicyResult {} without an error?
| return fmt.Sprintf("%s.%s.%s", headerB64, payloadB64, signatureB64), nil | ||
| } | ||
|
|
||
| func (v *vaultSigner) SignWithType(ctx context.Context, payload []byte, tokenType string) (string, error) { |
There was a problem hiding this comment.
Thi func is almost identical to Sign func.
Consider to separate common func logic
func Sign() {
return sign(false)
}
func SignWithType() {
return sign(true)
}
func sign(withType bool) {}
|
|
||
| c.PrometheusRegistry.MustRegister(requestCounter, durationHist, sizeHist) | ||
|
|
||
| // ID-JAG metrics. |
There was a problem hiding this comment.
Consider adding a check if ID-JAG is enabled. Otherwise there are phantom metrics that never increments, adding noise to monitoring dashboards
| userIdentity = &ui | ||
| } | ||
|
|
||
| // Skip approval if user already consented to the requested scopes for this client. |
There was a problem hiding this comment.
This check seems unrelated to token exchange. It modifies core auth flow
| type idJAGClaims struct { | ||
| Issuer string `json:"iss"` | ||
| Subject string `json:"sub"` | ||
| Audience string `json:"aud"` |
There was a problem hiding this comment.
I don;t know how to follow the RFC in a best way.
RFC allows aud to be either a string or an array. Current draft assumes only a string. So, if the ID-JAG draft allows multi-audience in the future, this will need a breaking change.
There was a problem hiding this comment.
Added a comment noting that audience is a single string per the current draft (draft-ietf-oauth-identity-assertion-authz-grant-02).
If multi-audience is added in a future revision, we can update then. Fixed in 9ce3053.
b7f4587 to
9ce3053
Compare
| } | ||
|
|
||
| if matched == nil { | ||
| return PolicyResult{ |
There was a problem hiding this comment.
Now no code path returns a non-nil error. Can we we change signature to func evaluateIDJAGPolicy(policies []TokenExchangePolicy, clientID, audience string, scopes []string) PolicyResult {} without an error?
| // IDJAGEnabled reports whether the ID-JAG token type is enabled. | ||
| func (c TokenExchangeConfig) IDJAGEnabled() bool { | ||
| for _, t := range c.TokenTypes { | ||
| if t == "urn:ietf:params:oauth:token-type:id-jag" { |
There was a problem hiding this comment.
There is a const tokenTypeIDJAG in oauth2.go
| return | ||
| } | ||
|
|
||
| if _, err := s.getConnector(ctx, connectorID); err != nil { |
There was a problem hiding this comment.
Please, add a comment explaining why the result from s.getConnector is unused. I figured it out only after checking log message
…exidp#4600) Add support for Identity Assertion JWT Authorization Grant (draft-ietf-oauth-identity-assertion-authz-grant-02) as a new requested_token_type in the RFC 8693 Token Exchange flow. - ID-JAG JWT with typ=oauth-id-jag+jwt, required claims (iss, sub, aud, client_id, jti, exp, iat) and optional claims (resource, scope) - Per-client policy with default-deny: allowedAudiences, allowedScopes - Scope filtering per Section 4.3.2 (granted scopes may differ from requested; response includes scope when modified) - subject_token audience validation against client_id (Section 4.3) - Public client rejection (Section 8.1) - OIDC Discovery: identity_chaining_requested_token_types_supported - SignWithType on signer interface for custom JWT typ header - Prometheus counters: dex_id_jag_requests_total, dex_id_jag_policy_rejections_total, dex_id_jag_scope_modifications_total - Structured logging on issuance and rejection with decision context Signed-off-by: Takuma Niwa <takuma@because-and.com> Signed-off-by: kanywst <niwatakuma@icloud.com>
…ange Signed-off-by: kanywst <niwatakuma@icloud.com>
…olicy Signed-off-by: kanywst <niwatakuma@icloud.com>
Signed-off-by: kanywst <niwatakuma@icloud.com>
Signed-off-by: kanywst <niwatakuma@icloud.com>
Signed-off-by: kanywst <niwatakuma@icloud.com>
No code path returns a non-nil error after the earlier refactor that moved policy denials into PolicyResult. Signed-off-by: kanywst <niwatakuma@icloud.com>
Signed-off-by: kanywst <niwatakuma@icloud.com>
…ange Signed-off-by: kanywst <niwatakuma@icloud.com>
9ce3053 to
f397972
Compare
…id-jag-phase1 # Conflicts: # config.yaml.dist # server/handlers.go
Overview
Implement ID-JAG (Identity Assertion JWT) token issuance via Token Exchange, per DEP #4600.
What this PR does / why we need it
Adds a new branch to the existing Token Exchange flow that issues ID-JAG tokens (
typ: oauth-id-jag+jwt) whenrequested_token_type=urn:ietf:params:oauth:token-type:id-jag. This enables cross-domain access brokered by the IdP,as specified in draft-ietf-oauth-identity-assertion-authz-grant-02.
Key points:
oauth2.tokenExchange.tokenTypesidJAGPoliciesinstaticClientscontrol allowed audiences/scopes (default-deny)identity_chaining_requested_token_types_supportedPhase 2 (accepting ID-JAG as upstream identity via RFC 7523) is out of scope.
Connected DEP: #4600
Special notes for your reviewer
Config example in
config.yaml.distandexamples/config-dev.yaml.