From ffd9b39bdbf6df06cd3762f3bb8bb7c5e7874bc5 Mon Sep 17 00:00:00 2001 From: gayathri <66356931+grvijayan@users.noreply.github.com> Date: Tue, 28 Jul 2026 15:20:46 +0200 Subject: [PATCH] feat: handle Zitadel provider IdP login via login v1 (#12469) # Which Problems Are Solved This PR adds support for using a ZITADEL IdP provider in the login v1 UI flow and conditionally grants a support-user instance membership based on a ZITADEL-specific project-roles token claim. # How the Problems Are Solved - Introduced ZITADEL IdP handling in the login v1 handler, including extracting the `urn:zitadel:iam:org:project:roles` claim and granting `IAM_OWNER_VIEWER` instance membership when configured orgs match. - Added a new `AddInstanceMemberFromLogin` to assign an instance membership during the login v1 ZITADEL-IdP flow without permission checks (as the authorization is established based on the project roles in the token claim) - Added unit tests # Additional Changes N/A # Additional Context - Closes https://github.com/zitadel/zitadel/issues/12446 --- .../idp/v2/integration_test/query_test.go | 6 - internal/api/grpc/management/idp_converter.go | 41 +-- .../api/grpc/management/idp_converter_test.go | 99 ------ .../management/integration_test/idp_test.go | 185 +--------- .../api/ui/login/external_provider_handler.go | 179 +++++++++- .../login/external_provider_handler_test.go | 315 ++++++++++++++++++ internal/command/instance_member.go | 79 ++++- internal/command/instance_member_test.go | 255 ++++++++++++++ internal/command/org_idp.go | 15 +- internal/command/org_idp_test.go | 200 +++-------- internal/domain/auth_request.go | 5 + internal/domain/roles.go | 1 + internal/integration/client.go | 6 - proto/zitadel/management.proto | 14 +- 14 files changed, 908 insertions(+), 492 deletions(-) diff --git a/internal/api/grpc/idp/v2/integration_test/query_test.go b/internal/api/grpc/idp/v2/integration_test/query_test.go index 06e4f82d9..14388911d 100644 --- a/internal/api/grpc/idp/v2/integration_test/query_test.go +++ b/internal/api/grpc/idp/v2/integration_test/query_test.go @@ -283,12 +283,6 @@ func TestServer_GetIDPByID(t *testing.T) { ClientId: "test-client", Issuer: "zitadel.example.com", Scopes: []string{"email", "profile"}, - InstanceRolesInfo: []*idp.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - }, }, }, Options: &idp.Options{ diff --git a/internal/api/grpc/management/idp_converter.go b/internal/api/grpc/management/idp_converter.go index d311813f1..fd5c80932 100644 --- a/internal/api/grpc/management/idp_converter.go +++ b/internal/api/grpc/management/idp_converter.go @@ -14,7 +14,6 @@ import ( "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore/v1/models" "github.com/zitadel/zitadel/internal/query" - "github.com/zitadel/zitadel/internal/repository/idp" "github.com/zitadel/zitadel/internal/zerrors" idp_pb "github.com/zitadel/zitadel/pkg/grpc/idp" mgmt_pb "github.com/zitadel/zitadel/pkg/grpc/management" @@ -544,38 +543,22 @@ func signatureAlgorithmToCommand(signatureAlgorithm idp_pb.SAMLSignatureAlgorith func addZitadelProviderToCommand(req *mgmt_pb.AddZitadelProviderRequest) command.ZitadelProvider { return command.ZitadelProvider{ - Name: req.Name, - Issuer: req.Issuer, - ClientID: req.ClientId, - ClientSecret: req.ClientSecret, - Scopes: req.Scopes, - IDPOptions: idp_grpc.OptionsToCommand(req.ProviderOptions), - InstanceRolesInfo: instanceRolesInfoToCommand(req.InstanceRolesInfo), + Name: req.Name, + Issuer: req.Issuer, + ClientID: req.ClientId, + ClientSecret: req.ClientSecret, + Scopes: req.Scopes, + IDPOptions: idp_grpc.OptionsToCommand(req.ProviderOptions), } } func updateZitadelProviderToCommand(req *mgmt_pb.UpdateZitadelProviderRequest) command.ZitadelProvider { return command.ZitadelProvider{ - Name: req.Name, - Issuer: req.Issuer, - ClientID: req.ClientId, - ClientSecret: req.ClientSecret, - Scopes: req.Scopes, - IDPOptions: idp_grpc.OptionsToCommand(req.ProviderOptions), - InstanceRolesInfo: instanceRolesInfoToCommand(req.InstanceRolesInfo), + Name: req.Name, + Issuer: req.Issuer, + ClientID: req.ClientId, + ClientSecret: req.ClientSecret, + Scopes: req.Scopes, + IDPOptions: idp_grpc.OptionsToCommand(req.ProviderOptions), } } - -func instanceRolesInfoToCommand(reqInstanceRolesInfo []*idp_pb.InstanceRolesInfo) []idp.RolesInfo { - if reqInstanceRolesInfo == nil { - return nil - } - instanceRolesInfo := make([]idp.RolesInfo, 0, len(reqInstanceRolesInfo)) - for _, info := range reqInstanceRolesInfo { - instanceRolesInfo = append(instanceRolesInfo, idp.RolesInfo{ - OrganizationID: info.OrganizationId, - OrganizationDomain: info.OrganizationDomain, - }) - } - return instanceRolesInfo -} diff --git a/internal/api/grpc/management/idp_converter_test.go b/internal/api/grpc/management/idp_converter_test.go index b27a09d48..00d9e0dfa 100644 --- a/internal/api/grpc/management/idp_converter_test.go +++ b/internal/api/grpc/management/idp_converter_test.go @@ -207,34 +207,6 @@ func Test_addZitadelProviderToCommand(t *testing.T) { req *mgmt_pb.AddZitadelProviderRequest want command.ZitadelProvider }{ - { - name: "without instance roles info", - req: &mgmt_pb.AddZitadelProviderRequest{ - Name: "Zitadel Support IdP", - ClientId: "test-client", - ClientSecret: "test-secret", - Scopes: []string{"email", "profile", "urn:zitadel:iam:org:project:roles"}, - ProviderOptions: &idp_pb.Options{ - IsLinkingAllowed: false, - IsCreationAllowed: true, - IsAutoCreation: false, - IsAutoUpdate: true, - AutoLinking: 0, - }, - }, - want: command.ZitadelProvider{ - Name: "Zitadel Support IdP", - ClientID: "test-client", - ClientSecret: "test-secret", - Scopes: []string{"email", "profile", "urn:zitadel:iam:org:project:roles"}, - IDPOptions: idp.Options{ - IsCreationAllowed: true, - IsAutoCreation: false, - IsLinkingAllowed: false, - IsAutoUpdate: true, - }, - }, - }, { name: "all fields filled", req: &mgmt_pb.AddZitadelProviderRequest{ @@ -249,17 +221,6 @@ func Test_addZitadelProviderToCommand(t *testing.T) { IsAutoUpdate: true, AutoLinking: 0, }, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - { - - OrganizationId: "org2", - OrganizationDomain: "org2.com", - }, - }, }, want: command.ZitadelProvider{ Name: "Zitadel Support IdP", @@ -272,16 +233,6 @@ func Test_addZitadelProviderToCommand(t *testing.T) { IsLinkingAllowed: false, IsAutoUpdate: true, }, - InstanceRolesInfo: []idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "org1.com", - }, - { - OrganizationID: "org2", - OrganizationDomain: "org2.com", - }, - }, }, }, } @@ -301,35 +252,6 @@ func Test_updateZitadelProviderToCommand(t *testing.T) { req *mgmt_pb.UpdateZitadelProviderRequest want command.ZitadelProvider }{ - { - name: "without instance roles info", - req: &mgmt_pb.UpdateZitadelProviderRequest{ - Name: "Zitadel Support IdP", - ClientId: "test-client", - ClientSecret: "test-secret", - Scopes: []string{"email", "profile", "urn:zitadel:iam:org:project:roles"}, - ProviderOptions: &idp_pb.Options{ - IsLinkingAllowed: false, - IsCreationAllowed: true, - IsAutoCreation: false, - IsAutoUpdate: true, - AutoLinking: 0, - }, - }, - want: command.ZitadelProvider{ - Name: "Zitadel Support IdP", - ClientID: "test-client", - ClientSecret: "test-secret", - Scopes: []string{"email", "profile", "urn:zitadel:iam:org:project:roles"}, - IDPOptions: idp.Options{ - IsCreationAllowed: true, - IsAutoCreation: false, - IsLinkingAllowed: false, - IsAutoUpdate: true, - }, - InstanceRolesInfo: nil, - }, - }, { name: "all fields filled", req: &mgmt_pb.UpdateZitadelProviderRequest{ @@ -344,17 +266,6 @@ func Test_updateZitadelProviderToCommand(t *testing.T) { IsAutoUpdate: true, AutoLinking: 0, }, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - { - - OrganizationId: "org2", - OrganizationDomain: "org2.com", - }, - }, }, want: command.ZitadelProvider{ Name: "Zitadel Support IdP", @@ -367,16 +278,6 @@ func Test_updateZitadelProviderToCommand(t *testing.T) { IsLinkingAllowed: false, IsAutoUpdate: true, }, - InstanceRolesInfo: []idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "org1.com", - }, - { - OrganizationID: "org2", - OrganizationDomain: "org2.com", - }, - }, }, }, } diff --git a/internal/api/grpc/management/integration_test/idp_test.go b/internal/api/grpc/management/integration_test/idp_test.go index 0c217d72b..f198d32e7 100644 --- a/internal/api/grpc/management/integration_test/idp_test.go +++ b/internal/api/grpc/management/integration_test/idp_test.go @@ -109,7 +109,7 @@ func Test_AddZitadelProvider(t *testing.T) { ), }, { - name: "missing org ID in instance roles info", + name: "valid request", args: args{ ctx: OrgCTX, req: &mgmt_pb.AddZitadelProviderRequest{ @@ -121,87 +121,6 @@ func Test_AddZitadelProvider(t *testing.T) { ProviderOptions: &idp_pb.Options{ IsCreationAllowed: true, }, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "", - }, - }, - }, - }, - wantErr: status.Error( - codes.InvalidArgument, - "invalid AddZitadelProviderRequest.InstanceRolesInfo[0]: embedded message failed validation | caused by: invalid InstanceRolesInfo.OrganizationId: value length must be between 1 and 200 runes, inclusive", - ), - }, - { - name: "missing org domain in instance roles info", - args: args{ - ctx: OrgCTX, - req: &mgmt_pb.AddZitadelProviderRequest{ - Name: "Zitadel Support IdP", - Issuer: "zitadel.example.com", - ClientId: "test-client", - ClientSecret: "test-secret", - Scopes: []string{"email", "profile"}, - ProviderOptions: &idp_pb.Options{ - IsCreationAllowed: true, - }, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - { - OrganizationId: "org2", - }, - }, - }, - }, - wantErr: status.Error( - codes.InvalidArgument, - "invalid AddZitadelProviderRequest.InstanceRolesInfo[1]: embedded message failed validation | caused by: invalid InstanceRolesInfo.OrganizationDomain: value length must be between 1 and 200 runes, inclusive", - ), - }, - { - name: "valid request without instance roles info", - args: args{ - ctx: OrgCTX, - req: &mgmt_pb.AddZitadelProviderRequest{ - Name: "Zitadel Support IdP", - Issuer: "zitadel.example.com", - ClientId: "test-client", - ClientSecret: "test-secret", - Scopes: []string{"email", "profile"}, - ProviderOptions: &idp_pb.Options{ - IsCreationAllowed: true, - }, - }, - }, - wantResponse: &mgmt_pb.AddZitadelProviderResponse{ - Details: &object_pb.ObjectDetails{ - ResourceOwner: Instance.DefaultOrg.Id, - }, - }, - }, - { - name: "valid request with instance roles info", - args: args{ - ctx: OrgCTX, - req: &mgmt_pb.AddZitadelProviderRequest{ - Name: "Zitadel Support IdP", - Issuer: "zitadel.example.com", - ClientId: "test-client", - ClientSecret: "test-secret", - Scopes: []string{"email", "profile"}, - ProviderOptions: &idp_pb.Options{ - IsCreationAllowed: true, - }, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - }, }, }, wantResponse: &mgmt_pb.AddZitadelProviderResponse{ @@ -318,16 +237,6 @@ func Test_UpdateZitadelProvider(t *testing.T) { Issuer: "acme.example.com", ClientId: "test-client", Scopes: []string{"email", "profile", "openid", "offline_access"}, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - { - OrganizationId: "org2", - OrganizationDomain: "org2.com", - }, - }, }, }, wantResponse: &mgmt_pb.UpdateZitadelProviderResponse{ @@ -347,16 +256,6 @@ func Test_UpdateZitadelProvider(t *testing.T) { Issuer: "acme.example.com", ClientId: "test-client", Scopes: []string{"email", "profile", "openid", "offline_access"}, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - { - OrganizationId: "org2", - OrganizationDomain: "org2.com", - }, - }, }, }, }, @@ -374,12 +273,6 @@ func Test_UpdateZitadelProvider(t *testing.T) { ProviderOptions: &idp_pb.Options{ IsAutoCreation: true, }, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org3", - OrganizationDomain: "org3.com", - }, - }, }, }, wantResponse: &mgmt_pb.UpdateZitadelProviderResponse{ @@ -401,52 +294,6 @@ func Test_UpdateZitadelProvider(t *testing.T) { Issuer: "acme.example.com", ClientId: "test-client", Scopes: nil, // unset scopes - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org3", - OrganizationDomain: "org3.com", - }, - }, - }, - }, - }, - }, - }, - { - name: "update with instance roles info not set, ok", - args: args{ - ctx: OrgCTX, - req: &mgmt_pb.UpdateZitadelProviderRequest{ - Name: "Zitadel Support IdP updated 2", - Issuer: "acme.example.com", - ClientId: "test-client", - Scopes: []string{"email", "openid"}, - ProviderOptions: &idp_pb.Options{ - IsCreationAllowed: true, - }, - InstanceRolesInfo: nil, // instance roles info isn't set -> will be unset by the API - }, - }, - wantResponse: &mgmt_pb.UpdateZitadelProviderResponse{ - Details: &object_pb.ObjectDetails{ - ResourceOwner: Instance.DefaultOrg.Id, - }, - }, - wantUpdatedProvider: &idp_pb.Provider{ - State: idp_pb.IDPState_IDP_STATE_ACTIVE, - Name: "Zitadel Support IdP updated 2", - Owner: idp_pb.IDPOwnerType_IDP_OWNER_TYPE_ORG, - Type: idp_pb.ProviderType_PROVIDER_TYPE_ZITADEL, - Config: &idp_pb.ProviderConfig{ - Options: &idp_pb.Options{ - IsCreationAllowed: true, - }, - Config: &idp_pb.ProviderConfig_Zitadel{ - Zitadel: &idp_pb.ZitadelConfig{ - Issuer: "acme.example.com", - ClientId: "test-client", - Scopes: []string{"email", "openid"}, - InstanceRolesInfo: nil, // instance roles info isn't set }, }, }, @@ -567,12 +414,6 @@ func Test_GetProviderByID(t *testing.T) { Issuer: "zitadel.example.com", ClientId: "test-client", Scopes: []string{"email", "profile"}, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - }, }, }, }, @@ -676,12 +517,6 @@ func Test_ListProviders(t *testing.T) { Issuer: "zitadel.example.com", ClientId: "test-client", Scopes: []string{"email", "profile"}, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - }, }, }, }, @@ -731,12 +566,6 @@ func Test_ListProviders(t *testing.T) { Issuer: "zitadel.example.com", ClientId: "test-client", Scopes: []string{"email", "profile"}, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - }, }, }, }, @@ -777,12 +606,6 @@ func Test_ListProviders(t *testing.T) { Issuer: "zitadel.example.com", ClientId: "test-client", Scopes: []string{"email", "profile"}, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - }, }, }, }, @@ -807,12 +630,6 @@ func Test_ListProviders(t *testing.T) { Issuer: "zitadel.example.com", ClientId: "test-client", Scopes: []string{"email", "profile"}, - InstanceRolesInfo: []*idp_pb.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - }, }, }, }, diff --git a/internal/api/ui/login/external_provider_handler.go b/internal/api/ui/login/external_provider_handler.go index fe8291667..f2d1fb6b1 100644 --- a/internal/api/ui/login/external_provider_handler.go +++ b/internal/api/ui/login/external_provider_handler.go @@ -20,6 +20,7 @@ import ( "github.com/zitadel/zitadel/internal/api/authz" http_utils "github.com/zitadel/zitadel/internal/api/http" http_mw "github.com/zitadel/zitadel/internal/api/http/middleware" + "github.com/zitadel/zitadel/internal/command" "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/domain/federatedlogout" @@ -36,6 +37,7 @@ import ( openid "github.com/zitadel/zitadel/internal/idp/providers/oidc" "github.com/zitadel/zitadel/internal/idp/providers/saml" "github.com/zitadel/zitadel/internal/idp/providers/saml/requesttracker" + "github.com/zitadel/zitadel/internal/idp/providers/zitadel" "github.com/zitadel/zitadel/internal/query" "github.com/zitadel/zitadel/internal/zerrors" ) @@ -46,6 +48,9 @@ const ( queryRelayState = "RelayState" queryMethod = "method" tmplExternalNotFoundOption = "externalnotfoundoption" + // zitadelProjectRolesClaim is the token claim a ZITADEL provider uses to convey + // a user's project roles ({role: {orgID: orgDomain}}). + zitadelProjectRolesClaim = "urn:zitadel:iam:org:project:roles" ) var ( @@ -190,6 +195,8 @@ func (l *Login) handleIDP(w http.ResponseWriter, r *http.Request, authReq *domai provider, err = l.ldapProvider(r.Context(), identityProvider) case domain.IDPTypeSAML: provider, err = l.samlProvider(r.Context(), identityProvider) + case domain.IDPTypeZitadel: + provider, err = l.zitadelProvider(r.Context(), identityProvider) case domain.IDPTypeUnspecified: fallthrough default: @@ -368,6 +375,13 @@ func (l *Login) handleExternalLoginCallback(w http.ResponseWriter, r *http.Reque l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) return } + case domain.IDPTypeZitadel: + provider, err := l.zitadelProvider(r.Context(), identityProvider) + if err != nil { + l.externalAuthCallbackFailed(w, r, authReq, nil, nil, err) + return + } + session = openid.NewSession(provider.Provider, data.Code, authReq.SelectedIDPConfigArgs) case domain.IDPTypeJWT, domain.IDPTypeLDAP, domain.IDPTypeUnspecified: @@ -459,6 +473,10 @@ func (l *Login) handleExternalUserAuthenticated( callback func(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest), ) { externalUser := mapIDPUserToExternalUser(user, provider.ID) + // a ZITADEL provider may convey support-user project roles, which are carried through to user creation + if provider.Type == domain.IDPTypeZitadel { + externalUser.ProjectRoles = projectRolesFromIDPUser(user) + } // ensure the linked IDP is added to the login policy if err := l.authRepo.SelectExternalIDP(r.Context(), authReq.ID, provider.ID, authReq.AgentID, authReq.SelectedIDPConfigArgs); err != nil { l.renderError(w, r, authReq, err) @@ -516,6 +534,17 @@ func (l *Login) handleExternalUserAuthenticated( return } } + // (re)assign the support-user instance membership for an existing user + // so that the role assignment remains idempotent + // in case user creation succeeded but role assignment failed in a previous request. + err = l.ensureSupportUserInstanceMembership(r, provider, authReq, externalUser) + if err != nil && !userLinked { + l.renderError(w, r, authReq, err) + return + } + // if a user was just linked we don't render further errors (see above), + // but a failed support-membership grant must still be observable, so logging it. + logging.WithFields("authReq", authReq.ID, "user", authReq.UserID).OnError(err).Error("unable to grant support user instance membership") callback(w, r, authReq) } @@ -653,12 +682,12 @@ func (l *Login) createOrLinkUser(w http.ResponseWriter, r *http.Request, authReq return } } - l.autoCreateExternalUser(w, r, authReq) + l.autoCreateExternalUser(w, r, provider, authReq) return false } // autoCreateExternalUser takes the externalUser and creates it automatically (without user interaction) -func (l *Login) autoCreateExternalUser(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest) { +func (l *Login) autoCreateExternalUser(w http.ResponseWriter, r *http.Request, provider *query.IDPTemplate, authReq *domain.AuthRequest) { if len(authReq.LinkingUsers) == 0 { l.renderError(w, r, authReq, zerrors.ThrowPreconditionFailed(nil, "LOGIN-asfg3", "Errors.ExternalIDP.NoExternalUserData")) return @@ -667,7 +696,7 @@ func (l *Login) autoCreateExternalUser(w http.ResponseWriter, r *http.Request, a // TODO (LS): how do we get multiple and why do we use the last of them (taken as is)? linkingUser := authReq.LinkingUsers[len(authReq.LinkingUsers)-1] - l.registerExternalUser(w, r, authReq, linkingUser) + l.registerExternalUser(w, r, provider, authReq, linkingUser) } // renderExternalNotFoundOption renders a page, where the user is able to edit the IDP data, @@ -790,14 +819,18 @@ func (l *Login) handleExternalNotFoundOptionCheck(w http.ResponseWriter, r *http return } linkingUser := mapExternalNotFoundOptionFormDataToLoginUser(data) - l.registerExternalUser(w, r, authReq, linkingUser) + // the form only carries the editable profile fields, so preserve the ZITADEL project + // roles captured at authentication (stored on the linking user) to keep the + // support-user instance membership grant working on the manual creation path. + preserveProjectRoles(linkingUser, authReq.LinkingUsers) + l.registerExternalUser(w, r, idpTemplate, authReq, linkingUser) } // registerExternalUser creates an externalUser with the provided data // incl. execution of pre and post creation actions // // it is called from either the [autoCreateExternalUser] or [handleExternalNotFoundOptionCheck] -func (l *Login) registerExternalUser(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest, externalUser *domain.ExternalUser) { +func (l *Login) registerExternalUser(w http.ResponseWriter, r *http.Request, provider *query.IDPTemplate, authReq *domain.AuthRequest, externalUser *domain.ExternalUser) { resourceOwner := determineResourceOwner(r.Context(), authReq) orgIamPolicy, err := l.getOrgDomainPolicy(r, resourceOwner) @@ -833,9 +866,81 @@ func (l *Login) registerExternalUser(w http.ResponseWriter, r *http.Request, aut l.renderError(w, r, authReq, err) return } + // a ZITADEL provider may grant a support user an instance membership based on the roles present in its token claim. + if err = l.ensureSupportUserInstanceMembership(r, provider, authReq, externalUser); err != nil { + l.renderError(w, r, authReq, err) + return + } l.renderNextStep(w, r, authReq) } +// ensureSupportUserInstanceMembership makes sure an external user holds the instance member +// roles conveyed by their `urn:zitadel:iam:org:project:roles` claim when they authenticated +// via a ZITADEL provider. +func (l *Login) ensureSupportUserInstanceMembership(r *http.Request, provider *query.IDPTemplate, authReq *domain.AuthRequest, externalUser *domain.ExternalUser) error { + roles := supportUserInstanceRoles(provider, externalUser) + if len(roles) == 0 { + return nil + } + member := &command.AddInstanceMember{ + InstanceID: authz.GetInstance(r.Context()).InstanceID(), + UserID: authReq.UserID, + Roles: roles, + } + _, err := l.command.EnsureInstanceMemberRolesFromLogin(setContext(r.Context(), ""), member) + if err != nil && !zerrors.IsErrorAlreadyExists(err) { + return err + } + return nil +} + +// supportUserInstanceRoles returns the instance member roles an external user must hold, +// derived from the IAM roles of their ZITADEL project roles claim. Only roles claimed for an +// organization configured in the IDP's InstanceRolesInfo are returned so that the set of +// organizations able to confer instance roles stays under the control of the instance admins. +func supportUserInstanceRoles(provider *query.IDPTemplate, externalUser *domain.ExternalUser) []string { + // instance-wide role grants may only originate from an instance-scoped IDP. + // An organization-scoped provider must never confer instance membership, even if + // its InstanceRolesInfo is somehow populated (e.g. stale data): honoring it would + // let an org owner escalate themselves to instance-level roles. + if provider.OwnerType != domain.IdentityProviderTypeSystem { + return nil + } + // only a ZITADEL provider conveys support-user project roles + if provider.Type != domain.IDPTypeZitadel || provider.ZitadelIDPTemplate == nil { + return nil + } + var roles []string + for role, claimOrgs := range externalUser.ProjectRoles { + // only an IAM role can be conferred by an instance membership + if !strings.HasPrefix(role, domain.IAMRolePrefix) { + continue + } + // a claim org must be the same as a configured organization (ID and domain) + if !claimMatchesConfiguredOrg(claimOrgs, provider.ZitadelIDPTemplate) { + continue + } + roles = append(roles, role) + } + // the claim is a map, so sort to keep the resulting role set deterministic + slices.Sort(roles) + return roles +} + +// claimMatchesConfiguredOrg reports whether any organization from the roles claim +// (orgID → orgDomain) exactly matches an organization (by ID and domain) +// configured in the ZITADEL IDP's InstanceRolesInfo. +func claimMatchesConfiguredOrg(claimOrgs map[string]string, zitadelTemplate *query.ZitadelIDPTemplate) bool { + for orgID, orgDomain := range claimOrgs { + for _, configured := range zitadelTemplate.InstanceRolesInfo { + if configured.OrganizationID == orgID && configured.OrganizationDomain == orgDomain { + return true + } + } + } + return false +} + // updateExternalUser will update the existing user (email, phone, profile) with data provided by the IDP func (l *Login) updateExternalUser(ctx context.Context, authReq *domain.AuthRequest, externalUser *domain.ExternalUser) error { user, err := l.query.GetUserByID(ctx, true, authReq.UserID) @@ -1290,6 +1395,22 @@ func (l *Login) appleProvider(ctx context.Context, identityProvider *query.IDPTe ) } +func (l *Login) zitadelProvider(ctx context.Context, identityProvider *query.IDPTemplate) (*zitadel.Provider, error) { + secret, err := crypto.DecryptString(identityProvider.ZitadelIDPTemplate.ClientSecret, l.idpConfigAlg) + if err != nil { + return nil, err + } + return zitadel.New( + identityProvider.ZitadelIDPTemplate.Issuer, + identityProvider.ZitadelIDPTemplate.ClientID, + secret, + l.baseURL(ctx)+EndpointExternalLoginCallback, + identityProvider.ZitadelIDPTemplate.Scopes, + l.httpClient, + openid.WithSelectAccount(), + ) +} + func (l *Login) appendUserGrants(ctx context.Context, userGrants []*domain.UserGrant, resourceOwner string) error { if len(userGrants) == 0 { return nil @@ -1629,3 +1750,51 @@ func WrapIdPError(err error) *IdPError { } return &IdPError{err: zerrors.CreateZitadelError(zerrors.KindPreconditionFailed, err, id, "Errors.User.ExternalIDP.LoginFailedSwitchLocal", 1)} } + +// projectRolesFromIDPUser extracts the ZITADEL project-roles claim from an IDP +// user's OIDC userinfo and returns it as a map {role: {orgID: orgDomain}}. +// It returns nil when the user is not an OIDC user or the claim is absent or malformed. +func projectRolesFromIDPUser(user idp.User) map[string]map[string]string { + oidcUser, ok := user.(*openid.User) + if !ok || oidcUser.UserInfo == nil { + return nil + } + rawRoles, ok := oidcUser.Claims[zitadelProjectRolesClaim].(map[string]any) + if !ok { + return nil + } + roles := make(map[string]map[string]string, len(rawRoles)) + for role, rawOrgs := range rawRoles { + orgs, ok := rawOrgs.(map[string]any) + if !ok { + continue + } + orgMap := make(map[string]string, len(orgs)) + for orgID, rawDomain := range orgs { + // skip entries whose domain is not a string. + domainName, ok := rawDomain.(string) + if !ok { + continue + } + orgMap[orgID] = domainName + } + if len(orgMap) == 0 { + continue + } + roles[role] = orgMap + } + if len(roles) == 0 { + return nil + } + return roles +} + +// preserveProjectRoles carries the ZITADEL project roles captured at authentication +// onto the form-mapped external user. The "external user not found" form only submits profile fields. +// The roles claim is restored from the most recent linking user stored on the auth request. +func preserveProjectRoles(user *domain.ExternalUser, linkingUsers []*domain.ExternalUser) { + if user == nil || len(linkingUsers) == 0 { + return + } + user.ProjectRoles = linkingUsers[len(linkingUsers)-1].ProjectRoles +} diff --git a/internal/api/ui/login/external_provider_handler_test.go b/internal/api/ui/login/external_provider_handler_test.go index b1724fc35..c425d588c 100644 --- a/internal/api/ui/login/external_provider_handler_test.go +++ b/internal/api/ui/login/external_provider_handler_test.go @@ -3,8 +3,14 @@ package login import ( "testing" + "github.com/stretchr/testify/assert" + "github.com/zitadel/oidc/v3/pkg/oidc" + "github.com/zitadel/zitadel/internal/domain" + "github.com/zitadel/zitadel/internal/idp" + openid "github.com/zitadel/zitadel/internal/idp/providers/oidc" "github.com/zitadel/zitadel/internal/query" + provideridp "github.com/zitadel/zitadel/internal/repository/idp" ) func Test_hasEmailChanged(t *testing.T) { @@ -284,3 +290,312 @@ func Test_hasPhoneChanged(t *testing.T) { }) } } + +func Test_projectRolesFromIDPUser(t *testing.T) { + tests := []struct { + name string + user idp.User + want map[string]map[string]string + }{ + { + "not an oidc user", + struct{ idp.User }{}, + nil, + }, + { + "no claims", + openid.NewUser(&oidc.UserInfo{Claims: nil}), + nil, + }, + { + "claim absent", + openid.NewUser(&oidc.UserInfo{Claims: map[string]any{"some": "other"}}), + nil, + }, + { + "claim wrong type", + openid.NewUser(&oidc.UserInfo{Claims: map[string]any{zitadelProjectRolesClaim: "not-a-map"}}), + nil, + }, + { + "role with non-map orgs is skipped", + openid.NewUser(&oidc.UserInfo{Claims: map[string]any{ + zitadelProjectRolesClaim: map[string]any{ + "IAM_OWNER_VIEWER": "not-a-map", + }}, + }), + nil, + }, + { + "single role and org", + openid.NewUser(&oidc.UserInfo{Claims: map[string]any{ + zitadelProjectRolesClaim: map[string]any{ + "IAM_OWNER_VIEWER": map[string]any{ + "orgID1": "org1.example.com", + }, + }}, + }), + map[string]map[string]string{ + "IAM_OWNER_VIEWER": {"orgID1": "org1.example.com"}, + }, + }, + { + "multiple roles and orgs", + openid.NewUser(&oidc.UserInfo{Claims: map[string]any{ + zitadelProjectRolesClaim: map[string]any{ + "IAM_OWNER_VIEWER": map[string]any{ + "orgID1": "org1.example.com", + "orgID2": "org2.example.com", + }, + "OTHER_ROLE": map[string]any{ + "orgID3": "org3.example.com", + }, + }}, + }), + map[string]map[string]string{ + "IAM_OWNER_VIEWER": {"orgID1": "org1.example.com", "orgID2": "org2.example.com"}, + "OTHER_ROLE": {"orgID3": "org3.example.com"}, + }, + }, + { + "non-string org domain is skipped", + openid.NewUser(&oidc.UserInfo{Claims: map[string]any{ + zitadelProjectRolesClaim: map[string]any{ + "IAM_OWNER_VIEWER": map[string]any{ + "orgID1": 55, + }, + }}, + }), + nil, + }, + { + "role keeps only orgs with string domains", + openid.NewUser(&oidc.UserInfo{Claims: map[string]any{ + zitadelProjectRolesClaim: map[string]any{ + "IAM_OWNER_VIEWER": map[string]any{ + "orgID1": "org1.example.com", + "orgID2": 55, + }, + }}, + }), + map[string]map[string]string{ + "IAM_OWNER_VIEWER": {"orgID1": "org1.example.com"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, projectRolesFromIDPUser(tt.user)) + }) + } +} + +func Test_claimMatchesConfiguredOrg(t *testing.T) { + tests := []struct { + name string + claimOrgs map[string]string + template *query.ZitadelIDPTemplate + want bool + }{ + { + "id and domain match", + map[string]string{"orgID1": "org1.example.com"}, + &query.ZitadelIDPTemplate{InstanceRolesInfo: []provideridp.RolesInfo{{OrganizationID: "orgID1", OrganizationDomain: "org1.example.com"}}}, + true, + }, + { + "id matches, domain differs", + map[string]string{"orgID1": "evil.example.com"}, + &query.ZitadelIDPTemplate{InstanceRolesInfo: []provideridp.RolesInfo{{OrganizationID: "orgID1", OrganizationDomain: "org1.example.com"}}}, + false, + }, + { + "domain matches, id differs", + map[string]string{"otherID": "org1.example.com"}, + &query.ZitadelIDPTemplate{InstanceRolesInfo: []provideridp.RolesInfo{{OrganizationID: "orgID1", OrganizationDomain: "org1.example.com"}}}, + false, + }, + { + "neither id nor domain match", + map[string]string{"orgIDx": "orgx.example.com"}, + &query.ZitadelIDPTemplate{InstanceRolesInfo: []provideridp.RolesInfo{{OrganizationID: "orgID1", OrganizationDomain: "org1.example.com"}}}, + false, + }, + { + "no configured orgs", + map[string]string{"orgID1": "org1.example.com"}, + &query.ZitadelIDPTemplate{}, + false, + }, + { + "no claim orgs", + map[string]string{}, + &query.ZitadelIDPTemplate{InstanceRolesInfo: []provideridp.RolesInfo{{OrganizationID: "orgID1", OrganizationDomain: "org1.example.com"}}}, + false, + }, + { + "one of multiple claim orgs matches", + map[string]string{"orgIDx": "orgx.example.com", "orgID1": "org1.example.com"}, + &query.ZitadelIDPTemplate{InstanceRolesInfo: []provideridp.RolesInfo{{OrganizationID: "orgID1", OrganizationDomain: "org1.example.com"}}}, + true, + }, + { + "one of multiple configured orgs matches", + map[string]string{"orgID2": "org2.example.com"}, + &query.ZitadelIDPTemplate{InstanceRolesInfo: []provideridp.RolesInfo{ + {OrganizationID: "orgID1", OrganizationDomain: "org1.example.com"}, + {OrganizationID: "orgID2", OrganizationDomain: "org2.example.com"}, + }}, + true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, claimMatchesConfiguredOrg(tt.claimOrgs, tt.template)) + }) + } +} + +func Test_preserveProjectRoles(t *testing.T) { + roles := map[string]map[string]string{ + "IAM_OWNER_VIEWER": {"orgID1": "org1.example.com"}, + } + tests := []struct { + name string + user *domain.ExternalUser + linkingUsers []*domain.ExternalUser + want map[string]map[string]string + }{ + { + // the form-mapped user has no roles claim; it must be restored from the linking user + name: "roles copied from the single linking user", + user: &domain.ExternalUser{}, + linkingUsers: []*domain.ExternalUser{{ProjectRoles: roles}}, + want: roles, + }, + { + // linking users accumulate on account-linking flows; the most recent one is mapped + name: "roles copied from the most recent linking user", + user: &domain.ExternalUser{}, + linkingUsers: []*domain.ExternalUser{ + {ProjectRoles: map[string]map[string]string{"OTHER_ROLE": {"orgIDx": "orgx.example.com"}}}, + {ProjectRoles: roles}, + }, + want: roles, + }, + { + name: "no linking users, nil", + user: &domain.ExternalUser{}, + linkingUsers: nil, + want: nil, + }, + { + name: "last linking user without roles, nil", + user: &domain.ExternalUser{}, + linkingUsers: []*domain.ExternalUser{{ProjectRoles: roles}, {}}, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + preserveProjectRoles(tt.user, tt.linkingUsers) + assert.Equal(t, tt.want, tt.user.ProjectRoles) + }) + } +} + +func Test_supportUserInstanceRoles(t *testing.T) { + configured := &query.ZitadelIDPTemplate{ + InstanceRolesInfo: []provideridp.RolesInfo{{OrganizationID: "orgID1", OrganizationDomain: "org1.example.com"}}, + } + supportClaim := map[string]map[string]string{ + domain.RoleIAMOwnerViewer: {"orgID1": "org1.example.com"}, + } + tests := []struct { + name string + provider *query.IDPTemplate + externalUser *domain.ExternalUser + want []string + }{ + { + name: "instance zitadel provider, support role claim, matching org", + provider: &query.IDPTemplate{OwnerType: domain.IdentityProviderTypeSystem, Type: domain.IDPTypeZitadel, ZitadelIDPTemplate: configured}, + externalUser: &domain.ExternalUser{ProjectRoles: supportClaim}, + want: []string{domain.RoleIAMOwnerViewer}, + }, + { + name: "multiple IAM roles claimed, all returned sorted", + provider: &query.IDPTemplate{Type: domain.IDPTypeZitadel, ZitadelIDPTemplate: configured}, + externalUser: &domain.ExternalUser{ProjectRoles: map[string]map[string]string{ + domain.RoleIAMOwnerViewer: {"orgID1": "org1.example.com"}, + domain.RoleIAMOwner: {"orgID1": "org1.example.com"}, + }}, + want: []string{domain.RoleIAMOwner, domain.RoleIAMOwnerViewer}, + }, + { + name: "only the roles of a configured org are returned", + provider: &query.IDPTemplate{Type: domain.IDPTypeZitadel, ZitadelIDPTemplate: configured}, + externalUser: &domain.ExternalUser{ProjectRoles: map[string]map[string]string{ + domain.RoleIAMOwnerViewer: {"orgID1": "org1.example.com"}, + domain.RoleIAMOwner: {"orgID2": "org2.example.com"}, + }}, + want: []string{domain.RoleIAMOwnerViewer}, + }, + { + name: "non-IAM roles are not returned", + provider: &query.IDPTemplate{Type: domain.IDPTypeZitadel, ZitadelIDPTemplate: configured}, + externalUser: &domain.ExternalUser{ProjectRoles: map[string]map[string]string{ + domain.RoleIAMOwnerViewer: {"orgID1": "org1.example.com"}, + domain.RoleOrgOwner: {"orgID1": "org1.example.com"}, + }}, + want: []string{domain.RoleIAMOwnerViewer}, + }, + { + name: "org-scoped zitadel provider, matching org, skipped", + provider: &query.IDPTemplate{OwnerType: domain.IdentityProviderTypeOrg, Type: domain.IDPTypeZitadel, ZitadelIDPTemplate: configured}, + externalUser: &domain.ExternalUser{ProjectRoles: supportClaim}, + want: nil, + }, + { + name: "non-zitadel provider, skipped", + provider: &query.IDPTemplate{Type: domain.IDPTypeOIDC, ZitadelIDPTemplate: configured}, + externalUser: &domain.ExternalUser{ProjectRoles: supportClaim}, + want: nil, + }, + { + name: "zitadel type but no zitadel template, skipped", + provider: &query.IDPTemplate{Type: domain.IDPTypeZitadel}, + externalUser: &domain.ExternalUser{ProjectRoles: supportClaim}, + want: nil, + }, + { + name: "no roles claim at all, skipped", + provider: &query.IDPTemplate{Type: domain.IDPTypeZitadel, ZitadelIDPTemplate: configured}, + externalUser: &domain.ExternalUser{}, + want: nil, + }, + { + name: "roles claim without an IAM role, skipped", + provider: &query.IDPTemplate{Type: domain.IDPTypeZitadel, ZitadelIDPTemplate: configured}, + externalUser: &domain.ExternalUser{ProjectRoles: map[string]map[string]string{"OTHER_ROLE": {"orgID1": "org1.example.com"}}}, + want: nil, + }, + { + name: "support role present but empty org set, skipped", + provider: &query.IDPTemplate{Type: domain.IDPTypeZitadel, ZitadelIDPTemplate: configured}, + externalUser: &domain.ExternalUser{ProjectRoles: map[string]map[string]string{domain.RoleIAMOwnerViewer: {}}}, + want: nil, + }, + { + name: "support role claim for a non-configured org, skipped", + provider: &query.IDPTemplate{Type: domain.IDPTypeZitadel, ZitadelIDPTemplate: configured}, + externalUser: &domain.ExternalUser{ProjectRoles: map[string]map[string]string{domain.RoleIAMOwnerViewer: {"orgID1": "evil.example.com"}}}, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, supportUserInstanceRoles(tt.provider, tt.externalUser)) + }) + } +} diff --git a/internal/command/instance_member.go b/internal/command/instance_member.go index 18a7e84f6..81953bc26 100644 --- a/internal/command/instance_member.go +++ b/internal/command/instance_member.go @@ -75,11 +75,88 @@ type AddInstanceMember struct { Roles []string } +// AddInstanceMember adds a user as a member of the instance with the given roles. func (c *Commands) AddInstanceMember(ctx context.Context, member *AddInstanceMember) (*domain.ObjectDetails, error) { - instanceAgg := instance.NewAggregate(member.InstanceID) if err := c.checkPermissionUpdateInstanceMember(ctx, member.InstanceID); err != nil { return nil, err } + return c.addInstanceMember(ctx, member) +} + +// EnsureInstanceMemberRolesFromLogin makes sure the user holds the given instance member roles +// during the login v1 ZITADEL-IdP flow. +// It adds the membership if the user is not a member yet and otherwise adds the missing roles +// to the existing membership. Roles the user already holds are never removed. +// +// It intentionally bypasses the standard instance-member permission check. +// Authorization is established by the validated `urn:zitadel:iam:org:project:roles` token claim +// and the configured InstanceRolesInfo for the Zitadel provider. +// Do not use it for any user-initiated API path. +func (c *Commands) EnsureInstanceMemberRolesFromLogin(ctx context.Context, member *AddInstanceMember) (*domain.ObjectDetails, error) { + if member.InstanceID == "" || member.UserID == "" { + return nil, zerrors.ThrowInvalidArgument(nil, "INSTA-Ee7ai", "Errors.Instance.MemberInvalid") + } + granted := configuredRolesOnly(member.Roles, c.zitadelRoles) + if len(granted) == 0 { + return nil, zerrors.ThrowInvalidArgument(nil, "INSTA-oo0Ai", "Errors.Instance.MemberInvalid") + } + existingMember, err := c.instanceMemberWriteModelByID(ctx, member.InstanceID, member.UserID) + if err != nil { + return nil, err + } + if !existingMember.State.Exists() { + return c.addInstanceMember(ctx, &AddInstanceMember{ + InstanceID: member.InstanceID, + UserID: member.UserID, + Roles: granted, + }) + } + // the roles are merged so that roles granted elsewhere are retained. + roles := missingRolesAdded(existingMember.Roles, granted) + if len(roles) == len(existingMember.Roles) { + return writeModelToObjectDetails(&existingMember.WriteModel), nil + } + pushedEvents, err := c.eventstore.Push(ctx, + instance.NewMemberChangedEvent(ctx, + InstanceAggregateFromWriteModel(&existingMember.WriteModel), + member.UserID, + roles..., + ), + ) + if err != nil { + return nil, err + } + if err = AppendAndReduce(existingMember, pushedEvents...); err != nil { + return nil, err + } + return writeModelToObjectDetails(&existingMember.WriteModel), nil +} + +// configuredRolesOnly returns the subset of roles that are configured as instance roles. +// The roles of a ZITADEL provider's claim originate from an external +// instance, whose role set may differ, so unknown roles are dropped. +func configuredRolesOnly(roles []string, zitadelRoles []authz.RoleMapping) []string { + invalid := domain.CheckForInvalidRoles(roles, domain.IAMRolePrefix, zitadelRoles) + return slices.DeleteFunc(slices.Clone(roles), func(role string) bool { + return slices.Contains(invalid, role) + }) +} + +// missingRolesAdded returns the existing roles with new roles to be added that are not already +// present, preserving the order of the existing roles. +func missingRolesAdded(existing, add []string) []string { + roles := slices.Clone(existing) + for _, role := range add { + if !slices.Contains(roles, role) { + roles = append(roles, role) + } + } + return roles +} + +// addInstanceMember performs the write without any permission check. +func (c *Commands) addInstanceMember(ctx context.Context, member *AddInstanceMember) (*domain.ObjectDetails, error) { + instanceAgg := instance.NewAggregate(member.InstanceID) //nolint:staticcheck cmds, err := preparation.PrepareCommands(ctx, c.eventstore.Filter, c.AddInstanceMemberCommand(instanceAgg, member.UserID, member.Roles...)) if err != nil { diff --git a/internal/command/instance_member_test.go b/internal/command/instance_member_test.go index 264e7d16a..d3daa9e69 100644 --- a/internal/command/instance_member_test.go +++ b/internal/command/instance_member_test.go @@ -276,6 +276,261 @@ func TestCommandSide_AddInstanceMember(t *testing.T) { } } +func TestCommandSide_EnsureInstanceMemberRolesFromLogin(t *testing.T) { + type fields struct { + eventstore func(t *testing.T) *eventstore.Eventstore + zitadelRoles []authz.RoleMapping + checkPermission domain.PermissionCheck + } + type args struct { + member *AddInstanceMember + } + type res struct { + want *domain.ObjectDetails + err func(error) bool + } + tests := []struct { + name string + fields fields + args args + res res + }{ + { + name: "no roles, invalid argument error", + fields: fields{ + eventstore: expectEventstore(), + checkPermission: newMockPermissionCheckNotAllowed(), + }, + args: args{ + member: &AddInstanceMember{ + InstanceID: "INSTANCE", + UserID: "user1", + }, + }, + res: res{ + err: zerrors.IsErrorInvalidArgument, + }, + }, + { + name: "member not existing, added, bypasses permission check", + fields: fields{ + eventstore: expectEventstore( + expectFilter(), + expectFilter( + eventFromEventPusherWithInstanceID( + "INSTANCE", + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username1", + "firstname1", + "lastname1", + "nickname1", + "displayname1", + language.German, + domain.GenderMale, + "email1", + true, + ), + ), + ), + expectFilter(), + expectPush( + instance.NewMemberAddedEvent(context.Background(), + &instance.NewAggregate("INSTANCE").Aggregate, + "user1", + []string{"IAM_OWNER_VIEWER"}..., + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + zitadelRoles: []authz.RoleMapping{ + { + Role: "IAM_OWNER_VIEWER", + }, + }, + }, + args: args{ + member: &AddInstanceMember{ + InstanceID: "INSTANCE", + UserID: "user1", + Roles: []string{"IAM_OWNER_VIEWER"}, + }, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "INSTANCE", + }, + }, + }, + { + name: "member existing without the role, role added", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + instance.NewMemberAddedEvent(context.Background(), + &instance.NewAggregate("INSTANCE").Aggregate, + "user1", + []string{"IAM_LOGIN_CLIENT"}..., + ), + ), + ), + expectPush( + instance.NewMemberChangedEvent(context.Background(), + &instance.NewAggregate("INSTANCE").Aggregate, + "user1", + []string{"IAM_LOGIN_CLIENT", "IAM_OWNER_VIEWER"}..., + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + zitadelRoles: []authz.RoleMapping{ + { + Role: "IAM_LOGIN_CLIENT", + }, + { + Role: "IAM_OWNER_VIEWER", + }, + }, + }, + args: args{ + member: &AddInstanceMember{ + InstanceID: "INSTANCE", + UserID: "user1", + Roles: []string{"IAM_OWNER_VIEWER"}, + }, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "INSTANCE", + }, + }, + }, + { + name: "member existing with the role, no push", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + instance.NewMemberAddedEvent(context.Background(), + &instance.NewAggregate("INSTANCE").Aggregate, + "user1", + []string{"IAM_OWNER", "IAM_OWNER_VIEWER"}..., + ), + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + zitadelRoles: []authz.RoleMapping{ + { + Role: "IAM_OWNER", + }, + { + Role: "IAM_OWNER_VIEWER", + }, + }, + }, + args: args{ + member: &AddInstanceMember{ + InstanceID: "INSTANCE", + UserID: "user1", + Roles: []string{"IAM_OWNER_VIEWER"}, + }, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "INSTANCE", + }, + }, + }, + { + name: "no role configured on instance, invalid argument error", + fields: fields{ + eventstore: expectEventstore(), + checkPermission: newMockPermissionCheckNotAllowed(), + zitadelRoles: []authz.RoleMapping{ + { + Role: "IAM_OWNER", + }, + }, + }, + args: args{ + member: &AddInstanceMember{ + InstanceID: "INSTANCE", + UserID: "user1", + Roles: []string{"IAM_OWNER_VIEWER"}, + }, + }, + res: res{ + err: zerrors.IsErrorInvalidArgument, + }, + }, + { + name: "role not configured on instance, dropped from the granted roles", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + instance.NewMemberAddedEvent(context.Background(), + &instance.NewAggregate("INSTANCE").Aggregate, + "user1", + []string{"IAM_LOGIN_CLIENT"}..., + ), + ), + ), + expectPush( + instance.NewMemberChangedEvent(context.Background(), + &instance.NewAggregate("INSTANCE").Aggregate, + "user1", + []string{"IAM_LOGIN_CLIENT", "IAM_OWNER_VIEWER"}..., + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + zitadelRoles: []authz.RoleMapping{ + { + Role: "IAM_LOGIN_CLIENT", + }, + { + Role: "IAM_OWNER_VIEWER", + }, + }, + }, + args: args{ + member: &AddInstanceMember{ + InstanceID: "INSTANCE", + UserID: "user1", + Roles: []string{"IAM_OWNER_VIEWER", "IAM_ROLE_OF_OTHER_INSTANCE"}, + }, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "INSTANCE", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := &Commands{ + eventstore: tt.fields.eventstore(t), + zitadelRoles: tt.fields.zitadelRoles, + checkPermission: tt.fields.checkPermission, + } + got, err := r.EnsureInstanceMemberRolesFromLogin(context.Background(), tt.args.member) + if tt.res.err == nil { + assert.NoError(t, err) + } + if tt.res.err != nil && !tt.res.err(err) { + t.Errorf("got wrong err: %v ", err) + } + if tt.res.err == nil { + assertObjectDetails(t, tt.res.want, got) + } + }) + } +} + func TestCommandSide_ChangeInstanceMember(t *testing.T) { type fields struct { eventstore func(t *testing.T) *eventstore.Eventstore diff --git a/internal/command/org_idp.go b/internal/command/org_idp.go index b41b16f8e..e237066b4 100644 --- a/internal/command/org_idp.go +++ b/internal/command/org_idp.go @@ -2049,15 +2049,12 @@ func (c *Commands) validateOrgZitadelProvider(provider *ZitadelProvider, create if create && provider.ClientSecret == "" { return zerrors.ThrowInvalidArgument(nil, "ORG-DxaSb9", "Errors.Invalid.Argument") } - for i := range provider.InstanceRolesInfo { - provider.InstanceRolesInfo[i].OrganizationID = strings.TrimSpace(provider.InstanceRolesInfo[i].OrganizationID) - if provider.InstanceRolesInfo[i].OrganizationID == "" { - return zerrors.ThrowInvalidArgument(nil, "ORG-M0QmjB", "Errors.Invalid.Argument") - } - provider.InstanceRolesInfo[i].OrganizationDomain = strings.TrimSpace(provider.InstanceRolesInfo[i].OrganizationDomain) - if provider.InstanceRolesInfo[i].OrganizationDomain == "" { - return zerrors.ThrowInvalidArgument(nil, "ORG-HAELEz", "Errors.Invalid.Argument") - } + // instance_roles_info must never be set on an organization-scoped Zitadel provider: + // it drives instance-wide role grants on login, and honoring it here would let an + // org owner escalate themselves to instance-level roles. Only the instance-scoped + // Admin API may configure it. + if len(provider.InstanceRolesInfo) > 0 { + return zerrors.ThrowInvalidArgument(nil, "ORG-ImOVF3", "Errors.Invalid.Argument") } return nil } diff --git a/internal/command/org_idp_test.go b/internal/command/org_idp_test.go index 436939616..02bbf59be 100644 --- a/internal/command/org_idp_test.go +++ b/internal/command/org_idp_test.go @@ -6167,66 +6167,6 @@ func TestCommandSide_AddOrgZitadelIDP(t *testing.T) { want: &domain.ObjectDetails{ResourceOwner: "org1"}, }, }, - { - name: "ok", - fields: fields{ - eventstore: expectEventstore( - expectFilter(), - expectPush( - org.NewZitadelIDPAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, - "id1", - "name", - "issuer", - "clientID", - &crypto.CryptoValue{ - CryptoType: crypto.TypeEncryption, - Algorithm: "enc", - KeyID: "id", - Crypted: []byte("clientSecret"), - }, - nil, - idp.Options{}, - []idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "example-org1.com", - }, - { - OrganizationID: "org2", - OrganizationDomain: "example-org2.com", - }, - }, - ), - ), - ), - idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "id1"), - secretCrypto: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), - }, - args: args{ - ctx: context.Background(), - resourceOwner: "org1", - provider: ZitadelProvider{ - Name: "name", - Issuer: "issuer", - ClientID: "clientID", - ClientSecret: "clientSecret", - InstanceRolesInfo: []idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "example-org1.com", - }, - { - OrganizationID: "org2", - OrganizationDomain: "example-org2.com", - }, - }, - }, - }, - res: res{ - id: "id1", - want: &domain.ObjectDetails{ResourceOwner: "org1"}, - }, - }, { name: "ok all set", fields: fields{ @@ -6251,16 +6191,7 @@ func TestCommandSide_AddOrgZitadelIDP(t *testing.T) { IsAutoCreation: true, IsAutoUpdate: true, }, - []idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "example-org1.com", - }, - { - OrganizationID: "org2", - OrganizationDomain: "example-org2.com", - }, - }, + nil, ), ), ), @@ -6282,16 +6213,6 @@ func TestCommandSide_AddOrgZitadelIDP(t *testing.T) { IsAutoCreation: true, IsAutoUpdate: true, }, - InstanceRolesInfo: []idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "example-org1.com", - }, - { - OrganizationID: "org2", - OrganizationDomain: "example-org2.com", - }, - }, }, }, res: res{ @@ -6299,6 +6220,34 @@ func TestCommandSide_AddOrgZitadelIDP(t *testing.T) { want: &domain.ObjectDetails{ResourceOwner: "org1"}, }, }, + { + name: "instance roles info not allowed", + fields: fields{ + eventstore: expectEventstore(), + idGenerator: id_mock.NewIDGeneratorExpectIDs(t, "id1"), + }, + args: args{ + ctx: context.Background(), + resourceOwner: "org1", + provider: ZitadelProvider{ + Name: "name", + Issuer: "issuer", + ClientID: "clientID", + ClientSecret: "clientSecret", + InstanceRolesInfo: []idp.RolesInfo{ + { + OrganizationID: "org1", + OrganizationDomain: "example-org1.com", + }, + }, + }, + }, + res: res{ + err: func(err error) bool { + return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "ORG-ImOVF3", "Errors.Invalid.Argument")) + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -6504,16 +6453,7 @@ func TestCommands_UpdateOrgZitadelIDP(t *testing.T) { }, []string{openid.ScopeOpenID, openid.ScopeEmail}, idp.Options{}, - []idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "example-org1.com", - }, - { - OrganizationID: "org2", - OrganizationDomain: "example-org2.com", - }, - }, + nil, ), ), ), @@ -6527,20 +6467,36 @@ func TestCommands_UpdateOrgZitadelIDP(t *testing.T) { Issuer: "issuer", ClientID: "clientID", Scopes: []string{openid.ScopeOpenID, openid.ScopeEmail}, + }, + }, + res: res{ + want: &domain.ObjectDetails{ResourceOwner: "org1"}, + }, + }, + { + name: "instance roles info not allowed", + fields: fields{ + eventstore: expectEventstore(), + }, + args: args{ + id: "idp-id", + resourceOwner: "org1", + provider: ZitadelProvider{ + Name: "name", + Issuer: "issuer", + ClientID: "clientID", InstanceRolesInfo: []idp.RolesInfo{ { OrganizationID: "org1", OrganizationDomain: "example-org1.com", }, - { - OrganizationID: "org2", - OrganizationDomain: "example-org2.com", - }, }, }, }, res: res{ - want: &domain.ObjectDetails{ResourceOwner: "org1"}, + err: func(err error) bool { + return errors.Is(err, zerrors.ThrowInvalidArgument(nil, "ORG-ImOVF3", "Errors.Invalid.Argument")) + }, }, }, { @@ -6562,12 +6518,7 @@ func TestCommands_UpdateOrgZitadelIDP(t *testing.T) { }, []string{openid.ScopeOpenID, openid.ScopeEmail}, idp.Options{}, - []idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "example-org1.com", - }, - }, + nil, )), ), expectPush( @@ -6592,16 +6543,6 @@ func TestCommands_UpdateOrgZitadelIDP(t *testing.T) { IsAutoCreation: &t, IsAutoUpdate: &t, }), - idp.ChangeZitadelIDPInstanceRolesInfo([]idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "example-org1.com", - }, - { - OrganizationID: "org2", - OrganizationDomain: "example-org2.com", - }, - }), }, ) return event @@ -6625,16 +6566,6 @@ func TestCommands_UpdateOrgZitadelIDP(t *testing.T) { IsAutoCreation: true, IsAutoUpdate: true, }, - InstanceRolesInfo: []idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "example-org1.com", - }, - { - OrganizationID: "org2", - OrganizationDomain: "example-org2.com", - }, - }, }, }, res: res{ @@ -6660,12 +6591,7 @@ func TestCommands_UpdateOrgZitadelIDP(t *testing.T) { }, []string{openid.ScopeOpenID, openid.ScopeEmail}, idp.Options{}, - []idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "example-org1.com", - }, - }, + nil, )), ), expectPush( @@ -6690,16 +6616,6 @@ func TestCommands_UpdateOrgZitadelIDP(t *testing.T) { IsAutoCreation: &t, IsAutoUpdate: &t, }), - idp.ChangeZitadelIDPInstanceRolesInfo([]idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "example-org1.com", - }, - { - OrganizationID: "org2", - OrganizationDomain: "example-org2.com", - }, - }), }, ) return event @@ -6723,16 +6639,6 @@ func TestCommands_UpdateOrgZitadelIDP(t *testing.T) { IsAutoCreation: true, IsAutoUpdate: true, }, - InstanceRolesInfo: []idp.RolesInfo{ - { - OrganizationID: "org1", - OrganizationDomain: "example-org1.com", - }, - { - OrganizationID: "org2", - OrganizationDomain: "example-org2.com", - }, - }, }, }, res: res{ diff --git a/internal/domain/auth_request.go b/internal/domain/auth_request.go index cd6594a30..2052ff807 100644 --- a/internal/domain/auth_request.go +++ b/internal/domain/auth_request.go @@ -104,6 +104,11 @@ type ExternalUser struct { Phone PhoneNumber IsPhoneVerified bool Metadatas []*Metadata + // ProjectRoles holds the ZITADEL `urn:zitadel:iam:org:project:roles` claim + // ({role: {orgID: orgDomain}}) as returned by a ZITADEL provider. It is + // captured at authentication and carried through to user creation, where it + // is used to grant the support-user instance membership. + ProjectRoles map[string]map[string]string } type Prompt int32 diff --git a/internal/domain/roles.go b/internal/domain/roles.go index 2cebd26d3..d3f7fc52d 100644 --- a/internal/domain/roles.go +++ b/internal/domain/roles.go @@ -14,6 +14,7 @@ const ( RoleOrgOwner = "ORG_OWNER" RoleOrgProjectCreator = "ORG_PROJECT_CREATOR" RoleIAMOwner = "IAM_OWNER" + RoleIAMOwnerViewer = "IAM_OWNER_VIEWER" RoleIAMLoginClient = "IAM_LOGIN_CLIENT" RoleProjectOwner = "PROJECT_OWNER" RoleProjectOwnerGlobal = "PROJECT_OWNER_GLOBAL" diff --git a/internal/integration/client.go b/internal/integration/client.go index 943ea2772..3615ca9d1 100644 --- a/internal/integration/client.go +++ b/internal/integration/client.go @@ -783,12 +783,6 @@ func (i *Instance) AddOrgZitadelProvider(ctx context.Context, name string) *mgmt ProviderOptions: &idp.Options{ IsCreationAllowed: true, }, - InstanceRolesInfo: []*idp.InstanceRolesInfo{ - { - OrganizationId: "org1", - OrganizationDomain: "org1.com", - }, - }, }) logging.OnError(err).Panic("create org zitadel idp") return resp diff --git a/proto/zitadel/management.proto b/proto/zitadel/management.proto index 6ef4603fa..1bd4c7f81 100644 --- a/proto/zitadel/management.proto +++ b/proto/zitadel/management.proto @@ -14128,9 +14128,10 @@ message AddZitadelProviderRequest { ]; zitadel.idp.v1.Options provider_options = 6; - // Optionally specify the details (organization_id, organization_domain) related to instance roles. - // It is used to determine if a user should be assigned instance administrator roles after authenticating with the provider. - repeated zitadel.idp.v1.InstanceRolesInfo instance_roles_info = 7 [(validate.rules).repeated = {max_items: 20}]; + // field 7 previously held instance_roles_info, which was removed as it allowed + // org-scoped providers to grant instance-wide role memberships on login. + reserved 7; + reserved "instance_roles_info"; } message AddZitadelProviderResponse { @@ -14182,9 +14183,10 @@ message UpdateZitadelProviderRequest { ]; zitadel.idp.v1.Options provider_options = 7; - // Optionally specify the details (organization_id, organization_domain) related to instance roles. - // It is used to determine if a user should be assigned instance administrator roles after authenticating with the provider. - repeated zitadel.idp.v1.InstanceRolesInfo instance_roles_info = 8 [(validate.rules).repeated = {max_items: 20}]; + // field 8 previously held instance_roles_info, which was removed as it allowed + // org-scoped providers to grant instance-wide role memberships on login. + reserved 8; + reserved "instance_roles_info"; } message UpdateZitadelProviderResponse {