diff --git a/models/organization/org.go b/models/organization/org.go index 7a4b327fde6..42628132c80 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -91,19 +91,6 @@ func (org *Organization) IsOwnedBy(ctx context.Context, uid int64) (bool, error) return IsOrganizationOwner(ctx, org.ID, uid) } -// CanChangeRepoTeamAccess reports whether a repository administrator can change team access. -func (org *Organization) CanChangeRepoTeamAccess(ctx context.Context, doer *user_model.User) (bool, error) { - if org.RepoAdminChangeTeamAccess || doer.IsAdmin { - return true, nil - } - return org.IsOwnedBy(ctx, doer.ID) -} - -// IsOrgAdmin returns true if given user is in the owner team or an admin team. -func (org *Organization) IsOrgAdmin(ctx context.Context, uid int64) (bool, error) { - return IsOrganizationAdmin(ctx, org.ID, uid) -} - // IsOrgMember returns true if given user is member of organization. func (org *Organization) IsOrgMember(ctx context.Context, uid int64) (bool, error) { return IsOrganizationMember(ctx, org.ID, uid) @@ -598,8 +585,8 @@ func RemoveOrgRepo(ctx context.Context, orgID, repoID int64) error { // GetUserTeams returns all teams that belong to user, // and that the user has joined. -func (org *Organization) GetUserTeams(ctx context.Context, userID int64, cols ...string) ([]*Team, error) { - teams := make([]*Team, 0, org.NumTeams) +func (org *Organization) GetUserTeams(ctx context.Context, userID int64, cols ...string) (TeamList, error) { + teams := make(TeamList, 0, org.NumTeams) return teams, db.GetEngine(ctx). Where("`team_user`.org_id = ?", org.ID). Join("INNER", "team_user", "`team_user`.team_id = team.id"). diff --git a/models/organization/org_user.go b/models/organization/org_user.go index 61102501af2..1e623b03e16 100644 --- a/models/organization/org_user.go +++ b/models/organization/org_user.go @@ -69,20 +69,6 @@ func IsOrganizationOwner(ctx context.Context, orgID, uid int64) (bool, error) { return IsTeamMember(ctx, orgID, ownerTeam.ID, uid) } -// IsOrganizationAdmin returns true if given user is in the owner team or an admin team. -func IsOrganizationAdmin(ctx context.Context, orgID, uid int64) (bool, error) { - teams, err := GetUserOrgTeams(ctx, orgID, uid) - if err != nil { - return false, err - } - for _, t := range teams { - if t.HasAdminAccess() { - return true, nil - } - } - return false, nil -} - // IsOrganizationMember returns true if given user is member of organization. func IsOrganizationMember(ctx context.Context, orgID, uid int64) (bool, error) { return db.GetEngine(ctx). diff --git a/models/organization/team.go b/models/organization/team.go index 06cc5c7e69a..d07a28c2452 100644 --- a/models/organization/team.go +++ b/models/organization/team.go @@ -74,19 +74,25 @@ const OwnerTeamName = "Owners" // Team represents a organization team. type Team struct { - ID int64 `xorm:"pk autoincr"` - OrgID int64 `xorm:"INDEX"` - LowerName string - Name string - Description string - AccessMode perm.AccessMode `xorm:"'authorize'"` - Members []*user_model.User `xorm:"-"` - NumRepos int - NumMembers int - Units []*TeamUnit `xorm:"-"` - IncludesAllRepositories bool `xorm:"NOT NULL DEFAULT false"` - CanCreateOrgRepo bool `xorm:"NOT NULL DEFAULT false"` - Visibility structs.VisibleType `xorm:"NOT NULL DEFAULT 2"` + ID int64 `xorm:"pk autoincr"` + OrgID int64 `xorm:"INDEX"` + LowerName string + Name string + Description string + AccessMode perm.AccessMode `xorm:"'authorize'"` + Members []*user_model.User `xorm:"-"` + NumRepos int + NumMembers int + Units []*TeamUnit `xorm:"-"` + + // All repos in the org are included in this team automatically + IncludesAllRepositories bool `xorm:"NOT NULL DEFAULT false"` + + // Any user with CanCreateOrgRepo permission can create a repository in the organization, regardless of team membership. + // And the user will become the repo's admin (via collaborator) after the creation. + CanCreateOrgRepo bool `xorm:"NOT NULL DEFAULT false"` + + Visibility structs.VisibleType `xorm:"NOT NULL DEFAULT 2"` } func (t *Team) IsPublic() bool { return t.Visibility.IsPublic() } @@ -176,10 +182,6 @@ func (t *Team) IsMember(ctx context.Context, userID int64) bool { return isMember } -func (t *Team) HasAdminAccess() bool { - return t.AccessMode >= perm.AccessModeAdmin -} - // LoadMembers returns paginated members in team of organization. func (t *Team) LoadMembers(ctx context.Context) (err error) { t.Members, err = GetTeamMembers(ctx, &SearchMembersOptions{ diff --git a/models/organization/team_list.go b/models/organization/team_list.go index 4e589c4544f..eab4a615e41 100644 --- a/models/organization/team_list.go +++ b/models/organization/team_list.go @@ -39,6 +39,18 @@ func (t TeamList) AnyRepoUnitMaxAccess(ctx context.Context, tp unit.Type) perm.A return maxAccess } +func (t TeamList) HasAllRepoAdminAccess() bool { + for _, team := range t { + if team.IsOwnerTeam() || team.AccessMode == perm.AccessModeOwner { + return true + } + if team.IncludesAllRepositories && team.AccessMode >= perm.AccessModeAdmin { + return true + } + } + return false +} + // SearchTeamOptions holds the search options type SearchTeamOptions struct { db.ListOptions diff --git a/models/perm/access/access.go b/models/perm/access/access.go index 53d14fca1b5..6b6698fe28e 100644 --- a/models/perm/access/access.go +++ b/models/perm/access/access.go @@ -33,7 +33,9 @@ func init() { db.RegisterModel(new(Access)) } -func accessLevel(ctx context.Context, user *user_model.User, repo *repo_model.Repository) (perm.AccessMode, error) { +// modeByOwnerAndAccess returns the access mode of a user to a repository, +// considering: the repository's owner (visibility and doer restriction), any explicit "access" records. +func modeByOwnerAndAccess(ctx context.Context, user *user_model.User, repo *repo_model.Repository) (perm.AccessMode, error) { mode := perm.AccessModeNone var userID int64 restricted := false @@ -69,14 +71,6 @@ func accessLevel(ctx context.Context, user *user_model.User, repo *repo_model.Re return a.Mode, nil } -func maxAccessMode(modes ...perm.AccessMode) perm.AccessMode { - maxMode := perm.AccessModeNone - for _, mode := range modes { - maxMode = max(maxMode, mode) - } - return maxMode -} - type userAccess struct { User *user_model.User Mode perm.AccessMode @@ -85,7 +79,7 @@ type userAccess struct { // updateUserAccess updates an access map so that user has at least mode func updateUserAccess(accessMap map[int64]*userAccess, user *user_model.User, mode perm.AccessMode) { if ua, ok := accessMap[user.ID]; ok { - ua.Mode = maxAccessMode(ua.Mode, mode) + ua.Mode = max(ua.Mode, mode) } else { accessMap[user.ID] = &userAccess{User: user, Mode: mode} } @@ -263,7 +257,7 @@ func RecalculateUserAccess(ctx context.Context, repo *repo_model.Repository, uid t.AccessMode = perm.AccessModeOwner } - accessMode = maxAccessMode(accessMode, t.AccessMode) + accessMode = max(accessMode, t.AccessMode) } } diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go index 7bf0fa4a6f9..ff677906afe 100644 --- a/models/perm/access/repo_permission.go +++ b/models/perm/access/repo_permission.go @@ -33,6 +33,8 @@ type Permission struct { everyoneAccessMode map[unit.Type]perm_model.AccessMode // the unit's minimal access mode for every signed-in user anonymousAccessMode map[unit.Type]perm_model.AccessMode // the unit's minimal access mode for anonymous (non-signed-in) user + + orgRepoTeams []*organization.Team } // IsOwner returns true if current user is the owner of repository. @@ -445,7 +447,7 @@ func GetIndividualUserRepoPermission(ctx context.Context, repo *repo_model.Repos } // plain user TODO: this check should be replaced, only need to check collaborator access mode - perm.AccessMode, err = accessLevel(ctx, user, repo) + perm.AccessMode, err = modeByOwnerAndAccess(ctx, user, repo) if err != nil { return perm, err } @@ -459,11 +461,11 @@ func GetIndividualUserRepoPermission(ctx context.Context, repo *repo_model.Repos perm.AccessMode = max(perm.AccessMode, minAccessMode) // get units mode from teams - teams, err := organization.GetUserRepoTeams(ctx, repo.OwnerID, user.ID, repo.ID) + perm.orgRepoTeams, err = organization.GetUserRepoTeams(ctx, repo.OwnerID, user.ID, repo.ID) if err != nil { return perm, err } - if len(teams) == 0 { + if len(perm.orgRepoTeams) == 0 { return perm, nil } @@ -477,8 +479,8 @@ func GetIndividualUserRepoPermission(ctx context.Context, repo *repo_model.Repos } // if user in an owner team - for _, team := range teams { - if team.HasAdminAccess() { + for _, team := range perm.orgRepoTeams { + if team.IsOwnerTeam() || team.AccessMode == perm_model.AccessModeOwner { perm.AccessMode = perm_model.AccessModeOwner perm.unitsMode = nil return perm, nil @@ -486,7 +488,7 @@ func GetIndividualUserRepoPermission(ctx context.Context, repo *repo_model.Repos } for _, u := range repo.Units { - for _, team := range teams { + for _, team := range perm.orgRepoTeams { teamMode, _ := team.UnitAccessModeEx(ctx, u.Type) unitAccessMode := max(perm.unitsMode[u.Type], minAccessMode, teamMode) perm.unitsMode[u.Type] = unitAccessMode @@ -496,52 +498,35 @@ func GetIndividualUserRepoPermission(ctx context.Context, repo *repo_model.Repos return perm, err } -// IsUserRealRepoAdmin check if this user is real repo admin -func IsUserRealRepoAdmin(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (bool, error) { - if repo.OwnerID == user.ID { - return true, nil - } - - if err := repo.LoadOwner(ctx); err != nil { - return false, err - } - - accessMode, err := accessLevel(ctx, user, repo) - if err != nil { - return false, err - } - - return accessMode >= perm_model.AccessModeAdmin, nil +func IsUserRepoAdmin(ctx context.Context, repo *repo_model.Repository, user *user_model.User) bool { + return (user != nil && user.IsAdmin) || IsUserRealRepoAdmin(ctx, repo, user) } -// IsUserRepoAdmin return true if user has admin right of a repo -func IsUserRepoAdmin(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (bool, error) { +// IsUserRealRepoAdmin check if this user is real repo admin (but not a site admin who also has repo admin access) +func IsUserRealRepoAdmin(ctx context.Context, repo *repo_model.Repository, user *user_model.User) bool { if user == nil || repo == nil { - return false, nil - } - if user.IsAdmin { - return true, nil + return false } - mode, err := accessLevel(ctx, user, repo) + mode, err := modeByOwnerAndAccess(ctx, user, repo) if err != nil { - return false, err + return false } if mode >= perm_model.AccessModeAdmin { - return true, nil + return true } teams, err := organization.GetUserRepoTeams(ctx, repo.OwnerID, user.ID, repo.ID) if err != nil { - return false, err + return false } for _, team := range teams { - if team.HasAdminAccess() { - return true, nil + if team.AccessMode >= perm_model.AccessModeAdmin { + return true } } - return false, nil + return false } // AccessLevel returns the Access a user has to a repository. Will return NoneAccess if the @@ -701,3 +686,39 @@ func CanReadWorkflowCrossRepo(ctx context.Context, targetRepo *repo_model.Reposi } return botPerm.AccessMode >= perm_model.AccessModeRead, nil } + +func CanDoerManageRepoDangerZone(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, perm *Permission) bool { + if perm.IsOwner() { + return true + } + + // FIXME: ORG-REPO-ADMIN-DANGER-ZONE: this is the legacy logic, "org repo admin" can delete a repo + // Ideally we need a new field in like "AdminManageDangerZone" to control this permission, but for now we keep the legacy logic + for _, team := range perm.orgRepoTeams { + if team.AccessMode >= perm_model.AccessModeAdmin { + return true + } + } + + // A special case: if the team allows to create repo, then the doer will be added as a collaborator with admin access. + // For this case, we also allow the doer to manage the danger zone as well, because the doer is effectively a repo admin. + // Since the admin permission from team is already allowed above (legacy logic), here nothing worse. + // Keep in mind: the newly created repo isn't in any org team, it only has the doer as a collaborator with admin access. + // So we need to get all the teams of the doer to check. + allowCreateRepo := false + doerOrgTeams, _ := organization.GetUserOrgTeams(ctx, repo.OwnerID, doer.ID) + for _, team := range doerOrgTeams { + if allowCreateRepo = team.CanCreateOrgRepo; allowCreateRepo { + break + } + } + return allowCreateRepo && perm.IsAdmin() +} + +func CanDoerManageOrgRepoCollaboratorTeam(ctx context.Context, repo *repo_model.Repository, perm *Permission) bool { + _ = repo.LoadOwner(ctx) + if repo.Owner == nil || !repo.Owner.IsOrganization() { + return false + } + return perm.IsOwner() || perm.IsAdmin() && repo.Owner.RepoAdminChangeTeamAccess +} diff --git a/models/user/user.go b/models/user/user.go index 804d2b7a4e0..fff576aa860 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -144,10 +144,16 @@ type User struct { NumRepos int // For organization - NumTeams int - NumMembers int - Visibility structs.VisibleType `xorm:"NOT NULL DEFAULT 0"` - RepoAdminChangeTeamAccess bool `xorm:"NOT NULL DEFAULT false"` + NumTeams int + NumMembers int + Visibility structs.VisibleType `xorm:"NOT NULL DEFAULT 0"` + + // Introduced by "Add teams to repo on collaboration page. (#8045)" + // Whether a repo admin can add/remove a team to/from the repo on the collaboration page + RepoAdminChangeTeamAccess bool `xorm:"NOT NULL DEFAULT false"` + + // FIXME: ORG-REPO-ADMIN-DANGER-ZONE: it needs a new field to decide whether a repo admin can manage the repo's danger zone + // Team won't work for this case, because a newly create org repo isn't in any team (same as above) // Preferences DiffViewStyle string `xorm:"NOT NULL DEFAULT ''"` diff --git a/modules/repository/delete.go b/modules/repository/delete.go deleted file mode 100644 index 68ec4f0d62b..00000000000 --- a/modules/repository/delete.go +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package repository - -import ( - "context" - - "gitea.dev/models/organization" - repo_model "gitea.dev/models/repo" - user_model "gitea.dev/models/user" -) - -// CanUserDelete returns true if user could delete the repository -func CanUserDelete(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (bool, error) { - if user.IsAdmin || user.ID == repo.OwnerID { - return true, nil - } - - if err := repo.LoadOwner(ctx); err != nil { - return false, err - } - - if repo.Owner.IsOrganization() { - isAdmin, err := organization.OrgFromUser(repo.Owner).IsOrgAdmin(ctx, user.ID) - if err != nil { - return false, err - } - return isAdmin, nil - } - - return false, nil -} diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 21b0790438b..1860f8b820f 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -427,6 +427,15 @@ func reqOwner() func(ctx *context.APIContext) { } } +func reqRepoDangerZone() func(ctx *context.APIContext) { + return func(ctx *context.APIContext) { + if !access_model.CanDoerManageRepoDangerZone(ctx, ctx.Doer, ctx.Repo.Repository, &ctx.Repo.Permission) { + ctx.APIError(http.StatusForbidden, "user has no permission to manage the danger zone") + return + } + } +} + // reqSelfOrAdmin doer should be the same as the contextUser or site admin func reqSelfOrAdmin() func(ctx *context.APIContext) { return func(ctx *context.APIContext) { @@ -1305,11 +1314,11 @@ func Routes() *web.Router { m.Get("/compare/*", reqRepoReader(unit.TypeCode), repo.CompareDiff) m.Combo("").Get(reqAnyRepoReader(), repo.Get). - Delete(reqToken(), reqOwner(), repo.Delete). + Delete(reqToken(), reqRepoDangerZone(), repo.Delete). Patch(reqToken(), reqAdmin(), bind(api.EditRepoOption{}), repo.Edit) m.Post("/generate", reqToken(), reqRepoReader(unit.TypeCode), bind(api.GenerateRepoOption{}), repo.Generate) m.Group("/transfer", func() { - m.Post("", reqOwner(), bind(api.TransferRepoOption{}), repo.Transfer) + m.Post("", reqRepoDangerZone(), bind(api.TransferRepoOption{}), repo.Transfer) m.Post("/accept", repo.AcceptTransfer) m.Post("/reject", repo.RejectTransfer) }, reqToken()) diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index 4782dfdb2a4..a256520e0da 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -613,7 +613,7 @@ func GetTeamRepo(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - repo := getRepositoryByParams(ctx) + repo, permission := getRepositoryByParams(ctx) if ctx.Written() { return } @@ -629,11 +629,6 @@ func GetTeamRepo(ctx *context.APIContext) { return } - permission, err := access_model.GetDoerRepoPermission(ctx, repo, ctx.Doer) - if err != nil { - ctx.APIErrorInternal(err) - return - } // The team may be reachable by a non-team-member via its visibility tier; // don't confirm the existence of a repo the doer cannot access. if !permission.HasAnyUnitAccessOrPublicAccess() { @@ -641,31 +636,28 @@ func GetTeamRepo(ctx *context.APIContext) { return } - ctx.JSON(http.StatusOK, convert.ToRepo(ctx, repo, permission)) + ctx.JSON(http.StatusOK, convert.ToRepo(ctx, repo, *permission)) } // getRepositoryByParams get repository by a team's organization ID and repo name -func getRepositoryByParams(ctx *context.APIContext) *repo_model.Repository { +func getRepositoryByParams(ctx *context.APIContext) (*repo_model.Repository, *access_model.Permission) { repo, err := repo_model.GetRepositoryByName(ctx, ctx.Org.Team.OrgID, ctx.PathParam("reponame")) if err != nil { - if repo_model.IsErrRepoNotExist(err) { - ctx.APIErrorNotFound() - } else { - ctx.APIErrorInternal(err) - } - return nil + ctx.APIErrorAuto(err) + return nil, nil } - return repo + perm, err := access_model.GetDoerRepoPermission(ctx, repo, ctx.Doer) + if err != nil { + ctx.APIErrorAuto(err) + return nil, nil + } + return repo, &perm } -func canChangeTeamRepository(ctx *context.APIContext) bool { - canChange, err := ctx.Org.Organization.CanChangeRepoTeamAccess(ctx, ctx.Doer) - if err != nil { - ctx.APIErrorInternal(err) - return false - } +func canManageRepoCollaboratorTeam(ctx *context.APIContext, repo *repo_model.Repository, perm *access_model.Permission) bool { + canChange := access_model.CanDoerManageOrgRepoCollaboratorTeam(ctx, repo, perm) if !canChange { - ctx.APIError(http.StatusForbidden, "Must be an organization owner") + ctx.APIError(http.StatusForbidden, "Must have permission to manage team repository access") return false } return true @@ -703,18 +695,11 @@ func AddTeamRepository(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - repo := getRepositoryByParams(ctx) + repo, perm := getRepositoryByParams(ctx) if ctx.Written() { return } - if !canChangeTeamRepository(ctx) { - return - } - if access, err := access_model.AccessLevel(ctx, ctx.Doer, repo); err != nil { - ctx.APIErrorInternal(err) - return - } else if access < perm.AccessModeAdmin { - ctx.APIError(http.StatusForbidden, "Must have admin-level access to the repository") + if !canManageRepoCollaboratorTeam(ctx, repo, perm) { return } if err := repo_service.TeamAddRepository(ctx, ctx.Org.Team, repo); err != nil { @@ -758,18 +743,11 @@ func RemoveTeamRepository(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - repo := getRepositoryByParams(ctx) + repo, perm := getRepositoryByParams(ctx) if ctx.Written() { return } - if !canChangeTeamRepository(ctx) { - return - } - if access, err := access_model.AccessLevel(ctx, ctx.Doer, repo); err != nil { - ctx.APIErrorInternal(err) - return - } else if access < perm.AccessModeAdmin { - ctx.APIError(http.StatusForbidden, "Must have admin-level access to the repository") + if !canManageRepoCollaboratorTeam(ctx, repo, perm) { return } if err := repo_service.RemoveRepositoryFromTeam(ctx, ctx.Org.Team, repo.ID); err != nil { diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 0fb3d304dc9..e264a9bf52a 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -1154,15 +1154,6 @@ func Delete(ctx *context.APIContext) { owner := ctx.Repo.Owner repo := ctx.Repo.Repository - canDelete, err := repo_module.CanUserDelete(ctx, repo, ctx.Doer) - if err != nil { - ctx.APIErrorInternal(err) - return - } else if !canDelete { - ctx.APIError(http.StatusForbidden, "Given user is not owner of organization.") - return - } - if ctx.Repo.GitRepo != nil { ctx.Repo.GitRepo.Close() } diff --git a/routers/api/v1/repo/teams.go b/routers/api/v1/repo/teams.go index afa8a3caa46..4aabb06a4bc 100644 --- a/routers/api/v1/repo/teams.go +++ b/routers/api/v1/repo/teams.go @@ -8,6 +8,7 @@ import ( "net/http" "gitea.dev/models/organization" + access_model "gitea.dev/models/perm/access" "gitea.dev/services/context" "gitea.dev/services/convert" repo_service "gitea.dev/services/repository" @@ -188,11 +189,7 @@ func DeleteTeam(ctx *context.APIContext) { } func changeRepoTeam(ctx *context.APIContext, add bool) { - if !ctx.Repo.Owner.IsOrganization() { - ctx.APIError(http.StatusMethodNotAllowed, "repo is not owned by an organization") - return - } - if !canChangeRepoTeam(ctx) { + if !canChangeOrgRepoTeam(ctx) { return } @@ -224,14 +221,10 @@ func changeRepoTeam(ctx *context.APIContext, add bool) { ctx.Status(http.StatusNoContent) } -func canChangeRepoTeam(ctx *context.APIContext) bool { - canChange, err := organization.OrgFromUser(ctx.Repo.Owner).CanChangeRepoTeamAccess(ctx, ctx.Doer) - if err != nil { - ctx.APIErrorInternal(err) - return false - } +func canChangeOrgRepoTeam(ctx *context.APIContext) bool { + canChange := access_model.CanDoerManageOrgRepoCollaboratorTeam(ctx, ctx.Repo.Repository, &ctx.Repo.Permission) if !canChange { - ctx.APIError(http.StatusForbidden, "Must be an organization owner") + ctx.APIError(http.StatusForbidden, "No permission to change organization repository's team") return false } return true diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index 203b40c3303..f5b99003aaa 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -74,11 +74,7 @@ func roleDescriptor(ctx *context.Context, repo *repo_model.Repository, poster *u return roleDesc, nil } // Otherwise (poster is site admin), check if poster is the real repo admin. - isRealRepoAdmin, err := access_model.IsUserRealRepoAdmin(ctx, repo, poster) - if err != nil { - return roleDesc, err - } - if isRealRepoAdmin { + if access_model.IsUserRealRepoAdmin(ctx, repo, poster) { roleDesc.RoleInRepo = issues_model.RoleRepoOwner return roleDesc, nil } diff --git a/routers/web/repo/setting/collaboration.go b/routers/web/repo/setting/collaboration.go index a1037dbb5fd..1a7418ec80a 100644 --- a/routers/web/repo/setting/collaboration.go +++ b/routers/web/repo/setting/collaboration.go @@ -10,6 +10,7 @@ import ( "gitea.dev/models/organization" "gitea.dev/models/perm" + "gitea.dev/models/perm/access" repo_model "gitea.dev/models/repo" unit_model "gitea.dev/models/unit" user_model "gitea.dev/models/user" @@ -43,14 +44,7 @@ func Collaboration(ctx *context.Context) { ctx.Data["OrgName"] = ctx.Repo.Repository.OwnerName ctx.Data["Org"] = ctx.Repo.Repository.Owner ctx.Data["Units"] = unit_model.Units - if ctx.Repo.Owner.IsOrganization() { - ctx.Data["CanChangeRepoTeamAccess"], err = organization.OrgFromUser(ctx.Repo.Owner).CanChangeRepoTeamAccess(ctx, ctx.Doer) - if err != nil { - ctx.ServerError("CanChangeRepoTeamAccess", err) - return - } - } - + ctx.Data["CanChangeRepoTeamAccess"] = access.CanDoerManageOrgRepoCollaboratorTeam(ctx, ctx.Repo.Repository, &ctx.Repo.Permission) ctx.HTML(http.StatusOK, tplCollaboration) } @@ -162,7 +156,7 @@ func DeleteCollaboration(ctx *context.Context) { // AddTeamPost response for adding a team to a repository func AddTeamPost(ctx *context.Context) { - if !canChangeRepoTeamAccess(ctx) { + if !canManageRepoCollaboratorTeam(ctx) { return } @@ -206,7 +200,7 @@ func AddTeamPost(ctx *context.Context) { // DeleteTeam response for deleting a team from a repository func DeleteTeam(ctx *context.Context) { - if !canChangeRepoTeamAccess(ctx) { + if !canManageRepoCollaboratorTeam(ctx) { return } @@ -225,12 +219,8 @@ func DeleteTeam(ctx *context.Context) { ctx.JSONRedirect(ctx.Repo.RepoLink + "/settings/collaboration") } -func canChangeRepoTeamAccess(ctx *context.Context) bool { - canChange, err := organization.OrgFromUser(ctx.Repo.Owner).CanChangeRepoTeamAccess(ctx, ctx.Doer) - if err != nil { - ctx.ServerError("CanChangeRepoTeamAccess", err) - return false - } +func canManageRepoCollaboratorTeam(ctx *context.Context) bool { + canChange := access.CanDoerManageOrgRepoCollaboratorTeam(ctx, ctx.Repo.Repository, &ctx.Repo.Permission) if !canChange { ctx.Flash.Error(ctx.Tr("repo.settings.change_team_access_not_allowed")) ctx.Redirect(ctx.Repo.RepoLink + "/settings/collaboration") diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index b8968cda706..5e7e33693d1 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -13,6 +13,7 @@ import ( "gitea.dev/models/db" "gitea.dev/models/organization" + access_model "gitea.dev/models/perm/access" repo_model "gitea.dev/models/repo" unit_model "gitea.dev/models/unit" user_model "gitea.dev/models/user" @@ -55,6 +56,14 @@ type selectOption struct { Selected bool } +func canManageRepoDangerZone(ctx *context.Context) bool { + if !access_model.CanDoerManageRepoDangerZone(ctx, ctx.Doer, ctx.Repo.Repository, &ctx.Repo.Permission) { + ctx.JSONErrorNotFound() + return false + } + return true +} + // SettingsCtxData is a middleware that sets all the general context data for the // settings template. func SettingsCtxData(ctx *context.Context) { @@ -67,6 +76,7 @@ func SettingsCtxData(ctx *context.Context) { ctx.Data["DefaultMirrorInterval"] = setting.Mirror.DefaultInterval ctx.Data["MinimumMirrorInterval"] = setting.Mirror.MinInterval ctx.Data["CanConvertFork"] = ctx.Repo.Repository.IsFork && ctx.Doer.CanCreateRepoIn(ctx.Repo.Repository.Owner) + ctx.Data["CanManagerDangerZone"] = access_model.CanDoerManageRepoDangerZone(ctx, ctx.Doer, ctx.Repo.Repository, &ctx.Repo.Permission) signing, _ := git.GetSigningKey(ctx) ctx.Data["SigningKeyAvailable"] = signing != nil @@ -774,12 +784,12 @@ func handleSettingsPostAdminIndex(ctx *context.Context) { } func handleSettingsPostConvert(ctx *context.Context) { - form := web.GetForm[*forms.RepoSettingForm](ctx) - repo := ctx.Repo.Repository - if !ctx.Repo.Permission.IsOwner() { - ctx.JSONErrorNotFound() + if !canManageRepoDangerZone(ctx) { return } + + form := web.GetForm[*forms.RepoSettingForm](ctx) + repo := ctx.Repo.Repository if repo.Name != form.RepoName { ctx.JSONError(ctx.Tr("form.enterred_invalid_repo_name")) return @@ -804,12 +814,12 @@ func handleSettingsPostConvert(ctx *context.Context) { } func handleSettingsPostConvertFork(ctx *context.Context) { - form := web.GetForm[*forms.RepoSettingForm](ctx) - repo := ctx.Repo.Repository - if !ctx.Repo.Permission.IsOwner() { - ctx.JSONErrorNotFound() + if !canManageRepoDangerZone(ctx) { return } + + form := web.GetForm[*forms.RepoSettingForm](ctx) + repo := ctx.Repo.Repository if err := repo.LoadOwner(ctx); err != nil { ctx.ServerError("Convert Fork", err) return @@ -844,12 +854,12 @@ func handleSettingsPostConvertFork(ctx *context.Context) { } func handleSettingsPostTransfer(ctx *context.Context) { - form := web.GetForm[*forms.RepoSettingForm](ctx) - repo := ctx.Repo.Repository - if !ctx.Repo.Permission.IsOwner() { - ctx.JSONErrorNotFound() + if !canManageRepoDangerZone(ctx) { return } + + form := web.GetForm[*forms.RepoSettingForm](ctx) + repo := ctx.Repo.Repository if repo.Name != form.RepoName { ctx.JSONError(ctx.Tr("form.enterred_invalid_repo_name")) return @@ -908,12 +918,11 @@ func handleSettingsPostTransfer(ctx *context.Context) { } func handleSettingsPostCancelTransfer(ctx *context.Context) { - repo := ctx.Repo.Repository - if !ctx.Repo.Permission.IsOwner() { - ctx.HTTPError(http.StatusNotFound) + if !canManageRepoDangerZone(ctx) { return } + repo := ctx.Repo.Repository repoTransfer, err := repo_model.GetPendingRepositoryTransfer(ctx, ctx.Repo.Repository) if err != nil { if repo_model.IsErrNoPendingTransfer(err) { @@ -936,12 +945,12 @@ func handleSettingsPostCancelTransfer(ctx *context.Context) { } func handleSettingsPostDelete(ctx *context.Context) { - form := web.GetForm[*forms.RepoSettingForm](ctx) - repo := ctx.Repo.Repository - if !ctx.Repo.Permission.IsOwner() { - ctx.JSONErrorNotFound() + if !canManageRepoDangerZone(ctx) { return } + + form := web.GetForm[*forms.RepoSettingForm](ctx) + repo := ctx.Repo.Repository if repo.Name != form.RepoName { ctx.JSONError(ctx.Tr("form.enterred_invalid_repo_name")) return @@ -963,12 +972,11 @@ func handleSettingsPostDelete(ctx *context.Context) { } func handleSettingsPostDeleteWiki(ctx *context.Context) { - form := web.GetForm[*forms.RepoSettingForm](ctx) - repo := ctx.Repo.Repository - if !ctx.Repo.Permission.IsOwner() { - ctx.JSONErrorNotFound() + if !canManageRepoDangerZone(ctx) { return } + form := web.GetForm[*forms.RepoSettingForm](ctx) + repo := ctx.Repo.Repository if repo.Name != form.RepoName { ctx.JSONError(ctx.Tr("form.enterred_invalid_repo_name")) return @@ -985,12 +993,11 @@ func handleSettingsPostDeleteWiki(ctx *context.Context) { } func handleSettingsPostArchive(ctx *context.Context) { - repo := ctx.Repo.Repository - if !ctx.Repo.Permission.IsOwner() { - ctx.HTTPError(http.StatusForbidden) + if !canManageRepoDangerZone(ctx) { return } + repo := ctx.Repo.Repository if repo.IsMirror { ctx.Flash.Error(ctx.Tr("repo.settings.archive.error_ismirror")) ctx.Redirect(ctx.Repo.RepoLink + "/settings") @@ -1018,12 +1025,11 @@ func handleSettingsPostArchive(ctx *context.Context) { } func handleSettingsPostUnarchive(ctx *context.Context) { - repo := ctx.Repo.Repository - if !ctx.Repo.Permission.IsOwner() { - ctx.HTTPError(http.StatusForbidden) + if !canManageRepoDangerZone(ctx) { return } + repo := ctx.Repo.Repository if err := repo_model.SetArchiveRepoState(ctx, repo, false); err != nil { log.Error("Tried to unarchive a repo: %s", err) ctx.Flash.Error(ctx.Tr("repo.settings.unarchive.error")) @@ -1047,6 +1053,10 @@ func handleSettingsPostUnarchive(ctx *context.Context) { } func handleSettingsPostVisibility(ctx *context.Context) { + if !canManageRepoDangerZone(ctx) { + return + } + repo := ctx.Repo.Repository if repo.IsFork { ctx.JSONError(ctx.Tr("repo.settings.visibility.fork_error")) @@ -1055,7 +1065,7 @@ func handleSettingsPostVisibility(ctx *context.Context) { private := ctx.FormOptionalBool("private").ValueOrDefault(true) // default to true for privacy & safety - // when ForcePrivate enabled, you could change public repo to private, but only admin users can change private to public + // when ForcePrivate enabled, you could change public repo to private, only site admin users can change private to public if !private && setting.Repository.ForcePrivate && !ctx.Doer.IsAdmin { ctx.JSONError(ctx.Tr("form.repository_force_private")) return diff --git a/routers/web/repo/setting/settings_test.go b/routers/web/repo/setting/settings_test.go index bd165a89e3e..b88940c83be 100644 --- a/routers/web/repo/setting/settings_test.go +++ b/routers/web/repo/setting/settings_test.go @@ -11,6 +11,7 @@ import ( asymkey_model "gitea.dev/models/asymkey" "gitea.dev/models/organization" "gitea.dev/models/perm" + access_model "gitea.dev/models/perm/access" repo_model "gitea.dev/models/repo" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" @@ -173,143 +174,48 @@ func TestCollaborationPost_NonExistentUser(t *testing.T) { func TestAddTeamPost(t *testing.T) { unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "org26/repo43") + org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 26}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 43}) + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 11}) + repo.Owner = org - ctx.Req.Form.Set("team", "team11") - - org := &user_model.User{ - LowerName: "org26", - Type: user_model.UserTypeOrganization, + testAddTeamPost := func(t *testing.T, teamName string, repoAdminChangeTeamAccess bool) *context.Context { + ctx, _ := contexttest.MockContext(t, "org26/repo43") + ctx.Req.Form.Set("team", teamName) + org.RepoAdminChangeTeamAccess = repoAdminChangeTeamAccess + ctx.Repo = &context.Repository{ + Permission: access_model.Permission{AccessMode: perm.AccessModeAdmin}, + Owner: repo.Owner, + Repository: repo, + } + ctx.Doer = &user_model.User{ID: 1, IsAdmin: true} + AddTeamPost(ctx) + return ctx } - team := &organization.Team{ - ID: 11, - OrgID: 26, - } - - re := &repo_model.Repository{ - ID: 43, - Owner: org, - OwnerID: 26, - } - - repo := &context.Repository{ - Owner: &user_model.User{ - ID: 26, - LowerName: "org26", - RepoAdminChangeTeamAccess: true, - }, - Repository: re, - } - - ctx.Repo = repo - - AddTeamPost(ctx) - - assert.True(t, repo_service.HasRepository(t.Context(), team, re.ID)) - assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus()) - assert.Empty(t, ctx.Flash.ErrorMsg) -} - -func TestAddTeamPost_NotAllowed(t *testing.T) { - unittest.PrepareTestEnv(t) - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 32}) - require.NoError(t, repo.LoadOwner(t.Context())) - adminTeam := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 12}) - targetTeam := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) - require.NoError(t, repo_service.TeamAddRepository(t.Context(), adminTeam, repo)) - doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28}) - repoContext := &context.Repository{Owner: repo.Owner, Repository: repo} - renderCtx, _ := contexttest.MockContext(t, repo.Link()+"/settings/collaboration") - renderCtx.Repo = repoContext - renderCtx.Doer = doer - Collaboration(renderCtx) - assert.Equal(t, false, renderCtx.Data["CanChangeRepoTeamAccess"]) - - ctx, _ := contexttest.MockContext(t, repo.Link()+"/settings/collaboration") - ctx.Req.Form.Set("team", targetTeam.Name) - ctx.Repo = repoContext - ctx.Doer = doer - - AddTeamPost(ctx) - - assert.False(t, repo_service.HasRepository(t.Context(), targetTeam, repo.ID)) - assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus()) - assert.NotEmpty(t, ctx.Flash.ErrorMsg) -} - -func TestAddTeamPost_AddTeamTwice(t *testing.T) { - unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "org26/repo43") - - ctx.Req.Form.Set("team", "team11") - - org := &user_model.User{ - LowerName: "org26", - Type: user_model.UserTypeOrganization, - } - - team := &organization.Team{ - ID: 11, - OrgID: 26, - } - - re := &repo_model.Repository{ - ID: 43, - Owner: org, - OwnerID: 26, - } - - repo := &context.Repository{ - Owner: &user_model.User{ - ID: 26, - LowerName: "org26", - RepoAdminChangeTeamAccess: true, - }, - Repository: re, - } - - ctx.Repo = repo - - AddTeamPost(ctx) - - AddTeamPost(ctx) - assert.True(t, repo_service.HasRepository(t.Context(), team, re.ID)) - assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus()) - assert.NotEmpty(t, ctx.Flash.ErrorMsg) -} - -func TestAddTeamPost_NonExistentTeam(t *testing.T) { - unittest.PrepareTestEnv(t) - ctx, _ := contexttest.MockContext(t, "org26/repo43") - - ctx.Req.Form.Set("team", "team-non-existent") - - org := &user_model.User{ - LowerName: "org26", - Type: user_model.UserTypeOrganization, - } - - re := &repo_model.Repository{ - ID: 43, - Owner: org, - OwnerID: 26, - } - - repo := &context.Repository{ - Owner: &user_model.User{ - ID: 26, - LowerName: "org26", - RepoAdminChangeTeamAccess: true, - }, - Repository: re, - } - - ctx.Repo = repo - - AddTeamPost(ctx) - assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus()) - assert.NotEmpty(t, ctx.Flash.ErrorMsg) + t.Run("NonExisting", func(t *testing.T) { + ctx := testAddTeamPost(t, "team-not-exist", true) + assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus()) + assert.Contains(t, ctx.Flash.ErrorMsg, "form.team_not_exist") + }) + t.Run("NotAllowed", func(t *testing.T) { + ctx := testAddTeamPost(t, team.Name, false) + assert.False(t, repo_service.HasRepository(t.Context(), team, repo.ID)) + assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus()) + assert.Contains(t, ctx.Flash.ErrorMsg, "repo.settings.change_team_access_not_allowed") + }) + t.Run("Allowed", func(t *testing.T) { + ctx := testAddTeamPost(t, team.Name, true) + assert.True(t, repo_service.HasRepository(t.Context(), team, repo.ID)) + assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus()) + assert.Empty(t, ctx.Flash.ErrorMsg) + t.Run("Twice", func(t *testing.T) { + ctx := testAddTeamPost(t, team.Name, true) + assert.True(t, repo_service.HasRepository(t.Context(), team, repo.ID)) + assert.Equal(t, http.StatusSeeOther, ctx.Resp.WrittenStatus()) + assert.Contains(t, ctx.Flash.ErrorMsg, "repo.settings.add_team_duplicate") + }) + }) } func TestDeleteTeam(t *testing.T) { @@ -317,37 +223,21 @@ func TestDeleteTeam(t *testing.T) { ctx, _ := contexttest.MockContext(t, "org3/team1/repo3") ctx.Req.Form.Set("id", "2") - - org := &user_model.User{ - LowerName: "org3", - Type: user_model.UserTypeOrganization, + org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3}) + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: 2}) + repo.Owner = org + org.RepoAdminChangeTeamAccess = true + ctx.Repo = &context.Repository{ + Permission: access_model.Permission{AccessMode: perm.AccessModeAdmin}, + Owner: repo.Owner, + Repository: repo, } + ctx.Doer = &user_model.User{ID: 1, IsAdmin: true} - team := &organization.Team{ - ID: 2, - OrgID: 3, - } - - re := &repo_model.Repository{ - ID: 3, - Owner: org, - OwnerID: 3, - } - - repo := &context.Repository{ - Owner: &user_model.User{ - ID: 3, - LowerName: "org3", - RepoAdminChangeTeamAccess: true, - }, - Repository: re, - } - - ctx.Repo = repo - + assert.True(t, repo_service.HasRepository(t.Context(), team, repo.ID)) DeleteTeam(ctx) - - assert.False(t, repo_service.HasRepository(t.Context(), team, re.ID)) + assert.False(t, repo_service.HasRepository(t.Context(), team, repo.ID)) } func TestHandleSettingsPostMirrorPreservesExistingUsername(t *testing.T) { diff --git a/routers/web/web.go b/routers/web/web.go index fcdad7dc65f..ed1593cf8fd 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -987,11 +987,9 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) { m.Post("/teams/{team}/action/repo/{action}", org.TeamsRepoAction) }, context.OrgAssignment(context.OrgAssignmentOptions{RequireMember: true, RequireTeamMember: true})) - // require member/team-admin permission (old logic is: requireMember=true, requireTeamAdmin=true) - // but it doesn't seem right: requireTeamAdmin does nothing m.Group("/{org}", func() { m.Get("/teams/-/search", org.SearchTeam) - }, context.OrgAssignment(context.OrgAssignmentOptions{RequireMember: true, RequireTeamAdmin: true})) + }, context.OrgAssignment(context.OrgAssignmentOptions{RequireMember: true})) // require owner permission m.Group("/{org}", func() { diff --git a/services/context/org.go b/services/context/org.go index 310e158c8ac..b0bb03bfa70 100644 --- a/services/context/org.go +++ b/services/context/org.go @@ -19,10 +19,10 @@ import ( // Organization contains organization context type Organization struct { - IsOwner bool - IsMember bool - IsTeamMember bool // Is member of team. - IsTeamAdmin bool // In owner team or team that has admin permission level. + IsOwner bool + IsMember bool + IsTeamMember bool // Is member of team. + Organization *organization.Organization OrgLink string CanCreateOrgRepo bool @@ -66,7 +66,6 @@ type OrgAssignmentOptions struct { RequireMember bool RequireOwner bool RequireTeamMember bool - RequireTeamAdmin bool } // OrgAssignment returns a middleware to handle organization assignment @@ -112,7 +111,6 @@ func OrgAssignment(orgAssignmentOpts OrgAssignmentOptions) func(ctx *Context) { ctx.Org.IsOwner = true ctx.Org.IsMember = true ctx.Org.IsTeamMember = true - ctx.Org.IsTeamAdmin = true ctx.Org.CanCreateOrgRepo = true } else if ctx.IsSigned { ctx.Org.IsOwner, err = org.IsOwnedBy(ctx, ctx.Doer.ID) @@ -124,7 +122,6 @@ func OrgAssignment(orgAssignmentOpts OrgAssignmentOptions) func(ctx *Context) { if ctx.Org.IsOwner { ctx.Org.IsMember = true ctx.Org.IsTeamMember = true - ctx.Org.IsTeamAdmin = true ctx.Org.CanCreateOrgRepo = true } else { ctx.Org.IsMember, err = org.IsOrgMember(ctx, ctx.Doer.ID) @@ -236,14 +233,6 @@ func OrgAssignment(orgAssignmentOpts OrgAssignmentOptions) func(ctx *Context) { ctx.NotFound(err) return } - - isTeamOwnerOrAdmin := ctx.Org.Team.IsOwnerTeam() || ctx.Org.Team.HasAdminAccess() - ctx.Org.IsTeamAdmin = ctx.Org.IsOwner || (ctx.Org.IsTeamMember && isTeamOwnerOrAdmin) - ctx.Data["IsTeamAdmin"] = ctx.Org.IsTeamAdmin - if opts.RequireTeamAdmin && !ctx.Org.IsTeamAdmin { - ctx.NotFound(err) - return - } } ctx.Data["ContextUser"] = ctx.ContextUser @@ -277,10 +266,5 @@ func UserShouldSeeAllOrgTeams(ctx *Context) (bool, error) { if err != nil { return false, err } - for _, team := range teams { - if team.IncludesAllRepositories && team.HasAdminAccess() { - return true, nil - } - } - return false, nil + return teams.HasAllRepoAdminAccess(), nil } diff --git a/services/issue/comments.go b/services/issue/comments.go index 400ca775a04..649f4bfd180 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -30,7 +30,7 @@ func CreateRefComment(ctx context.Context, doer *user_model.User, repo *repo_mod } if user_model.IsUserBlockedBy(ctx, doer, issue.PosterID, repo.OwnerID) { - if isAdmin, _ := access_model.IsUserRepoAdmin(ctx, repo, doer); !isAdmin { + if !access_model.IsUserRepoAdmin(ctx, repo, doer) { return user_model.ErrBlockedUser } } @@ -61,7 +61,7 @@ func CreateRefComment(ctx context.Context, doer *user_model.User, repo *repo_mod // CreateIssueComment creates a plain issue comment. func CreateIssueComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, content string, attachments []string) (*issues_model.Comment, error) { if user_model.IsUserBlockedBy(ctx, doer, issue.PosterID, repo.OwnerID) { - if isAdmin, _ := access_model.IsUserRepoAdmin(ctx, repo, doer); !isAdmin { + if !access_model.IsUserRepoAdmin(ctx, repo, doer) { return nil, user_model.ErrBlockedUser } } @@ -104,7 +104,7 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, contentVersion } if user_model.IsUserBlockedBy(ctx, doer, c.Issue.PosterID, c.Issue.Repo.OwnerID) { - if isAdmin, _ := access_model.IsUserRepoAdmin(ctx, c.Issue.Repo, doer); !isAdmin { + if !access_model.IsUserRepoAdmin(ctx, c.Issue.Repo, doer) { return user_model.ErrBlockedUser } } diff --git a/services/issue/content.go b/services/issue/content.go index 41f30e09983..70fc4422c1d 100644 --- a/services/issue/content.go +++ b/services/issue/content.go @@ -19,7 +19,7 @@ func ChangeContent(ctx context.Context, issue *issues_model.Issue, doer *user_mo } if user_model.IsUserBlockedBy(ctx, doer, issue.PosterID, issue.Repo.OwnerID) { - if isAdmin, _ := access_model.IsUserRepoAdmin(ctx, issue.Repo, doer); !isAdmin { + if !access_model.IsUserRepoAdmin(ctx, issue.Repo, doer) { return user_model.ErrBlockedUser } } diff --git a/services/issue/issue.go b/services/issue/issue.go index 5d32279649f..b903f15b0b1 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -97,7 +97,7 @@ func ChangeTitle(ctx context.Context, issue *issues_model.Issue, doer *user_mode } if user_model.IsUserBlockedBy(ctx, doer, issue.PosterID, issue.Repo.OwnerID) { - if isAdmin, _ := access_model.IsUserRepoAdmin(ctx, issue.Repo, doer); !isAdmin { + if !access_model.IsUserRepoAdmin(ctx, issue.Repo, doer) { return user_model.ErrBlockedUser } } diff --git a/services/packages/package_update.go b/services/packages/package_update.go index 880d0fd9127..b378b95553b 100644 --- a/services/packages/package_update.go +++ b/services/packages/package_update.go @@ -9,6 +9,7 @@ import ( org_model "gitea.dev/models/organization" packages_model "gitea.dev/models/packages" + "gitea.dev/models/perm" access_model "gitea.dev/models/perm/access" repo_model "gitea.dev/models/repo" "gitea.dev/models/unit" @@ -38,42 +39,42 @@ func LinkToRepository(ctx context.Context, pkg *packages_model.Package, repo *re return nil } +func canDoerManagePackage(ctx context.Context, pkg *packages_model.Package, doer *user_model.User) bool { + owner, err := user_model.GetUserByID(ctx, pkg.OwnerID) + if err != nil { + return false + } + if doer.IsAdmin { + return true + } + if !owner.IsOrganization() { + return doer.ID == pkg.OwnerID + } + + teams, _ := org_model.GetUserOrgTeams(ctx, owner.ID, doer.ID) + if teams.HasAllRepoAdminAccess() { + return true + } + + if pkg.RepoID != 0 { + // old behavior: repo admin can manage the package linked to the repo + teams, _ = org_model.GetUserRepoTeams(ctx, owner.ID, doer.ID, pkg.RepoID) + for _, team := range teams { + if team.AccessMode >= perm.AccessModeAdmin { + return true + } + } + } + + return false +} + func UnlinkFromRepository(ctx context.Context, pkg *packages_model.Package, doer *user_model.User) error { if pkg.RepoID == 0 { return util.ErrInvalidArgument } - - repo, err := repo_model.GetRepositoryByID(ctx, pkg.RepoID) - if err != nil && !repo_model.IsErrRepoNotExist(err) { - return fmt.Errorf("error getting repository %d: %w", pkg.RepoID, err) - } - if err == nil { - perms, err := access_model.GetDoerRepoPermission(ctx, repo, doer) - if err != nil { - return fmt.Errorf("error getting permissions for user %d on repository %d: %w", doer.ID, repo.ID, err) - } - if !perms.CanWrite(unit.TypePackages) { - return util.ErrPermissionDenied - } - } - - user, err := user_model.GetUserByID(ctx, pkg.OwnerID) - if err != nil { - return err - } - if !doer.IsAdmin { - if !user.IsOrganization() { - if doer.ID != pkg.OwnerID { - return fmt.Errorf("no permission to unlink package '%v' from its repository, or packages are disabled", pkg.Name) - } - } else { - isOrgAdmin, err := org_model.OrgFromUser(user).IsOrgAdmin(ctx, doer.ID) - if err != nil { - return err - } else if !isOrgAdmin { - return fmt.Errorf("no permission to unlink package '%v' from its repository, or packages are disabled", pkg.Name) - } - } + if !canDoerManagePackage(ctx, pkg, doer) { + return util.ErrPermissionDenied } return packages_model.UnlinkRepository(ctx, pkg.ID) } diff --git a/services/packages/packages_test.go b/services/packages/packages_test.go new file mode 100644 index 00000000000..b4563cf74ff --- /dev/null +++ b/services/packages/packages_test.go @@ -0,0 +1,44 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package packages + +import ( + "testing" + + "gitea.dev/models/db" + "gitea.dev/models/organization" + packages_model "gitea.dev/models/packages" + repo_model "gitea.dev/models/repo" + unit_model "gitea.dev/models/unit" + "gitea.dev/models/unittest" + user_model "gitea.dev/models/user" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMain(m *testing.M) { + unittest.MainTest(m) +} + +func TestUnlinkFromRepositoryRequiresTargetRepoAdmin(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + repo := &repo_model.Repository{OwnerID: 3, OwnerName: "org3", Name: "package-repo", LowerName: "package-repo", IsPrivate: true} + require.NoError(t, db.Insert(t.Context(), repo)) + require.NoError(t, db.Insert(t.Context(), + &repo_model.RepoUnit{RepoID: repo.ID, Type: unit_model.TypeCode}, + &repo_model.RepoUnit{RepoID: repo.ID, Type: unit_model.TypePackages}, + &organization.TeamRepo{OrgID: repo.OwnerID, TeamID: 14, RepoID: repo.ID}, + )) + pkg := &packages_model.Package{OwnerID: repo.OwnerID, RepoID: repo.ID, Type: packages_model.TypeGeneric, Name: "package", LowerName: "package"} + require.NoError(t, db.Insert(t.Context(), pkg)) + doer := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28}) + + assert.Error(t, UnlinkFromRepository(t.Context(), pkg, doer)) + assert.Equal(t, repo.ID, unittest.AssertExistsAndLoadBean(t, &packages_model.Package{ID: pkg.ID}).RepoID) + + require.NoError(t, db.Insert(t.Context(), &organization.TeamRepo{OrgID: repo.OwnerID, TeamID: 12, RepoID: repo.ID})) + require.NoError(t, UnlinkFromRepository(t.Context(), pkg, doer)) + assert.Zero(t, unittest.AssertExistsAndLoadBean(t, &packages_model.Package{ID: pkg.ID}).RepoID) +} diff --git a/services/pull/check.go b/services/pull/check.go index bf06e476ee8..9c1aa016770 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -190,11 +190,7 @@ func CheckPullMergeable(stdCtx context.Context, doer *user_model.User, perm *acc // * if the doer tries to "Force Merge", check whether it is really allowed if forceMerge { - isRepoAdmin, errForceMerge := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer) - if errForceMerge != nil { - return fmt.Errorf("IsUserRepoAdmin failed, repo: %v, doer: %v, err: %w", pr.BaseRepoID, doer.ID, errForceMerge) - } - + isRepoAdmin := access_model.IsUserRepoAdmin(ctx, pr.BaseRepo, doer) protectedBranchRule, errForceMerge := git_model.GetFirstMatchProtectedBranchRule(ctx, pr.BaseRepoID, pr.BaseBranch) if errForceMerge != nil { return fmt.Errorf("GetFirstMatchProtectedBranchRule failed, repo: %v, base branch: %v, err: %w", pr.BaseRepoID, pr.BaseBranch, errForceMerge) diff --git a/services/repository/create.go b/services/repository/create.go index 4df886c2dc2..50875cd544e 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -424,9 +424,7 @@ func createRepositoryInDB(ctx context.Context, doer, u *user_model.User, repo *r } } - if isAdmin, err := access_model.IsUserRepoAdmin(ctx, repo, doer); err != nil { - return fmt.Errorf("IsUserRepoAdmin: %w", err) - } else if !isAdmin { + if !access_model.IsUserRepoAdmin(ctx, repo, doer) { // Make creator repo admin if it wasn't assigned automatically if err = AddOrUpdateCollaborator(ctx, repo, doer, perm.AccessModeAdmin); err != nil { return fmt.Errorf("AddCollaborator: %w", err) diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index d4491e58d84..4d50f51e0a1 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -780,7 +780,7 @@ {{end}} - {{if .Permission.IsOwner}} + {{if $.CanManagerDangerZone}}

{{ctx.Locale.Tr "repo.settings.danger_zone"}}

@@ -899,7 +899,7 @@ {{template "repo/settings/layout_footer" .}} -{{if .Permission.IsOwner}} +{{if $.CanManagerDangerZone}} {{if .Repository.IsMirror}}