diff --git a/internal/api/scim/authz.go b/internal/api/scim/authz.go index 759ee3e84d..a89df38061 100644 --- a/internal/api/scim/authz.go +++ b/internal/api/scim/authz.go @@ -10,4 +10,7 @@ var AuthMapping = authz.MethodMapping{ "POST:/scim/v2/" + http.OrgIdInPathVariable + "/Users": { Permission: domain.PermissionUserWrite, }, + "DELETE:/scim/v2/" + http.OrgIdInPathVariable + "/Users/{id}": { + Permission: domain.PermissionUserDelete, + }, } diff --git a/internal/api/scim/integration_test/users_create_test.go b/internal/api/scim/integration_test/users_create_test.go index cad0fcbd33..b7d97e342f 100644 --- a/internal/api/scim/integration_test/users_create_test.go +++ b/internal/api/scim/integration_test/users_create_test.go @@ -5,7 +5,7 @@ package integration_test import ( "context" _ "embed" - "errors" + "github.com/brianvoe/gofakeit/v6" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/zitadel/zitadel/internal/api/scim/schemas" @@ -16,7 +16,6 @@ import ( "google.golang.org/grpc/codes" "net/http" "path" - "strconv" "testing" ) @@ -139,20 +138,14 @@ func TestCreateUser(t *testing.T) { } if err != nil { - assert.IsType(t, new(scim.ScimError), err) - - var scimErr *scim.ScimError - errors.As(err, &scimErr) - assert.Equal(t, tt.scimErrorType, scimErr.ScimType) - statusCode := tt.errorStatus if statusCode == 0 { statusCode = http.StatusBadRequest } - assert.Equal(t, strconv.Itoa(statusCode), scimErr.Status) - + scimErr := scim.RequireScimError(t, statusCode, err) + assert.Equal(t, tt.scimErrorType, scimErr.Error.ScimType) if tt.zitadelErrID != "" { - assert.Equal(t, tt.zitadelErrID, scimErr.ZitadelDetail.ID) + assert.Equal(t, tt.zitadelErrID, scimErr.Error.ZitadelDetail.ID) } return @@ -175,13 +168,8 @@ func TestCreateUser_duplicate(t *testing.T) { require.NoError(t, err) _, err = Instance.Client.SCIM.Users.Create(CTX, Instance.DefaultOrg.Id, minimalUserJson) - require.Error(t, err) - assert.IsType(t, new(scim.ScimError), err) - - var scimErr *scim.ScimError - errors.As(err, &scimErr) - assert.Equal(t, strconv.Itoa(http.StatusConflict), scimErr.Status) - assert.Equal(t, "User already exists", scimErr.Detail) + scimErr := scim.RequireScimError(t, http.StatusConflict, err) + assert.Equal(t, "User already exists", scimErr.Error.Detail) _, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID}) require.NoError(t, err) @@ -248,3 +236,9 @@ func TestCreateUser_scopedExternalID(t *testing.T) { _, err = Instance.Client.UserV2.DeleteUser(CTX, &user.DeleteUserRequest{UserId: createdUser.ID}) require.NoError(t, err) } + +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 new file mode 100644 index 0000000000..6d3f73a71e --- /dev/null +++ b/internal/api/scim/integration_test/users_delete_test.go @@ -0,0 +1,84 @@ +//go:build integration + +package integration_test + +import ( + "context" + "github.com/brianvoe/gofakeit/v6" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zitadel/zitadel/internal/integration" + "github.com/zitadel/zitadel/internal/integration/scim" + "github.com/zitadel/zitadel/pkg/grpc/user/v2" + "google.golang.org/grpc/codes" + "net/http" + "testing" +) + +func TestDeleteUser_errors(t *testing.T) { + tests := []struct { + name string + ctx context.Context + errorStatus int + }{ + { + name: "not authenticated", + ctx: context.Background(), + errorStatus: http.StatusUnauthorized, + }, + { + name: "no permissions", + ctx: Instance.WithAuthorization(CTX, integration.UserTypeNoPermission), + errorStatus: http.StatusNotFound, + }, + { + name: "unknown user id", + errorStatus: http.StatusNotFound, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := tt.ctx + if ctx == nil { + ctx = CTX + } + + err := Instance.Client.SCIM.Users.Delete(ctx, Instance.DefaultOrg.Id, "1") + + statusCode := tt.errorStatus + if statusCode == 0 { + statusCode = http.StatusBadRequest + } + + scim.RequireScimError(t, statusCode, err) + }) + } +} + +func TestDeleteUser_ensureReallyDeleted(t *testing.T) { + // create user and dependencies + createUserResp := Instance.CreateHumanUser(CTX) + proj, err := Instance.CreateProject(CTX) + require.NoError(t, err) + + Instance.CreateProjectUserGrant(t, CTX, proj.Id, createUserResp.UserId) + + // delete user via scim + err = Instance.Client.SCIM.Users.Delete(CTX, Instance.DefaultOrg.Id, createUserResp.UserId) + assert.NoError(t, err) + + // ensure it is really deleted => try to delete again => should 404 + err = Instance.Client.SCIM.Users.Delete(CTX, Instance.DefaultOrg.Id, createUserResp.UserId) + scim.RequireScimError(t, http.StatusNotFound, err) + + // try to get user via api => should 404 + _, err = Instance.Client.UserV2.GetUserByID(CTX, &user.GetUserByIDRequest{UserId: createUserResp.UserId}) + integration.AssertGrpcStatus(t, codes.NotFound, err) +} + +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/resources/resource_handler.go b/internal/api/scim/resources/resource_handler.go index c624253ee9..2d601fd1fc 100644 --- a/internal/api/scim/resources/resource_handler.go +++ b/internal/api/scim/resources/resource_handler.go @@ -19,6 +19,7 @@ type ResourceHandler[T ResourceHolder] interface { NewResource() T Create(ctx context.Context, resource T) (T, error) + Delete(ctx context.Context, id string) error } type Resource struct { diff --git a/internal/api/scim/resources/resource_handler_adapter.go b/internal/api/scim/resources/resource_handler_adapter.go index 79f74f7bfb..979fdad99a 100644 --- a/internal/api/scim/resources/resource_handler_adapter.go +++ b/internal/api/scim/resources/resource_handler_adapter.go @@ -5,6 +5,8 @@ import ( "net/http" "slices" + "github.com/gorilla/mux" + "github.com/zitadel/zitadel/internal/api/scim/schemas" "github.com/zitadel/zitadel/internal/api/scim/serrors" "github.com/zitadel/zitadel/internal/zerrors" @@ -45,6 +47,11 @@ func (adapter *ResourceHandlerAdapter[T]) Create(r *http.Request) (T, error) { return adapter.handler.Create(r.Context(), entity) } +func (adapter *ResourceHandlerAdapter[T]) Delete(r *http.Request) error { + id := mux.Vars(r)["id"] + return adapter.handler.Delete(r.Context(), id) +} + func (adapter *ResourceHandlerAdapter[T]) readEntityFromBody(r *http.Request) (T, error) { entity := adapter.handler.NewResource() err := json.NewDecoder(r.Body).Decode(entity) diff --git a/internal/api/scim/resources/user.go b/internal/api/scim/resources/user.go index fef9a34c6c..14f5af6115 100644 --- a/internal/api/scim/resources/user.go +++ b/internal/api/scim/resources/user.go @@ -7,7 +7,7 @@ import ( "github.com/zitadel/zitadel/internal/api/authz" scim_config "github.com/zitadel/zitadel/internal/api/scim/config" - schemas2 "github.com/zitadel/zitadel/internal/api/scim/schemas" + scim_schemas "github.com/zitadel/zitadel/internal/api/scim/schemas" "github.com/zitadel/zitadel/internal/command" "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/query" @@ -22,26 +22,26 @@ type UsersHandler struct { type ScimUser struct { *Resource - ID string `json:"id"` - ExternalID string `json:"externalId,omitempty"` - UserName string `json:"userName,omitempty"` - Name *ScimUserName `json:"name,omitempty"` - DisplayName string `json:"displayName,omitempty"` - NickName string `json:"nickName,omitempty"` - ProfileUrl *schemas2.HttpURL `json:"profileUrl,omitempty"` - Title string `json:"title,omitempty"` - PreferredLanguage language.Tag `json:"preferredLanguage,omitempty"` - Locale string `json:"locale,omitempty"` - Timezone string `json:"timezone,omitempty"` - Active bool `json:"active,omitempty"` - Emails []*ScimEmail `json:"emails,omitempty"` - PhoneNumbers []*ScimPhoneNumber `json:"phoneNumbers,omitempty"` - Password *schemas2.WriteOnlyString `json:"password,omitempty"` - Ims []*ScimIms `json:"ims,omitempty"` - Addresses []*ScimAddress `json:"addresses,omitempty"` - Photos []*ScimPhoto `json:"photos,omitempty"` - Entitlements []*ScimEntitlement `json:"entitlements,omitempty"` - Roles []*ScimRole `json:"roles,omitempty"` + ID string `json:"id"` + ExternalID string `json:"externalId,omitempty"` + UserName string `json:"userName,omitempty"` + Name *ScimUserName `json:"name,omitempty"` + DisplayName string `json:"displayName,omitempty"` + NickName string `json:"nickName,omitempty"` + ProfileUrl *scim_schemas.HttpURL `json:"profileUrl,omitempty"` + Title string `json:"title,omitempty"` + PreferredLanguage language.Tag `json:"preferredLanguage,omitempty"` + Locale string `json:"locale,omitempty"` + Timezone string `json:"timezone,omitempty"` + Active *bool `json:"active,omitempty"` + Emails []*ScimEmail `json:"emails,omitempty"` + PhoneNumbers []*ScimPhoneNumber `json:"phoneNumbers,omitempty"` + Password *scim_schemas.WriteOnlyString `json:"password,omitempty"` + Ims []*ScimIms `json:"ims,omitempty"` + Addresses []*ScimAddress `json:"addresses,omitempty"` + Photos []*ScimPhoto `json:"photos,omitempty"` + Entitlements []*ScimEntitlement `json:"entitlements,omitempty"` + Roles []*ScimRole `json:"roles,omitempty"` } type ScimEntitlement struct { @@ -59,10 +59,10 @@ type ScimRole struct { } type ScimPhoto struct { - Value schemas2.HttpURL `json:"value"` - Display string `json:"display,omitempty"` - Type string `json:"type"` - Primary bool `json:"primary,omitempty"` + Value scim_schemas.HttpURL `json:"value"` + Display string `json:"display,omitempty"` + Type string `json:"type"` + Primary bool `json:"primary,omitempty"` } type ScimAddress struct { @@ -108,12 +108,12 @@ func NewUsersHandler( return &UsersHandler{command, query, userCodeAlg, config} } -func (h *UsersHandler) ResourceNameSingular() schemas2.ScimResourceTypeSingular { - return schemas2.UserResourceType +func (h *UsersHandler) ResourceNameSingular() scim_schemas.ScimResourceTypeSingular { + return scim_schemas.UserResourceType } -func (h *UsersHandler) ResourceNamePlural() schemas2.ScimResourceTypePlural { - return schemas2.UsersResourceType +func (h *UsersHandler) ResourceNamePlural() scim_schemas.ScimResourceTypePlural { + return scim_schemas.UsersResourceType } func (u *ScimUser) GetResource() *Resource { @@ -124,8 +124,8 @@ func (h *UsersHandler) NewResource() *ScimUser { return new(ScimUser) } -func (h *UsersHandler) SchemaType() schemas2.ScimSchemaType { - return schemas2.IdUser +func (h *UsersHandler) SchemaType() scim_schemas.ScimSchemaType { + return scim_schemas.IdUser } func (h *UsersHandler) Create(ctx context.Context, user *ScimUser) (*ScimUser, error) { @@ -142,5 +142,43 @@ func (h *UsersHandler) Create(ctx context.Context, user *ScimUser) (*ScimUser, e user.ID = addHuman.Details.ID user.Resource = buildResource(ctx, h, addHuman.Details) - return user, err + return user, nil +} + +func (h *UsersHandler) Delete(ctx context.Context, id string) error { + memberships, grants, err := h.queryUserDependencies(ctx, id) + if err != nil { + return err + } + + _, err = h.command.RemoveUserV2(ctx, id, memberships, grants...) + return err +} + +func (h *UsersHandler) queryUserDependencies(ctx context.Context, userID string) ([]*command.CascadingMembership, []string, error) { + userGrantUserQuery, err := query.NewUserGrantUserIDSearchQuery(userID) + if err != nil { + return nil, nil, err + } + + grants, err := h.query.UserGrants(ctx, &query.UserGrantsQueries{ + Queries: []query.SearchQuery{userGrantUserQuery}, + }, true) + if err != nil { + return nil, nil, err + } + + membershipsUserQuery, err := query.NewMembershipUserIDQuery(userID) + if err != nil { + return nil, nil, err + } + + memberships, err := h.query.Memberships(ctx, &query.MembershipSearchQuery{ + Queries: []query.SearchQuery{membershipsUserQuery}, + }, false) + + if err != nil { + return nil, nil, err + } + return cascadingMemberships(memberships.Memberships), userGrantsToIDs(grants.UserGrants), nil } diff --git a/internal/api/scim/resources/user_mapping.go b/internal/api/scim/resources/user_mapping.go index 8ee7cd511c..bc40005382 100644 --- a/internal/api/scim/resources/user_mapping.go +++ b/internal/api/scim/resources/user_mapping.go @@ -7,6 +7,7 @@ import ( "github.com/zitadel/zitadel/internal/command" "github.com/zitadel/zitadel/internal/domain" + "github.com/zitadel/zitadel/internal/query" ) func (h *UsersHandler) mapToAddHuman(ctx context.Context, scimUser *ScimUser) (*command.AddHuman, error) { @@ -79,3 +80,54 @@ func (h *UsersHandler) mapPrimaryPhone(scimUser *ScimUser) command.Phone { return command.Phone{} } + +func cascadingMemberships(memberships []*query.Membership) []*command.CascadingMembership { + cascades := make([]*command.CascadingMembership, len(memberships)) + for i, membership := range memberships { + cascades[i] = &command.CascadingMembership{ + UserID: membership.UserID, + ResourceOwner: membership.ResourceOwner, + IAM: cascadingIAMMembership(membership.IAM), + Org: cascadingOrgMembership(membership.Org), + Project: cascadingProjectMembership(membership.Project), + ProjectGrant: cascadingProjectGrantMembership(membership.ProjectGrant), + } + } + return cascades +} + +func cascadingIAMMembership(membership *query.IAMMembership) *command.CascadingIAMMembership { + if membership == nil { + return nil + } + return &command.CascadingIAMMembership{IAMID: membership.IAMID} +} + +func cascadingOrgMembership(membership *query.OrgMembership) *command.CascadingOrgMembership { + if membership == nil { + return nil + } + return &command.CascadingOrgMembership{OrgID: membership.OrgID} +} + +func cascadingProjectMembership(membership *query.ProjectMembership) *command.CascadingProjectMembership { + if membership == nil { + return nil + } + return &command.CascadingProjectMembership{ProjectID: membership.ProjectID} +} + +func cascadingProjectGrantMembership(membership *query.ProjectGrantMembership) *command.CascadingProjectGrantMembership { + if membership == nil { + return nil + } + return &command.CascadingProjectGrantMembership{ProjectID: membership.ProjectID, GrantID: membership.GrantID} +} + +func userGrantsToIDs(userGrants []*query.UserGrant) []string { + converted := make([]string, len(userGrants)) + for i, grant := range userGrants { + converted[i] = grant.ID + } + return converted +} diff --git a/internal/api/scim/server.go b/internal/api/scim/server.go index c23aadf247..a2f9c7e7bf 100644 --- a/internal/api/scim/server.go +++ b/internal/api/scim/server.go @@ -54,6 +54,7 @@ func mapResource[T sresources.ResourceHolder](router *mux.Router, mw zhttp_middl resourceRouter := router.PathPrefix("/" + path.Join(zhttp.OrgIdInPathVariable, string(handler.ResourceNamePlural()))).Subrouter() resourceRouter.Handle("", mw(handleResourceCreatedResponse(adapter.Create))).Methods(http.MethodPost) + resourceRouter.Handle("/{id}", mw(handleEmptyResponse(adapter.Delete))).Methods(http.MethodDelete) } func handleResourceCreatedResponse[T sresources.ResourceHolder](next func(*http.Request) (T, error)) zhttp_middlware.HandlerFuncWithError { @@ -72,3 +73,15 @@ func handleResourceCreatedResponse[T sresources.ResourceHolder](next func(*http. return nil } } + +func handleEmptyResponse(next func(*http.Request) error) zhttp_middlware.HandlerFuncWithError { + return func(w http.ResponseWriter, r *http.Request) error { + err := next(r) + if err != nil { + return err + } + + w.WriteHeader(http.StatusNoContent) + return nil + } +} diff --git a/internal/integration/scim/assertions.go b/internal/integration/scim/assertions.go new file mode 100644 index 0000000000..a91c33da82 --- /dev/null +++ b/internal/integration/scim/assertions.go @@ -0,0 +1,22 @@ +package scim + +import ( + "errors" + "strconv" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type AssertedScimError struct { + Error *ScimError +} + +func RequireScimError(t require.TestingT, httpStatus int, err error) AssertedScimError { + require.Error(t, err) + + var scimErr *ScimError + assert.True(t, errors.As(err, &scimErr)) + assert.Equal(t, strconv.Itoa(httpStatus), scimErr.Status) + return AssertedScimError{scimErr} // wrap it, otherwise error handling is enforced +} diff --git a/internal/integration/scim/client.go b/internal/integration/scim/client.go index d6b5066f45..478c831826 100644 --- a/internal/integration/scim/client.go +++ b/internal/integration/scim/client.go @@ -58,6 +58,19 @@ func (c *ResourceClient) Create(ctx context.Context, orgID string, body []byte) return user, err } +func (c *ResourceClient) Delete(ctx context.Context, orgID, id string) error { + return c.do(ctx, http.MethodDelete, orgID, id) +} + +func (c *ResourceClient) do(ctx context.Context, method, orgID, url string) error { + req, err := http.NewRequestWithContext(ctx, method, c.buildURL(orgID, url), nil) + if err != nil { + return err + } + + return c.doReq(req, nil) +} + func (c *ResourceClient) doWithBody(ctx context.Context, method, orgID, url string, body io.Reader, responseEntity interface{}) error { req, err := http.NewRequestWithContext(ctx, method, c.buildURL(orgID, url), body) if err != nil {