From 563f74640ec62193304b2a7c81ae5879539858a8 Mon Sep 17 00:00:00 2001 From: Lars Date: Thu, 30 Jan 2025 16:43:13 +0100 Subject: [PATCH] fix: scim v2 endpoints enforce user resource owner (#9273) # Which Problems Are Solved - If a SCIM endpoint is called with an orgID in the URL that is not the resource owner, no error is returned, and the action is executed. # How the Problems Are Solved - The orgID provided in the SCIM URL path must match the resource owner of the target user. Otherwise, an error will be returned. # Additional Context Part of https://github.com/zitadel/zitadel/issues/8140 --- internal/api/grpc/user/v2/user.go | 2 +- internal/api/grpc/user/v2beta/user.go | 2 +- .../api/scim/integration_test/scim_test.go | 12 +++++- .../integration_test/users_create_test.go | 24 +++++++---- .../integration_test/users_delete_test.go | 26 +++++++---- .../scim/integration_test/users_get_test.go | 31 ++++++++----- .../integration_test/users_replace_test.go | 43 ++++++++++++++----- .../integration_test/users_update_test.go | 16 ++++--- internal/api/scim/resources/user.go | 10 +++-- internal/api/scim/resources/user_mapping.go | 6 ++- internal/command/user_v2.go | 8 ++-- internal/command/user_v2_human.go | 18 ++++---- internal/command/user_v2_model_test.go | 14 +++--- internal/command/user_v2_test.go | 2 +- internal/query/user.go | 5 +++ internal/query/user_by_id.sql | 12 +++--- 16 files changed, 153 insertions(+), 78 deletions(-) diff --git a/internal/api/grpc/user/v2/user.go b/internal/api/grpc/user/v2/user.go index 10b21e8918..9a092afacf 100644 --- a/internal/api/grpc/user/v2/user.go +++ b/internal/api/grpc/user/v2/user.go @@ -275,7 +275,7 @@ func (s *Server) DeleteUser(ctx context.Context, req *user.DeleteUserRequest) (_ if err != nil { return nil, err } - details, err := s.command.RemoveUserV2(ctx, req.UserId, memberships, grants...) + details, err := s.command.RemoveUserV2(ctx, req.UserId, "", memberships, grants...) if err != nil { return nil, err } diff --git a/internal/api/grpc/user/v2beta/user.go b/internal/api/grpc/user/v2beta/user.go index 274c1d867f..52da057906 100644 --- a/internal/api/grpc/user/v2beta/user.go +++ b/internal/api/grpc/user/v2beta/user.go @@ -278,7 +278,7 @@ func (s *Server) DeleteUser(ctx context.Context, req *user.DeleteUserRequest) (_ if err != nil { return nil, err } - details, err := s.command.RemoveUserV2(ctx, req.UserId, memberships, grants...) + details, err := s.command.RemoveUserV2(ctx, req.UserId, "", memberships, grants...) if err != nil { return nil, err } diff --git a/internal/api/scim/integration_test/scim_test.go b/internal/api/scim/integration_test/scim_test.go index 7980a34337..7b16671f1f 100644 --- a/internal/api/scim/integration_test/scim_test.go +++ b/internal/api/scim/integration_test/scim_test.go @@ -9,12 +9,16 @@ import ( "testing" "time" + "github.com/brianvoe/gofakeit/v6" + "github.com/zitadel/zitadel/internal/integration" + "github.com/zitadel/zitadel/pkg/grpc/org/v2" ) var ( - Instance *integration.Instance - CTX context.Context + Instance *integration.Instance + SecondaryOrganization *org.AddOrganizationResponse + CTX context.Context // remove comments in the json, as the default golang json unmarshaler cannot handle them // some test files (e.g. bulk, patch) are much easier to maintain with comments @@ -29,6 +33,10 @@ func TestMain(m *testing.M) { Instance = integration.NewInstance(ctx) CTX = Instance.WithAuthorization(ctx, integration.UserTypeOrgOwner) + + iamOwnerCtx := Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) + SecondaryOrganization = Instance.CreateOrganization(iamOwnerCtx, gofakeit.Name(), gofakeit.Email()) + return m.Run() }()) } diff --git a/internal/api/scim/integration_test/users_create_test.go b/internal/api/scim/integration_test/users_create_test.go index 7bc9fe0348..bebb7fd00e 100644 --- a/internal/api/scim/integration_test/users_create_test.go +++ b/internal/api/scim/integration_test/users_create_test.go @@ -10,7 +10,6 @@ import ( "testing" "time" - "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -164,6 +163,7 @@ func TestCreateUser(t *testing.T) { name string body []byte ctx context.Context + orgID string want *resources.ScimUser wantErr bool scimErrorType string @@ -275,6 +275,13 @@ func TestCreateUser(t *testing.T) { wantErr: true, errorStatus: http.StatusNotFound, }, + { + name: "another org", + body: minimalUserJson, + orgID: SecondaryOrganization.OrganizationId, + wantErr: true, + errorStatus: http.StatusNotFound, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -283,7 +290,12 @@ func TestCreateUser(t *testing.T) { ctx = CTX } - createdUser, err := Instance.Client.SCIM.Users.Create(ctx, Instance.DefaultOrg.Id, tt.body) + orgID := tt.orgID + if orgID == "" { + orgID = Instance.DefaultOrg.Id + } + + createdUser, err := Instance.Client.SCIM.Users.Create(ctx, orgID, tt.body) if (err != nil) != tt.wantErr { t.Errorf("CreateUser() error = %v, wantErr %v", err, tt.wantErr) return @@ -311,7 +323,7 @@ func TestCreateUser(t *testing.T) { assert.EqualValues(t, []schemas.ScimSchemaType{"urn:ietf:params:scim:schemas:core:2.0:User"}, createdUser.Resource.Schemas) assert.Equal(t, schemas.ScimResourceTypeSingular("User"), createdUser.Resource.Meta.ResourceType) - assert.Equal(t, "http://"+Instance.Host()+path.Join(schemas.HandlerPrefix, Instance.DefaultOrg.Id, "Users", createdUser.ID), createdUser.Resource.Meta.Location) + assert.Equal(t, "http://"+Instance.Host()+path.Join(schemas.HandlerPrefix, orgID, "Users", createdUser.ID), createdUser.Resource.Meta.Location) assert.Nil(t, createdUser.Password) if tt.want != nil { @@ -423,9 +435,3 @@ func TestCreateUser_scopedExternalID(t *testing.T) { assert.Equal(tt, "701984", string(md.Metadata.Value)) }, retryDuration, tick) } - -func TestCreateUser_anotherOrg(t *testing.T) { - org := Instance.CreateOrganization(Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner), gofakeit.Name(), gofakeit.Email()) - _, err := Instance.Client.SCIM.Users.Create(CTX, org.OrganizationId, fullUserJson) - scim.RequireScimError(t, http.StatusNotFound, err) -} diff --git a/internal/api/scim/integration_test/users_delete_test.go b/internal/api/scim/integration_test/users_delete_test.go index bfdd0eae88..86e88f46c4 100644 --- a/internal/api/scim/integration_test/users_delete_test.go +++ b/internal/api/scim/integration_test/users_delete_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - "github.com/brianvoe/gofakeit/v6" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" @@ -22,6 +21,7 @@ func TestDeleteUser_errors(t *testing.T) { tests := []struct { name string ctx context.Context + orgID string errorStatus int }{ { @@ -38,6 +38,17 @@ func TestDeleteUser_errors(t *testing.T) { name: "unknown user id", errorStatus: http.StatusNotFound, }, + { + name: "another org", + orgID: SecondaryOrganization.OrganizationId, + errorStatus: http.StatusNotFound, + }, + { + name: "another org with permissions", + orgID: SecondaryOrganization.OrganizationId, + ctx: Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner), + errorStatus: http.StatusNotFound, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -46,7 +57,11 @@ func TestDeleteUser_errors(t *testing.T) { ctx = CTX } - err := Instance.Client.SCIM.Users.Delete(ctx, Instance.DefaultOrg.Id, "1") + orgID := tt.orgID + if orgID == "" { + orgID = Instance.DefaultOrg.Id + } + err := Instance.Client.SCIM.Users.Delete(ctx, orgID, "1") statusCode := tt.errorStatus if statusCode == 0 { @@ -81,10 +96,3 @@ func TestDeleteUser_ensureReallyDeleted(t *testing.T) { integration.AssertGrpcStatus(tt, codes.NotFound, err) }, retryDuration, tick) } - -func TestDeleteUser_anotherOrg(t *testing.T) { - createUserResp := Instance.CreateHumanUser(CTX) - org := Instance.CreateOrganization(Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner), gofakeit.Name(), gofakeit.Email()) - err := Instance.Client.SCIM.Users.Delete(CTX, org.OrganizationId, createUserResp.UserId) - scim.RequireScimError(t, http.StatusNotFound, err) -} diff --git a/internal/api/scim/integration_test/users_get_test.go b/internal/api/scim/integration_test/users_get_test.go index 7984b5d63b..76e59c373d 100644 --- a/internal/api/scim/integration_test/users_get_test.go +++ b/internal/api/scim/integration_test/users_get_test.go @@ -9,7 +9,6 @@ import ( "testing" "time" - "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -27,6 +26,7 @@ import ( func TestGetUser(t *testing.T) { tests := []struct { name string + orgID string buildUserID func() string cleanup func(userID string) ctx context.Context @@ -46,6 +46,19 @@ func TestGetUser(t *testing.T) { errorStatus: http.StatusNotFound, wantErr: true, }, + { + name: "another org", + orgID: SecondaryOrganization.OrganizationId, + errorStatus: http.StatusNotFound, + wantErr: true, + }, + { + name: "another org with permissions", + orgID: SecondaryOrganization.OrganizationId, + ctx: Instance.WithAuthorization(CTX, integration.UserTypeNoPermission), + errorStatus: http.StatusNotFound, + wantErr: true, + }, { name: "unknown user id", buildUserID: func() string { @@ -237,11 +250,16 @@ func TestGetUser(t *testing.T) { userID = createUserResp.UserId } + orgID := tt.orgID + if orgID == "" { + orgID = Instance.DefaultOrg.Id + } + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(CTX, time.Minute) var fetchedUser *resources.ScimUser var err error require.EventuallyWithT(t, func(ttt *assert.CollectT) { - fetchedUser, err = Instance.Client.SCIM.Users.Get(ctx, Instance.DefaultOrg.Id, userID) + fetchedUser, err = Instance.Client.SCIM.Users.Get(ctx, orgID, userID) if tt.wantErr { statusCode := tt.errorStatus if statusCode == 0 { @@ -255,7 +273,7 @@ func TestGetUser(t *testing.T) { assert.Equal(ttt, userID, fetchedUser.ID) assert.EqualValues(ttt, []schemas.ScimSchemaType{"urn:ietf:params:scim:schemas:core:2.0:User"}, fetchedUser.Schemas) assert.Equal(ttt, schemas.ScimResourceTypeSingular("User"), fetchedUser.Resource.Meta.ResourceType) - assert.Equal(ttt, "http://"+Instance.Host()+path.Join(schemas.HandlerPrefix, Instance.DefaultOrg.Id, "Users", fetchedUser.ID), fetchedUser.Resource.Meta.Location) + assert.Equal(ttt, "http://"+Instance.Host()+path.Join(schemas.HandlerPrefix, orgID, "Users", fetchedUser.ID), fetchedUser.Resource.Meta.Location) assert.Nil(ttt, fetchedUser.Password) if !test.PartiallyDeepEqual(tt.want, fetchedUser) { ttt.Errorf("GetUser() got = %#v, want %#v", fetchedUser, tt.want) @@ -268,10 +286,3 @@ func TestGetUser(t *testing.T) { }) } } - -func TestGetUser_anotherOrg(t *testing.T) { - createUserResp := Instance.CreateHumanUser(CTX) - org := Instance.CreateOrganization(Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner), gofakeit.Name(), gofakeit.Email()) - _, err := Instance.Client.SCIM.Users.Get(CTX, org.OrganizationId, createUserResp.UserId) - scim.RequireScimError(t, http.StatusNotFound, err) -} diff --git a/internal/api/scim/integration_test/users_replace_test.go b/internal/api/scim/integration_test/users_replace_test.go index 0eeaf76e22..bd4f34cef7 100644 --- a/internal/api/scim/integration_test/users_replace_test.go +++ b/internal/api/scim/integration_test/users_replace_test.go @@ -37,14 +37,16 @@ var ( func TestReplaceUser(t *testing.T) { tests := []struct { - name string - body []byte - ctx context.Context - want *resources.ScimUser - wantErr bool - scimErrorType string - errorStatus int - zitadelErrID string + name string + body []byte + ctx context.Context + createUserOrgID string + replaceUserOrgID string + want *resources.ScimUser + wantErr bool + scimErrorType string + errorStatus int + zitadelErrID string }{ { name: "minimal user", @@ -207,10 +209,26 @@ func TestReplaceUser(t *testing.T) { wantErr: true, errorStatus: http.StatusNotFound, }, + { + name: "another org", + body: minimalUserJson, + replaceUserOrgID: SecondaryOrganization.OrganizationId, + wantErr: true, + errorStatus: http.StatusNotFound, + }, + { + name: "another org with permissions", + body: minimalUserJson, + replaceUserOrgID: SecondaryOrganization.OrganizationId, + ctx: Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner), + wantErr: true, + errorStatus: http.StatusNotFound, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - createdUser, err := Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, fullUserJson) + // use iam owner => we don't want to test permissions of the create endpoint. + createdUser, err := Instance.Client.SCIM.Users.Create(Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner), Instance.DefaultOrg.Id, fullUserJson) require.NoError(t, err) defer func() { @@ -223,7 +241,12 @@ func TestReplaceUser(t *testing.T) { ctx = CTX } - replacedUser, err := Instance.Client.SCIM.Users.Replace(ctx, Instance.DefaultOrg.Id, createdUser.ID, tt.body) + replaceUserOrgID := tt.replaceUserOrgID + if replaceUserOrgID == "" { + replaceUserOrgID = Instance.DefaultOrg.Id + } + + replacedUser, err := Instance.Client.SCIM.Users.Replace(ctx, replaceUserOrgID, createdUser.ID, tt.body) if (err != nil) != tt.wantErr { t.Errorf("ReplaceUser() error = %v, wantErr %v", err, tt.wantErr) } diff --git a/internal/api/scim/integration_test/users_update_test.go b/internal/api/scim/integration_test/users_update_test.go index f9a807c357..8334fad2e5 100644 --- a/internal/api/scim/integration_test/users_update_test.go +++ b/internal/api/scim/integration_test/users_update_test.go @@ -10,7 +10,6 @@ import ( "testing" "time" - "github.com/brianvoe/gofakeit/v6" "github.com/muhlemmer/gu" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -28,7 +27,7 @@ var ( //go:embed testdata/users_update_test_full.json fullUserUpdateJson []byte - minimalUserUpdateJson = simpleReplacePatchBody("nickname", "foo") + minimalUserUpdateJson = simpleReplacePatchBody("nickname", "\"foo\"") ) func init() { @@ -44,9 +43,6 @@ func TestUpdateUser(t *testing.T) { require.NoError(t, err) }() - iamOwnerCtx := Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner) - secondaryOrg := Instance.CreateOrganization(iamOwnerCtx, gofakeit.Name(), gofakeit.Email()) - tests := []struct { name string body []byte @@ -74,7 +70,15 @@ func TestUpdateUser(t *testing.T) { }, { name: "other org", - orgID: secondaryOrg.OrganizationId, + orgID: SecondaryOrganization.OrganizationId, + body: minimalUserUpdateJson, + wantErr: true, + errorStatus: http.StatusNotFound, + }, + { + name: "other org with permissions", + ctx: Instance.WithAuthorization(CTX, integration.UserTypeIAMOwner), + orgID: SecondaryOrganization.OrganizationId, body: minimalUserUpdateJson, wantErr: true, errorStatus: http.StatusNotFound, diff --git a/internal/api/scim/resources/user.go b/internal/api/scim/resources/user.go index f2a8d9cfbe..08997967d9 100644 --- a/internal/api/scim/resources/user.go +++ b/internal/api/scim/resources/user.go @@ -180,7 +180,8 @@ func (h *UsersHandler) Replace(ctx context.Context, id string, user *ScimUser) ( } func (h *UsersHandler) Update(ctx context.Context, id string, operations patch.OperationCollection) error { - userWM, err := h.command.UserHumanWriteModel(ctx, id, true, true, true, true, false, false, true) + orgID := authz.GetCtxData(ctx).OrgID + userWM, err := h.command.UserHumanWriteModel(ctx, id, orgID, true, true, true, true, false, false, true) if err != nil { return err } @@ -191,6 +192,9 @@ func (h *UsersHandler) Update(ctx context.Context, id string, operations patch.O return err } + // ensure the identity of the user is not modified + changeHuman.ID = id + changeHuman.ResourceOwner = orgID return h.command.ChangeUserHuman(ctx, changeHuman, h.userCodeAlg) } @@ -200,12 +204,12 @@ func (h *UsersHandler) Delete(ctx context.Context, id string) error { return err } - _, err = h.command.RemoveUserV2(ctx, id, memberships, grants...) + _, err = h.command.RemoveUserV2(ctx, id, authz.GetCtxData(ctx).OrgID, memberships, grants...) return err } func (h *UsersHandler) Get(ctx context.Context, id string) (*ScimUser, error) { - user, err := h.query.GetUserByID(ctx, false, id) + user, err := h.query.GetUserByIDWithResourceOwner(ctx, false, id, authz.GetCtxData(ctx).OrgID) if err != nil { return nil, err } diff --git a/internal/api/scim/resources/user_mapping.go b/internal/api/scim/resources/user_mapping.go index 4fb0940fa6..79c75ab113 100644 --- a/internal/api/scim/resources/user_mapping.go +++ b/internal/api/scim/resources/user_mapping.go @@ -9,6 +9,7 @@ import ( "github.com/zitadel/logging" "golang.org/x/text/language" + "github.com/zitadel/zitadel/internal/api/authz" "github.com/zitadel/zitadel/internal/api/scim/metadata" "github.com/zitadel/zitadel/internal/api/scim/schemas" "github.com/zitadel/zitadel/internal/command" @@ -73,8 +74,9 @@ func (h *UsersHandler) mapToAddHuman(ctx context.Context, scimUser *ScimUser) (* func (h *UsersHandler) mapToChangeHuman(ctx context.Context, scimUser *ScimUser) (*command.ChangeHuman, error) { human := &command.ChangeHuman{ - ID: scimUser.ID, - Username: &scimUser.UserName, + ID: scimUser.ID, + ResourceOwner: authz.GetCtxData(ctx).OrgID, + Username: &scimUser.UserName, Profile: &command.Profile{ NickName: &scimUser.NickName, DisplayName: &scimUser.DisplayName, diff --git a/internal/command/user_v2.go b/internal/command/user_v2.go index 033a16eb9a..00ca85aaf4 100644 --- a/internal/command/user_v2.go +++ b/internal/command/user_v2.go @@ -159,12 +159,12 @@ func (c *Commands) userStateWriteModel(ctx context.Context, userID string) (writ return writeModel, nil } -func (c *Commands) RemoveUserV2(ctx context.Context, userID string, cascadingUserMemberships []*CascadingMembership, cascadingGrantIDs ...string) (*domain.ObjectDetails, error) { +func (c *Commands) RemoveUserV2(ctx context.Context, userID, resourceOwner string, cascadingUserMemberships []*CascadingMembership, cascadingGrantIDs ...string) (*domain.ObjectDetails, error) { if userID == "" { return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-vaipl7s13l", "Errors.User.UserIDMissing") } - existingUser, err := c.userRemoveWriteModel(ctx, userID) + existingUser, err := c.userRemoveWriteModel(ctx, userID, resourceOwner) if err != nil { return nil, err } @@ -210,11 +210,11 @@ func (c *Commands) RemoveUserV2(ctx context.Context, userID string, cascadingUse return writeModelToObjectDetails(&existingUser.WriteModel), nil } -func (c *Commands) userRemoveWriteModel(ctx context.Context, userID string) (writeModel *UserV2WriteModel, err error) { +func (c *Commands) userRemoveWriteModel(ctx context.Context, userID, resourceOwner string) (writeModel *UserV2WriteModel, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() - writeModel = NewUserRemoveWriteModel(userID, "") + writeModel = NewUserRemoveWriteModel(userID, resourceOwner) err = c.eventstore.FilterToQueryReducer(ctx, writeModel) if err != nil { return nil, err diff --git a/internal/command/user_v2_human.go b/internal/command/user_v2_human.go index 72f76b6730..f88e2017d5 100644 --- a/internal/command/user_v2_human.go +++ b/internal/command/user_v2_human.go @@ -14,12 +14,13 @@ import ( ) type ChangeHuman struct { - ID string - State *domain.UserState - Username *string - Profile *Profile - Email *Email - Phone *Phone + ID string + ResourceOwner string + State *domain.UserState + Username *string + Profile *Profile + Email *Email + Phone *Phone Metadata []*domain.Metadata MetadataKeysToRemove []string @@ -267,6 +268,7 @@ func (c *Commands) ChangeUserHuman(ctx context.Context, human *ChangeHuman, alg existingHuman, err := c.UserHumanWriteModel( ctx, human.ID, + human.ResourceOwner, human.Profile != nil, human.Email != nil, human.Phone != nil, @@ -525,11 +527,11 @@ func (c *Commands) userExistsWriteModel(ctx context.Context, userID string) (wri return writeModel, nil } -func (c *Commands) UserHumanWriteModel(ctx context.Context, userID string, profileWM, emailWM, phoneWM, passwordWM, avatarWM, idpLinksWM, metadataWM bool) (writeModel *UserV2WriteModel, err error) { +func (c *Commands) UserHumanWriteModel(ctx context.Context, userID, resourceOwner string, profileWM, emailWM, phoneWM, passwordWM, avatarWM, idpLinksWM, metadataWM bool) (writeModel *UserV2WriteModel, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() - writeModel = NewUserHumanWriteModel(userID, "", profileWM, emailWM, phoneWM, passwordWM, avatarWM, idpLinksWM, metadataWM) + writeModel = NewUserHumanWriteModel(userID, resourceOwner, profileWM, emailWM, phoneWM, passwordWM, avatarWM, idpLinksWM, metadataWM) err = c.eventstore.FilterToQueryReducer(ctx, writeModel) if err != nil { return nil, err diff --git a/internal/command/user_v2_model_test.go b/internal/command/user_v2_model_test.go index 5bbc17a54a..cb12c8fcda 100644 --- a/internal/command/user_v2_model_test.go +++ b/internal/command/user_v2_model_test.go @@ -567,7 +567,7 @@ func TestCommandSide_userHumanWriteModel_profile(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, true, false, false, false, false, false, false) + wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, "", true, false, false, false, false, false, false) if tt.res.err == nil { if !assert.NoError(t, err) { t.FailNow() @@ -912,7 +912,7 @@ func TestCommandSide_userHumanWriteModel_email(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, false, true, false, false, false, false, false) + wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, "", false, true, false, false, false, false, false) if tt.res.err == nil { if !assert.NoError(t, err) { t.FailNow() @@ -1344,7 +1344,7 @@ func TestCommandSide_userHumanWriteModel_phone(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, false, false, true, false, false, false, false) + wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, "", false, false, true, false, false, false, false) if tt.res.err == nil { if !assert.NoError(t, err) { t.FailNow() @@ -1605,7 +1605,7 @@ func TestCommandSide_userHumanWriteModel_password(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, false, false, false, true, false, false, false) + wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, "", false, false, false, true, false, false, false) if tt.res.err == nil { if !assert.NoError(t, err) { t.FailNow() @@ -2132,7 +2132,7 @@ func TestCommandSide_userHumanWriteModel_avatar(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, false, false, false, false, true, false, false) + wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, "", false, false, false, false, true, false, false) if tt.res.err == nil { if !assert.NoError(t, err) { t.FailNow() @@ -2441,7 +2441,7 @@ func TestCommandSide_userHumanWriteModel_idpLinks(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - wm, err := r.userRemoveWriteModel(tt.args.ctx, tt.args.userID) + wm, err := r.userRemoveWriteModel(tt.args.ctx, tt.args.userID, "") if tt.res.err == nil { if !assert.NoError(t, err) { t.FailNow() @@ -2744,7 +2744,7 @@ func TestCommandSide_userHumanWriteModel_metadata(t *testing.T) { r := &Commands{ eventstore: tt.fields.eventstore(t), } - wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, false, false, false, false, false, false, true) + wm, err := r.UserHumanWriteModel(tt.args.ctx, tt.args.userID, "", false, false, false, false, false, false, true) if tt.res.err == nil { if !assert.NoError(t, err) { t.FailNow() diff --git a/internal/command/user_v2_test.go b/internal/command/user_v2_test.go index e156c92f08..3eb9ecd6f7 100644 --- a/internal/command/user_v2_test.go +++ b/internal/command/user_v2_test.go @@ -1358,7 +1358,7 @@ func TestCommandSide_RemoveUserV2(t *testing.T) { eventstore: tt.fields.eventstore(t), checkPermission: tt.fields.checkPermission, } - got, err := r.RemoveUserV2(tt.args.ctx, tt.args.userID, tt.args.cascadingMemberships, tt.args.grantIDs...) + got, err := r.RemoveUserV2(tt.args.ctx, tt.args.userID, "", tt.args.cascadingMemberships, tt.args.grantIDs...) if tt.res.err == nil { assert.NoError(t, err) } diff --git a/internal/query/user.go b/internal/query/user.go index 31a2a58e58..bb76e51f66 100644 --- a/internal/query/user.go +++ b/internal/query/user.go @@ -368,6 +368,10 @@ func (q *Queries) GetUserByIDWithPermission(ctx context.Context, shouldTriggerBu } func (q *Queries) GetUserByID(ctx context.Context, shouldTriggerBulk bool, userID string) (user *User, err error) { + return q.GetUserByIDWithResourceOwner(ctx, shouldTriggerBulk, userID, "") +} + +func (q *Queries) GetUserByIDWithResourceOwner(ctx context.Context, shouldTriggerBulk bool, userID, resourceOwner string) (user *User, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -382,6 +386,7 @@ func (q *Queries) GetUserByID(ctx context.Context, shouldTriggerBulk bool, userI }, userByIDQuery, userID, + resourceOwner, authz.GetInstance(ctx).InstanceID(), ) return user, err diff --git a/internal/query/user_by_id.sql b/internal/query/user_by_id.sql index 366e6552f6..2ce741f9b7 100644 --- a/internal/query/user_by_id.sql +++ b/internal/query/user_by_id.sql @@ -20,8 +20,8 @@ WITH login_names AS (SELECT WHERE u.instance_id = p.instance_id AND ( - (p.is_default IS TRUE AND p.instance_id = $2) - OR (p.instance_id = $2 AND p.resource_owner = u.resource_owner) + (p.is_default IS TRUE AND p.instance_id = $3) + OR (p.instance_id = $3 AND p.resource_owner = u.resource_owner) ) ORDER BY is_default LIMIT 1 @@ -32,8 +32,9 @@ WITH login_names AS (SELECT u.instance_id = d.instance_id AND u.resource_owner = d.resource_owner WHERE - u.instance_id = $2 - AND u.id = $1 + u.id = $1 + AND (u.resource_owner = $2 OR $2 = '') + AND u.instance_id = $3 ) SELECT u.id @@ -80,6 +81,7 @@ LEFT JOIN AND u.instance_id = m.instance_id WHERE u.id = $1 - AND u.instance_id = $2 + AND (u.resource_owner = $2 OR $2 = '') + AND u.instance_id = $3 LIMIT 1 ; \ No newline at end of file