From ec58fc218848d7e0235035f7397a1b09064a8e34 Mon Sep 17 00:00:00 2001 From: Simon Eisenmann Date: Wed, 14 Oct 2026 14:10:29 +0200 Subject: [PATCH] Add RFC 9207 iss to authorization responses, errors included A client talking to several authorization servers cannot tell from a bare code and state which server answered. A malicious server can then get it to send a code issued by an honest one to the attacker's token endpoint, the mix-up attack of RFC 9207 section 1. RFC 9207 lets the authorization server name itself in every response with iss, and section 2.4 makes a client which sees the metadata announce support reject a response without iss or with a different one, by simple string comparison. Every authorization response redirected to the client, success and error alike, now carries iss, the issuer identifier exactly as in the discovery document, and the metadata announces authorization_response_iss_parameter_supported: true, as RFC 9207 section 2.3 ties the two together. AuthenticationSuccess gains Iss, set in AuthorizeResponse from the provider's issuer identifier. AuthenticationError and AuthenticationBadRequest gain Iss for the URL only, tagged json "-", since they are also rendered as JSON bodies where iss has no place. It is filled in NewError and NewBadRequest of AuthenticationRequest from the provider metadata the request already holds. Every error redirected to a client goes through these two constructors, from the identity managers, denied consent, required login or interaction, invalid_target and the OAuth2Error conversion, so no call site changes. Logout redirects, which also use AuthenticationSuccess, are not authorization responses and stay without iss. HTML error pages are unchanged. Clients which do not know RFC 9207 ignore the extra parameter, as RFC 6749 section 4.1.2 requires of unrecognized response parameters. The API spec lists RFC 9207 in its info section, describes iss for the authorize endpoint and its error convention and gains the metadata field, and the README lists RFC 9207 among the standards. The new TestAuthorizeResponseIss checks that the metadata announces support and that both a success redirect and an error redirect, an invalid_target for an unregistered resource, carry iss equal to the discovery issuer. It fails without the change. The helper getTestProviderMetadata fetches the discovery document for it. Verified live, the built licod announces the flag, and an error redirect for an unknown resource carries error=invalid_target, its description and state, and iss=https%3A%2F%2Flocalhost%3A18781. --- README.md | 3 ++ docs/libregraph-connect-api-v1.yaml | 14 ++++- oidc/payload/authentication.go | 21 ++++++++ oidc/provider/handlers.go | 1 + oidc/provider/handlers_test.go | 79 +++++++++++++++++++++++++++++ oidc/provider/provider.go | 5 ++ 6 files changed, 121 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 10e646e2..85bb2c60 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,9 @@ the limits worth knowing before you build on it. the same document the OpenID Connect discovery endpoint serves - [RFC 8707 - Resource Indicators](https://www.rfc-editor.org/rfc/rfc8707.html) - the `resource` parameter binds an access token to the resource it is for +- [RFC 9207 - Authorization Server Issuer Identification](https://datatracker.ietf.org/doc/html/rfc9207) - + every authorization response redirected to the client, errors included, + carries `iss` - [OAuth 2.0 Multiple Response Type Encoding Practices](https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html) ### JOSE and cryptography diff --git a/docs/libregraph-connect-api-v1.yaml b/docs/libregraph-connect-api-v1.yaml index 3a32fa6c..bf9232eb 100644 --- a/docs/libregraph-connect-api-v1.yaml +++ b/docs/libregraph-connect-api-v1.yaml @@ -15,6 +15,7 @@ info: - OAuth 2.0 Authorization Server Metadata (RFC 8414) - Proof Key for Code Exchange (RFC 7636), `S256` only - Resource Indicators for OAuth 2.0 (RFC 8707) + - OAuth 2.0 Authorization Server Issuer Identification (RFC 9207) - Dynamic Client Registration (only when explicitly enabled, see below) - SAML2 and OAuth2 integration with external identity providers @@ -44,7 +45,7 @@ info: - The **OIDC/OAuth2 protocol endpoints** (`token`) return the JSON `OIDCError` shape. - `authorize` reports errors by **302 redirect** to the client's `redirect_uri` with - `error`/`error_description`/`state` in the query or fragment. It does not return JSON. + `error`/`error_description`/`state`/`iss` in the query or fragment. It does not return JSON. - The **identifier JSON endpoints** (`{signin}/identifier/_/*`) never return a JSON error body. Failures are `text/plain` from Go's `http.Error`, shaped `" [ - ]"`, e.g. `400 Bad Request - failed to decode request JSON`. @@ -155,10 +156,14 @@ paths: **Errors are not returned as JSON.** A request that validates far enough to know the client and `redirect_uri` is answered with a 302 to that `redirect_uri` carrying - `error`, `error_description` and `state`, in the query string or the fragment + `error`, `error_description`, `state` and `iss`, in the query string or the fragment depending on the response type. Requests that fail before that point render an HTML error page. + Every redirect to the client, success or error, carries `iss`, the issuer identifier + exactly as in the discovery document (RFC 9207), so a client talking to several + authorization servers can tell which one answered. + A request naming a `resource` binds the tokens it leads to to that resource, which must be registered with this server, and narrows the requested scopes to the ones the resource accepts. A client which must use PKCE is not refused here: a code @@ -1226,6 +1231,11 @@ components: is therefore not advertised. items: type: string + authorization_response_iss_parameter_supported: + type: boolean + description: | + Emitted as `true`. Every authorization response redirected to the client carries + `iss` (RFC 9207), errors included, so a client may reject one which does not. subject_types_supported: type: array items: diff --git a/oidc/payload/authentication.go b/oidc/payload/authentication.go index c501adb5..821a56fe 100644 --- a/oidc/payload/authentication.go +++ b/oidc/payload/authentication.go @@ -414,6 +414,7 @@ func (ar *AuthenticationRequest) NewError(id string, description string) *Authen ErrorID: id, ErrorDescription: description, State: ar.State, + Iss: ar.issuer(), } } @@ -424,9 +425,19 @@ func (ar *AuthenticationRequest) NewBadRequest(id string, description string) *A ErrorID: id, ErrorDescription: description, State: ar.State, + Iss: ar.issuer(), } } +// issuer returns the issuer identifier of the provider the associated request +// was made to, as an error redirected back to the client has to carry it. +func (ar *AuthenticationRequest) issuer() string { + if ar.providerMetadata == nil { + return "" + } + return ar.providerMetadata.Issuer +} + // AuthenticationSuccess holds the outgoind data for a successful OpenID // Connect 1.0 authorize request as specified at // http://openid.net/specs/openid-connect-core-1_0.html#AuthResponse and @@ -440,6 +451,11 @@ type AuthenticationSuccess struct { State string `url:"state"` ExpiresIn int64 `url:"expires_in,omitempty"` + // Iss identifies the issuer of the response, so a client talking to more + // than one authorization server can tell which one answered, as specified + // at https://www.rfc-editor.org/rfc/rfc9207.html#section-2. + Iss string `url:"iss,omitempty"` + Scope string `url:"scope,omitempty"` SessionState string `url:"session_state,omitempty"` @@ -453,6 +469,9 @@ type AuthenticationError struct { ErrorID string `url:"error" json:"error"` ErrorDescription string `url:"error_description,omitempty" json:"error_description,omitempty"` State string `url:"state,omitempty" json:"state,omitempty"` + // Iss as in AuthenticationSuccess. It belongs to the redirect alone and + // has no place in an error rendered as a JSON body. + Iss string `url:"iss,omitempty" json:"-"` } // Error interface implementation. @@ -472,6 +491,8 @@ type AuthenticationBadRequest struct { ErrorID string `url:"error" json:"error"` ErrorDescription string `url:"error_description,omitempty" json:"error_description,omitempty"` State string `url:"state,omitempty" json:"state,omitempty"` + // Iss as in AuthenticationSuccess. + Iss string `url:"iss,omitempty" json:"-"` } // Error interface implementation. diff --git a/oidc/provider/handlers.go b/oidc/provider/handlers.go index 6b413b4a..3f86523d 100644 --- a/oidc/provider/handlers.go +++ b/oidc/provider/handlers.go @@ -346,6 +346,7 @@ done: // http://openid.net/specs/openid-connect-core-1_0.html#ImplicitAuthResponse response := &payload.AuthenticationSuccess{ State: ar.State, + Iss: p.issuerIdentifier, Scope: strings.Join(authorizedScopesList, " "), SessionState: sessionState, diff --git a/oidc/provider/handlers_test.go b/oidc/provider/handlers_test.go index 3ced580c..fb406f97 100644 --- a/oidc/provider/handlers_test.go +++ b/oidc/provider/handlers_test.go @@ -449,6 +449,85 @@ func TestAuthorizeAndTokenWithResource(t *testing.T) { }) } +// TestAuthorizeResponseIss covers the iss parameter of RFC 9207 on the +// redirect back to the client, which a client compares with the issuer of the +// discovery document by simple string comparison, for an error as much as for +// a success, and which the discovery document announces. +func TestAuthorizeResponseIss(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + httpServer, _, router, config := NewTestProvider(ctx, t) + defer httpServer.Close() + + metadata := getTestProviderMetadata(t, router, config.WellKnownPath) + if !metadata.AuthorizationResponseIssParameterSupported { + t.Error("authorization_response_iss_parameter_supported is not announced") + } + + for _, tc := range []struct { + name string + resource string + wantError string + }{ + {name: "success", wantError: ""}, + {name: "error", resource: "https://unregistered.example.com/mcp", wantError: konnectoidc.ErrorCodeOAuth2InvalidTarget}, + } { + t.Run(tc.name, func(t *testing.T) { + query := url.Values{ + "response_type": []string{oidc.ResponseTypeCode}, + "client_id": []string{"unittestclient"}, + "redirect_uri": []string{"http://localhost:33418/callback"}, + "scope": []string{oidc.ScopeOpenID}, + "state": []string{"unitteststate"}, + } + if tc.resource != "" { + query.Set("resource", tc.resource) + } + req, err := http.NewRequest("GET", config.AuthorizationPath+"?"+query.Encode(), http.NoBody) + if err != nil { + t.Fatal(err) + } + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + if rr.Code != http.StatusFound { + t.Fatalf("authorize returned wrong status code: got %v want %v, body %s", rr.Code, http.StatusFound, rr.Body.String()) + } + location, err := url.Parse(rr.Header().Get("Location")) + if err != nil { + t.Fatal(err) + } + values := location.Query() + + if errorID := values.Get("error"); errorID != tc.wantError { + t.Fatalf("error was incorrect, got %q, want %q", errorID, tc.wantError) + } + if iss := values.Get("iss"); iss != metadata.Issuer { + t.Errorf("iss was incorrect, got %q, want the discovery issuer %q", iss, metadata.Issuer) + } + }) + } +} + +// getTestProviderMetadata returns the discovery document served at the +// provided path. +func getTestProviderMetadata(t *testing.T, router http.Handler, path string) *konnectoidc.ProviderMetadata { + t.Helper() + + req, err := http.NewRequest("GET", path, http.NoBody) + if err != nil { + t.Fatal(err) + } + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + metadata := &konnectoidc.ProviderMetadata{} + if err := json.Unmarshal(rr.Body.Bytes(), metadata); err != nil { + t.Fatal(err) + } + + return metadata +} + // TestTokenRequiresPKCE covers that a client registration which demands PKCE // cannot exchange a code which was not bound to a code challenge, and can when // it was. diff --git a/oidc/provider/provider.go b/oidc/provider/provider.go index 08595480..d1445882 100644 --- a/oidc/provider/provider.go +++ b/oidc/provider/provider.go @@ -483,6 +483,11 @@ func (p *Provider) InitializeMetadata() error { CodeChallengeMethodsSupported: []string{ oidc.S256CodeChallengeMethod, }, + // Every authorization response redirected to a client carries iss, + // successes from AuthorizeResponse and errors from the constructors of + // payload.AuthenticationRequest. Clients reject a response without it + // once this is set, see RFC 9207 section 2.4. + AuthorizationResponseIssParameterSupported: true, } return nil