mirror of
https://github.com/zitadel/zitadel.git
synced 2025-08-11 21:37:32 +00:00
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
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
@@ -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
|
||||
}
|
||||
|
@@ -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()
|
||||
}())
|
||||
}
|
||||
|
@@ -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)
|
||||
}
|
||||
|
@@ -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)
|
||||
}
|
||||
|
@@ -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)
|
||||
}
|
||||
|
@@ -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)
|
||||
}
|
||||
|
@@ -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,
|
||||
|
@@ -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
|
||||
}
|
||||
|
@@ -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,
|
||||
|
Reference in New Issue
Block a user