From 5e7fc110b21043c8804aa7767549a5ef82e2427f Mon Sep 17 00:00:00 2001 From: Akshay Gaikwad Date: Wed, 11 May 2022 18:47:11 +0530 Subject: [PATCH 1/6] Add IdP groups in Identities table The idp_groups is list of groups IdP user belongs to that is returning in the OIdC providers token response. The flow of Idp Group mapping is as follows: OIdC Provider (OP) return custom claim with groups in a token when authentication event | The value of custom claim is mapped to `idp_groups` of identity traint using JsonNet mapper. | On inserting/updating/deleting `identities` table, Postgresql sends a pg_notification with `PG_OPERATION,IDENTITY_ID,IDENTITY_TRAIN` as a payload. | The `pkg/service/user.UserService.UpdateIdpUserGroupPolicy` update the casbin policies for each notification based on payload received. --- _kratos/identity.schema.json | 7 ++++ main.go | 30 ++++++++++++- .../000038_pg_identities_triggers.down.sql | 2 + .../000038_pg_identities_triggers.up.sql | 23 ++++++++++ pkg/service/user.go | 42 +++++++++++++++++++ 5 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 persistence/migrations/admindb/000038_pg_identities_triggers.down.sql create mode 100644 persistence/migrations/admindb/000038_pg_identities_triggers.up.sql diff --git a/_kratos/identity.schema.json b/_kratos/identity.schema.json index 09e1199..dd03208 100644 --- a/_kratos/identity.schema.json +++ b/_kratos/identity.schema.json @@ -40,6 +40,13 @@ "via": "email" } } + }, + "idp_groups": { + "title": "IDP groups", + "type": "array", + "items": { + "type": "string" + } } }, "required": [ diff --git a/main.go b/main.go index 150f8d5..38bf593 100644 --- a/main.go +++ b/main.go @@ -431,11 +431,11 @@ func run() { go runRelayPeerRPC(&wg, ctx) go runDebug(&wg, ctx) go runEventHandlers(&wg, ctx) + go runIdpGroupSync(&wg, ctx) <-ctx.Done() _log.Infow("shutting down, waiting for children to die") wg.Wait() - } func runAPI(wg *sync.WaitGroup, ctx context.Context) { @@ -675,6 +675,34 @@ func runDebug(wg *sync.WaitGroup, ctx context.Context) { s.Shutdown(ctx) } +func runIdpGroupSync(wg *sync.WaitGroup, ctx context.Context) { + defer wg.Done() + channel := "identities:changed" + ln := pgdriver.NewListener(db) +listen: + if err := ln.Listen(ctx, channel); err != nil { + _log.Errorf("error listening for notification on channel %q: %s", channel, err) + time.Sleep(2 * time.Second) + goto listen + } + + _log.Infof("Listening for notifications on channel %q", channel) + for n := range ln.Channel() { + _log.Info("A identities table notification received") + splitPl := strings.SplitN(n.Payload, ",", 3) + op := splitPl[0] + id := splitPl[1] + traits := splitPl[2] + err := us.UpdateIdpUserGroupPolicy(ctx, op, id, traits) + if err != nil { + _log.Warnf("Failed updating policy for IDP user with id %s: %s", id, err) + } else { + _log.Infof("Policies are updated successfully for IDP user with id %s", id) + } + } + <-ctx.Done() +} + func main() { setup() run() diff --git a/persistence/migrations/admindb/000038_pg_identities_triggers.down.sql b/persistence/migrations/admindb/000038_pg_identities_triggers.down.sql new file mode 100644 index 0000000..22e658d --- /dev/null +++ b/persistence/migrations/admindb/000038_pg_identities_triggers.down.sql @@ -0,0 +1,2 @@ +DROP FUNCTION IF EXISTS identities_after_change() CASCADE; +DROP TRIGGER IF EXISTS trigger_identities_update ON identities; diff --git a/persistence/migrations/admindb/000038_pg_identities_triggers.up.sql b/persistence/migrations/admindb/000038_pg_identities_triggers.up.sql new file mode 100644 index 0000000..3b57a95 --- /dev/null +++ b/persistence/migrations/admindb/000038_pg_identities_triggers.up.sql @@ -0,0 +1,23 @@ +CREATE OR REPLACE FUNCTION identities_after_change() RETURNS TRIGGER AS $$ + DECLARE + row RECORD; + output TEXT; + + BEGIN + IF (TG_OP = 'DELETE') THEN + row = OLD; + ELSE + row = NEW; + END IF; + + output = TG_OP || ',' || row.id || ',' || row.traits; + PERFORM pg_notify('identities:changed',output); + RETURN NULL; + END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER trigger_identities_update + AFTER INSERT OR UPDATE OR DELETE + ON identities + FOR EACH ROW + EXECUTE PROCEDURE identities_after_change(); diff --git a/pkg/service/user.go b/pkg/service/user.go index 3b6521e..08f190d 100644 --- a/pkg/service/user.go +++ b/pkg/service/user.go @@ -3,6 +3,7 @@ package service import ( "context" "database/sql" + "encoding/json" "fmt" "strings" "time" @@ -48,6 +49,8 @@ type UserService interface { List(context.Context, ...query.Option) (*userv3.UserList, error) // retrieve the cli config for the logged in user RetrieveCliConfig(ctx context.Context, req *userrpcv3.ApiKeyRequest) (*common.CliConfigDownloadData, error) + // Update UserGroup casbin for OIdC/Idp users + UpdateIdpUserGroupPolicy(context.Context, string, string, string) error } type userService struct { @@ -65,6 +68,7 @@ type userTraits struct { FirstName string LastName string Description string + IdpGroups []string `json:"idp_groups"` } // FIXME: find a better way to do this @@ -850,3 +854,41 @@ func (s *userService) RetrieveCliConfig(ctx context.Context, req *userrpcv3.ApiK return cliConfig, nil } + +func (s *userService) UpdateIdpUserGroupPolicy(ctx context.Context, op, id, traits string) error { + var userInfo userTraits + err := json.Unmarshal([]byte(traits), &userInfo) + if err != nil { + return fmt.Errorf("Encounterd error unmarshing payload to userInfo: %s", err) + } + switch op { + case "DELETE": + _, err = s.azc.DeleteUserGroups(ctx, &authzv1.UserGroup{Grp: "u:" + userInfo.Email}) + if err != nil { + return fmt.Errorf("error deleting UserGroups policy: %s", err) + } + case "UPDATE": + // delete old policies + _, err = s.azc.DeleteUserGroups(ctx, &authzv1.UserGroup{Grp: "u:" + userInfo.Email}) + if err != nil { + return fmt.Errorf("error deleting UserGroups policy: %s", err) + } + // create new policies + fallthrough + case "INSERT": + var ugs []*authzv1.UserGroup + for _, g := range utils.Unique(userInfo.IdpGroups) { + ugs = append(ugs, &authzv1.UserGroup{ + Grp: "g:" + g, + User: "u:" + userInfo.Email, + }) + } + _, err = s.azc.CreateUserGroups(ctx, &authzv1.UserGroups{UserGroups: ugs}) + if err != nil { + return fmt.Errorf("error creating UserGroups policy: %s", err) + } + default: + return fmt.Errorf("Unsupported %s operation in payload", op) + } + return nil +} From a0424f4000639ff8f88be53740354f63b0fd98ae Mon Sep 17 00:00:00 2001 From: Akshay Gaikwad Date: Thu, 12 May 2022 18:11:10 +0530 Subject: [PATCH 2/6] Modify groupaccount table instead of just policy rules --- pkg/service/user.go | 57 ++++++++++++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/pkg/service/user.go b/pkg/service/user.go index 08f190d..8ff8764 100644 --- a/pkg/service/user.go +++ b/pkg/service/user.go @@ -251,7 +251,7 @@ func (s *userService) createUserRoleRelations(ctx context.Context, db bun.IDB, u } // Update the groups mapped to each user(account) -func (s *userService) createGroupAccountRelations(ctx context.Context, db bun.IDB, userId uuid.UUID, usr *userv3.User) (*userv3.User, []uuid.UUID, error) { +func (s *userService) createGroupAccountRelations(ctx context.Context, db bun.IDB, userId uuid.UUID, usr *userv3.User, ignoreGrp bool) (*userv3.User, []uuid.UUID, error) { var grpaccs []models.GroupAccount var ugs []*authzv1.UserGroup var ids []uuid.UUID @@ -259,6 +259,9 @@ func (s *userService) createGroupAccountRelations(ctx context.Context, db bun.ID // FIXME: do combined lookup entity, err := dao.GetByName(ctx, s.db, group, &models.Group{}) if err != nil { + if ignoreGrp { + continue + } return &userv3.User{}, nil, fmt.Errorf("unable to find group '%v'", group) } if grp, ok := entity.(*models.Group); ok { @@ -302,7 +305,7 @@ func (s *userService) deleteGroupAccountRelations(ctx context.Context, db bun.ID return &userv3.User{}, ids, fmt.Errorf("unable to delete user; %v", err) } - _, err = s.azc.DeleteUserGroups(ctx, &authzv1.UserGroup{Grp: "u:" + usr.GetMetadata().GetName()}) + _, err = s.azc.DeleteUserGroups(ctx, &authzv1.UserGroup{User: "u:" + usr.GetMetadata().GetName()}) if err != nil { return &userv3.User{}, ids, fmt.Errorf("unable to delete group-user relations from authz; %v", err) } @@ -359,7 +362,7 @@ func (s *userService) Create(ctx context.Context, user *userv3.User) (*userv3.Us return &userv3.User{}, err } - user, groupsAfter, err := s.createGroupAccountRelations(ctx, tx, uuid.MustParse(id), user) + user, groupsAfter, err := s.createGroupAccountRelations(ctx, tx, uuid.MustParse(id), user, false) if err != nil { tx.Rollback() return &userv3.User{}, err @@ -626,7 +629,7 @@ func (s *userService) Update(ctx context.Context, user *userv3.User) (*userv3.Us return &userv3.User{}, err } - user, groupsAfter, err := s.createGroupAccountRelations(ctx, tx, usr.ID, user) + user, groupsAfter, err := s.createGroupAccountRelations(ctx, tx, usr.ID, user, false) if err != nil { tx.Rollback() return &userv3.User{}, err @@ -856,36 +859,52 @@ func (s *userService) RetrieveCliConfig(ctx context.Context, req *userrpcv3.ApiK } func (s *userService) UpdateIdpUserGroupPolicy(ctx context.Context, op, id, traits string) error { - var userInfo userTraits - err := json.Unmarshal([]byte(traits), &userInfo) + var ( + userInfo userTraits + user *userv3.User + userUUID uuid.UUID + ) + userUUID, err := uuid.Parse(id) + if err != nil { + return fmt.Errorf("error parsing id %s: %s", id, err) + } + err = json.Unmarshal([]byte(traits), &userInfo) if err != nil { return fmt.Errorf("Encounterd error unmarshing payload to userInfo: %s", err) } + // TODO: Revisit to only run by IDP users and not by any other + // user + if len(userInfo.IdpGroups) == 0 { + return fmt.Errorf("Empty idp groups for user with id %s", id) + } + user = &userv3.User{ + Metadata: &v3.Metadata{ + Name: userInfo.Email, + }, + Spec: &userv3.UserSpec{ + FirstName: userInfo.FirstName, + LastName: userInfo.LastName, + Groups: userInfo.IdpGroups, + }, + } switch op { case "DELETE": - _, err = s.azc.DeleteUserGroups(ctx, &authzv1.UserGroup{Grp: "u:" + userInfo.Email}) + _, _, err = s.deleteGroupAccountRelations(ctx, s.db, userUUID, user) if err != nil { - return fmt.Errorf("error deleting UserGroups policy: %s", err) + return err } case "UPDATE": // delete old policies - _, err = s.azc.DeleteUserGroups(ctx, &authzv1.UserGroup{Grp: "u:" + userInfo.Email}) + _, _, err = s.deleteGroupAccountRelations(ctx, s.db, userUUID, user) if err != nil { - return fmt.Errorf("error deleting UserGroups policy: %s", err) + return err } // create new policies fallthrough case "INSERT": - var ugs []*authzv1.UserGroup - for _, g := range utils.Unique(userInfo.IdpGroups) { - ugs = append(ugs, &authzv1.UserGroup{ - Grp: "g:" + g, - User: "u:" + userInfo.Email, - }) - } - _, err = s.azc.CreateUserGroups(ctx, &authzv1.UserGroups{UserGroups: ugs}) + _, _, err = s.createGroupAccountRelations(ctx, s.db, userUUID, user, true) if err != nil { - return fmt.Errorf("error creating UserGroups policy: %s", err) + return err } default: return fmt.Errorf("Unsupported %s operation in payload", op) From f3de101f940e7b434037abe4c15bd000fcd2c60d Mon Sep 17 00:00:00 2001 From: Abin Simon Date: Fri, 13 May 2022 13:31:03 +0530 Subject: [PATCH 3/6] Update User spec to include IDPGroups --- gen/openapi/proto/rpc/user/user.swagger.json | 51 ++++++++++++ pkg/service/user.go | 73 ++++++++++++++--- pkg/service/user_test.go | 86 +++++++++++++++++++- proto/types/userpb/v3/user.pb.go | 58 +++++++++---- proto/types/userpb/v3/user.proto | 26 ++++-- 5 files changed, 258 insertions(+), 36 deletions(-) diff --git a/gen/openapi/proto/rpc/user/user.swagger.json b/gen/openapi/proto/rpc/user/user.swagger.json index 887c61e..f79d576 100644 --- a/gen/openapi/proto/rpc/user/user.swagger.json +++ b/gen/openapi/proto/rpc/user/user.swagger.json @@ -202,6 +202,17 @@ }, "collectionFormat": "multi" }, + { + "name": "spec.idpGroups", + "description": "Idp Group. Idp Groups the user belongs to", + "in": "query", + "required": false, + "type": "array", + "items": { + "type": "string" + }, + "collectionFormat": "multi" + }, { "name": "spec.emailVerified", "description": "EmailVerified. Flag to show if the email of the user was verified", @@ -412,6 +423,17 @@ }, "collectionFormat": "multi" }, + { + "name": "spec.idpGroups", + "description": "Idp Group. Idp Groups the user belongs to", + "in": "query", + "required": false, + "type": "array", + "items": { + "type": "string" + }, + "collectionFormat": "multi" + }, { "name": "spec.emailVerified", "description": "EmailVerified. Flag to show if the email of the user was verified", @@ -762,6 +784,17 @@ }, "collectionFormat": "multi" }, + { + "name": "spec.idpGroups", + "description": "Idp Group. Idp Groups the user belongs to", + "in": "query", + "required": false, + "type": "array", + "items": { + "type": "string" + }, + "collectionFormat": "multi" + }, { "name": "spec.emailVerified", "description": "EmailVerified. Flag to show if the email of the user was verified", @@ -1478,6 +1511,15 @@ "title": "Group", "readOnly": true }, + "idpGroups": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Idp Groups the user belongs to", + "title": "Idp Group", + "readOnly": true + }, "permissions": { "type": "array", "items": { @@ -1571,6 +1613,15 @@ "title": "Group", "readOnly": true }, + "idpGroups": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Idp Groups the user belongs to", + "title": "Idp Group", + "readOnly": true + }, "projectNamespaceRoles": { "type": "array", "items": { diff --git a/pkg/service/user.go b/pkg/service/user.go index 8ff8764..c11393b 100644 --- a/pkg/service/user.go +++ b/pkg/service/user.go @@ -21,7 +21,6 @@ import ( "github.com/RafayLabs/rcloud-base/pkg/utils" userrpcv3 "github.com/RafayLabs/rcloud-base/proto/rpc/user" authzv1 "github.com/RafayLabs/rcloud-base/proto/types/authz" - commonv3 "github.com/RafayLabs/rcloud-base/proto/types/commonpb/v3" v3 "github.com/RafayLabs/rcloud-base/proto/types/commonpb/v3" userv3 "github.com/RafayLabs/rcloud-base/proto/types/userpb/v3" ) @@ -101,11 +100,16 @@ func getUserTraits(traits map[string]interface{}) userTraits { if !ok { desc = "" } + ig, ok := traits["idp_groups"] + if !ok { + ig = []string{} + } return userTraits{ Email: email.(string), FirstName: fname.(string), LastName: lname.(string), Description: desc.(string), + IdpGroups: ig.([]string), } } @@ -251,17 +255,16 @@ func (s *userService) createUserRoleRelations(ctx context.Context, db bun.IDB, u } // Update the groups mapped to each user(account) -func (s *userService) createGroupAccountRelations(ctx context.Context, db bun.IDB, userId uuid.UUID, usr *userv3.User, ignoreGrp bool) (*userv3.User, []uuid.UUID, error) { +func (s *userService) createGroupAccountRelations(ctx context.Context, db bun.IDB, userId uuid.UUID, usr *userv3.User) (*userv3.User, []uuid.UUID, error) { var grpaccs []models.GroupAccount var ugs []*authzv1.UserGroup var ids []uuid.UUID + + // Add managed groups for _, group := range utils.Unique(usr.GetSpec().GetGroups()) { // FIXME: do combined lookup entity, err := dao.GetByName(ctx, s.db, group, &models.Group{}) if err != nil { - if ignoreGrp { - continue - } return &userv3.User{}, nil, fmt.Errorf("unable to find group '%v'", group) } if grp, ok := entity.(*models.Group); ok { @@ -281,6 +284,35 @@ func (s *userService) createGroupAccountRelations(ctx context.Context, db bun.ID }) } } + + // Add idp groups + for _, group := range utils.Unique(usr.GetSpec().GetIdpGroups()) { + entity, err := dao.GetByName(ctx, s.db, group, &models.Group{}) + if err != nil { + // It is possible that a group that has been mapped via + // Idp is not available in our system. As of now, we + // ignore such cases, later when the group becomes + // available we will associate them to the group. + continue + } + if grp, ok := entity.(*models.Group); ok { + grp := models.GroupAccount{ + CreatedAt: time.Now(), + ModifiedAt: time.Now(), + Trash: false, + AccountId: userId, + GroupId: grp.ID, + Active: true, + } + ids = append(ids, grp.ID) + grpaccs = append(grpaccs, grp) + ugs = append(ugs, &authzv1.UserGroup{ + Grp: "g:" + group, + User: "u:" + usr.Metadata.Name, + }) + } + } + if len(grpaccs) == 0 { return usr, []uuid.UUID{}, nil } @@ -338,6 +370,8 @@ func (s *userService) Create(ctx context.Context, user *userv3.User) (*userv3.Us return nil, fmt.Errorf("unable to get partner and org id") } + user.Spec.IdpGroups = []string{} // we should not be taking idp groups as input on user creation + // Kratos checks if the user is already available id, err := s.ap.Create(ctx, map[string]interface{}{ "email": user.GetMetadata().GetName(), // can be just username for API access @@ -362,7 +396,7 @@ func (s *userService) Create(ctx context.Context, user *userv3.User) (*userv3.Us return &userv3.User{}, err } - user, groupsAfter, err := s.createGroupAccountRelations(ctx, tx, uuid.MustParse(id), user, false) + user, groupsAfter, err := s.createGroupAccountRelations(ctx, tx, uuid.MustParse(id), user) if err != nil { tx.Rollback() return &userv3.User{}, err @@ -387,6 +421,7 @@ func (s *userService) Create(ctx context.Context, user *userv3.User) (*userv3.Us func (s *userService) identitiesModelToUser(ctx context.Context, db bun.IDB, user *userv3.User, usr *models.KratosIdentities) (*userv3.User, error) { traits := getUserTraits(usr.Traits) + idpGroups := traits.IdpGroups groups, err := dao.GetGroups(ctx, db, usr.ID) if err != nil { return &userv3.User{}, err @@ -424,6 +459,7 @@ func (s *userService) identitiesModelToUser(ctx context.Context, db bun.IDB, use FirstName: traits.FirstName, LastName: traits.LastName, Groups: groupNames, + IdpGroups: idpGroups, ProjectNamespaceRoles: roles, } @@ -586,7 +622,7 @@ func (s *userService) deleteUserRoleRelations(ctx context.Context, db bun.IDB, u func (s *userService) Update(ctx context.Context, user *userv3.User) (*userv3.User, error) { name := user.GetMetadata().GetName() - entity, err := dao.GetIdByTraits(ctx, s.db, name, &models.KratosIdentities{}) + entity, err := dao.GetByTraits(ctx, s.db, name, &models.KratosIdentities{}) if err != nil { return &userv3.User{}, fmt.Errorf("no user found with name '%v'", name) } @@ -629,7 +665,9 @@ func (s *userService) Update(ctx context.Context, user *userv3.User) (*userv3.Us return &userv3.User{}, err } - user, groupsAfter, err := s.createGroupAccountRelations(ctx, tx, usr.ID, user, false) + // Add idp groups to user so that it gets added on update + user.Spec.IdpGroups = getUserTraits(usr.Traits).IdpGroups + user, groupsAfter, err := s.createGroupAccountRelations(ctx, tx, usr.ID, user) if err != nil { tx.Rollback() return &userv3.User{}, err @@ -705,7 +743,7 @@ func (s *userService) List(ctx context.Context, opts ...query.Option) (*userv3.U }, } - queryOptions := commonv3.QueryOptions{} + queryOptions := v3.QueryOptions{} for _, opt := range opts { opt(&queryOptions) } @@ -870,13 +908,23 @@ func (s *userService) UpdateIdpUserGroupPolicy(ctx context.Context, op, id, trai } err = json.Unmarshal([]byte(traits), &userInfo) if err != nil { - return fmt.Errorf("Encounterd error unmarshing payload to userInfo: %s", err) + return fmt.Errorf("Encountered error unmarshing payload to userInfo: %s", err) } // TODO: Revisit to only run by IDP users and not by any other // user if len(userInfo.IdpGroups) == 0 { return fmt.Errorf("Empty idp groups for user with id %s", id) } + + // Get existing user group so that the update does not wipe them out + userGroups, err := dao.GetGroups(ctx, s.db, userUUID) + ugn := []string{} + for _, g := range userGroups { + ugn = append(ugn, g.Name) + } + if err != nil { + return fmt.Errorf("Empty to find existing groups for user with id %s", id) + } user = &userv3.User{ Metadata: &v3.Metadata{ Name: userInfo.Email, @@ -884,7 +932,8 @@ func (s *userService) UpdateIdpUserGroupPolicy(ctx context.Context, op, id, trai Spec: &userv3.UserSpec{ FirstName: userInfo.FirstName, LastName: userInfo.LastName, - Groups: userInfo.IdpGroups, + Groups: ugn, + IdpGroups: userInfo.IdpGroups, }, } switch op { @@ -902,7 +951,7 @@ func (s *userService) UpdateIdpUserGroupPolicy(ctx context.Context, op, id, trai // create new policies fallthrough case "INSERT": - _, _, err = s.createGroupAccountRelations(ctx, s.db, userUUID, user, true) + _, _, err = s.createGroupAccountRelations(ctx, s.db, userUUID, user) if err != nil { return err } diff --git a/pkg/service/user_test.go b/pkg/service/user_test.go index dc43821..79687bb 100644 --- a/pkg/service/user_test.go +++ b/pkg/service/user_test.go @@ -170,7 +170,7 @@ func TestUpdateUser(t *testing.T) { us := NewUserService(ap, db, &mazc, nil, common.CliConfigDownloadData{}, getLogger(), true) // performing update - uuuid := addUserIdFetchExpectation(mock) + uuuid := addUserFetchExpectation(mock) puuid, ouuid := addParterOrgFetchExpectation(mock) mock.ExpectBegin() _ = addUserRoleMappingsUpdateExpectation(mock, uuuid) @@ -197,6 +197,90 @@ func TestUpdateUser(t *testing.T) { performBasicAuthProviderChecks(t, *ap, 0, 1, 0, 0) } +func TestUpdateUserWithGroup(t *testing.T) { + db, mock := getDB(t) + defer db.Close() + + ap := &mockAuthProvider{} + mazc := mockAuthzClient{} + us := NewUserService(ap, db, &mazc, nil, common.CliConfigDownloadData{}, getLogger(), true) + + // performing update + uuuid := addUserFetchExpectation(mock) + puuid, ouuid := addParterOrgFetchExpectation(mock) + mock.ExpectBegin() + _ = addUserRoleMappingsUpdateExpectation(mock, uuuid) + addUserGroupMappingsUpdateExpectation(mock, uuuid) + ruuid := addResourceRoleFetchExpectation(mock, "project") + pruuid := addFetchExpectation(mock, "project") + mock.ExpectQuery(`INSERT INTO "authsrv_projectaccountresourcerole"`). + WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(uuid.New().String())) + addFetchExpectation(mock, "group") + mock.ExpectQuery(`INSERT INTO "authsrv_groupaccount"`). + WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(uuid.New().String())) + mock.ExpectCommit() + + var ns int64 = 7 + user := &userv3.User{ + Metadata: &v3.Metadata{Partner: "partner-" + puuid, Organization: "org-" + ouuid, Name: "user-" + uuuid}, + Spec: &userv3.UserSpec{ + Groups: []string{"group"}, + ProjectNamespaceRoles: []*userv3.ProjectNamespaceRole{{Project: idnamea(pruuid, "project"), Namespace: &ns, Role: idname(ruuid, "role")}}, + }, + } + user, err := us.Update(context.Background(), user) + if err != nil { + t.Fatal("could not create user:", err) + } + performUserBasicChecks(t, user, uuuid) + if user.GetMetadata().GetName() != "user-"+uuuid { + t.Errorf("expected name 'user-%v'; got '%v'", uuuid, user.GetMetadata().GetName()) + } + performBasicAuthProviderChecks(t, *ap, 0, 1, 0, 0) +} + +func TestUpdateUserInvalid(t *testing.T) { + db, mock := getDB(t) + defer db.Close() + + ap := &mockAuthProvider{} + mazc := mockAuthzClient{} + us := NewUserService(ap, db, &mazc, nil, common.CliConfigDownloadData{}, getLogger(), true) + + // performing update + uuuid := addUserFetchExpectation(mock) + puuid, ouuid := addParterOrgFetchExpectation(mock) + mock.ExpectBegin() + _ = addUserRoleMappingsUpdateExpectation(mock, uuuid) + addUserGroupMappingsUpdateExpectation(mock, uuuid) + ruuid := addResourceRoleFetchExpectation(mock, "project") + pruuid := addFetchExpectation(mock, "project") + mock.ExpectQuery(`INSERT INTO "authsrv_projectaccountresourcerole"`). + WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow(uuid.New().String())) + mock.ExpectCommit() + + var ns int64 = 7 + user := &userv3.User{ + Metadata: &v3.Metadata{Partner: "partner-" + puuid, Organization: "org-" + ouuid, Name: "user-" + uuuid}, + Spec: &userv3.UserSpec{ + IdpGroups: []string{"unnecessary"}, + ProjectNamespaceRoles: []*userv3.ProjectNamespaceRole{{Project: idnamea(pruuid, "project"), Namespace: &ns, Role: idname(ruuid, "role")}}, + }, + } + user, err := us.Update(context.Background(), user) + if err != nil { + t.Fatal("could not create user:", err) + } + performUserBasicChecks(t, user, uuuid) + if len(user.Spec.IdpGroups) != 0 { + t.Errorf("Idp groups added to local user") + } + if user.GetMetadata().GetName() != "user-"+uuuid { + t.Errorf("expected name 'user-%v'; got '%v'", uuuid, user.GetMetadata().GetName()) + } + performBasicAuthProviderChecks(t, *ap, 0, 1, 0, 0) +} + func TestUserGetByName(t *testing.T) { db, mock := getDB(t) defer db.Close() diff --git a/proto/types/userpb/v3/user.pb.go b/proto/types/userpb/v3/user.pb.go index 4d7aa9b..e07389d 100644 --- a/proto/types/userpb/v3/user.pb.go +++ b/proto/types/userpb/v3/user.pb.go @@ -111,9 +111,10 @@ type UserInfoSpec struct { LastName string `protobuf:"bytes,2,opt,name=lastName,proto3" json:"lastName,omitempty"` Phone string `protobuf:"bytes,4,opt,name=phone,proto3" json:"phone,omitempty"` Groups []string `protobuf:"bytes,6,rep,name=groups,proto3" json:"groups,omitempty"` - Permissions []*Permission `protobuf:"bytes,7,rep,name=permissions,proto3" json:"permissions,omitempty"` - EmailVerified bool `protobuf:"varint,8,opt,name=emailVerified,proto3" json:"emailVerified,omitempty"` - PhoneVerified bool `protobuf:"varint,9,opt,name=phoneVerified,proto3" json:"phoneVerified,omitempty"` + IdpGroups []string `protobuf:"bytes,7,rep,name=idpGroups,proto3" json:"idpGroups,omitempty"` + Permissions []*Permission `protobuf:"bytes,8,rep,name=permissions,proto3" json:"permissions,omitempty"` + EmailVerified bool `protobuf:"varint,9,opt,name=emailVerified,proto3" json:"emailVerified,omitempty"` + PhoneVerified bool `protobuf:"varint,10,opt,name=phoneVerified,proto3" json:"phoneVerified,omitempty"` } func (x *UserInfoSpec) Reset() { @@ -176,6 +177,13 @@ func (x *UserInfoSpec) GetGroups() []string { return nil } +func (x *UserInfoSpec) GetIdpGroups() []string { + if x != nil { + return x.IdpGroups + } + return nil +} + func (x *UserInfoSpec) GetPermissions() []*Permission { if x != nil { return x.Permissions @@ -286,10 +294,11 @@ type UserSpec struct { Phone string `protobuf:"bytes,4,opt,name=phone,proto3" json:"phone,omitempty"` Password string `protobuf:"bytes,5,opt,name=password,proto3" json:"password,omitempty"` Groups []string `protobuf:"bytes,6,rep,name=groups,proto3" json:"groups,omitempty"` - ProjectNamespaceRoles []*ProjectNamespaceRole `protobuf:"bytes,7,rep,name=projectNamespaceRoles,proto3" json:"projectNamespaceRoles,omitempty"` - EmailVerified bool `protobuf:"varint,8,opt,name=emailVerified,proto3" json:"emailVerified,omitempty"` - PhoneVerified bool `protobuf:"varint,9,opt,name=phoneVerified,proto3" json:"phoneVerified,omitempty"` - RecoveryUrl *string `protobuf:"bytes,10,opt,name=recoveryUrl,proto3,oneof" json:"recoveryUrl,omitempty"` + IdpGroups []string `protobuf:"bytes,7,rep,name=idpGroups,proto3" json:"idpGroups,omitempty"` + ProjectNamespaceRoles []*ProjectNamespaceRole `protobuf:"bytes,8,rep,name=projectNamespaceRoles,proto3" json:"projectNamespaceRoles,omitempty"` + EmailVerified bool `protobuf:"varint,9,opt,name=emailVerified,proto3" json:"emailVerified,omitempty"` + PhoneVerified bool `protobuf:"varint,10,opt,name=phoneVerified,proto3" json:"phoneVerified,omitempty"` + RecoveryUrl *string `protobuf:"bytes,11,opt,name=recoveryUrl,proto3,oneof" json:"recoveryUrl,omitempty"` } func (x *UserSpec) Reset() { @@ -359,6 +368,13 @@ func (x *UserSpec) GetGroups() []string { return nil } +func (x *UserSpec) GetIdpGroups() []string { + if x != nil { + return x.IdpGroups + } + return nil +} + func (x *UserSpec) GetProjectNamespaceRoles() []*ProjectNamespaceRole { if x != nil { return x.ProjectNamespaceRoles @@ -566,7 +582,7 @@ var file_proto_types_userpb_v3_user_proto_rawDesc = []byte{ 0x0a, 0x3b, 0x2a, 0x08, 0x55, 0x73, 0x65, 0x72, 0x49, 0x6e, 0x66, 0x6f, 0x32, 0x09, 0x55, 0x73, 0x65, 0x72, 0x20, 0x69, 0x6e, 0x66, 0x6f, 0xd2, 0x01, 0x0a, 0x61, 0x70, 0x69, 0x56, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0xd2, 0x01, 0x04, 0x6b, 0x69, 0x6e, 0x64, 0xd2, 0x01, 0x08, 0x6d, 0x65, - 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0xd2, 0x01, 0x04, 0x73, 0x70, 0x65, 0x63, 0x22, 0xa2, 0x05, + 0x74, 0x61, 0x64, 0x61, 0x74, 0x61, 0xd2, 0x01, 0x04, 0x73, 0x70, 0x65, 0x63, 0x22, 0xf2, 0x05, 0x0a, 0x0c, 0x55, 0x73, 0x65, 0x72, 0x49, 0x6e, 0x66, 0x6f, 0x53, 0x70, 0x65, 0x63, 0x12, 0x44, 0x0a, 0x09, 0x66, 0x69, 0x72, 0x73, 0x74, 0x4e, 0x61, 0x6d, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x42, 0x26, 0x92, 0x41, 0x23, 0x2a, 0x09, 0x46, 0x69, 0x72, 0x73, 0x74, 0x4e, 0x61, 0x6d, @@ -584,22 +600,27 @@ var file_proto_types_userpb_v3_user_proto_rawDesc = []byte{ 0x28, 0x09, 0x42, 0x28, 0x92, 0x41, 0x25, 0x2a, 0x05, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x32, 0x1a, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x73, 0x20, 0x74, 0x68, 0x65, 0x20, 0x75, 0x73, 0x65, 0x72, 0x20, 0x62, 0x65, 0x6c, 0x6f, 0x6e, 0x67, 0x73, 0x20, 0x74, 0x6f, 0x40, 0x01, 0x52, 0x06, 0x67, 0x72, + 0x6f, 0x75, 0x70, 0x73, 0x12, 0x4e, 0x0a, 0x09, 0x69, 0x64, 0x70, 0x47, 0x72, 0x6f, 0x75, 0x70, + 0x73, 0x18, 0x07, 0x20, 0x03, 0x28, 0x09, 0x42, 0x30, 0x92, 0x41, 0x2d, 0x2a, 0x09, 0x49, 0x64, + 0x70, 0x20, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x32, 0x1e, 0x49, 0x64, 0x70, 0x20, 0x47, 0x72, 0x6f, + 0x75, 0x70, 0x73, 0x20, 0x74, 0x68, 0x65, 0x20, 0x75, 0x73, 0x65, 0x72, 0x20, 0x62, 0x65, 0x6c, + 0x6f, 0x6e, 0x67, 0x73, 0x20, 0x74, 0x6f, 0x40, 0x01, 0x52, 0x09, 0x69, 0x64, 0x70, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x73, 0x12, 0x71, 0x0a, 0x0b, 0x70, 0x65, 0x72, 0x6d, 0x69, 0x73, 0x73, 0x69, - 0x6f, 0x6e, 0x73, 0x18, 0x07, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x23, 0x2e, 0x72, 0x61, 0x66, 0x61, + 0x6f, 0x6e, 0x73, 0x18, 0x08, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x23, 0x2e, 0x72, 0x61, 0x66, 0x61, 0x79, 0x2e, 0x64, 0x65, 0x76, 0x2e, 0x74, 0x79, 0x70, 0x65, 0x73, 0x2e, 0x75, 0x73, 0x65, 0x72, 0x2e, 0x76, 0x33, 0x2e, 0x50, 0x65, 0x72, 0x6d, 0x69, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x42, 0x2a, 0x92, 0x41, 0x27, 0x2a, 0x0b, 0x50, 0x65, 0x72, 0x6d, 0x69, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x32, 0x18, 0x50, 0x65, 0x72, 0x6d, 0x69, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x20, 0x66, 0x6f, 0x72, 0x20, 0x74, 0x68, 0x65, 0x20, 0x75, 0x73, 0x65, 0x72, 0x52, 0x0b, 0x70, 0x65, 0x72, 0x6d, 0x69, 0x73, 0x73, 0x69, 0x6f, 0x6e, 0x73, 0x12, 0x6e, 0x0a, 0x0d, 0x65, 0x6d, 0x61, 0x69, 0x6c, - 0x56, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x18, 0x08, 0x20, 0x01, 0x28, 0x08, 0x42, 0x48, + 0x56, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x18, 0x09, 0x20, 0x01, 0x28, 0x08, 0x42, 0x48, 0x92, 0x41, 0x45, 0x2a, 0x0d, 0x45, 0x6d, 0x61, 0x69, 0x6c, 0x56, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x32, 0x32, 0x46, 0x6c, 0x61, 0x67, 0x20, 0x74, 0x6f, 0x20, 0x73, 0x68, 0x6f, 0x77, 0x20, 0x69, 0x66, 0x20, 0x74, 0x68, 0x65, 0x20, 0x65, 0x6d, 0x61, 0x69, 0x6c, 0x20, 0x6f, 0x66, 0x20, 0x74, 0x68, 0x65, 0x20, 0x75, 0x73, 0x65, 0x72, 0x20, 0x77, 0x61, 0x73, 0x20, 0x76, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x40, 0x01, 0x52, 0x0d, 0x65, 0x6d, 0x61, 0x69, 0x6c, 0x56, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x12, 0x71, 0x0a, 0x0d, 0x70, 0x68, 0x6f, 0x6e, 0x65, - 0x56, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x18, 0x09, 0x20, 0x01, 0x28, 0x08, 0x42, 0x4b, + 0x56, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x18, 0x0a, 0x20, 0x01, 0x28, 0x08, 0x42, 0x4b, 0x92, 0x41, 0x48, 0x2a, 0x0d, 0x50, 0x68, 0x6f, 0x6e, 0x65, 0x56, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x32, 0x35, 0x46, 0x6c, 0x61, 0x67, 0x20, 0x74, 0x6f, 0x20, 0x73, 0x68, 0x6f, 0x77, 0x20, 0x69, 0x66, 0x20, 0x70, 0x68, 0x6f, 0x6e, 0x65, 0x20, 0x6e, 0x75, 0x6d, 0x62, 0x65, 0x72, @@ -642,7 +663,7 @@ var file_proto_types_userpb_v3_user_proto_rawDesc = []byte{ 0x74, 0x75, 0x73, 0x3a, 0x37, 0x92, 0x41, 0x34, 0x0a, 0x32, 0x2a, 0x04, 0x55, 0x73, 0x65, 0x72, 0x32, 0x04, 0x55, 0x73, 0x65, 0x72, 0xd2, 0x01, 0x0a, 0x61, 0x70, 0x69, 0x56, 0x65, 0x72, 0x73, 0x69, 0x6f, 0x6e, 0xd2, 0x01, 0x04, 0x6b, 0x69, 0x6e, 0x64, 0xd2, 0x01, 0x08, 0x6d, 0x65, 0x74, - 0x61, 0x64, 0x61, 0x74, 0x61, 0xd2, 0x01, 0x04, 0x73, 0x70, 0x65, 0x63, 0x22, 0x92, 0x07, 0x0a, + 0x61, 0x64, 0x61, 0x74, 0x61, 0xd2, 0x01, 0x04, 0x73, 0x70, 0x65, 0x63, 0x22, 0xe2, 0x07, 0x0a, 0x08, 0x55, 0x73, 0x65, 0x72, 0x53, 0x70, 0x65, 0x63, 0x12, 0x44, 0x0a, 0x09, 0x66, 0x69, 0x72, 0x73, 0x74, 0x4e, 0x61, 0x6d, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x42, 0x26, 0x92, 0x41, 0x23, 0x2a, 0x09, 0x46, 0x69, 0x72, 0x73, 0x74, 0x4e, 0x61, 0x6d, 0x65, 0x32, 0x16, 0x46, 0x69, @@ -664,8 +685,13 @@ var file_proto_types_userpb_v3_user_proto_rawDesc = []byte{ 0x92, 0x41, 0x25, 0x2a, 0x05, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x32, 0x1a, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x73, 0x20, 0x74, 0x68, 0x65, 0x20, 0x75, 0x73, 0x65, 0x72, 0x20, 0x62, 0x65, 0x6c, 0x6f, 0x6e, 0x67, 0x73, 0x20, 0x74, 0x6f, 0x40, 0x01, 0x52, 0x06, 0x67, 0x72, 0x6f, 0x75, 0x70, 0x73, + 0x12, 0x4e, 0x0a, 0x09, 0x69, 0x64, 0x70, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x73, 0x18, 0x07, 0x20, + 0x03, 0x28, 0x09, 0x42, 0x30, 0x92, 0x41, 0x2d, 0x2a, 0x09, 0x49, 0x64, 0x70, 0x20, 0x47, 0x72, + 0x6f, 0x75, 0x70, 0x32, 0x1e, 0x49, 0x64, 0x70, 0x20, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x73, 0x20, + 0x74, 0x68, 0x65, 0x20, 0x75, 0x73, 0x65, 0x72, 0x20, 0x62, 0x65, 0x6c, 0x6f, 0x6e, 0x67, 0x73, + 0x20, 0x74, 0x6f, 0x40, 0x01, 0x52, 0x09, 0x69, 0x64, 0x70, 0x47, 0x72, 0x6f, 0x75, 0x70, 0x73, 0x12, 0xaf, 0x01, 0x0a, 0x15, 0x70, 0x72, 0x6f, 0x6a, 0x65, 0x63, 0x74, 0x4e, 0x61, 0x6d, 0x65, - 0x73, 0x70, 0x61, 0x63, 0x65, 0x52, 0x6f, 0x6c, 0x65, 0x73, 0x18, 0x07, 0x20, 0x03, 0x28, 0x0b, + 0x73, 0x70, 0x61, 0x63, 0x65, 0x52, 0x6f, 0x6c, 0x65, 0x73, 0x18, 0x08, 0x20, 0x03, 0x28, 0x0b, 0x32, 0x2d, 0x2e, 0x72, 0x61, 0x66, 0x61, 0x79, 0x2e, 0x64, 0x65, 0x76, 0x2e, 0x74, 0x79, 0x70, 0x65, 0x73, 0x2e, 0x75, 0x73, 0x65, 0x72, 0x2e, 0x76, 0x33, 0x2e, 0x50, 0x72, 0x6f, 0x6a, 0x65, 0x63, 0x74, 0x4e, 0x61, 0x6d, 0x65, 0x73, 0x70, 0x61, 0x63, 0x65, 0x52, 0x6f, 0x6c, 0x65, 0x42, @@ -676,21 +702,21 @@ var file_proto_types_userpb_v3_user_proto_rawDesc = []byte{ 0x6e, 0x73, 0x20, 0x66, 0x6f, 0x72, 0x20, 0x75, 0x73, 0x65, 0x72, 0x52, 0x15, 0x70, 0x72, 0x6f, 0x6a, 0x65, 0x63, 0x74, 0x4e, 0x61, 0x6d, 0x65, 0x73, 0x70, 0x61, 0x63, 0x65, 0x52, 0x6f, 0x6c, 0x65, 0x73, 0x12, 0x6e, 0x0a, 0x0d, 0x65, 0x6d, 0x61, 0x69, 0x6c, 0x56, 0x65, 0x72, 0x69, 0x66, - 0x69, 0x65, 0x64, 0x18, 0x08, 0x20, 0x01, 0x28, 0x08, 0x42, 0x48, 0x92, 0x41, 0x45, 0x2a, 0x0d, + 0x69, 0x65, 0x64, 0x18, 0x09, 0x20, 0x01, 0x28, 0x08, 0x42, 0x48, 0x92, 0x41, 0x45, 0x2a, 0x0d, 0x45, 0x6d, 0x61, 0x69, 0x6c, 0x56, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x32, 0x32, 0x46, 0x6c, 0x61, 0x67, 0x20, 0x74, 0x6f, 0x20, 0x73, 0x68, 0x6f, 0x77, 0x20, 0x69, 0x66, 0x20, 0x74, 0x68, 0x65, 0x20, 0x65, 0x6d, 0x61, 0x69, 0x6c, 0x20, 0x6f, 0x66, 0x20, 0x74, 0x68, 0x65, 0x20, 0x75, 0x73, 0x65, 0x72, 0x20, 0x77, 0x61, 0x73, 0x20, 0x76, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x40, 0x01, 0x52, 0x0d, 0x65, 0x6d, 0x61, 0x69, 0x6c, 0x56, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x12, 0x71, 0x0a, 0x0d, 0x70, 0x68, 0x6f, 0x6e, 0x65, 0x56, 0x65, 0x72, 0x69, 0x66, - 0x69, 0x65, 0x64, 0x18, 0x09, 0x20, 0x01, 0x28, 0x08, 0x42, 0x4b, 0x92, 0x41, 0x48, 0x2a, 0x0d, + 0x69, 0x65, 0x64, 0x18, 0x0a, 0x20, 0x01, 0x28, 0x08, 0x42, 0x4b, 0x92, 0x41, 0x48, 0x2a, 0x0d, 0x50, 0x68, 0x6f, 0x6e, 0x65, 0x56, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x32, 0x35, 0x46, 0x6c, 0x61, 0x67, 0x20, 0x74, 0x6f, 0x20, 0x73, 0x68, 0x6f, 0x77, 0x20, 0x69, 0x66, 0x20, 0x70, 0x68, 0x6f, 0x6e, 0x65, 0x20, 0x6e, 0x75, 0x6d, 0x62, 0x65, 0x72, 0x20, 0x6f, 0x66, 0x20, 0x74, 0x68, 0x65, 0x20, 0x75, 0x73, 0x65, 0x72, 0x20, 0x77, 0x61, 0x73, 0x20, 0x76, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x40, 0x01, 0x52, 0x0d, 0x70, 0x68, 0x6f, 0x6e, 0x65, 0x56, 0x65, 0x72, 0x69, 0x66, 0x69, 0x65, 0x64, 0x12, 0x6b, 0x0a, 0x0b, 0x72, 0x65, 0x63, 0x6f, 0x76, 0x65, 0x72, - 0x79, 0x55, 0x72, 0x6c, 0x18, 0x0a, 0x20, 0x01, 0x28, 0x09, 0x42, 0x44, 0x92, 0x41, 0x41, 0x2a, + 0x79, 0x55, 0x72, 0x6c, 0x18, 0x0b, 0x20, 0x01, 0x28, 0x09, 0x42, 0x44, 0x92, 0x41, 0x41, 0x2a, 0x0c, 0x52, 0x65, 0x63, 0x6f, 0x76, 0x65, 0x72, 0x79, 0x20, 0x55, 0x72, 0x6c, 0x32, 0x2f, 0x49, 0x6e, 0x69, 0x74, 0x69, 0x61, 0x6c, 0x20, 0x73, 0x69, 0x67, 0x6e, 0x75, 0x70, 0x20, 0x55, 0x52, 0x4c, 0x20, 0x72, 0x65, 0x74, 0x75, 0x72, 0x6e, 0x65, 0x64, 0x20, 0x61, 0x66, 0x74, 0x65, 0x72, diff --git a/proto/types/userpb/v3/user.proto b/proto/types/userpb/v3/user.proto index fee2498..dcf0e31 100644 --- a/proto/types/userpb/v3/user.proto +++ b/proto/types/userpb/v3/user.proto @@ -75,18 +75,24 @@ message UserInfoSpec { description : "Groups the user belongs to" read_only : true, } ]; - repeated rafay.dev.types.user.v3.Permission permissions = 7 + repeated string idpGroups = 7 + [ (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + title : "Idp Group" + description : "Idp Groups the user belongs to" + read_only : true, + } ]; + repeated rafay.dev.types.user.v3.Permission permissions = 8 [ (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { title : "Permissions" description : "Permissions for the user" } ]; - bool emailVerified = 8 + bool emailVerified = 9 [ (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { title : "EmailVerified" description : "Flag to show if the email of the user was verified" read_only : true } ]; - bool phoneVerified = 9 + bool phoneVerified = 10 [ (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { title : "PhoneVerified" description : "Flag to show if phone number of the user was verified" @@ -167,24 +173,30 @@ message UserSpec { description : "Groups the user belongs to" read_only : true, } ]; - repeated rafay.dev.types.user.v3.ProjectNamespaceRole projectNamespaceRoles = 7 + repeated string idpGroups = 7 + [ (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + title : "Idp Group" + description : "Idp Groups the user belongs to" + read_only : true, + } ]; + repeated rafay.dev.types.user.v3.ProjectNamespaceRole projectNamespaceRoles = 8 [ (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { title : "ProjectNamespaceRoles" description : "Project, namespace, role associations for user" } ]; - bool emailVerified = 8 + bool emailVerified = 9 [ (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { title : "EmailVerified" description : "Flag to show if the email of the user was verified" read_only : true } ]; - bool phoneVerified = 9 + bool phoneVerified = 10 [ (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { title : "PhoneVerified" description : "Flag to show if phone number of the user was verified" read_only : true } ]; - optional string recoveryUrl = 10 + optional string recoveryUrl = 11 [ (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { title : "Recovery Url", description : "Initial signup URL returned after user creation" From 5c68a335372822ee0ffe75d397679339fd1b5f05 Mon Sep 17 00:00:00 2001 From: Abin Simon Date: Fri, 13 May 2022 16:24:58 +0530 Subject: [PATCH 4/6] Prevent combining idp and managed groups --- internal/dao/common.go | 13 +++++++++++++ internal/dao/user.go | 13 +++++++++++++ pkg/service/test_utils.go | 7 +++++++ pkg/service/user.go | 39 +++++++++++++++++++++++++-------------- pkg/service/user_test.go | 6 +++--- 5 files changed, 61 insertions(+), 17 deletions(-) diff --git a/internal/dao/common.go b/internal/dao/common.go index e3b6597..de363ec 100644 --- a/internal/dao/common.go +++ b/internal/dao/common.go @@ -292,6 +292,19 @@ func GetByTraits(ctx context.Context, db bun.IDB, name string, entity interface{ return entity, nil } +func GetByTraitsFull(ctx context.Context, db bun.IDB, name string, entity interface{}) (interface{}, error) { + err := db.NewSelect().Model(entity). + Where("traits ->> 'email' = ?", name). + Relation("IdentityCredential"). + Relation("IdentityCredential.IdentityCredentialType"). + Scan(ctx) + if err != nil { + return nil, err + } + + return entity, nil +} + func GetIdByTraits(ctx context.Context, db bun.IDB, name string, entity interface{}) (interface{}, error) { // TODO: better name and possibly pass in trait name err := db.NewSelect().Column("id").Model(entity). diff --git a/internal/dao/user.go b/internal/dao/user.go index 3a0526a..2576a76 100644 --- a/internal/dao/user.go +++ b/internal/dao/user.go @@ -9,6 +9,19 @@ import ( "github.com/uptrace/bun" ) +func GetUserType(ctx context.Context, db bun.IDB, id uuid.UUID) (string, error) { + var user = models.KratosIdentities{} + q := db.NewSelect().Model(&user) + q.Relation("IdentityCredential"). + Relation("IdentityCredential.IdentityCredentialType") + q.Where("id = ?", id) + err := q.Scan(ctx) + if err != nil { + return "", err + } + return user.IdentityCredential.IdentityCredentialType.Name, nil +} + func GetGroups(ctx context.Context, db bun.IDB, id uuid.UUID) ([]models.Group, error) { var entities = []models.Group{} err := db.NewSelect().Model(&entities). diff --git a/pkg/service/test_utils.go b/pkg/service/test_utils.go index 9e697c1..2176560 100644 --- a/pkg/service/test_utils.go +++ b/pkg/service/test_utils.go @@ -145,6 +145,13 @@ func addUserFetchExpectation(mock sqlmock.Sqlmock) string { return uid } +func addUserFullFetchExpectation(mock sqlmock.Sqlmock) string { + uid := uuid.New().String() + mock.ExpectQuery(`SELECT "identities"."id", "identities"."schema_id", "identities"."traits", "identities"."created_at", "identities"."updated_at", "identities"."state", "identities"."state_changed_at", "identities"."nid", "identity_credential"."id" AS "identity_credential__id", "identity_credential"."identity_id" AS "identity_credential__identity_id", "identity_credential"."identity_credential_type_id" AS "identity_credential__identity_credential_type_id", "identity_credential__identity_credential_type"."id" AS "identity_credential__identity_credential_type__id", "identity_credential__identity_credential_type"."name" AS "identity_credential__identity_credential_type__name" FROM "identities" LEFT JOIN "identity_credentials" AS "identity_credential" ON ."identity_credential"."identity_id" = "identities"."id". LEFT JOIN "identity_credential_types" AS "identity_credential__identity_credential_type" ON ."identity_credential__identity_credential_type"."id" = "identity_credential"."identity_credential_type_id". WHERE .traits ->> 'email' = 'user-`+uid+`'.`). + WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id", "traits", "identity_credential__identity_credential_type__name"}).AddRow(uid, []byte(`{"email":"user-`+uid+`", "first_name": "John", "last_name": "Doe", "description": "The OG user."}`), "password")) + return uid +} + func addUsersGroupFetchExpectation(mock sqlmock.Sqlmock, user string) string { uid := uuid.New().String() mock.ExpectQuery(`SELECT "group"."id".* FROM "authsrv_group" AS "group" JOIN authsrv_groupaccount ON authsrv_groupaccount.group_id="group".id WHERE .authsrv_groupaccount.account_id = '` + user + `'`). diff --git a/pkg/service/user.go b/pkg/service/user.go index c11393b..9eec6f3 100644 --- a/pkg/service/user.go +++ b/pkg/service/user.go @@ -370,7 +370,8 @@ func (s *userService) Create(ctx context.Context, user *userv3.User) (*userv3.Us return nil, fmt.Errorf("unable to get partner and org id") } - user.Spec.IdpGroups = []string{} // we should not be taking idp groups as input on user creation + // we should not be taking idp groups as input on local user creation + user.Spec.IdpGroups = []string{} // Kratos checks if the user is already available id, err := s.ap.Create(ctx, map[string]interface{}{ @@ -429,14 +430,18 @@ func (s *userService) identitiesModelToUser(ctx context.Context, db bun.IDB, use groupNames := []string{} allAssociatedRoles := []*userv3.ProjectNamespaceRole{} for _, g := range groups { - groupNames = append(groupNames, g.Name) - - //group roles + // group roles (both idp and non idp) groupRoles, err := dao.GetGroupRoles(ctx, db, g.ID) if err != nil { return &userv3.User{}, err } allAssociatedRoles = append(allAssociatedRoles, groupRoles...) + + // idp groups will be available in both traits and groups and + // needs to be filetered out + if !utils.Contains(idpGroups, g.Name) { + groupNames = append(groupNames, g.Name) + } } labels := make(map[string]string) @@ -622,7 +627,7 @@ func (s *userService) deleteUserRoleRelations(ctx context.Context, db bun.IDB, u func (s *userService) Update(ctx context.Context, user *userv3.User) (*userv3.User, error) { name := user.GetMetadata().GetName() - entity, err := dao.GetByTraits(ctx, s.db, name, &models.KratosIdentities{}) + entity, err := dao.GetByTraitsFull(ctx, s.db, name, &models.KratosIdentities{}) if err != nil { return &userv3.User{}, fmt.Errorf("no user found with name '%v'", name) } @@ -632,14 +637,18 @@ func (s *userService) Update(ctx context.Context, user *userv3.User) (*userv3.Us if err != nil { return nil, fmt.Errorf("unable to get partner and org id") } - err = s.ap.Update(ctx, usr.ID.String(), map[string]interface{}{ - "email": user.GetMetadata().GetName(), - "first_name": user.GetSpec().GetFirstName(), - "last_name": user.GetSpec().GetLastName(), - "description": user.GetMetadata().GetDescription(), - }) - if err != nil { - return &userv3.User{}, err + + if usr.IdentityCredential.IdentityCredentialType.Name == "password" { + // Don't update details for non local(IDP) users + err = s.ap.Update(ctx, usr.ID.String(), map[string]interface{}{ + "email": user.GetMetadata().GetName(), + "first_name": user.GetSpec().GetFirstName(), + "last_name": user.GetSpec().GetLastName(), + "description": user.GetMetadata().GetDescription(), + }) + if err != nil { + return &userv3.User{}, err + } } tx, err := s.db.BeginTx(ctx, &sql.TxOptions{}) @@ -920,7 +929,9 @@ func (s *userService) UpdateIdpUserGroupPolicy(ctx context.Context, op, id, trai userGroups, err := dao.GetGroups(ctx, s.db, userUUID) ugn := []string{} for _, g := range userGroups { - ugn = append(ugn, g.Name) + if !utils.Contains(userInfo.IdpGroups, g.Name) { + ugn = append(ugn, g.Name) + } } if err != nil { return fmt.Errorf("Empty to find existing groups for user with id %s", id) diff --git a/pkg/service/user_test.go b/pkg/service/user_test.go index 79687bb..77411a2 100644 --- a/pkg/service/user_test.go +++ b/pkg/service/user_test.go @@ -170,7 +170,7 @@ func TestUpdateUser(t *testing.T) { us := NewUserService(ap, db, &mazc, nil, common.CliConfigDownloadData{}, getLogger(), true) // performing update - uuuid := addUserFetchExpectation(mock) + uuuid := addUserFullFetchExpectation(mock) puuid, ouuid := addParterOrgFetchExpectation(mock) mock.ExpectBegin() _ = addUserRoleMappingsUpdateExpectation(mock, uuuid) @@ -206,7 +206,7 @@ func TestUpdateUserWithGroup(t *testing.T) { us := NewUserService(ap, db, &mazc, nil, common.CliConfigDownloadData{}, getLogger(), true) // performing update - uuuid := addUserFetchExpectation(mock) + uuuid := addUserFullFetchExpectation(mock) puuid, ouuid := addParterOrgFetchExpectation(mock) mock.ExpectBegin() _ = addUserRoleMappingsUpdateExpectation(mock, uuuid) @@ -248,7 +248,7 @@ func TestUpdateUserInvalid(t *testing.T) { us := NewUserService(ap, db, &mazc, nil, common.CliConfigDownloadData{}, getLogger(), true) // performing update - uuuid := addUserFetchExpectation(mock) + uuuid := addUserFullFetchExpectation(mock) puuid, ouuid := addParterOrgFetchExpectation(mock) mock.ExpectBegin() _ = addUserRoleMappingsUpdateExpectation(mock, uuuid) From 2e4d8029954cbcded87d9ec41dd4071bb21d4287 Mon Sep 17 00:00:00 2001 From: Akshay Gaikwad Date: Tue, 17 May 2022 12:55:37 +0530 Subject: [PATCH 5/6] Fix: Type assertion error on IdPGroups --- pkg/service/user.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/service/user.go b/pkg/service/user.go index 9eec6f3..db32c1f 100644 --- a/pkg/service/user.go +++ b/pkg/service/user.go @@ -100,16 +100,23 @@ func getUserTraits(traits map[string]interface{}) userTraits { if !ok { desc = "" } + + igStr := []string{} ig, ok := traits["idp_groups"] - if !ok { - ig = []string{} + if ok { + igList := ig.([]interface{}) + igStr = make([]string, len(igList)) + for i, g := range igList { + igStr[i] = g.(string) + } } + return userTraits{ Email: email.(string), FirstName: fname.(string), LastName: lname.(string), Description: desc.(string), - IdpGroups: ig.([]string), + IdpGroups: igStr, } } From d26dfa5e5548edc437379b92c72ea73ad4da9f5a Mon Sep 17 00:00:00 2001 From: Akshay Gaikwad Date: Tue, 17 May 2022 13:20:10 +0530 Subject: [PATCH 6/6] Fix: go formatting --- pkg/service/test_utils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/service/test_utils.go b/pkg/service/test_utils.go index 2176560..e1d5e60 100644 --- a/pkg/service/test_utils.go +++ b/pkg/service/test_utils.go @@ -147,7 +147,7 @@ func addUserFetchExpectation(mock sqlmock.Sqlmock) string { func addUserFullFetchExpectation(mock sqlmock.Sqlmock) string { uid := uuid.New().String() - mock.ExpectQuery(`SELECT "identities"."id", "identities"."schema_id", "identities"."traits", "identities"."created_at", "identities"."updated_at", "identities"."state", "identities"."state_changed_at", "identities"."nid", "identity_credential"."id" AS "identity_credential__id", "identity_credential"."identity_id" AS "identity_credential__identity_id", "identity_credential"."identity_credential_type_id" AS "identity_credential__identity_credential_type_id", "identity_credential__identity_credential_type"."id" AS "identity_credential__identity_credential_type__id", "identity_credential__identity_credential_type"."name" AS "identity_credential__identity_credential_type__name" FROM "identities" LEFT JOIN "identity_credentials" AS "identity_credential" ON ."identity_credential"."identity_id" = "identities"."id". LEFT JOIN "identity_credential_types" AS "identity_credential__identity_credential_type" ON ."identity_credential__identity_credential_type"."id" = "identity_credential"."identity_credential_type_id". WHERE .traits ->> 'email' = 'user-`+uid+`'.`). + mock.ExpectQuery(`SELECT "identities"."id", "identities"."schema_id", "identities"."traits", "identities"."created_at", "identities"."updated_at", "identities"."state", "identities"."state_changed_at", "identities"."nid", "identity_credential"."id" AS "identity_credential__id", "identity_credential"."identity_id" AS "identity_credential__identity_id", "identity_credential"."identity_credential_type_id" AS "identity_credential__identity_credential_type_id", "identity_credential__identity_credential_type"."id" AS "identity_credential__identity_credential_type__id", "identity_credential__identity_credential_type"."name" AS "identity_credential__identity_credential_type__name" FROM "identities" LEFT JOIN "identity_credentials" AS "identity_credential" ON ."identity_credential"."identity_id" = "identities"."id". LEFT JOIN "identity_credential_types" AS "identity_credential__identity_credential_type" ON ."identity_credential__identity_credential_type"."id" = "identity_credential"."identity_credential_type_id". WHERE .traits ->> 'email' = 'user-` + uid + `'.`). WithArgs().WillReturnRows(sqlmock.NewRows([]string{"id", "traits", "identity_credential__identity_credential_type__name"}).AddRow(uid, []byte(`{"email":"user-`+uid+`", "first_name": "John", "last_name": "Doe", "description": "The OG user."}`), "password")) return uid }