From f14f299a6fee64919fcd5b32604861543b5f7c89 Mon Sep 17 00:00:00 2001 From: Wim Date: Sun, 19 Jun 2022 21:06:46 +0200 Subject: [PATCH 1/5] Do not allow organisation owners add themselves as collaborator We're already checking for repo owners, but we also need to check for organisation owners. Closes #17966 --- routers/web/repo/setting.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/routers/web/repo/setting.go b/routers/web/repo/setting.go index fae62c102077c..f47b5fc1f6d19 100644 --- a/routers/web/repo/setting.go +++ b/routers/web/repo/setting.go @@ -892,6 +892,16 @@ func CollaborationPost(ctx *context.Context) { return } + // find the owner team of the organization the repo belongs too and + // check if the user we're trying to add is an owner. + teams, err := organization.GetRepoTeams(ctx, ctx.Repo.Repository) + for _, team := range teams { + if team.IsOwnerTeam() && team.IsMember(u.ID) { + ctx.Redirect(setting.AppSubURL + ctx.Req.URL.EscapedPath()) + return + } + } + if !u.IsActive { ctx.Flash.Error(ctx.Tr("repo.settings.add_collaborator_inactive_user")) ctx.Redirect(setting.AppSubURL + ctx.Req.URL.EscapedPath()) From 532c36a6b7593439c2822eb356964e295c2f7736 Mon Sep 17 00:00:00 2001 From: Wim Date: Sun, 19 Jun 2022 21:29:13 +0200 Subject: [PATCH 2/5] Apply code review and check error --- routers/web/repo/setting.go | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/routers/web/repo/setting.go b/routers/web/repo/setting.go index f47b5fc1f6d19..c07afddbc3ba8 100644 --- a/routers/web/repo/setting.go +++ b/routers/web/repo/setting.go @@ -892,16 +892,6 @@ func CollaborationPost(ctx *context.Context) { return } - // find the owner team of the organization the repo belongs too and - // check if the user we're trying to add is an owner. - teams, err := organization.GetRepoTeams(ctx, ctx.Repo.Repository) - for _, team := range teams { - if team.IsOwnerTeam() && team.IsMember(u.ID) { - ctx.Redirect(setting.AppSubURL + ctx.Req.URL.EscapedPath()) - return - } - } - if !u.IsActive { ctx.Flash.Error(ctx.Tr("repo.settings.add_collaborator_inactive_user")) ctx.Redirect(setting.AppSubURL + ctx.Req.URL.EscapedPath()) @@ -921,6 +911,23 @@ func CollaborationPost(ctx *context.Context) { return } + // find the owner team of the organization the repo belongs too and + // check if the user we're trying to add is an owner. + if ctx.Repo.Repository.Owner.IsOrganization() { + teams, err := organization.GetRepoTeams(ctx, ctx.Repo.Repository) + if err != nil { + ctx.ServerError("GetRepoTeams", err) + return + } + + for _, team := range teams { + if team.IsOwnerTeam() && team.IsMember(u.ID) { + ctx.Redirect(setting.AppSubURL + ctx.Req.URL.EscapedPath()) + return + } + } + } + if err = models.AddCollaborator(ctx.Repo.Repository, u); err != nil { ctx.ServerError("AddCollaborator", err) return From 149db5034bafb36eff959289200fa20513776867 Mon Sep 17 00:00:00 2001 From: Wim Date: Mon, 20 Jun 2022 00:52:01 +0200 Subject: [PATCH 3/5] Use IsOrganizationOwner --- routers/web/repo/setting.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/routers/web/repo/setting.go b/routers/web/repo/setting.go index c07afddbc3ba8..66e39e429a76d 100644 --- a/routers/web/repo/setting.go +++ b/routers/web/repo/setting.go @@ -914,17 +914,12 @@ func CollaborationPost(ctx *context.Context) { // find the owner team of the organization the repo belongs too and // check if the user we're trying to add is an owner. if ctx.Repo.Repository.Owner.IsOrganization() { - teams, err := organization.GetRepoTeams(ctx, ctx.Repo.Repository) - if err != nil { - ctx.ServerError("GetRepoTeams", err) + if isOwner, err := organization.IsOrganizationOwner(ctx, ctx.Repo.Repository.Owner.ID, u.ID); err != nil { + ctx.ServerError("IsOrganizationOwner", err) + return + } else if isOwner { + ctx.Redirect(setting.AppSubURL + ctx.Req.URL.EscapedPath()) return - } - - for _, team := range teams { - if team.IsOwnerTeam() && team.IsMember(u.ID) { - ctx.Redirect(setting.AppSubURL + ctx.Req.URL.EscapedPath()) - return - } } } From 318ef820af270bf488e9d4c6a5f0c457f362d9d9 Mon Sep 17 00:00:00 2001 From: Wim Date: Tue, 21 Jun 2022 21:00:57 +0200 Subject: [PATCH 4/5] Add flash --- options/locale/locale_en-US.ini | 1 + routers/web/repo/setting.go | 1 + 2 files changed, 2 insertions(+) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 72253b3640866..763e482627d3a 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -1891,6 +1891,7 @@ settings.confirm_delete = Delete Repository settings.add_collaborator = Add Collaborator settings.add_collaborator_success = The collaborator has been added. settings.add_collaborator_inactive_user = Can not add an inactive user as a collaborator. +settings.add_collaborator_owner = Can not add an owner as a collaborator. settings.add_collaborator_duplicate = The collaborator is already added to this repository. settings.delete_collaborator = Remove settings.collaborator_deletion = Remove Collaborator diff --git a/routers/web/repo/setting.go b/routers/web/repo/setting.go index 66e39e429a76d..0b501d146ef6d 100644 --- a/routers/web/repo/setting.go +++ b/routers/web/repo/setting.go @@ -918,6 +918,7 @@ func CollaborationPost(ctx *context.Context) { ctx.ServerError("IsOrganizationOwner", err) return } else if isOwner { + ctx.Flash.Error(ctx.Tr("settings.add_collaborator_owner")) ctx.Redirect(setting.AppSubURL + ctx.Req.URL.EscapedPath()) return } From f94505c8db2a3515f249f759b6aa7426c31c980a Mon Sep 17 00:00:00 2001 From: Wim Date: Fri, 15 Jul 2022 00:16:30 +0200 Subject: [PATCH 5/5] Use repo section --- routers/web/repo/setting.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/repo/setting.go b/routers/web/repo/setting.go index 0b501d146ef6d..15abeb8fcc8c2 100644 --- a/routers/web/repo/setting.go +++ b/routers/web/repo/setting.go @@ -918,7 +918,7 @@ func CollaborationPost(ctx *context.Context) { ctx.ServerError("IsOrganizationOwner", err) return } else if isOwner { - ctx.Flash.Error(ctx.Tr("settings.add_collaborator_owner")) + ctx.Flash.Error(ctx.Tr("repo.settings.add_collaborator_owner")) ctx.Redirect(setting.AppSubURL + ctx.Req.URL.EscapedPath()) return }