diff --git a/apps/docs/content/apis/openidoauth/endpoints.mdx b/apps/docs/content/apis/openidoauth/endpoints.mdx index bf9be6fbd..10dc4b8be 100644 --- a/apps/docs/content/apis/openidoauth/endpoints.mdx +++ b/apps/docs/content/apis/openidoauth/endpoints.mdx @@ -686,7 +686,12 @@ curl --request POST \ }' ``` -See the [Dynamic Client Registration guide](/guides/integrate/dynamic-client-registration) for the supported metadata, the available registration modes and the current limitations. +A successful registration also returns a `registration_access_token` and a `registration_client_uri` +(`${CUSTOM_DOMAIN}/oauth/v2/register/{client_id}`). They let the client read, update and delete its own registration +through [OAuth 2.0 Dynamic Client Registration Management (RFC 7592)](https://datatracker.ietf.org/doc/html/rfc7592), +served whenever dynamic client registration is enabled in the instance's security settings. + +See the [Dynamic Client Registration guide](/guides/integrate/dynamic-client-registration) for the supported metadata, the available registration modes, managing a registration and the current limitations. ## OAuth 2.0 metadata diff --git a/apps/docs/content/guides/integrate/dynamic-client-registration.mdx b/apps/docs/content/guides/integrate/dynamic-client-registration.mdx index 5a2be8e20..d7af9b6bb 100644 --- a/apps/docs/content/guides/integrate/dynamic-client-registration.mdx +++ b/apps/docs/content/guides/integrate/dynamic-client-registration.mdx @@ -11,6 +11,10 @@ themselves as OIDC applications at runtime, instead of being created upfront in This is required by clients that self-register before any user context exists, most notably [Model Context Protocol (MCP)](https://modelcontextprotocol.io) clients such as Claude Desktop, claude.ai and the MCP SDKs. +A registered client can later read, update and delete its own registration through the +[OAuth 2.0 Dynamic Client Registration Management Protocol (RFC 7592)](https://datatracker.ietf.org/doc/html/rfc7592), +using the registration access token it receives at registration. See [Manage a registration](#manage-a-registration). + Dynamic client registration is disabled by default. Only enable it if you understand the trade-offs described in [Registration modes](#registration-modes) below. @@ -82,6 +86,8 @@ A successful registration returns `HTTP 201` with the client information respons { "client_id": "340396026519785524", "client_id_issued_at": 1718000000, + "registration_access_token": "kFqx...redacted...", + "registration_client_uri": "https://${CUSTOM_DOMAIN}/oauth/v2/register/340396026519785524", "client_name": "My MCP Client", "redirect_uris": ["https://client.example.com/callback"], "grant_types": ["authorization_code", "refresh_token"], @@ -97,6 +103,10 @@ The returned `client_id` can be used immediately to start an [authorization code flow](/guides/integrate/login/oidc/login-users). Public clients (`token_endpoint_auth_method` of `none`) must use [PKCE](https://datatracker.ietf.org/doc/html/rfc7636). +The `registration_access_token` and `registration_client_uri` let the client manage its own registration afterwards, see +[Manage a registration](#manage-a-registration). Store the registration access token securely: it is only returned at +registration and when the registration is updated, and it is never stored in clear text on the server. + ## Supported metadata | Member | Notes | @@ -114,15 +124,91 @@ for example `invalid_redirect_uri` or `invalid_client_metadata`, with HTTP statu [RFC 6750 §3.1](https://datatracker.ietf.org/doc/html/rfc6750#section-3.1) codes `invalid_token` (`401`) and `insufficient_scope` (`403`). +## Manage a registration + +ZITADEL implements the [OAuth 2.0 Dynamic Client Registration Management Protocol (RFC 7592)](https://datatracker.ietf.org/doc/html/rfc7592). +A registered client manages itself at the client configuration endpoint returned as `registration_client_uri` +(`${CUSTOM_DOMAIN}/oauth/v2/register/{client_id}`), authenticating with the `registration_access_token` from the +registration response. The endpoints are served whenever `dynamicClientRegistration.enabled` is set, the same switch as +the registration endpoint itself: a client that may register may manage what it registered. `allowUnauthenticated` +selects the [registration mode](#registration-modes) and does not apply here, as the registration access token +authorizes these endpoints in both modes. + +The registration access token is bound to a single client. Send it as an `Authorization: Bearer` header. A missing or +invalid token is rejected with `401 Unauthorized`; once a client has been deleted its endpoint returns `404 Not Found`. + +The registration access token is the only authorization path: a client manages itself and needs no user permission, so +neither `project.app.register_dynamic` (which governs who may *create* clients) nor `project.app.write` applies. As +operators cannot obtain the token, they manage dynamically registered clients through the Management API and the +Console like any other application, not through these endpoints. + +### Read + +```bash +curl --request GET \ + --url ${CUSTOM_DOMAIN}/oauth/v2/register/340396026519785524 \ + --header 'Authorization: Bearer ' +``` + +Returns `HTTP 200` with the current client information response. The `client_secret` is never returned, as it is not +stored in clear text. + +### Update + +An update replaces the stored metadata, it does not merge into it. [RFC 7592 §2.2](https://datatracker.ietf.org/doc/html/rfc7592#section-2.2) +therefore requires the client to include **all** metadata fields it received from the registration, read or update +response, not only the ones it wants to change. Send the full set, with the new values in place: + +```bash +curl --request PUT \ + --url ${CUSTOM_DOMAIN}/oauth/v2/register/340396026519785524 \ + --header 'Authorization: Bearer ' \ + --header 'Content-Type: application/json' \ + --data '{ + "client_id": "340396026519785524", + "redirect_uris": ["https://client.example.com/callback", "https://client.example.com/callback2"], + "grant_types": ["authorization_code", "refresh_token"], + "response_types": ["code"], + "token_endpoint_auth_method": "none" + }' +``` + +Omitted members are not preserved: they are re-defaulted exactly as during registration, using the same validation and +defaulting rules. Dropping `grant_types` from the body above would reset the client to the default +`["authorization_code"]` and silently lose `refresh_token`, and dropping `token_endpoint_auth_method` would re-derive it +from the application type. Read the registration back first if the client does not keep its metadata. + +Returns `HTTP 200` with the updated registration. An update rotates the registration access token: the response contains +a new `registration_access_token`, so the client must replace its stored token with the new value. Every update rotates +the token, including an update that does not change any metadata. The new token works immediately; the previous token is +invalidated through the eventually consistent projection, so it may keep working for a brief moment after the update. + +### Delete + +```bash +curl --request DELETE \ + --url ${CUSTOM_DOMAIN}/oauth/v2/register/340396026519785524 \ + --header 'Authorization: Bearer ' +``` + +Returns `HTTP 204 No Content` and removes the client. + ## Limitations -This first version intentionally keeps a small scope: +This version intentionally keeps a small scope: - Free-form metadata such as `logo_uri`, `client_uri` or `contacts` is accepted but not persisted. - The `private_key_jwt` authentication method and the `jwks`/`jwks_uri` members are rejected with `invalid_client_metadata`. -- Reading, updating and deleting a registration ([RFC 7592](https://datatracker.ietf.org/doc/html/rfc7592)) is not yet supported. - Dynamically registered clients share the audience of the `ZITADEL DCR` project. A JWT access token issued to one of them carries every client ID registered in that project, plus the project ID, in its `aud` claim. The `resource` parameter ([RFC 8707](https://datatracker.ietf.org/doc/html/rfc8707)) is accepted on the authorization code flow but ignored, so it does not narrow `aud`. Do not rely on `aud` alone to tell one dynamically registered client from another; validate the `client_id` or `azp` claim instead. +- A registration access token is invalidated by rotating it (through an update) or by deleting the client; there is no + separate per-token revocation endpoint. +- The read and update responses return the current metadata but not `client_name` or `client_id_issued_at` (not + persisted in this version), and never return `client_secret` (only its hash is stored). +- Changing `token_endpoint_auth_method` to a confidential method (`client_secret_basic` or `client_secret_post`) through + an update does not issue a `client_secret`; register a new confidential client instead. +- A client whose application, project or organization is deactivated (not deleted) returns `404` from the management + endpoints until it is reactivated. diff --git a/cmd/setup/74.go b/cmd/setup/74.go new file mode 100644 index 000000000..77aed31b7 --- /dev/null +++ b/cmd/setup/74.go @@ -0,0 +1,27 @@ +package setup + +import ( + "context" + _ "embed" + + "github.com/zitadel/zitadel/internal/database" + "github.com/zitadel/zitadel/internal/eventstore" +) + +var ( + //go:embed 74.sql + addOIDCConfigRegistrationToken string +) + +type Apps7OIDCConfigsAddRegistrationToken struct { + dbClient *database.DB +} + +func (mig *Apps7OIDCConfigsAddRegistrationToken) Execute(ctx context.Context, _ eventstore.Event) error { + _, err := mig.dbClient.ExecContext(ctx, addOIDCConfigRegistrationToken) + return err +} + +func (mig *Apps7OIDCConfigsAddRegistrationToken) String() string { + return "74_apps7_oidc_configs_add_registration_token" +} diff --git a/cmd/setup/74.sql b/cmd/setup/74.sql new file mode 100644 index 000000000..5f31374ca --- /dev/null +++ b/cmd/setup/74.sql @@ -0,0 +1,2 @@ +ALTER TABLE IF EXISTS projections.apps7_oidc_configs +ADD COLUMN IF NOT EXISTS registration_token text; diff --git a/cmd/setup/config.go b/cmd/setup/config.go index cd8f117c5..c8d696864 100644 --- a/cmd/setup/config.go +++ b/cmd/setup/config.go @@ -193,6 +193,7 @@ type Steps struct { s71JWTProvideAddAudienceColumn *JWTProvideAddAudienceColumn s72AddColumnsToLoginNamesView *AddColumnsToLoginNamesView s73FixUserGrantRoles *FixUserGrantRoles + s74Apps7OIDCConfigsAddRegistrationToken *Apps7OIDCConfigsAddRegistrationToken RelationalTables *TransactionalTables } diff --git a/cmd/setup/setup.go b/cmd/setup/setup.go index d2831a581..cebf085b3 100644 --- a/cmd/setup/setup.go +++ b/cmd/setup/setup.go @@ -256,6 +256,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string) steps.s71JWTProvideAddAudienceColumn = &JWTProvideAddAudienceColumn{dbClient: dbClient} steps.s72AddColumnsToLoginNamesView = &AddColumnsToLoginNamesView{dbClient: dbClient} steps.s73FixUserGrantRoles = &FixUserGrantRoles{eventstore: eventstoreClient} + steps.s74Apps7OIDCConfigsAddRegistrationToken = &Apps7OIDCConfigsAddRegistrationToken{dbClient: dbClient} err = projection.Create(ctx, dbClient, eventstoreClient, config.Projections, nil, nil, nil) if err != nil { @@ -388,6 +389,7 @@ func Setup(ctx context.Context, config *Config, steps *Steps, masterKey string) steps.s71JWTProvideAddAudienceColumn, steps.s72AddColumnsToLoginNamesView, steps.s73FixUserGrantRoles, + steps.s74Apps7OIDCConfigsAddRegistrationToken, } { setupErr = executeMigration(ctx, eventstoreClient, step, "migration failed") if setupErr != nil { diff --git a/internal/api/oidc/client_registration.go b/internal/api/oidc/client_registration.go index d6f87428e..dac3988e1 100644 --- a/internal/api/oidc/client_registration.go +++ b/internal/api/oidc/client_registration.go @@ -78,7 +78,14 @@ func (s *Server) dynamicClientRegistration(w http.ResponseWriter, r *http.Reques return } - s.writeRegistrationJSON(ctx, w, http.StatusCreated, newClientRegistrationResponse(registered, req.ClientName, time.Now().Unix())) + resp := newClientRegistrationResponse(registered, req.ClientName, time.Now().Unix()) + resp.RegistrationAccessToken, err = s.registrationAccessToken(registered.ClientID, resourceOwner, registered.RegistrationAccessToken) + if err != nil { + s.writeRegistrationServerError(ctx, w, err) + return + } + resp.RegistrationClientURI = s.registrationClientURI(ctx, registered.ClientID) + s.writeRegistrationJSON(ctx, w, http.StatusCreated, resp) } // dynamicClientRegistrationResourceOwner authorizes the registration and returns the @@ -186,9 +193,12 @@ func (s *Server) writeRegistrationError(ctx context.Context, w http.ResponseWrit s.writeRegistrationJSON(ctx, w, http.StatusBadRequest, regErr) } +// writeRegistrationUnauthorized answers a 401 for both the registration endpoint (a missing +// or invalid initial access token) and the management endpoints (a missing or invalid +// registration access token), so the description is kept token-neutral. func (s *Server) writeRegistrationUnauthorized(ctx context.Context, w http.ResponseWriter) { w.Header().Set("WWW-Authenticate", `Bearer error="invalid_token"`) - s.writeRegistrationJSON(ctx, w, http.StatusUnauthorized, newRegistrationError("invalid_token", "a valid access token is required to register a client")) + s.writeRegistrationJSON(ctx, w, http.StatusUnauthorized, newRegistrationError("invalid_token", "a valid bearer token is required")) } func (s *Server) writeRegistrationForbidden(ctx context.Context, w http.ResponseWriter) { @@ -196,9 +206,12 @@ func (s *Server) writeRegistrationForbidden(ctx context.Context, w http.Response s.writeRegistrationJSON(ctx, w, http.StatusForbidden, newRegistrationError("insufficient_scope", "the access token is not allowed to register clients")) } +// writeRegistrationServerError answers a 500 for both the registration endpoint and the +// management endpoints, so like the 401 above the description is kept operation-neutral. The +// cause is logged rather than returned, as it is not the client's to act on. func (s *Server) writeRegistrationServerError(ctx context.Context, w http.ResponseWriter, err error) { s.getLogger(ctx).ErrorContext(ctx, "dynamic client registration", "err", err) - s.writeRegistrationJSON(ctx, w, http.StatusInternalServerError, newRegistrationError("server_error", "the client could not be registered")) + s.writeRegistrationJSON(ctx, w, http.StatusInternalServerError, newRegistrationError("server_error", "the request could not be processed")) } func (s *Server) writeRegistrationCommandError(ctx context.Context, w http.ResponseWriter, err error) { @@ -253,13 +266,19 @@ type clientRegistrationRequest struct { } // clientRegistrationResponse is the client information response of a successful -// registration (RFC 7591 §3.2.1). +// registration (RFC 7591 §3.2.1) and of the RFC 7592 read and update operations. type clientRegistrationResponse struct { ClientID string `json:"client_id"` ClientSecret string `json:"client_secret,omitempty"` ClientIDIssuedAt int64 `json:"client_id_issued_at,omitempty"` ClientSecretExpiresAt *int64 `json:"client_secret_expires_at,omitempty"` + // RegistrationAccessToken and RegistrationClientURI let the client manage its own + // registration (RFC 7592 §3). The token is only returned when it is (re)issued, i.e. on + // registration and on update; a read does not rotate it and therefore omits it. + RegistrationAccessToken string `json:"registration_access_token,omitempty"` + RegistrationClientURI string `json:"registration_client_uri,omitempty"` + RedirectURIs []string `json:"redirect_uris,omitempty"` ResponseTypes []string `json:"response_types,omitempty"` GrantTypes []string `json:"grant_types,omitempty"` diff --git a/internal/api/oidc/client_registration_management.go b/internal/api/oidc/client_registration_management.go new file mode 100644 index 000000000..ad64e470a --- /dev/null +++ b/internal/api/oidc/client_registration_management.go @@ -0,0 +1,235 @@ +package oidc + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "strings" + + "github.com/go-chi/chi/v5" + "github.com/zitadel/oidc/v3/pkg/op" + + "github.com/zitadel/zitadel/internal/api/authz" + "github.com/zitadel/zitadel/internal/query" + "github.com/zitadel/zitadel/internal/zerrors" +) + +// getDynamicClientRegistration handles GET requests to the client configuration endpoint +// (RFC 7592 §2.1) and returns the current registration of the client. The read does not +// rotate the registration access token, so the response omits it; the client keeps the token +// it authenticated with. +func (s *Server) getDynamicClientRegistration(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + client, _, ok := s.authorizeClientManagement(w, r) + if !ok { + return + } + + resp := clientRegistrationResponseFromClient(client) + resp.RegistrationClientURI = s.registrationClientURI(ctx, client.ClientID) + s.writeRegistrationJSON(ctx, w, http.StatusOK, resp) +} + +// updateDynamicClientRegistration handles PUT requests to the client configuration endpoint +// (RFC 7592 §2.2). It replaces the client metadata with the submitted values and rotates the +// registration access token, returning the updated registration with the new token. +func (s *Server) updateDynamicClientRegistration(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + client, binding, ok := s.authorizeClientManagement(w, r) + if !ok { + return + } + + var req clientRegistrationRequest + if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, registrationMaxBodyBytes)).Decode(&req); err != nil { + var maxBytesErr *http.MaxBytesError + if errors.As(err, &maxBytesErr) { + s.writeRegistrationJSON(ctx, w, http.StatusRequestEntityTooLarge, newRegistrationError(registrationErrorInvalidClientMetadata, "the request body is too large")) + return + } + s.writeRegistrationError(ctx, w, newRegistrationError(registrationErrorInvalidClientMetadata, "the request body could not be parsed")) + return + } + + app, regErr := req.toOIDCApp() + if regErr != nil { + s.writeRegistrationError(ctx, w, regErr) + return + } + app.AggregateID = client.ProjectID + app.AppID = client.AppID + + updated, err := s.command.UpdateDynamicOIDCClient(ctx, app, binding.orgID) + if err != nil { + s.writeManagementCommandError(ctx, w, err) + return + } + + // The update response is a client information response (RFC 7592 §3); client_id_issued_at + // reflects the original registration, which is not echoed back here. + resp := newClientRegistrationResponse(updated, req.ClientName, 0) + resp.RegistrationAccessToken, err = s.registrationAccessToken(client.ClientID, binding.orgID, updated.RegistrationAccessToken) + if err != nil { + s.writeRegistrationServerError(ctx, w, err) + return + } + resp.RegistrationClientURI = s.registrationClientURI(ctx, client.ClientID) + s.writeRegistrationJSON(ctx, w, http.StatusOK, resp) +} + +// deleteDynamicClientRegistration handles DELETE requests to the client configuration +// endpoint (RFC 7592 §2.3). It removes the client and answers with 204 No Content. +func (s *Server) deleteDynamicClientRegistration(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + client, binding, ok := s.authorizeClientManagement(w, r) + if !ok { + return + } + + if _, err := s.command.RemoveDynamicOIDCClient(ctx, client.ProjectID, client.AppID, binding.orgID); err != nil { + s.writeManagementCommandError(ctx, w, err) + return + } + + w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Pragma", "no-cache") + w.WriteHeader(http.StatusNoContent) +} + +// registrationAccessTokenBinding holds the values recovered from a registration access token. +type registrationAccessTokenBinding struct { + clientID string + orgID string + secret string +} + +// authorizeClientManagement performs the authorization shared by the RFC 7592 management +// endpoints. It requires dynamic client registration to be enabled in the instance's security +// settings, a registration access token that decrypts and is bound to the client_id in the +// path, an existing client, and a secret that matches the stored token hash. On any failure it +// writes the appropriate response (404 when registration is disabled or the client does not +// exist, 401 for token problems, 500 for an unexpected failure) and returns ok=false. +// +// The registration access token is the only authorization path here (RFC 7592 §3): it is +// issued at registration and bound to a single client, so a client manages only itself and +// needs no user permission. In particular neither project.app.register_dynamic, which gates +// who may create clients, nor project.app.write applies. Operators manage dynamically +// registered clients through the Management API and Console as they do any other application. +func (s *Server) authorizeClientManagement(w http.ResponseWriter, r *http.Request) (*query.OIDCClient, *registrationAccessTokenBinding, bool) { + ctx := r.Context() + if !authz.GetInstance(ctx).EnableDynamicClientRegistration() { + http.NotFound(w, r) + return nil, nil, false + } + + clientID := chi.URLParam(r, "client_id") + token := bearerToken(r) + if token == "" { + s.writeRegistrationUnauthorized(ctx, w) + return nil, nil, false + } + binding, err := s.parseRegistrationAccessToken(token) + if err != nil || binding.clientID != clientID { + s.writeRegistrationUnauthorized(ctx, w) + return nil, nil, false + } + + client, err := s.query.ActiveOIDCClientByID(ctx, clientID, false) + if err != nil { + // Only a genuinely absent client is a 404. An unexpected failure (a database + // problem, say) must not be reported as "this client does not exist", which would + // hide the incident from both the client and whoever is on call. + if !zerrors.IsNotFound(err) { + s.writeRegistrationServerError(ctx, w, err) + return nil, nil, false + } + s.writeRegistrationNotFound(ctx, w) + return nil, nil, false + } + if err = s.verifyRegistrationAccessToken(ctx, client, binding.orgID, binding.secret); err != nil { + // Same reasoning as above: only a token that does not match is a 401. + if !zerrors.IsUnauthenticated(err) { + s.writeRegistrationServerError(ctx, w, err) + return nil, nil, false + } + s.writeRegistrationUnauthorized(ctx, w) + return nil, nil, false + } + return client, binding, true +} + +// verifyRegistrationAccessToken checks the presented registration access token secret against +// the client's stored token hash. The hash is served from the projection for an O(1) read on +// the common path; on a miss (most importantly a token that was just rotated and is not +// projected yet) it falls back to a strongly consistent read from the eventstore, so a rotated +// token is accepted immediately. +// +// A token that does not match is reported as unauthenticated; any other error is passed +// through unchanged so the caller can tell a rejected token from a failed lookup. +func (s *Server) verifyRegistrationAccessToken(ctx context.Context, client *query.OIDCClient, orgID, secret string) error { + if client.RegistrationTokenHash != "" { + if _, err := s.hasher.Verify(client.RegistrationTokenHash, secret); err == nil { + return nil + } + } + return s.command.VerifyDynamicClientRegistrationToken(ctx, client.ProjectID, client.AppID, orgID, secret) +} + +// registrationAccessToken builds the opaque registration access token (RFC 7592 §3) handed to +// a client on registration and rotation. It binds the plain secret to the client and its +// organization through authenticated encryption with the instance key (the same mechanism as +// refresh tokens), so the management endpoints can recover the binding before checking the +// secret against the stored hash. +func (s *Server) registrationAccessToken(clientID, orgID, secret string) (string, error) { + return s.encAlg.EncryptToken(strings.Join([]string{clientID, orgID, secret}, ":")) +} + +// parseRegistrationAccessToken decrypts a registration access token back into its binding. +func (s *Server) parseRegistrationAccessToken(token string) (*registrationAccessTokenBinding, error) { + plain, err := s.encAlg.DecryptToken(token) + if err != nil { + return nil, err + } + parts := strings.SplitN(plain, ":", 3) + if len(parts) != 3 { + return nil, ErrInvalidTokenFormat + } + return ®istrationAccessTokenBinding{clientID: parts[0], orgID: parts[1], secret: parts[2]}, nil +} + +// registrationClientURI builds the RFC 7592 §3 client configuration endpoint URL of a client. +func (s *Server) registrationClientURI(ctx context.Context, clientID string) string { + return op.IssuerFromContext(ctx) + s.registrationEndpoint.Relative() + "/" + clientID +} + +func (s *Server) writeRegistrationNotFound(ctx context.Context, w http.ResponseWriter) { + s.writeRegistrationJSON(ctx, w, http.StatusNotFound, newRegistrationError("invalid_client", "the client does not exist or the registration access token is no longer valid")) +} + +// writeManagementCommandError maps a command error from the update and delete endpoints. A +// client that no longer exists (a concurrent or retried delete, or a delete racing an update) +// is reported as 404, consistent with a read of a missing client; other errors fall back to +// the shared registration error mapping. +func (s *Server) writeManagementCommandError(ctx context.Context, w http.ResponseWriter, err error) { + if zerrors.IsNotFound(err) { + s.writeRegistrationNotFound(ctx, w) + return + } + s.writeRegistrationCommandError(ctx, w, err) +} + +// clientRegistrationResponseFromClient maps a persisted OIDC client to an RFC 7592 read +// response. The client secret is not returned because only its hash is stored; the original +// client name and issuance time are likewise not persisted in this version. +func clientRegistrationResponseFromClient(client *query.OIDCClient) *clientRegistrationResponse { + return &clientRegistrationResponse{ + ClientID: client.ClientID, + RedirectURIs: client.RedirectURIs, + ResponseTypes: responseTypesToRegistration(client.ResponseTypes), + GrantTypes: grantTypesToRegistration(client.GrantTypes), + ApplicationType: applicationTypeToRegistration(client.ApplicationType), + TokenEndpointAuthMethod: authMethodToRegistration(client.AuthMethodType), + PostLogoutRedirectURIs: client.PostLogoutRedirectURIs, + } +} diff --git a/internal/api/oidc/client_registration_management_test.go b/internal/api/oidc/client_registration_management_test.go new file mode 100644 index 000000000..3e85ffc8c --- /dev/null +++ b/internal/api/oidc/client_registration_management_test.go @@ -0,0 +1,98 @@ +package oidc + +import ( + "errors" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zitadel/zitadel/internal/domain" + "github.com/zitadel/zitadel/internal/query" +) + +// fakeRegistrationTokenAlg is a reversible stand-in for the instance AuthAlgorithm. It only +// has to round-trip a string so the binding logic can be exercised without real keys. +type fakeRegistrationTokenAlg struct{} + +func (fakeRegistrationTokenAlg) EncryptToken(data string) (string, error) { + return "enc(" + data + ")", nil +} + +func (fakeRegistrationTokenAlg) DecryptToken(token string) (string, error) { + if !strings.HasPrefix(token, "enc(") || !strings.HasSuffix(token, ")") { + return "", errors.New("not a registration access token") + } + return strings.TrimSuffix(strings.TrimPrefix(token, "enc("), ")"), nil +} + +func (fakeRegistrationTokenAlg) LegacyTokenEnabled() bool { return false } + +func Test_registrationAccessToken_roundTrip(t *testing.T) { + s := &Server{encAlg: fakeRegistrationTokenAlg{}} + + tests := []struct { + name string + clientID string + orgID string + secret string + }{ + {"plain", "client1", "org1", "topsecret"}, + {"secret with colon is preserved", "client1", "org1", "top:sec:ret"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + token, err := s.registrationAccessToken(tt.clientID, tt.orgID, tt.secret) + require.NoError(t, err) + + binding, err := s.parseRegistrationAccessToken(token) + require.NoError(t, err) + assert.Equal(t, tt.clientID, binding.clientID) + assert.Equal(t, tt.orgID, binding.orgID) + assert.Equal(t, tt.secret, binding.secret) + }) + } +} + +func Test_parseRegistrationAccessToken_errors(t *testing.T) { + s := &Server{encAlg: fakeRegistrationTokenAlg{}} + + t.Run("undecryptable token errors", func(t *testing.T) { + _, err := s.parseRegistrationAccessToken("not-encrypted") + assert.Error(t, err) + }) + + t.Run("wrong number of parts is an invalid token format", func(t *testing.T) { + token, err := fakeRegistrationTokenAlg{}.EncryptToken("client1:org1") + require.NoError(t, err) + _, err = s.parseRegistrationAccessToken(token) + assert.ErrorIs(t, err, ErrInvalidTokenFormat) + }) +} + +func Test_clientRegistrationResponseFromClient(t *testing.T) { + client := &query.OIDCClient{ + ClientID: "client1", + RedirectURIs: []string{"https://client.example.com/callback"}, + ResponseTypes: []domain.OIDCResponseType{domain.OIDCResponseTypeCode}, + GrantTypes: []domain.OIDCGrantType{domain.OIDCGrantTypeAuthorizationCode}, + ApplicationType: domain.OIDCApplicationTypeWeb, + AuthMethodType: domain.OIDCAuthMethodTypeNone, + PostLogoutRedirectURIs: []string{"https://client.example.com/logout"}, + HashedSecret: "$plain$$secret", + } + + resp := clientRegistrationResponseFromClient(client) + + assert.Equal(t, "client1", resp.ClientID) + assert.Equal(t, []string{"https://client.example.com/callback"}, resp.RedirectURIs) + assert.Equal(t, []string{"code"}, resp.ResponseTypes) + assert.Equal(t, []string{"authorization_code"}, resp.GrantTypes) + assert.Equal(t, "web", resp.ApplicationType) + assert.Equal(t, "none", resp.TokenEndpointAuthMethod) + assert.Equal(t, []string{"https://client.example.com/logout"}, resp.PostLogoutRedirectURIs) + // The stored secret hash must never be returned, and a read does not (re)issue a token. + assert.Empty(t, resp.ClientSecret) + assert.Empty(t, resp.RegistrationAccessToken) +} diff --git a/internal/api/oidc/integration_test/client_registration_management_test.go b/internal/api/oidc/integration_test/client_registration_management_test.go new file mode 100644 index 000000000..a903962c2 --- /dev/null +++ b/internal/api/oidc/integration_test/client_registration_management_test.go @@ -0,0 +1,168 @@ +//go:build integration + +package oidc_test + +import ( + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zitadel/zitadel/internal/integration" +) + +// TestServer_DynamicClientRegistration_Management covers the OAuth 2.0 Dynamic Client +// Registration Management Protocol (RFC 7592): read, update and delete of a dynamically +// registered client, authenticated with the registration access token issued at registration. +// +// The registration access token is the only credential involved: none of the calls below +// carries a user access token, so the test also pins down that the management endpoints +// require no permission of their own. +func TestServer_DynamicClientRegistration_Management(t *testing.T) { + // Open registration, so the client below can register itself without a token; the + // management endpoints are gated on Enabled alone. + enableDynamicClientRegistration(t, CTXIAM, Instance, true) + + issuer := Instance.OIDCIssuer() + + status, reg := registerDynamicClient(t, issuer, "", map[string]any{ + "client_name": "lifecycle client", + "redirect_uris": []string{redirectURI}, + "token_endpoint_auth_method": "none", + }) + require.Equal(t, http.StatusCreated, status) + clientID, _ := reg["client_id"].(string) + require.NotEmpty(t, clientID) + token, _ := reg["registration_access_token"].(string) + require.NotEmpty(t, token) + clientURI, _ := reg["registration_client_uri"].(string) + require.Equal(t, issuer+"/oauth/v2/register/"+clientID, clientURI) + + // The client and its dedicated project were just created, so wait for the projection to + // catch up before the first read. + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) + require.EventuallyWithT(t, func(tt *assert.CollectT) { + getStatus, body := manageDynamicClient(t, http.MethodGet, clientURI, token, nil) + assert.Equal(tt, http.StatusOK, getStatus) + assert.Equal(tt, clientID, body["client_id"]) + assert.Equal(tt, []any{redirectURI}, body["redirect_uris"]) + // A read does not rotate the token, so it is not returned again. + assert.Empty(tt, body["registration_access_token"]) + }, retryDuration, tick, "registered client not readable") + + t.Run("missing or wrong registration access token is unauthorized", func(t *testing.T) { + noToken, _ := manageDynamicClient(t, http.MethodGet, clientURI, "", nil) + assert.Equal(t, http.StatusUnauthorized, noToken) + + wrongToken, _ := manageDynamicClient(t, http.MethodGet, clientURI, token+"tampered", nil) + assert.Equal(t, http.StatusUnauthorized, wrongToken) + }) + + var rotatedToken string + t.Run("update replaces metadata and rotates the token", func(t *testing.T) { + putStatus, body := manageDynamicClient(t, http.MethodPut, clientURI, token, map[string]any{ + "redirect_uris": []string{redirectURI + "/updated"}, + "token_endpoint_auth_method": "none", + }) + require.Equal(t, http.StatusOK, putStatus) + assert.Equal(t, clientID, body["client_id"]) + assert.Equal(t, []any{redirectURI + "/updated"}, body["redirect_uris"]) + rotatedToken, _ = body["registration_access_token"].(string) + require.NotEmpty(t, rotatedToken) + assert.NotEqual(t, token, rotatedToken) + }) + + t.Run("the previous token is eventually rejected after rotation", func(t *testing.T) { + // Rotation invalidates the previous token, but verification is served from the + // eventually consistent projection (the new token is accepted immediately through a + // strongly consistent fallback). The previous token therefore stops working once the + // rotation has been projected. + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) + require.EventuallyWithT(t, func(tt *assert.CollectT) { + oldStatus, _ := manageDynamicClient(t, http.MethodGet, clientURI, token, nil) + assert.Equal(tt, http.StatusUnauthorized, oldStatus) + }, retryDuration, tick, "previous token still accepted after rotation") + }) + + t.Run("read reflects the update with the rotated token", func(t *testing.T) { + // The update is read back from the projection, which is eventually consistent, so + // retry until the changed redirect_uris show up. + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) + require.EventuallyWithT(t, func(tt *assert.CollectT) { + getStatus, body := manageDynamicClient(t, http.MethodGet, clientURI, rotatedToken, nil) + assert.Equal(tt, http.StatusOK, getStatus) + assert.Equal(tt, []any{redirectURI + "/updated"}, body["redirect_uris"]) + }, retryDuration, tick, "update not reflected in read") + }) + + t.Run("a token bound to another client is rejected", func(t *testing.T) { + _, other := registerDynamicClient(t, issuer, "", map[string]any{ + "redirect_uris": []string{redirectURI}, + "token_endpoint_auth_method": "none", + }) + otherURI, _ := other["registration_client_uri"].(string) + require.NotEmpty(t, otherURI) + // Using the first client's token against another client's endpoint must be rejected + // before any lookup: the token's client_id does not match the path. + status, _ := manageDynamicClient(t, http.MethodGet, otherURI, rotatedToken, nil) + assert.Equal(t, http.StatusUnauthorized, status) + }) + + t.Run("delete removes the client", func(t *testing.T) { + delStatus, _ := manageDynamicClient(t, http.MethodDelete, clientURI, rotatedToken, nil) + require.Equal(t, http.StatusNoContent, delStatus) + + // After deletion the client is gone; wait for the projection to catch up. + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) + require.EventuallyWithT(t, func(tt *assert.CollectT) { + getStatus, _ := manageDynamicClient(t, http.MethodGet, clientURI, rotatedToken, nil) + assert.Equal(tt, http.StatusNotFound, getStatus) + }, retryDuration, tick, "client still readable after deletion") + }) +} + +// TestServer_DynamicClientRegistration_Management_disabled verifies that the management +// endpoints are not served on an instance where dynamic client registration is disabled, +// which is the default. +func TestServer_DynamicClientRegistration_Management_disabled(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + instance := integration.NewInstance(ctx) + issuer := instance.OIDCIssuer() + + status, _ := manageDynamicClient(t, http.MethodGet, issuer+"/oauth/v2/register/whatever", "sometoken", nil) + assert.Equal(t, http.StatusNotFound, status) +} + +func manageDynamicClient(t testing.TB, method, uri, token string, body map[string]any) (int, map[string]any) { + t.Helper() + var reader io.Reader + if body != nil { + raw, err := json.Marshal(body) + require.NoError(t, err) + reader = bytes.NewReader(raw) + } + req, err := http.NewRequest(method, uri, reader) + require.NoError(t, err) + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer func() { + _, _ = io.Copy(io.Discard, resp.Body) + resp.Body.Close() + }() + var respBody map[string]any + _ = json.NewDecoder(resp.Body).Decode(&respBody) + return resp.StatusCode, respBody +} diff --git a/internal/api/oidc/integration_test/client_registration_test.go b/internal/api/oidc/integration_test/client_registration_test.go index c4f4cb3b2..a66608c5f 100644 --- a/internal/api/oidc/integration_test/client_registration_test.go +++ b/internal/api/oidc/integration_test/client_registration_test.go @@ -14,6 +14,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/zitadel/oidc/v3/pkg/oidc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/integration" @@ -193,6 +195,12 @@ func TestServer_DynamicClientRegistration_disabled(t *testing.T) { // enableDynamicClientRegistration turns the endpoint on through the instance's security // settings and waits until the change is observable, i.e. until the projection behind the // cached instance has caught up and discovery advertises the endpoint. +// +// It is safe to call more than once on the same instance. The security settings are instance +// wide and several tests here share an instance, so the requested values may already be in +// place from an earlier test. ZITADEL answers a write that changes nothing with a failed +// precondition ("Errors.NoChangesFound"), which for this helper means the desired state is +// already reached rather than a failure. func enableDynamicClientRegistration(t *testing.T, ctx context.Context, instance *integration.Instance, allowUnauthenticated bool) { t.Helper() _, err := instance.Client.SettingsV2.SetSecuritySettings(ctx, &settings.SetSecuritySettingsRequest{ @@ -201,7 +209,9 @@ func enableDynamicClientRegistration(t *testing.T, ctx context.Context, instance AllowUnauthenticated: allowUnauthenticated, }, }) - require.NoError(t, err) + if status.Code(err) != codes.FailedPrecondition { + require.NoError(t, err) + } issuer := instance.OIDCIssuer() retryDuration, tick := integration.WaitForAndTickWithMaxDuration(ctx, time.Minute) diff --git a/internal/api/oidc/op.go b/internal/api/oidc/op.go index 839450481..61440cdf2 100644 --- a/internal/api/oidc/op.go +++ b/internal/api/oidc/op.go @@ -224,6 +224,9 @@ func NewServer( op.WithSetRouter(func(r chi.Router) { r.HandleFunc(server.Endpoints().Authorization.Relative()+authCallbackPathSuffix, server.authorizeCallbackHandler) r.Method(http.MethodPost, server.registrationEndpoint.Relative(), http.HandlerFunc(server.dynamicClientRegistration)) + r.Method(http.MethodGet, server.registrationEndpoint.Relative()+"/{client_id}", http.HandlerFunc(server.getDynamicClientRegistration)) + r.Method(http.MethodPut, server.registrationEndpoint.Relative()+"/{client_id}", http.HandlerFunc(server.updateDynamicClientRegistration)) + r.Method(http.MethodDelete, server.registrationEndpoint.Relative()+"/{client_id}", http.HandlerFunc(server.deleteDynamicClientRegistration)) }), ) diff --git a/internal/command/project_application_oidc.go b/internal/command/project_application_oidc.go index 1b0f32dfd..ae1b7392e 100644 --- a/internal/command/project_application_oidc.go +++ b/internal/command/project_application_oidc.go @@ -196,9 +196,13 @@ func (c *Commands) addOIDCApplicationWithID(ctx context.Context, oidcApp *domain // pushOIDCApplication writes the application-added and oidc-config-added events for a new // OIDC application onto the project aggregate. Authorization must already have been // performed by the caller: either through an app.write permission check (see -// addOIDCApplicationWithID) or, for dynamic client registration, through the feature flag -// and registration mode (see AddDynamicOIDCClient). -func (c *Commands) pushOIDCApplication(ctx context.Context, addedApplication *OIDCApplicationWriteModel, oidcApp *domain.OIDCApp, appID string) (_ *domain.OIDCApp, err error) { +// addOIDCApplicationWithID) or, for dynamic client registration, at the registration endpoint +// through the instance's security settings and registration mode (see AddDynamicOIDCClient). +// +// extraEvents are appended to the same push, so callers can persist additional state about +// the application atomically with its creation (dynamic client registration uses this to +// store the registration access token hash). +func (c *Commands) pushOIDCApplication(ctx context.Context, addedApplication *OIDCApplicationWriteModel, oidcApp *domain.OIDCApp, appID string, extraEvents ...eventstore.Command) (_ *domain.OIDCApp, err error) { projectAgg := ProjectAggregateFromWriteModel(&addedApplication.WriteModel) oidcApp.AppID = appID @@ -249,6 +253,8 @@ func (c *Commands) pushOIDCApplication(ctx context.Context, addedApplication *OI strings.TrimSpace(gu.Value(oidcApp.LoginBaseURI)), )) + events = append(events, extraEvents...) + addedApplication.AppID = oidcApp.AppID postCommit, err := c.applicationCreatedMilestone(ctx, &events) if err != nil { @@ -305,12 +311,40 @@ func (c *Commands) UpdateOIDCApplication(ctx context.Context, oidc *domain.OIDCA return nil, err } - projectAgg := ProjectAggregateFromWriteModel(&existingOIDC.WriteModel) + changedEvent, hasChanged, err := c.oidcApplicationChangeEvent(ctx, existingOIDC, oidc) + if err != nil { + return nil, err + } + if !hasChanged { + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-1m88i", "Errors.NoChangesFound") + } + + pushedEvents, err := c.eventstore.Push(ctx, changedEvent) + if err != nil { + return nil, err + } + err = AppendAndReduce(existingOIDC, pushedEvents...) + if err != nil { + return nil, err + } + + result := oidcWriteModelToOIDCConfig(existingOIDC) + result.FillCompliance() + return result, nil +} + +// oidcApplicationChangeEvent builds the oidc-config-changed event that brings an existing +// OIDC application to the desired state. It assumes the existing write model has been loaded +// and reduced and that authorization has already been performed by the caller (an app.write +// permission check for UpdateOIDCApplication, the registration access token for +// UpdateDynamicOIDCClient). It reports whether anything actually changed. +func (c *Commands) oidcApplicationChangeEvent(ctx context.Context, existingOIDC *OIDCApplicationWriteModel, oidc *domain.OIDCApp) (*project_repo.OIDCConfigChangedEvent, bool, error) { + projectAgg := ProjectAggregateFromWriteModelWithCTX(ctx, &existingOIDC.WriteModel) var backChannelLogout, loginBaseURI *string if oidc.BackChannelLogoutURI != nil { bcl, err := c.validateBackchannelLogoutURI(oidc) if err != nil { - return nil, err + return nil, false, err } backChannelLogout = gu.Ptr(bcl) } @@ -319,7 +353,7 @@ func (c *Commands) UpdateOIDCApplication(ctx context.Context, oidc *domain.OIDCA loginBaseURI = gu.Ptr(strings.TrimSpace(*oidc.LoginBaseURI)) } - changedEvent, hasChanged, err := existingOIDC.NewChangedEvent( + return existingOIDC.NewChangedEvent( ctx, projectAgg, oidc.AppID, @@ -342,25 +376,6 @@ func (c *Commands) UpdateOIDCApplication(ctx context.Context, oidc *domain.OIDCA oidc.LoginVersion, loginBaseURI, ) - if err != nil { - return nil, err - } - if !hasChanged { - return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-1m88i", "Errors.NoChangesFound") - } - - pushedEvents, err := c.eventstore.Push(ctx, changedEvent) - if err != nil { - return nil, err - } - err = AppendAndReduce(existingOIDC, pushedEvents...) - if err != nil { - return nil, err - } - - result := oidcWriteModelToOIDCConfig(existingOIDC) - result.FillCompliance() - return result, nil } // Deprecated: use [ChangeApplicationSecret], which supports both OIDC and API applications. diff --git a/internal/command/project_application_oidc_dcr.go b/internal/command/project_application_oidc_dcr.go index 2f3059f65..7e1b6dfdd 100644 --- a/internal/command/project_application_oidc_dcr.go +++ b/internal/command/project_application_oidc_dcr.go @@ -192,7 +192,107 @@ func (c *Commands) AddDynamicOIDCClient(ctx context.Context, projectID, resource return nil, err } - return c.pushOIDCApplication(ctx, addedApplication, oidcApp, appID) + // Persist the registration access token (RFC 7592 §3) atomically with the application, + // so a registered client can always be managed and never ends up without a token. + projectAgg := ProjectAggregateFromWriteModelWithCTX(ctx, &addedApplication.WriteModel) + registrationTokenEvent, registrationToken, err := c.newOIDCRegistrationTokenEvent(ctx, projectAgg, appID) + if err != nil { + return nil, err + } + + registered, err := c.pushOIDCApplication(ctx, addedApplication, oidcApp, appID, registrationTokenEvent) + if err != nil { + return nil, err + } + registered.RegistrationAccessToken = registrationToken + return registered, nil +} + +// UpdateDynamicOIDCClient applies an RFC 7592 update to a dynamically registered client and +// rotates its registration access token in the same push. Unlike UpdateOIDCApplication it does +// not perform an app.write permission check: the caller is authorized through the registration +// access token (see VerifyDynamicClientRegistrationToken). The returned application carries the +// new registration access token, which the client must use from then on. Updating metadata to +// the values it already has is not an error: the token is rotated and the current state is +// returned, as RFC 7592 expects a successful read-back from an update. +func (c *Commands) UpdateDynamicOIDCClient(ctx context.Context, oidcApp *domain.OIDCApp, resourceOwner string) (_ *domain.OIDCApp, err error) { + ctx, span := tracing.NewSpan(ctx) + defer func() { span.EndWithError(err) }() + + if oidcApp == nil || !oidcApp.IsValid() || oidcApp.AppID == "" || oidcApp.AggregateID == "" { + return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-Aef8a", "Errors.Project.App.OIDCConfigInvalid") + } + + existingOIDC, err := c.getOIDCAppWriteModel(ctx, oidcApp.AggregateID, oidcApp.AppID, resourceOwner) + if err != nil { + return nil, err + } + if existingOIDC.State == domain.AppStateUnspecified || existingOIDC.State == domain.AppStateRemoved { + return nil, zerrors.ThrowNotFound(nil, "COMMAND-Voo8i", "Errors.Project.App.NotExisting") + } + if !existingOIDC.IsOIDC() { + return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-Geph9", "Errors.Project.App.IsNotOIDC") + } + + changedEvent, hasChanged, err := c.oidcApplicationChangeEvent(ctx, existingOIDC, oidcApp) + if err != nil { + return nil, err + } + + projectAgg := ProjectAggregateFromWriteModelWithCTX(ctx, &existingOIDC.WriteModel) + rotationEvent, registrationToken, err := c.newOIDCRegistrationTokenEvent(ctx, projectAgg, oidcApp.AppID) + if err != nil { + return nil, err + } + + events := make([]eventstore.Command, 0, 2) + if hasChanged { + events = append(events, changedEvent) + } + events = append(events, rotationEvent) + + pushedEvents, err := c.eventstore.Push(ctx, events...) + if err != nil { + return nil, err + } + if err = AppendAndReduce(existingOIDC, pushedEvents...); err != nil { + return nil, err + } + + result := oidcWriteModelToOIDCConfig(existingOIDC) + result.FillCompliance() + result.RegistrationAccessToken = registrationToken + return result, nil +} + +// RemoveDynamicOIDCClient deletes a dynamically registered client (RFC 7592 §2.3). Unlike +// RemoveApplication it does not perform an app.delete permission check: the caller is +// authorized through the registration access token. Removing the application also invalidates +// its registration access token. +func (c *Commands) RemoveDynamicOIDCClient(ctx context.Context, projectID, appID, resourceOwner string) (_ *domain.ObjectDetails, err error) { + ctx, span := tracing.NewSpan(ctx) + defer func() { span.EndWithError(err) }() + + if projectID == "" || appID == "" { + return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-Eiv0u", "Errors.IDMissing") + } + existingApp, err := c.getApplicationWriteModel(ctx, projectID, appID, resourceOwner) + if err != nil { + return nil, err + } + if existingApp.State == domain.AppStateUnspecified || existingApp.State == domain.AppStateRemoved { + return nil, zerrors.ThrowNotFound(nil, "COMMAND-Quu7n", "Errors.Project.App.NotExisting") + } + + projectAgg := ProjectAggregateFromWriteModelWithCTX(ctx, &existingApp.WriteModel) + pushedEvents, err := c.eventstore.Push(ctx, project.NewApplicationRemovedEvent(ctx, projectAgg, appID, existingApp.Name, "")) + if err != nil { + return nil, err + } + if err = AppendAndReduce(existingApp, pushedEvents...); err != nil { + return nil, err + } + return writeModelToObjectDetails(&existingApp.WriteModel), nil } // dynamicOIDCClientName builds a per-project unique application name for a dynamically @@ -205,3 +305,93 @@ func dynamicOIDCClientName(requestedName, appID string) string { } return requestedName + " (" + appID + ")" } + +// newOIDCRegistrationTokenEvent generates a fresh registration access token secret for the +// application, returning the event that persists its hash and the plain secret to hand back +// to the client. Like a client secret, only the hash is ever stored; the plain secret leaves +// the server exactly once. The caller is responsible for pushing the event. +func (c *Commands) newOIDCRegistrationTokenEvent(ctx context.Context, projectAgg *eventstore.Aggregate, appID string) (eventstore.Command, string, error) { + encodedHash, plain, err := c.newHashedSecret(ctx, c.eventstore.Filter) //nolint:staticcheck + if err != nil { + return nil, "", err + } + return project.NewOIDCConfigRegistrationTokenChangedEvent(ctx, projectAgg, appID, encodedHash), plain, nil +} + +// VerifyDynamicClientRegistrationToken checks a presented registration access token secret +// (RFC 7592 §3) against the stored hash of the application's current token. It returns an +// unauthenticated error when no token is set or the secret does not match, so the management +// endpoints can answer with 401. The hash is read strongly consistently from the eventstore; +// the management endpoints serve the common case from the projection and only fall back here, +// most importantly for a token that was just rotated and is not projected yet. +func (c *Commands) VerifyDynamicClientRegistrationToken(ctx context.Context, projectID, appID, resourceOwner, secret string) (err error) { + ctx, span := tracing.NewSpan(ctx) + defer func() { span.EndWithError(err) }() + + if projectID == "" || appID == "" || secret == "" { + return zerrors.ThrowUnauthenticated(nil, "COMMAND-Ohj6e", "Errors.Token.Invalid") + } + wm := newOIDCRegistrationTokenWriteModel(projectID, appID, resourceOwner) + if err = c.eventstore.FilterToQueryReducer(ctx, wm); err != nil { + return err + } + if wm.hashedToken == "" { + return zerrors.ThrowUnauthenticated(nil, "COMMAND-Eereu", "Errors.Token.Invalid") + } + // The registration access token is short lived (it is rotated on every update), so an + // outdated hash is not persisted back; the updated hash from Verify is intentionally + // ignored. + if _, err = c.secretHasher.Verify(wm.hashedToken, secret); err != nil { + return zerrors.ThrowUnauthenticated(err, "COMMAND-Ush2a", "Errors.Token.Invalid") + } + return nil +} + +// oidcRegistrationTokenWriteModel resolves the current registration access token hash of an +// application from the eventstore. It tracks the latest token-changed event and clears the +// hash when the application is removed. +type oidcRegistrationTokenWriteModel struct { + eventstore.WriteModel + appID string + hashedToken string +} + +func newOIDCRegistrationTokenWriteModel(projectID, appID, resourceOwner string) *oidcRegistrationTokenWriteModel { + return &oidcRegistrationTokenWriteModel{ + WriteModel: eventstore.WriteModel{ + AggregateID: projectID, + ResourceOwner: resourceOwner, + }, + appID: appID, + } +} + +func (wm *oidcRegistrationTokenWriteModel) Reduce() error { + for _, event := range wm.Events { + switch e := event.(type) { + case *project.OIDCConfigRegistrationTokenChangedEvent: + if e.AppID == wm.appID { + wm.hashedToken = e.HashedToken + } + case *project.ApplicationRemovedEvent: + if e.AppID == wm.appID { + wm.hashedToken = "" + } + } + } + return wm.WriteModel.Reduce() +} + +func (wm *oidcRegistrationTokenWriteModel) Query() *eventstore.SearchQueryBuilder { + return eventstore.NewSearchQueryBuilder(eventstore.ColumnsEvent). + AwaitOpenTransactions(). + ResourceOwner(wm.ResourceOwner). + AddQuery(). + AggregateTypes(project.AggregateType). + AggregateIDs(wm.AggregateID). + EventTypes( + project.OIDCConfigRegistrationTokenChangedType, + project.ApplicationRemovedType, + ). + Builder() +} diff --git a/internal/command/project_application_oidc_dcr_test.go b/internal/command/project_application_oidc_dcr_test.go index 9f112c73f..a84e11cd1 100644 --- a/internal/command/project_application_oidc_dcr_test.go +++ b/internal/command/project_application_oidc_dcr_test.go @@ -235,6 +235,14 @@ func TestCommandSide_AddDynamicOIDCClient(t *testing.T) { domain.LoginVersionUnspecified, "", ), + // The registration access token (RFC 7592 §3) is persisted in the same + // push as the application, so a registered client is never left + // unmanageable. + project.NewOIDCConfigRegistrationTokenChangedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, + "app1", + "secret", + ), ), ), idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "app1", "client1"), @@ -264,6 +272,7 @@ func TestCommandSide_AddDynamicOIDCClient(t *testing.T) { AppName: "app (app1)", ClientID: "client1", ClientSecretString: "secret", + RegistrationAccessToken: "secret", AuthMethodType: gu.Ptr(domain.OIDCAuthMethodTypeBasic), OIDCVersion: gu.Ptr(domain.OIDCVersionV1), RedirectUris: []string{"https://client.example.com/callback"}, @@ -328,6 +337,11 @@ func TestCommandSide_AddDynamicOIDCClient(t *testing.T) { domain.LoginVersionUnspecified, "", ), + project.NewOIDCConfigRegistrationTokenChangedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, + "app1", + "secret", + ), ), ), idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "app1", "client1"), @@ -356,6 +370,7 @@ func TestCommandSide_AddDynamicOIDCClient(t *testing.T) { AppID: "app1", AppName: "DCR Client app1", ClientID: "client1", + RegistrationAccessToken: "secret", AuthMethodType: gu.Ptr(domain.OIDCAuthMethodTypeNone), OIDCVersion: gu.Ptr(domain.OIDCVersionV1), RedirectUris: []string{"https://client.example.com/callback"}, @@ -493,3 +508,155 @@ func Test_dcrProjectWriteModel_Reduce(t *testing.T) { }) } } + +func TestCommandSide_VerifyDynamicClientRegistrationToken(t *testing.T) { + t.Parallel() + + regToken := func() *project.OIDCConfigRegistrationTokenChangedEvent { + return project.NewOIDCConfigRegistrationTokenChangedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, "app1", "$plain$$secret") + } + + tests := []struct { + name string + eventstore func(*testing.T) *eventstore.Eventstore + secret string + wantErr func(error) bool + }{ + { + name: "valid token", + eventstore: expectEventstore(expectFilter(eventFromEventPusher(regToken()))), + secret: "secret", + wantErr: nil, + }, + { + name: "wrong secret is unauthenticated", + eventstore: expectEventstore(expectFilter(eventFromEventPusher(regToken()))), + secret: "wrong", + wantErr: zerrors.IsUnauthenticated, + }, + { + name: "no token set is unauthenticated", + eventstore: expectEventstore(expectFilter()), + secret: "secret", + wantErr: zerrors.IsUnauthenticated, + }, + { + name: "empty secret is unauthenticated", + eventstore: expectEventstore(), + secret: "", + wantErr: zerrors.IsUnauthenticated, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + c := &Commands{ + eventstore: tt.eventstore(t), + secretHasher: mockPasswordHasher(""), + } + err := c.VerifyDynamicClientRegistrationToken(context.Background(), "project1", "app1", "org1", tt.secret) + if tt.wantErr == nil { + assert.NoError(t, err) + return + } + assert.True(t, tt.wantErr(err), "got wrong err: %v", err) + }) + } +} + +func TestCommandSide_UpdateDynamicOIDCClient(t *testing.T) { + t.Parallel() + + existingClient := func() []eventstore.Event { + return []eventstore.Event{ + eventFromEventPusher(project.NewApplicationAddedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, "app1", "DCR Client app1")), + eventFromEventPusher(project.NewOIDCConfigAddedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, + domain.OIDCVersionV1, "app1", "client1", "", + []string{"https://client.example.com/callback"}, + []domain.OIDCResponseType{domain.OIDCResponseTypeCode}, + []domain.OIDCGrantType{domain.OIDCGrantTypeAuthorizationCode}, + domain.OIDCApplicationTypeWeb, domain.OIDCAuthMethodTypeNone, + []string{}, false, domain.OIDCTokenTypeBearer, + false, false, false, 0, []string{}, false, "", + domain.LoginVersionUnspecified, "")), + } + } + sameMetadata := &domain.OIDCApp{ + ObjectRoot: models.ObjectRoot{AggregateID: "project1"}, + AppID: "app1", + RedirectUris: []string{"https://client.example.com/callback"}, + ResponseTypes: []domain.OIDCResponseType{domain.OIDCResponseTypeCode}, + GrantTypes: []domain.OIDCGrantType{domain.OIDCGrantTypeAuthorizationCode}, + ApplicationType: gu.Ptr(domain.OIDCApplicationTypeWeb), + AuthMethodType: gu.Ptr(domain.OIDCAuthMethodTypeNone), + OIDCVersion: gu.Ptr(domain.OIDCVersionV1), + AccessTokenType: gu.Ptr(domain.OIDCTokenTypeBearer), + } + + t.Run("unchanged metadata still rotates the token, no permission required", func(t *testing.T) { + t.Parallel() + c := &Commands{ + eventstore: expectEventstore( + // A single filter: getOIDCAppWriteModel already reduces the write model. + expectFilter(existingClient()...), + expectPush( + project.NewOIDCConfigRegistrationTokenChangedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, "app1", "secret"), + ), + )(t), + newHashedSecret: mockHashedSecret("secret"), + checkPermission: newMockPermissionCheckNotAllowed(), + } + got, err := c.UpdateDynamicOIDCClient(context.Background(), sameMetadata, "org1") + assert.NoError(t, err) + assert.Equal(t, "client1", got.ClientID) + assert.Equal(t, "secret", got.RegistrationAccessToken) + }) + + t.Run("not existing client is not found", func(t *testing.T) { + t.Parallel() + c := &Commands{ + eventstore: expectEventstore(expectFilter())(t), + checkPermission: newMockPermissionCheckNotAllowed(), + } + _, err := c.UpdateDynamicOIDCClient(context.Background(), sameMetadata, "org1") + assert.True(t, zerrors.IsNotFound(err), "got wrong err: %v", err) + }) +} + +func TestCommandSide_RemoveDynamicOIDCClient(t *testing.T) { + t.Parallel() + + t.Run("remove existing client, no permission required", func(t *testing.T) { + t.Parallel() + c := &Commands{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(project.NewApplicationAddedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, "app1", "DCR Client app1")), + ), + expectPush( + project.NewApplicationRemovedEvent(context.Background(), + &project.NewAggregate("project1", "org1").Aggregate, "app1", "DCR Client app1", ""), + ), + )(t), + checkPermission: newMockPermissionCheckNotAllowed(), + } + got, err := c.RemoveDynamicOIDCClient(context.Background(), "project1", "app1", "org1") + assert.NoError(t, err) + assertObjectDetails(t, &domain.ObjectDetails{ResourceOwner: "org1"}, got) + }) + + t.Run("not existing client is not found", func(t *testing.T) { + t.Parallel() + c := &Commands{ + eventstore: expectEventstore(expectFilter())(t), + checkPermission: newMockPermissionCheckNotAllowed(), + } + _, err := c.RemoveDynamicOIDCClient(context.Background(), "project1", "app1", "org1") + assert.True(t, zerrors.IsNotFound(err), "got wrong err: %v", err) + }) +} diff --git a/internal/domain/application_oidc.go b/internal/domain/application_oidc.go index 5194ad2c2..83746925f 100644 --- a/internal/domain/application_oidc.go +++ b/internal/domain/application_oidc.go @@ -20,11 +20,16 @@ const ( type OIDCApp struct { models.ObjectRoot - AppID string - AppName string - ClientID string - EncodedHash string - ClientSecretString string + AppID string + AppName string + ClientID string + EncodedHash string + ClientSecretString string + // RegistrationAccessToken is the plain registration access token (RFC 7592 §3) of a + // dynamically registered client. It is only populated transiently on registration and + // rotation so the registration endpoint can return it to the client; it is never + // persisted (only its hash is, see project.OIDCConfigRegistrationTokenChangedEvent). + RegistrationAccessToken string RedirectUris []string ResponseTypes []OIDCResponseType GrantTypes []OIDCGrantType diff --git a/internal/query/oidc_client.go b/internal/query/oidc_client.go index 1592aa93d..c7bca8342 100644 --- a/internal/query/oidc_client.go +++ b/internal/query/oidc_client.go @@ -25,6 +25,7 @@ type OIDCClient struct { ClientID string `json:"client_id,omitempty"` BackChannelLogoutURI string `json:"back_channel_logout_uri,omitempty"` HashedSecret string `json:"client_secret,omitempty"` + RegistrationTokenHash string `json:"registration_token,omitempty"` RedirectURIs []string `json:"redirect_uris,omitempty"` ResponseTypes []domain.OIDCResponseType `json:"response_types,omitempty"` GrantTypes []domain.OIDCGrantType `json:"grant_types,omitempty"` diff --git a/internal/query/oidc_client_by_id.sql b/internal/query/oidc_client_by_id.sql index 7d783bbb5..f54fa9e44 100644 --- a/internal/query/oidc_client_by_id.sql +++ b/internal/query/oidc_client_by_id.sql @@ -4,7 +4,7 @@ with client as ( c.grant_types, c.application_type, c.auth_method_type, c.post_logout_redirect_uris, c.is_dev_mode, c.access_token_type, c.access_token_role_assertion, c.id_token_role_assertion, c.id_token_userinfo_assertion, c.clock_skew, c.additional_origins, a.project_id, p.project_role_assertion, - c.login_version, c.login_base_uri + c.login_version, c.login_base_uri, c.registration_token from projections.apps7_oidc_configs c join projections.apps7 a on a.id = c.app_id and a.instance_id = c.instance_id and a.state = 1 join projections.projects4 p on p.id = a.project_id and p.instance_id = a.instance_id and p.state = 1 diff --git a/internal/query/projection/app.go b/internal/query/projection/app.go index c50bf03f4..0105b5566 100644 --- a/internal/query/projection/app.go +++ b/internal/query/projection/app.go @@ -61,6 +61,7 @@ const ( AppOIDCConfigColumnBackChannelLogoutURI = "back_channel_logout_uri" AppOIDCConfigColumnLoginVersion = "login_version" AppOIDCConfigColumnLoginBaseURI = "login_base_uri" + AppOIDCConfigColumnRegistrationToken = "registration_token" appSAMLTableSuffix = "saml_configs" AppSAMLConfigColumnAppID = "app_id" @@ -133,6 +134,7 @@ func (*appProjection) Init() *old_handler.Check { handler.NewColumn(AppOIDCConfigColumnBackChannelLogoutURI, handler.ColumnTypeText, handler.Nullable()), handler.NewColumn(AppOIDCConfigColumnLoginVersion, handler.ColumnTypeEnum, handler.Nullable()), handler.NewColumn(AppOIDCConfigColumnLoginBaseURI, handler.ColumnTypeText, handler.Nullable()), + handler.NewColumn(AppOIDCConfigColumnRegistrationToken, handler.ColumnTypeText, handler.Nullable()), }, handler.NewPrimaryKey(AppOIDCConfigColumnInstanceID, AppOIDCConfigColumnAppID), appOIDCTableSuffix, @@ -217,6 +219,10 @@ func (p *appProjection) Reducers() []handler.AggregateReducer { Event: project.OIDCConfigSecretHashUpdatedType, Reduce: p.reduceOIDCConfigSecretHashUpdated, }, + { + Event: project.OIDCConfigRegistrationTokenChangedType, + Reduce: p.reduceOIDCConfigRegistrationTokenChanged, + }, { Event: project.SAMLConfigAddedType, Reduce: p.reduceSAMLConfigAdded, @@ -648,6 +654,36 @@ func (p *appProjection) reduceOIDCConfigSecretChanged(event eventstore.Event) (* ), nil } +func (p *appProjection) reduceOIDCConfigRegistrationTokenChanged(event eventstore.Event) (*handler.Statement, error) { + e, ok := event.(*project.OIDCConfigRegistrationTokenChangedEvent) + if !ok { + return nil, zerrors.ThrowInvalidArgumentf(nil, "HANDL-Eem6u", "reduce.wrong.event.type %s", project.OIDCConfigRegistrationTokenChangedType) + } + return handler.NewMultiStatement( + e, + handler.AddUpdateStatement( + []handler.Column{ + handler.NewCol(AppOIDCConfigColumnRegistrationToken, e.HashedToken), + }, + []handler.Condition{ + handler.NewCond(AppOIDCConfigColumnAppID, e.AppID), + handler.NewCond(AppOIDCConfigColumnInstanceID, e.Aggregate().InstanceID), + }, + handler.WithTableSuffix(appOIDCTableSuffix), + ), + handler.AddUpdateStatement( + []handler.Column{ + handler.NewCol(AppColumnChangeDate, e.CreationDate()), + handler.NewCol(AppColumnSequence, e.Sequence()), + }, + []handler.Condition{ + handler.NewCond(AppColumnID, e.AppID), + handler.NewCond(AppColumnInstanceID, e.Aggregate().InstanceID), + }, + ), + ), nil +} + func (p *appProjection) reduceOIDCConfigSecretHashUpdated(event eventstore.Event) (*handler.Statement, error) { e, ok := event.(*project.OIDCConfigSecretHashUpdatedEvent) if !ok { diff --git a/internal/query/projection/app_test.go b/internal/query/projection/app_test.go index 8177fe7d1..c693d7c3c 100644 --- a/internal/query/projection/app_test.go +++ b/internal/query/projection/app_test.go @@ -862,6 +862,46 @@ func TestAppProjection_reduces(t *testing.T) { }, }, }, + { + name: "project reduceOIDCConfigRegistrationTokenChanged", + args: args{ + event: getEvent( + testEvent( + project.OIDCConfigRegistrationTokenChangedType, + project.AggregateType, + []byte(`{ + "appId": "app-id", + "hashedToken": "token-hash" + }`), + ), eventstore.GenericEventMapper[project.OIDCConfigRegistrationTokenChangedEvent]), + }, + reduce: (&appProjection{}).reduceOIDCConfigRegistrationTokenChanged, + want: wantReduce{ + aggregateType: eventstore.AggregateType("project"), + sequence: 15, + executer: &testExecuter{ + executions: []execution{ + { + expectedStmt: "UPDATE projections.apps7_oidc_configs SET registration_token = $1 WHERE (app_id = $2) AND (instance_id = $3)", + expectedArgs: []interface{}{ + "token-hash", + "app-id", + "instance-id", + }, + }, + { + expectedStmt: "UPDATE projections.apps7 SET (change_date, sequence) = ($1, $2) WHERE (id = $3) AND (instance_id = $4)", + expectedArgs: []interface{}{ + anyArg{}, + uint64(15), + "app-id", + "instance-id", + }, + }, + }, + }, + }, + }, { name: "project reduceOIDCConfigSecretHashUpdated", args: args{ diff --git a/internal/repository/project/eventstore.go b/internal/repository/project/eventstore.go index 2648737d3..4364b24e8 100644 --- a/internal/repository/project/eventstore.go +++ b/internal/repository/project/eventstore.go @@ -36,6 +36,7 @@ func init() { eventstore.RegisterFilterEventMapper(AggregateType, OIDCConfigChangedType, OIDCConfigChangedEventMapper) eventstore.RegisterFilterEventMapper(AggregateType, OIDCConfigSecretChangedType, OIDCConfigSecretChangedEventMapper) eventstore.RegisterFilterEventMapper(AggregateType, OIDCConfigSecretHashUpdatedType, eventstore.GenericEventMapper[OIDCConfigSecretHashUpdatedEvent]) + eventstore.RegisterFilterEventMapper(AggregateType, OIDCConfigRegistrationTokenChangedType, eventstore.GenericEventMapper[OIDCConfigRegistrationTokenChangedEvent]) eventstore.RegisterFilterEventMapper(AggregateType, APIConfigAddedType, APIConfigAddedEventMapper) eventstore.RegisterFilterEventMapper(AggregateType, APIConfigChangedType, APIConfigChangedEventMapper) eventstore.RegisterFilterEventMapper(AggregateType, APIConfigSecretChangedType, APIConfigSecretChangedEventMapper) diff --git a/internal/repository/project/oidc_config.go b/internal/repository/project/oidc_config.go index dd7d3a85b..5df0fa1ec 100644 --- a/internal/repository/project/oidc_config.go +++ b/internal/repository/project/oidc_config.go @@ -15,6 +15,8 @@ const ( OIDCConfigChangedType = applicationEventTypePrefix + "config.oidc.changed" OIDCConfigSecretChangedType = applicationEventTypePrefix + "config.oidc.secret.changed" OIDCConfigSecretHashUpdatedType = applicationEventTypePrefix + "config.oidc.secret.updated" + + OIDCConfigRegistrationTokenChangedType = applicationEventTypePrefix + "config.oidc.registration_token.changed" ) type OIDCConfigAddedEvent struct { @@ -493,3 +495,45 @@ func (e *OIDCConfigSecretHashUpdatedEvent) Payload() interface{} { func (e *OIDCConfigSecretHashUpdatedEvent) UniqueConstraints() []*eventstore.UniqueConstraint { return nil } + +// OIDCConfigRegistrationTokenChangedEvent stores the hash of the registration access token +// (RFC 7592 §3) of a dynamically registered OIDC client. The token authenticates the read, +// update and delete operations on the client's registration. As with a client secret only the +// hash is persisted (and projected); the token itself is returned to the client once and never +// stored in clear text. The event is written on registration and again whenever the token is +// rotated (on update). +type OIDCConfigRegistrationTokenChangedEvent struct { + *eventstore.BaseEvent `json:"-"` + + AppID string `json:"appId"` + HashedToken string `json:"hashedToken,omitempty"` +} + +func NewOIDCConfigRegistrationTokenChangedEvent( + ctx context.Context, + aggregate *eventstore.Aggregate, + appID string, + hashedToken string, +) *OIDCConfigRegistrationTokenChangedEvent { + return &OIDCConfigRegistrationTokenChangedEvent{ + BaseEvent: eventstore.NewBaseEventForPush( + ctx, + aggregate, + OIDCConfigRegistrationTokenChangedType, + ), + AppID: appID, + HashedToken: hashedToken, + } +} + +func (e *OIDCConfigRegistrationTokenChangedEvent) SetBaseEvent(b *eventstore.BaseEvent) { + e.BaseEvent = b +} + +func (e *OIDCConfigRegistrationTokenChangedEvent) Payload() interface{} { + return e +} + +func (e *OIDCConfigRegistrationTokenChangedEvent) UniqueConstraints() []*eventstore.UniqueConstraint { + return nil +}