diff --git a/README.md b/README.md index 10e646e..85bb2c6 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 3a32fa6..bf9232e 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 c501adb..821a56f 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 6b413b4..3f86523 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 3ced580..fb406f9 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 0859548..d144588 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