From cde10961646ce1363dfe55e931cd533e6380a704 Mon Sep 17 00:00:00 2001 From: Akshita <110122283+akshita317@users.noreply.github.com> Date: Sat, 24 Oct 2026 00:58:07 +0530 Subject: [PATCH] auth/extauth: validate the RFC 9207 iss in PerformOIDCLogin AuthorizationCodeHandler validates the iss parameter of the authorization response (RFC 9207), the mitigation for mix-up attacks, but PerformOIDCLogin only compared the state. The iss reported by the AuthorizationCodeFetcher was ignored, so a response naming another issuer, or lacking iss from an IdP that advertises authorization_response_iss_parameter_supported, was accepted and its code exchanged at the IdP's token endpoint. Check it after the state: iss is required when the IdP's metadata advertises support, and a present iss must equal the issuer from that metadata. A matching iss is accepted even when support is not advertised, as RFC 9207, section 2.4, asks clients to compare it. The fake IdP gets an IssParameterSupported option that advertises support and adds iss to its redirects. TestPerformOIDCLogin_Issuer covers both cases with a missing, matching and foreign iss; the three rejected cases fail without the change. --- auth/extauth/oidc_login.go | 35 +++++++++- auth/extauth/oidc_login_test.go | 93 +++++++++++++++++++++++++++ internal/oauthtest/fake_idp_server.go | 18 +++++- 3 files changed, 140 insertions(+), 6 deletions(-) diff --git a/auth/extauth/oidc_login.go b/auth/extauth/oidc_login.go index cf5d92721..5a0b75f96 100644 --- a/auth/extauth/oidc_login.go +++ b/auth/extauth/oidc_login.go @@ -88,6 +88,9 @@ func PerformOIDCLogin( if authResult.State != authReq.state { return nil, fmt.Errorf("state mismatch: expected %q, got %q", authReq.state, authResult.State) } + if err := checkAuthorizationResponseIssuer(authResult.Iss, authReq.issuer, authReq.issParameterSupported); err != nil { + return nil, err + } tokens, err := completeOIDCLogin(ctx, config, oauth2Config, authResult.Code, authReq.codeVerifier) if err != nil { @@ -102,6 +105,10 @@ type oidcAuthorizationRequest struct { authURL string state string codeVerifier string + // issuer and issParameterSupported come from the IdP's metadata, to + // validate the iss parameter of the authorization response (RFC 9207). + issuer string + issParameterSupported bool } // initiateOIDCLogin initiates an OIDC Authorization Code flow with PKCE. @@ -177,12 +184,34 @@ func initiateOIDCLogin( authURL := oauth2Config.AuthCodeURL(state, authURLOpts...) return &oidcAuthorizationRequest{ - authURL: authURL, - state: state, - codeVerifier: codeVerifier, + authURL: authURL, + state: state, + codeVerifier: codeVerifier, + issuer: meta.Issuer, + issParameterSupported: meta.AuthorizationResponseIssParameterSupported, }, oauth2Config, nil } +// checkAuthorizationResponseIssuer validates the iss parameter of an +// authorization response, the mitigation for mix-up attacks in [RFC 9207]. +// If the IdP advertises authorization_response_iss_parameter_supported, iss +// is required. A present iss must equal the issuer from the IdP's metadata, +// whether or not support is advertised (RFC 9207, section 2.4). +// +// [RFC 9207]: https://www.rfc-editor.org/rfc/rfc9207 +func checkAuthorizationResponseIssuer(iss, issuer string, issParameterSupported bool) error { + if iss == "" { + if issParameterSupported { + return fmt.Errorf("IdP advertises RFC 9207 iss parameter support but none was received in the authorization response") + } + return nil + } + if iss != issuer { + return fmt.Errorf("authorization response issuer %q does not match expected issuer %q", iss, issuer) + } + return nil +} + // completeOIDCLogin completes the OIDC Authorization Code flow by exchanging // the authorization code for tokens. func completeOIDCLogin( diff --git a/auth/extauth/oidc_login_test.go b/auth/extauth/oidc_login_test.go index 9cd9b9354..7db0e0ed0 100644 --- a/auth/extauth/oidc_login_test.go +++ b/auth/extauth/oidc_login_test.go @@ -261,3 +261,96 @@ func TestPerformOIDCLogin_ValidationErrors(t *testing.T) { }) } } + +// TestPerformOIDCLogin_Issuer checks the iss parameter of the authorization +// response (RFC 9207), the mitigation for mix-up attacks. +func TestPerformOIDCLogin_Issuer(t *testing.T) { + const otherIssuer = "https://attacker.example.com" + tests := []struct { + name string + advertised bool + // iss returns the iss the fetcher reports, given the one in the + // IdP's redirect and the IdP's issuer. + iss func(fromRedirect, issuer string) string + wantErr string + }{ + { + name: "advertised, iss from the IdP", + advertised: true, + iss: func(fromRedirect, _ string) string { return fromRedirect }, + }, + { + name: "advertised, iss missing", + advertised: true, + iss: func(string, string) string { return "" }, + wantErr: "none was received", + }, + { + name: "advertised, other issuer", + advertised: true, + iss: func(string, string) string { return otherIssuer }, + wantErr: "does not match expected issuer", + }, + { + name: "not advertised, iss missing", + iss: func(string, string) string { return "" }, + }, + { + name: "not advertised, matching iss", + iss: func(_, issuer string) string { return issuer }, + }, + { + name: "not advertised, other issuer", + iss: func(string, string) string { return otherIssuer }, + wantErr: "does not match expected issuer", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + idpServer := oauthtest.NewFakeIdPServer(oauthtest.IdPConfig{ + PreregisteredClients: map[string]oauthtest.ClientInfo{ + "test-client": { + Secret: "test-secret", + RedirectURIs: []string{testRedirectURL}, + }, + }, + IssParameterSupported: tt.advertised, + }) + idpServer.Start(t) + + fetch := func(ctx context.Context, args *auth.AuthorizationArgs) (*auth.AuthorizationResult, error) { + client := &http.Client{ + CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }, + } + resp, err := client.Get(args.URL) + if err != nil { + return nil, err + } + defer resp.Body.Close() + loc, err := resp.Location() + if err != nil { + return nil, err + } + q := loc.Query() + return &auth.AuthorizationResult{ + Code: q.Get("code"), + State: q.Get("state"), + Iss: tt.iss(q.Get("iss"), idpServer.URL()), + }, nil + } + + _, err := PerformOIDCLogin(context.Background(), validOIDCLoginConfig(idpServer.URL()), fetch) + if tt.wantErr == "" { + if err != nil { + t.Fatalf("PerformOIDCLogin() error = %v", err) + } + return + } + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("PerformOIDCLogin() error = %v, want error containing %q", err, tt.wantErr) + } + }) + } +} diff --git a/internal/oauthtest/fake_idp_server.go b/internal/oauthtest/fake_idp_server.go index b7057ffdb..b12f1098b 100644 --- a/internal/oauthtest/fake_idp_server.go +++ b/internal/oauthtest/fake_idp_server.go @@ -13,6 +13,7 @@ import ( "maps" "net/http" "net/http/httptest" + "net/url" "slices" "testing" "time" @@ -35,6 +36,10 @@ type IdPConfig struct { // TokenExchangeConfig enables RFC 8693 token exchange at the /token endpoint. // If non-nil, the server accepts grant_type=urn:ietf:params:oauth:grant-type:token-exchange. TokenExchangeConfig *TokenExchangeConfig + + // IssParameterSupported advertises authorization_response_iss_parameter_supported + // and adds the iss parameter to authorization responses (RFC 9207). + IssParameterSupported bool } // FakeIdPServer is a fake OIDC Identity Provider for testing. @@ -89,8 +94,7 @@ func (s *FakeIdPServer) handleMetadata(w http.ResponseWriter, r *http.Request) { if s.config.TokenExchangeConfig != nil { grantTypes = append(grantTypes, oauthex.GrantTypeTokenExchange) } - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(map[string]any{ + meta := map[string]any{ "issuer": s.URL(), "authorization_endpoint": s.URL() + "/authorize", "token_endpoint": s.URL() + "/token", @@ -98,7 +102,12 @@ func (s *FakeIdPServer) handleMetadata(w http.ResponseWriter, r *http.Request) { "response_types_supported": []string{"code"}, "code_challenge_methods_supported": []string{"S256"}, "grant_types_supported": grantTypes, - }) + } + if s.config.IssParameterSupported { + meta["authorization_response_iss_parameter_supported"] = true + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(meta) } func (s *FakeIdPServer) handleAuthorize(w http.ResponseWriter, r *http.Request) { @@ -130,6 +139,9 @@ func (s *FakeIdPServer) handleAuthorize(w http.ResponseWriter, r *http.Request) s.codes[code] = codeInfo{CodeChallenge: codeChallenge} state := r.URL.Query().Get("state") redirectURL := fmt.Sprintf("%s?code=%s&state=%s", redirectURI, code, state) + if s.config.IssParameterSupported { + redirectURL += "&iss=" + url.QueryEscape(s.URL()) + } http.Redirect(w, r, redirectURL, http.StatusFound) }