{"action":"created","comment":{"url":"https://api.github.com/repos/ovh/cds/pulls/comments/316139713","pull_request_review_id":277736527,"id":316139713,"node_id":"MDI0OlB1bGxSZXF1ZXN0UmV2aWV3Q29tbWVudDMxNjEzOTcxMw==","diff_hunk":"@@ -14,352 +14,387 @@ import (\n \t\"github.com/ovh/cds/sdk\"\n )\n \n-func (api *API) getGroupHandler() service.Handler {\n+func (api *API) getGroupsHandler() service.Handler {\n \treturn func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {\n-\t\t// Get group name in URL\n-\t\tvars := mux.Vars(r)\n-\t\tname := vars[\"permGroupName\"]\n+\t\tvar groups []sdk.Group\n+\t\tvar err error\n \n-\t\tg, errl := group.LoadGroup(api.mustDB(), name)\n-\t\tif errl != nil {\n-\t\t\treturn sdk.WrapError(errl, \"Cannot load group from db\")\n+\t\twithoutDefault := FormBool(r, \"withoutDefault\")\n+\t\tif isMaintainer(ctx) {\n+\t\t\tgroups, err = group.LoadAll(ctx, api.mustDB())\n+\t\t} else {\n+\t\t\tgroups, err = group.LoadAllByDeprecatedUserID(ctx, api.mustDB(), getAPIConsumer(ctx).AuthentifiedUser.OldUserStruct.ID)\n \t\t}\n-\n-\t\tif err := group.LoadUserGroup(api.mustDB(), g); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot load user group from db\")\n+\t\tif err != nil {\n+\t\t\treturn err\n \t\t}\n \n-\t\tisGroupAdmin := false\n-\t\tcurrentUser := deprecatedGetUser(ctx)\n-\t\tfor _, grAdmin := range g.Admins {\n-\t\t\tif currentUser.ID == grAdmin.ID {\n-\t\t\t\tisGroupAdmin = true\n-\t\t\t}\n-\t\t}\n-\t\tif isGroupAdmin {\n-\t\t\ttokens, errT := group.LoadTokens(api.mustDB(), name)\n-\t\t\tif errT != nil {\n-\t\t\t\treturn sdk.WrapError(errT, \"Cannot load tokens group from db\")\n+\t\t// withoutDefault is use by project add, to avoid selecting the default group on project creation\n+\t\tif withoutDefault {\n+\t\t\tvar filteredGroups []sdk.Group\n+\t\t\tfor _, g := range groups {\n+\t\t\t\tif !group.IsDefaultGroupID(g.ID) {\n+\t\t\t\t\tfilteredGroups = append(filteredGroups, g)\n+\t\t\t\t}\n \t\t\t}\n-\t\t\tg.Tokens = tokens\n+\t\t\treturn service.WriteJSON(w, filteredGroups, http.StatusOK)\n \t\t}\n \n-\t\treturn service.WriteJSON(w, g, http.StatusOK)\n+\t\treturn service.WriteJSON(w, groups, http.StatusOK)\n \t}\n }\n \n-func (api *API) deleteGroupHandler() service.Handler {\n+func (api *API) getGroupHandler() service.Handler {\n \treturn func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {\n-\t\t// Get group name in URL\n \t\tvars := mux.Vars(r)\n \t\tname := vars[\"permGroupName\"]\n-\t\tu := deprecatedGetUser(ctx)\n \n-\t\tg, errl := group.LoadGroup(api.mustDB(), name)\n-\t\tif errl != nil {\n-\t\t\treturn sdk.WrapError(errl, \"Cannot load %s\", name)\n+\t\tg, err := group.LoadByName(ctx, api.mustDB(), name, group.LoadOptions.Default)\n+\t\tif err != nil {\n+\t\t\treturn err\n \t\t}\n \n-\t\tprojPerms, err := project.LoadPermissions(api.mustDB(), g.ID)\n-\t\tif err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot load projects for group\")\n+\t\treturn service.WriteJSON(w, g, http.StatusOK)\n+\t}\n+}\n+\n+func (api *API) postGroupHandler() service.Handler {\n+\treturn func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {\n+\t\tvar newGroup sdk.Group\n+\t\tif err := service.UnmarshalBody(r, &newGroup); err != nil {\n+\t\t\treturn err\n+\t\t}\n+\t\tif err := newGroup.IsValid(); err != nil {\n+\t\t\treturn err\n \t\t}\n \n-\t\ttx, errb := api.mustDB().Begin()\n-\t\tif errb != nil {\n-\t\t\treturn sdk.WrapError(errb, \"Cannot start transaction\")\n+\t\ttx, err := api.mustDB().Begin()\n+\t\tif err != nil {\n+\t\t\treturn sdk.WrapError(err, \"cannot begin tx\")\n \t\t}\n \t\tdefer tx.Rollback()\n \n-\t\tif err := group.DeleteGroupAndDependencies(tx, g); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot delete group\")\n+\t\texistingGroup, err := group.LoadByName(ctx, tx, newGroup.Name)\n+\t\tif err != nil && !sdk.ErrorIs(err, sdk.ErrNotFound) {\n+\t\t\treturn err\n+\t\t}\n+\t\tif existingGroup != nil {\n+\t\t\treturn sdk.WithStack(sdk.ErrGroupPresent)\n+\t\t}\n+\n+\t\tconsumer := getAPIConsumer(ctx)\n+\t\tif err := group.Create(tx, &newGroup, consumer.AuthentifiedUser.OldUserStruct.ID); err != nil {\n+\t\t\treturn err\n \t\t}\n \n \t\tif err := tx.Commit(); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot commit transaction\")\n+\t\t\treturn sdk.WrapError(err, \"cannot commit tx\")\n \t\t}\n \n-\t\tgroupPerm := sdk.GroupPermission{Group: *g}\n-\t\tfor _, pg := range projPerms {\n-\t\t\tevent.PublishDeleteProjectPermission(&pg.Project, groupPerm, u)\n+\t\tif err := group.LoadOptions.Default(ctx, api.mustDB(), &newGroup); err != nil {\n+\t\t\treturn err\n \t\t}\n \n-\t\treturn nil\n+\t\treturn service.WriteJSON(w, &newGroup, http.StatusCreated)\n \t}\n }\n \n-func (api *API) updateGroupHandler() service.Handler {\n+func (api *API) putGroupHandler() service.Handler {\n \treturn func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {\n-\t\t// Get group name in URL\n \t\tvars := mux.Vars(r)\n-\t\toldName := vars[\"permGroupName\"]\n+\t\tgroupName := vars[\"permGroupName\"]\n \n-\t\tvar updatedGroup sdk.Group\n-\t\tif err := service.UnmarshalBody(r, &updatedGroup); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot unmarshal\")\n+\t\tvar data sdk.Group\n+\t\tif err := service.UnmarshalBody(r, &data); err != nil {\n+\t\t\treturn err\n \t\t}\n-\n-\t\tif len(updatedGroup.Admins) == 0 {\n-\t\t\treturn sdk.WrapError(sdk.ErrGroupNeedAdmin, \"Cannot Delete all admins for group %s\", updatedGroup.Name)\n+\t\tif err := data.IsValid(); err != nil {\n+\t\t\treturn err\n \t\t}\n \n-\t\tg, errl := group.LoadGroup(api.mustDB(), oldName)\n-\t\tif errl != nil {\n-\t\t\treturn sdk.WrapError(errl, \"Cannot load %s\", oldName)\n-\t\t}\n-\n-\t\tupdatedGroup.ID = g.ID\n-\t\ttx, errb := api.mustDB().Begin()\n-\t\tif errb != nil {\n-\t\t\treturn sdk.WrapError(errb, \"Cannot start transaction\")\n+\t\ttx, err := api.mustDB().Begin()\n+\t\tif err != nil {\n+\t\t\treturn sdk.WrapError(err, \"cannot start transaction\")\n \t\t}\n \t\tdefer tx.Rollback()\n \n-\t\tif err := group.UpdateGroup(tx, &updatedGroup, oldName); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot update group %s\", oldName)\n-\t\t}\n-\n-\t\tif err := group.DeleteGroupUserByGroup(tx, &updatedGroup); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot delete users in group %s\", oldName)\n+\t\toldGroup, err := group.LoadByName(ctx, tx, groupName)\n+\t\tif err != nil {\n+\t\t\treturn sdk.WrapError(err, \"cannot load group: %s\", groupName)\n \t\t}\n \n-\t\tfor _, a := range updatedGroup.Admins {\n-\t\t\tu, errlu := user.LoadUserWithoutAuth(tx, a.Username)\n-\t\t\tif errlu != nil {\n-\t\t\t\treturn sdk.WrapError(errlu, \"Cannot load user(admins) %s\", a.Username)\n+\t\t// In case of rename, checks that new name is not already used\n+\t\tif data.Name != oldGroup.Name {\n+\t\t\texstingGroup, err := group.LoadByName(ctx, tx, data.Name)\n+\t\t\tif err != nil && !sdk.ErrorIs(err, sdk.ErrNotFound) {\n+\t\t\t\treturn err\n \t\t\t}\n-\n-\t\t\tif err := group.InsertUserInGroup(tx, updatedGroup.ID, u.ID, true); err != nil {\n-\t\t\t\treturn sdk.WrapError(err, \"Cannot insert admin %s in group %s\", a.Username, updatedGroup.Name)\n+\t\t\tif exstingGroup != nil {\n+\t\t\t\treturn sdk.WithStack(sdk.ErrGroupPresent)\n \t\t\t}\n \t\t}\n \n-\t\tfor _, a := range updatedGroup.Users {\n-\t\t\tu, errlu := user.LoadUserWithoutAuth(tx, a.Username)\n-\t\t\tif errlu != nil {\n-\t\t\t\treturn sdk.WrapError(errlu, \"Cannot load user(members) %s\", a.Username)\n-\t\t\t}\n+\t\tnewGroup := *oldGroup\n+\t\tnewGroup.Name = data.Name\n \n-\t\t\tif err := group.InsertUserInGroup(tx, updatedGroup.ID, u.ID, false); err != nil {\n-\t\t\t\treturn sdk.WrapError(err, \"Cannot insert member %s in group %s\", a.Username, updatedGroup.Name)\n-\t\t\t}\n+\t\tif err := group.Update(tx, &newGroup); err != nil {\n+\t\t\treturn sdk.WrapError(err, \"cannot update group with id: %d\", newGroup.ID)\n \t\t}\n \n+\t\t// TODO Update all requirements that was using the group name\n+\n \t\tif err := tx.Commit(); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot commit transaction\")\n+\t\t\treturn sdk.WrapError(err, \"cannot commit transaction\")\n+\t\t}\n+\n+\t\t// Load extra data for group\n+\t\tif err := group.LoadOptions.Default(ctx, api.mustDB(), &newGroup); err != nil {\n+\t\t\treturn err\n \t\t}\n \n-\t\treturn service.WriteJSON(w, updatedGroup, http.StatusOK)\n+\t\treturn service.WriteJSON(w, newGroup, http.StatusOK)\n \t}\n }\n \n-func (api *API) getGroupsHandler() service.Handler {\n+func (api *API) deleteGroupHandler() service.Handler {\n \treturn func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {\n-\t\tvar groups []sdk.Group\n-\t\tvar err error\n+\t\tvars := mux.Vars(r)\n+\t\tname := vars[\"permGroupName\"]\n+\t\tu := getAPIConsumer(ctx)\n \n-\t\tpublic := FormBool(r, \"withPublic\")\n-\t\twithoutDefault := FormBool(r, \"withoutDefault\")\n-\t\tif deprecatedGetUser(ctx).Admin {\n-\t\t\tgroups, err = group.LoadGroups(api.mustDB())\n-\t\t} else {\n-\t\t\tgroups, err = group.LoadGroupByUser(api.mustDB(), deprecatedGetUser(ctx).ID)\n-\t\t\tif public {\n-\t\t\t\tpublicGroups, errl := group.LoadPublicGroups(api.mustDB())\n-\t\t\t\tif errl != nil {\n-\t\t\t\t\treturn sdk.WrapError(errl, \"Cannot load group from db\")\n-\t\t\t\t}\n-\t\t\t\tgroups = append(groups, publicGroups...)\n-\t\t\t}\n+\t\ttx, err := api.mustDB().Begin()\n+\t\tif err != nil {\n+\t\t\treturn sdk.WrapError(err, \"cannot start transaction\")\n \t\t}\n+\t\tdefer tx.Rollback()\n+\n+\t\tg, err := group.LoadByName(ctx, tx, name)\n \t\tif err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot load group from db\")\n+\t\t\treturn sdk.WrapError(err, \"cannot load %s\", name)\n \t\t}\n \n-\t\t// withoutDefault is use by project Add, to avoid\n-\t\t// user select the default group on project creation\n-\t\tif withoutDefault {\n-\t\t\tvar filteredGroups []sdk.Group\n-\t\t\tfor _, g := range groups {\n-\t\t\t\tif group.IsDefaultGroupID(g.ID) {\n-\t\t\t\t\tcontinue\n-\t\t\t\t} else {\n-\t\t\t\t\tfilteredGroups = append(filteredGroups, g)\n-\t\t\t\t}\n-\t\t\t}\n-\t\t\treturn service.WriteJSON(w, filteredGroups, http.StatusOK)\n+\t\tprojPerms, err := project.LoadPermissions(tx, g.ID)\n+\t\tif err != nil {\n+\t\t\treturn sdk.WrapError(err, \"cannot load projects for group\")\n \t\t}\n \n-\t\treturn service.WriteJSON(w, groups, http.StatusOK)\n-\t}\n-}\n+\t\tif err := group.Delete(ctx, tx, g); err != nil {\n+\t\t\treturn sdk.WrapError(err, \"cannot delete group\")\n+\t\t}\n \n-func (api *API) getPublicGroupsHandler() service.Handler {\n-\treturn func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {\n-\t\tgroups, err := group.LoadPublicGroups(api.mustDB())\n-\t\tif err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot load group from db\")\n+\t\tif err := tx.Commit(); err != nil {\n+\t\t\treturn sdk.WrapError(err, \"cannot commit transaction\")\n \t\t}\n-\t\treturn service.WriteJSON(w, groups, http.StatusOK)\n+\n+\t\t// TODO refact events\n+\t\tgroupPerm := sdk.GroupPermission{Group: *g}\n+\t\tfor _, pg := range projPerms {\n+\t\t\tevent.PublishDeleteProjectPermission(&pg.Project, groupPerm, u)\n+\t\t}\n+\n+\t\treturn service.WriteJSON(w, nil, http.StatusOK)\n \t}\n }\n \n-func (api *API) addGroupHandler() service.Handler {\n+func (api *API) postGroupUserHandler() service.Handler {\n \treturn func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {\n-\t\tg := &sdk.Group{}\n-\t\tif err := service.UnmarshalBody(r, g); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot unmarshal\")\n+\t\tvars := mux.Vars(r)\n+\t\tgroupName := vars[\"permGroupName\"]\n+\n+\t\tvar data sdk.GroupMember\n+\t\tif err := service.UnmarshalBody(r, &data); err != nil {\n+\t\t\treturn err\n+\t\t}\n+\t\tif data.ID == \"\" && data.Username == \"\" {\n+\t\t\treturn sdk.NewErrorFrom(sdk.ErrWrongRequest, \"invalid given user id or username\")\n \t\t}\n \n-\t\ttx, errb := api.mustDB().Begin()\n-\t\tif errb != nil {\n-\t\t\treturn sdk.WrapError(errb, \"Cannot begin tx\")\n+\t\ttx, err := api.mustDB().Begin()\n+\t\tif err != nil {\n+\t\t\treturn sdk.WithStack(err)\n \t\t}\n \t\tdefer tx.Rollback()\n \n-\t\tif _, _, err := group.AddGroup(tx, g); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot add group\")\n+\t\tg, err := group.LoadByName(ctx, tx, groupName)\n+\t\tif err != nil {\n+\t\t\treturn sdk.WrapError(err, \"cannot load group with name: %s\", groupName)\n+\t\t}\n+\n+\t\tvar u *sdk.AuthentifiedUser\n+\t\tif data.ID != \"\" {\n+\t\t\tu, err = user.LoadByID(ctx, tx, data.ID, user.LoadOptions.WithDeprecatedUser)\n+\t\t} else {\n+\t\t\tu, err = user.LoadByUsername(ctx, tx, data.Username, user.LoadOptions.WithDeprecatedUser)\n+\t\t}\n+\t\tif err != nil {\n+\t\t\treturn err\n \t\t}\n \n-\t\t// Add caller into group\n-\t\tif err := group.InsertUserInGroup(tx, g.ID, deprecatedGetUser(ctx).ID, false); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot add user %s in group %s\", deprecatedGetUser(ctx).Username, g.Name)\n+\t\t// If the user is already in group return an error\n+\t\tlink, err := group.LoadLinkGroupUserForGroupIDAndUserID(ctx, tx, g.ID, u.OldUserStruct.ID)\n+\t\tif err != nil && !sdk.ErrorIs(err, sdk.ErrNotFound) {\n+\t\t\treturn err\n \t\t}\n-\t\t// and set it admin\n-\t\tif err := group.SetUserGroupAdmin(tx, g.ID, deprecatedGetUser(ctx).ID); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot set user group admin\")\n+\t\tif link != nil {\n+\t\t\treturn sdk.NewErrorFrom(sdk.ErrForbidden, \"given user is already in group\")\n+\t\t}\n+\n+\t\t// Create the link between group and user with admin flag from request\n+\t\tif err := group.InsertLinkGroupUser(tx, &group.LinkGroupUser{\n+\t\t\tGroupID: g.ID,\n+\t\t\tUserID:  u.OldUserStruct.ID,\n+\t\t\tAdmin:   data.Admin,\n+\t\t}); err != nil {\n+\t\t\treturn sdk.WrapError(err, \"cannot add user %s in group %s\", u.Username, g.Name)\n \t\t}\n \n \t\tif err := tx.Commit(); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot commit tx\")\n+\t\t\treturn sdk.WrapError(err, \"cannot commit transaction\")\n+\t\t}\n+\n+\t\t// Load extra data for group\n+\t\tif err := group.LoadOptions.Default(ctx, api.mustDB(), g); err != nil {\n+\t\t\treturn err\n \t\t}\n \n \t\treturn service.WriteJSON(w, g, http.StatusCreated)\n \t}\n }\n \n-func (api *API) removeUserFromGroupHandler() service.Handler {\n+func (api *API) putGroupUserHandler() service.Handler {\n \treturn func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {\n-\t\t// Get group name in URL\n \t\tvars := mux.Vars(r)\n-\t\tname := vars[\"permGroupName\"]\n-\t\tuserName := vars[\"user\"]\n+\t\tgroupName := vars[\"permGroupName\"]\n+\t\tusername := vars[\"username\"]\n \n-\t\tg, errl := group.LoadGroup(api.mustDB(), name)\n-\t\tif errl != nil {\n-\t\t\treturn sdk.WrapError(errl, \"Cannot load %s\", name)\n+\t\tvar data sdk.GroupMember\n+\t\tif err := service.UnmarshalBody(r, &data); err != nil {\n+\t\t\treturn err\n \t\t}\n \n-\t\tuserID, err := user.FindUserIDByName(api.mustDB(), userName)\n+\t\ttx, err := api.mustDB().Begin()\n \t\tif err != nil {\n-\t\t\treturn sdk.WrapError(sdk.ErrNotFound, \"Unknown user %s\", userName)\n+\t\t\treturn sdk.WithStack(err)\n \t\t}\n+\t\tdefer tx.Rollback()\n \n-\t\tuserInGroup, errc := group.CheckUserInGroup(api.mustDB(), g.ID, userID)\n-\t\tif errc != nil {\n-\t\t\treturn sdk.WrapError(errc, \"Cannot check if user %s is already in the group %s\", userName, g.Name)\n+\t\tg, err := group.LoadByName(ctx, tx, groupName)\n+\t\tif err != nil {\n+\t\t\treturn sdk.WrapError(err, \"cannot load group with name: %s\", groupName)\n \t\t}\n \n-\t\tif !userInGroup {\n-\t\t\treturn sdk.WrapError(sdk.ErrWrongRequest, \"User %s is not in group %s\", userName, name)\n+\t\tu, err := user.LoadByUsername(ctx, tx, username, user.LoadOptions.WithDeprecatedUser)\n+\t\tif err != nil {\n+\t\t\treturn err\n \t\t}\n \n-\t\tif err := group.DeleteUserFromGroup(api.mustDB(), g.ID, userID); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot delete user %s from group %s\", userName, g.Name)\n+\t\tlink, err := group.LoadLinkGroupUserForGroupIDAndUserID(ctx, tx, g.ID, u.OldUserStruct.ID)\n+\t\tif err != nil {\n+\t\t\treturn err\n \t\t}\n \n-\t\treturn nil\n-\t}\n-}\n-\n-func (api *API) addUserInGroupHandler() service.Handler {\n-\treturn func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {\n-\t\t// Get group name in URL\n-\t\tvars := mux.Vars(r)\n-\t\tname := vars[\"permGroupName\"]\n+\t\t// In case we are removing admin rights to user, we need to check that it's not the last admin\n+\t\tif link.Admin && !data.Admin {\n+\t\t\tlinks, err := group.LoadLinksGroupUserForGroupIDs(ctx, tx, []int64{g.ID})\n+\t\t\tif err != nil {\n+\t\t\t\treturn err\n+\t\t\t}\n \n-\t\tvar users []string\n-\t\tif err := service.UnmarshalBody(r, &users); err != nil {\n-\t\t\treturn sdk.WrapError(err, \"Cannot unmarshal\")\n+\t\t\tvar adminFound bool\n+\t\t\tfor i := range links {\n+\t\t\t\tif links[i].UserID != u.OldUserStruct.ID && links[i].Admin {\n+\t\t\t\t\tadminFound = true\n+\t\t\t\t\tbreak\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tif !adminFound {\n+\t\t\t\treturn sdk.NewErrorFrom(sdk.ErrGroupNeedAdmin, \"cannot remove the last admin of the group\")\n+\t\t\t}\n \t\t}\n \n-\t\tg, errl := group.LoadGroup(api.mustDB(), name)\n-\t\tif errl != nil {\n-\t\t\treturn sdk.WrapError(errl, \"Cannot load group %s\", name)\n+\t\tlink.Admin = data.Admin\n+\n+\t\tif err := group.UpdateLinkGroupUser(tx, link); err != nil {\n+\t\t\treturn err\n \t\t}\n \n-\t\ttx, errb := api.mustDB().Begin()\n-\t\tif errb != nil {\n-\t\t\treturn errb\n+\t\tif err := tx.Commit(); err != nil {\n+\t\t\treturn sdk.WrapError(err, \"cannot commit transaction\")\n \t\t}\n-\t\tdefer tx.Rollback()\n \n-\t\tfor _, u := range users {\n-\t\t\tuserID, errf := user.FindUserIDByName(api.mustDB(), u)\n-\t\t\tif errf != nil {\n-\t\t\t\treturn sdk.WrapError(errf, \"Unknown user '%s'\", u)\n-\t\t\t}\n-\t\t\tuserInGroup, errc := group.CheckUserInGroup(api.mustDB(), g.ID, userID)\n-\t\t\tif errc != nil {\n-\t\t\t\treturn sdk.WrapError(errc, \"Cannot check if user %s is already in the group %s\", u, g.Name)\n-\t\t\t}\n-\t\t\tif !userInGroup {\n-\t\t\t\tif err := group.InsertUserInGroup(api.mustDB(), g.ID, userID, false); err != nil {\n-\t\t\t\t\treturn sdk.WrapError(err, \"Cannot add user %s in group %s\", u, g.Name)\n-\t\t\t\t}\n-\t\t\t}\n+\t\t// Load extra data for group\n+\t\tif err := group.LoadOptions.Default(ctx, api.mustDB(), g); err != nil {\n+\t\t\treturn err\n \t\t}\n \n-\t\treturn tx.Commit()\n+\t\treturn service.WriteJSON(w, g, http.StatusOK)\n \t}\n }\n \n-func (api *API) setUserGroupAdminHandler() service.Handler {\n+func (api *API) deleteGroupUserHandler() service.Handler {\n \treturn func(ctx context.Context, w http.ResponseWriter, r *http.Request) error {\n-\t\t// Get group name in URL\n \t\tvars := mux.Vars(r)\n-\t\tname := vars[\"permGroupName\"]\n-\t\tuserName := vars[\"user\"]\n+\t\tgroupName := vars[\"permGroupName\"]\n+\t\tusername := vars[\"username\"]\n+\n+\t\ttx, err := api.mustDB().Begin()\n+\t\tif err != nil {\n+\t\t\treturn sdk.WithStack(err)\n+\t\t}\n+\t\tdefer tx.Rollback()","path":"engine/api/group.go","position":522,"original_position":522,"commit_id":"f6787fc57fc2b7dde149cb430ed163a0d2e1570d","original_commit_id":"f6787fc57fc2b7dde149cb430ed163a0d2e1570d","user":{"login":"golangcibot","id":42910462,"node_id":"MDQ6VXNlcjQyOTEwNDYy","avatar_url":"https://avatars1.githubusercontent.com/u/42910462?v=4","gravatar_id":"","url":"https://api.github.com/users/golangcibot","html_url":"https://github.com/golangcibot","followers_url":"https://api.github.com/users/golangcibot/followers","following_url":"https://api.github.com/users/golangcibot/following{/other_user}","gists_url":"https://api.github.com/users/golangcibot/gists{/gist_id}","starred_url":"https://api.github.com/users/golangcibot/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/golangcibot/subscriptions","organizations_url":"https://api.github.com/users/golangcibot/orgs","repos_url":"https://api.github.com/users/golangcibot/repos","events_url":"https://api.github.com/users/golangcibot/events{/privacy}","received_events_url":"https://api.github.com/users/golangcibot/received_events","type":"User","site_admin":false},"body":"Error return value of `tx.Rollback` is not checked (from `errcheck`)","created_at":"2019-08-21T11:45:43Z","updated_at":"2019-08-21T11:45:49Z","html_url":"https://github.com/ovh/cds/pull/4290#discussion_r316139713","pull_request_url":"https://api.github.com/repos/ovh/cds/pulls/4290","author_association":"NONE","_links":{"self":{"href":"https://api.github.com/repos/ovh/cds/pulls/comments/316139713"},"html":{"href":"https://github.com/ovh/cds/pull/4290#discussion_r316139713"},"pull_request":{"href":"https://api.github.com/repos/ovh/cds/pulls/4290"}}},"pull_request":{"url":"https://api.github.com/repos/ovh/cds/pulls/4290","id":279097110,"node_id":"MDExOlB1bGxSZXF1ZXN0Mjc5MDk3MTEw","html_url":"https://github.com/ovh/cds/pull/4290","diff_url":"https://github.com/ovh/cds/pull/4290.diff","patch_url":"https://github.com/ovh/cds/pull/4290.patch","issue_url":"https://api.github.com/repos/ovh/cds/issues/4290","number":4290,"state":"open","locked":false,"title":"Refactor/users","user":{"login":"fsamin","id":684151,"node_id":"MDQ6VXNlcjY4NDE1MQ==","avatar_url":"https://avatars1.githubusercontent.com/u/684151?v=4","gravatar_id":"","url":"https://api.github.com/users/fsamin","html_url":"https://github.com/fsamin","followers_url":"https://api.github.com/users/fsamin/followers","following_url":"https://api.github.com/users/fsamin/following{/other_user}","gists_url":"https://api.github.com/users/fsamin/gists{/gist_id}","starred_url":"https://api.github.com/users/fsamin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fsamin/subscriptions","organizations_url":"https://api.github.com/users/fsamin/orgs","repos_url":"https://api.github.com/users/fsamin/repos","events_url":"https://api.github.com/users/fsamin/events{/privacy}","received_events_url":"https://api.github.com/users/fsamin/received_events","type":"User","site_admin":false},"body":"1. Description\r\n\r\n- [ ] When a user is removed from a group, removed from admins, when a group is deleted, check if some consumers need to be revoked or updated.\r\n- [ ] Use of a builtin signin token in CLI, should use the client that automatically signin. We can also signin manually.\r\n- [ ] Consumer \"data\" map should be scoped to authentication package.\r\n\r\n TODOs\r\n - [ ] cdsclt consumer & sessions management\r\n - [ ] `getActionsForGroupHandler, getWorkerModelsForGroupHandler`: move this in a permGroupID middleware\r\n - [x] api startup authentication drivers initialization\r\n - [x] LDAP authentication drivers\r\n - [ ] api debug handlers `getProfileIndexHandler`\r\n - [x] `updateGroupHandler`checks number of remaining admins\r\n - [ ] `GetUserWorkflowEvents`send email notifications\r\n - [ ] `Test_checkWorkerModelPermissionsByUser`\r\n - [x] `Test_checkWorkflowPermissionsByUser`\r\n - [x] `postServiceRegisterHandler`: for hatcheries, the user who created the used token must be admin of all groups in the token\r\n  - [x] `serviceAPIHeartbeat`: fake user ?\r\n  - [x] `bookWorkerModelHandler`: permModelID middleware\r\n  - [x] Implement a unit test for checkGroupPermissions\r\n  - [ ] Implement a unit test for checkWorkerModelPermissions\r\n  - [ ] Implement a unit test for checkActionPermissions\r\n  - [ ] Implement permActionBuiltinName\r\n  - [ ] Implement a unit tests for checkTemplateSlugPermissions\r\n  - [ ] Check the \"triggered by\" data when a workflow is run by a uService\r\n  - [ ] [ERROR] error executing query UPDATE services SET sig = $2 WHERE id = $1 with parameters services, id: pq: payload string too long \r\n - [ ] Implement session clean goroutine on API side\r\n - [ ] Add XSRF signin local and ldap\r\n - [ ] username check syntax backend\r\n - [ ] Update all requirements that was using the group name","created_at":"2019-05-15T13:25:52Z","updated_at":"2019-08-21T11:45:49Z","closed_at":null,"merged_at":null,"merge_commit_sha":"07c373ba16f10abf300fefac4ed959f095b97687","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":1099241641,"node_id":"MDU6TGFiZWwxMDk5MjQxNjQx","url":"https://api.github.com/repos/ovh/cds/labels/api","name":"api","color":"000080","default":false},{"id":851152631,"node_id":"MDU6TGFiZWw4NTExNTI2MzE=","url":"https://api.github.com/repos/ovh/cds/labels/refactor","name":"refactor","color":"CD5C5C","default":false}],"milestone":null,"commits_url":"https://api.github.com/repos/ovh/cds/pulls/4290/commits","review_comments_url":"https://api.github.com/repos/ovh/cds/pulls/4290/comments","review_comment_url":"https://api.github.com/repos/ovh/cds/pulls/comments{/number}","comments_url":"https://api.github.com/repos/ovh/cds/issues/4290/comments","statuses_url":"https://api.github.com/repos/ovh/cds/statuses/f6787fc57fc2b7dde149cb430ed163a0d2e1570d","head":{"label":"ovh:refactor/users","ref":"refactor/users","sha":"f6787fc57fc2b7dde149cb430ed163a0d2e1570d","user":{"login":"ovh","id":1698434,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE2OTg0MzQ=","avatar_url":"https://avatars3.githubusercontent.com/u/1698434?v=4","gravatar_id":"","url":"https://api.github.com/users/ovh","html_url":"https://github.com/ovh","followers_url":"https://api.github.com/users/ovh/followers","following_url":"https://api.github.com/users/ovh/following{/other_user}","gists_url":"https://api.github.com/users/ovh/gists{/gist_id}","starred_url":"https://api.github.com/users/ovh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ovh/subscriptions","organizations_url":"https://api.github.com/users/ovh/orgs","repos_url":"https://api.github.com/users/ovh/repos","events_url":"https://api.github.com/users/ovh/events{/privacy}","received_events_url":"https://api.github.com/users/ovh/received_events","type":"Organization","site_admin":false},"repo":{"id":70572539,"node_id":"MDEwOlJlcG9zaXRvcnk3MDU3MjUzOQ==","name":"cds","full_name":"ovh/cds","private":false,"owner":{"login":"ovh","id":1698434,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE2OTg0MzQ=","avatar_url":"https://avatars3.githubusercontent.com/u/1698434?v=4","gravatar_id":"","url":"https://api.github.com/users/ovh","html_url":"https://github.com/ovh","followers_url":"https://api.github.com/users/ovh/followers","following_url":"https://api.github.com/users/ovh/following{/other_user}","gists_url":"https://api.github.com/users/ovh/gists{/gist_id}","starred_url":"https://api.github.com/users/ovh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ovh/subscriptions","organizations_url":"https://api.github.com/users/ovh/orgs","repos_url":"https://api.github.com/users/ovh/repos","events_url":"https://api.github.com/users/ovh/events{/privacy}","received_events_url":"https://api.github.com/users/ovh/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/ovh/cds","description":"Enterprise-Grade Continuous Delivery & DevOps Automation Open Source Platform","fork":false,"url":"https://api.github.com/repos/ovh/cds","forks_url":"https://api.github.com/repos/ovh/cds/forks","keys_url":"https://api.github.com/repos/ovh/cds/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ovh/cds/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ovh/cds/teams","hooks_url":"https://api.github.com/repos/ovh/cds/hooks","issue_events_url":"https://api.github.com/repos/ovh/cds/issues/events{/number}","events_url":"https://api.github.com/repos/ovh/cds/events","assignees_url":"https://api.github.com/repos/ovh/cds/assignees{/user}","branches_url":"https://api.github.com/repos/ovh/cds/branches{/branch}","tags_url":"https://api.github.com/repos/ovh/cds/tags","blobs_url":"https://api.github.com/repos/ovh/cds/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ovh/cds/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ovh/cds/git/refs{/sha}","trees_url":"https://api.github.com/repos/ovh/cds/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ovh/cds/statuses/{sha}","languages_url":"https://api.github.com/repos/ovh/cds/languages","stargazers_url":"https://api.github.com/repos/ovh/cds/stargazers","contributors_url":"https://api.github.com/repos/ovh/cds/contributors","subscribers_url":"https://api.github.com/repos/ovh/cds/subscribers","subscription_url":"https://api.github.com/repos/ovh/cds/subscription","commits_url":"https://api.github.com/repos/ovh/cds/commits{/sha}","git_commits_url":"https://api.github.com/repos/ovh/cds/git/commits{/sha}","comments_url":"https://api.github.com/repos/ovh/cds/comments{/number}","issue_comment_url":"https://api.github.com/repos/ovh/cds/issues/comments{/number}","contents_url":"https://api.github.com/repos/ovh/cds/contents/{+path}","compare_url":"https://api.github.com/repos/ovh/cds/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ovh/cds/merges","archive_url":"https://api.github.com/repos/ovh/cds/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ovh/cds/downloads","issues_url":"https://api.github.com/repos/ovh/cds/issues{/number}","pulls_url":"https://api.github.com/repos/ovh/cds/pulls{/number}","milestones_url":"https://api.github.com/repos/ovh/cds/milestones{/number}","notifications_url":"https://api.github.com/repos/ovh/cds/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ovh/cds/labels{/name}","releases_url":"https://api.github.com/repos/ovh/cds/releases{/id}","deployments_url":"https://api.github.com/repos/ovh/cds/deployments","created_at":"2016-10-11T08:28:23Z","updated_at":"2019-08-21T11:17:27Z","pushed_at":"2019-08-21T10:00:24Z","git_url":"git://github.com/ovh/cds.git","ssh_url":"git@github.com:ovh/cds.git","clone_url":"https://github.com/ovh/cds.git","svn_url":"https://github.com/ovh/cds","homepage":"https://ovh.github.io/cds/","size":198844,"stargazers_count":2326,"watchers_count":2326,"language":"Go","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":178,"mirror_url":null,"archived":false,"disabled":false,"open_issues_count":168,"license":{"key":"other","name":"Other","spdx_id":"NOASSERTION","url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":178,"open_issues":168,"watchers":2326,"default_branch":"master"}},"base":{"label":"ovh:master","ref":"master","sha":"3b329d500313a2b535114b77982087d70ffc6162","user":{"login":"ovh","id":1698434,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE2OTg0MzQ=","avatar_url":"https://avatars3.githubusercontent.com/u/1698434?v=4","gravatar_id":"","url":"https://api.github.com/users/ovh","html_url":"https://github.com/ovh","followers_url":"https://api.github.com/users/ovh/followers","following_url":"https://api.github.com/users/ovh/following{/other_user}","gists_url":"https://api.github.com/users/ovh/gists{/gist_id}","starred_url":"https://api.github.com/users/ovh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ovh/subscriptions","organizations_url":"https://api.github.com/users/ovh/orgs","repos_url":"https://api.github.com/users/ovh/repos","events_url":"https://api.github.com/users/ovh/events{/privacy}","received_events_url":"https://api.github.com/users/ovh/received_events","type":"Organization","site_admin":false},"repo":{"id":70572539,"node_id":"MDEwOlJlcG9zaXRvcnk3MDU3MjUzOQ==","name":"cds","full_name":"ovh/cds","private":false,"owner":{"login":"ovh","id":1698434,"node_id":"MDEyOk9yZ2FuaXphdGlvbjE2OTg0MzQ=","avatar_url":"https://avatars3.githubusercontent.com/u/1698434?v=4","gravatar_id":"","url":"https://api.github.com/users/ovh","html_url":"https://github.com/ovh","followers_url":"https://api.github.com/users/ovh/followers","following_url":"https://api.github.com/users/ovh/following{/other_user}","gists_url":"https://api.github.com/users/ovh/gists{/gist_id}","starred_url":"https://api.github.com/users/ovh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ovh/subscriptions","organizations_url":"https://api.github.com/users/ovh/orgs","repos_url":"https://api.github.com/users/ovh/repos","events_url":"https://api.github.com/users/ovh/events{/privacy}","received_events_url":"https://api.github.com/users/ovh/received_events","type":"Organization","site_admin":false},"html_url":"https://github.com/ovh/cds","description":"Enterprise-Grade Continuous Delivery & DevOps Automation Open Source Platform","fork":false,"url":"https://api.github.com/repos/ovh/cds","forks_url":"https://api.github.com/repos/ovh/cds/forks","keys_url":"https://api.github.com/repos/ovh/cds/keys{/key_id}","collaborators_url":"https://api.github.com/repos/ovh/cds/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/ovh/cds/teams","hooks_url":"https://api.github.com/repos/ovh/cds/hooks","issue_events_url":"https://api.github.com/repos/ovh/cds/issues/events{/number}","events_url":"https://api.github.com/repos/ovh/cds/events","assignees_url":"https://api.github.com/repos/ovh/cds/assignees{/user}","branches_url":"https://api.github.com/repos/ovh/cds/branches{/branch}","tags_url":"https://api.github.com/repos/ovh/cds/tags","blobs_url":"https://api.github.com/repos/ovh/cds/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/ovh/cds/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/ovh/cds/git/refs{/sha}","trees_url":"https://api.github.com/repos/ovh/cds/git/trees{/sha}","statuses_url":"https://api.github.com/repos/ovh/cds/statuses/{sha}","languages_url":"https://api.github.com/repos/ovh/cds/languages","stargazers_url":"https://api.github.com/repos/ovh/cds/stargazers","contributors_url":"https://api.github.com/repos/ovh/cds/contributors","subscribers_url":"https://api.github.com/repos/ovh/cds/subscribers","subscription_url":"https://api.github.com/repos/ovh/cds/subscription","commits_url":"https://api.github.com/repos/ovh/cds/commits{/sha}","git_commits_url":"https://api.github.com/repos/ovh/cds/git/commits{/sha}","comments_url":"https://api.github.com/repos/ovh/cds/comments{/number}","issue_comment_url":"https://api.github.com/repos/ovh/cds/issues/comments{/number}","contents_url":"https://api.github.com/repos/ovh/cds/contents/{+path}","compare_url":"https://api.github.com/repos/ovh/cds/compare/{base}...{head}","merges_url":"https://api.github.com/repos/ovh/cds/merges","archive_url":"https://api.github.com/repos/ovh/cds/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/ovh/cds/downloads","issues_url":"https://api.github.com/repos/ovh/cds/issues{/number}","pulls_url":"https://api.github.com/repos/ovh/cds/pulls{/number}","milestones_url":"https://api.github.com/repos/ovh/cds/milestones{/number}","notifications_url":"https://api.github.com/repos/ovh/cds/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/ovh/cds/labels{/name}","releases_url":"https://api.github.com/repos/ovh/cds/releases{/id}","deployments_url":"https://api.github.com/repos/ovh/cds/deployments","created_at":"2016-10-11T08:28:23Z","updated_at":"2019-08-21T11:17:27Z","pushed_at":"2019-08-21T10:00:24Z","git_url":"git://github.com/ovh/cds.git","ssh_url":"git@github.com:ovh/cds.git","clone_url":"https://github.com/ovh/cds.git","svn_url":"https://github.com/ovh/cds","homepage":"https://ovh.github.io/cds/","size":198844,"stargazers_count":2326,"watchers_count":2326,"language":"Go","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":true,"forks_count":178,"mirror_url":null,"archived":false,"disabled":false,"open_issues_count":168,"license":{"key":"other","name":"Other","spdx_id":"NOASSERTION","url":null,"node_id":"MDc6TGljZW5zZTA="},"forks":178,"open_issues":168,"watchers":2326,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/ovh/cds/pulls/4290"},"html":{"href":"https://github.com/ovh/cds/pull/4290"},"issue":{"href":"https://api.github.com/repos/ovh/cds/issues/4290"},"comments":{"href":"https://api.github.com/repos/ovh/cds/issues/4290/comments"},"review_comments":{"href":"https://api.github.com/repos/ovh/cds/pulls/4290/comments"},"review_comment":{"href":"https://api.github.com/repos/ovh/cds/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/ovh/cds/pulls/4290/commits"},"statuses":{"href":"https://api.github.com/repos/ovh/cds/statuses/f6787fc57fc2b7dde149cb430ed163a0d2e1570d"}},"author_association":"MEMBER"}}
{"actor":{"display_login":"golangcibot"}}