andwati/zitadel
1
0
Fork 0
mirror of https://github.com/andwati/zitadel.git synced 2026-10-21 23:14:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: handle Zitadel provider IdP login via login v1 (#12469)

Browse Source 
# 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
This commit is contained in:
gayathri
2026-07-28 15:20:46 +02:00
committed by GitHub
parent 318acd4bc3
commit ffd9b39bdb
14 changed files with 908 additions and 492 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/command/instance_member.go
+78 -1
View File
@@ -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 {
internal/command/instance_member_test.go
+255
View File
@@ -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
internal/command/org_idp.go
+6 -9
View File
@@ -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
}
internal/command/org_idp_test.go
+53 -147
View File
@@ -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{