diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index bc39199766..234634e367 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -11,6 +11,7 @@ import ( "gitea.dev/models/db" "gitea.dev/models/organization" + "gitea.dev/models/perm" repo_model "gitea.dev/models/repo" "gitea.dev/models/unit" user_model "gitea.dev/models/user" @@ -287,8 +288,8 @@ func applyConditions(sess db.Session, opts *IssuesOptions) { } } -// teamUnitsRepoCond returns query condition for those repo id in the special org team with special units access -func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Type) builder.Cond { +// teamUnitsRepoReaderCond returns query condition for those repo id in the special org team with special units access +func teamUnitsRepoReaderCond(id string, userID, orgID, teamID int64, units ...unit.Type) builder.Cond { return builder.In(id, builder.Select("repo_id").From("team_repo").Where( builder.Eq{ @@ -316,12 +317,19 @@ func teamUnitsRepoCond(id string, userID, orgID, teamID int64, units ...unit.Typ }), ), )).And( - builder.In( - "team_id", builder.Select("team_id").From("team_unit").Where( - builder.Eq{ - "`team_unit`.org_id": orgID, - }.And( - builder.In("`team_unit`.type", units), + builder.Or( + builder.In( + "team_id", builder.Select("id").From("team").Where( + builder.Eq{"id": teamID}.And(builder.Gt{"authorize": perm.AccessModeNone}), + ), + ), + builder.In( + "team_id", builder.Select("team_id").From("team_unit").Where( + builder.Eq{ + "`team_unit`.org_id": orgID, + }.And( + builder.In("`team_unit`.type", units), + ), ), ), ), @@ -338,7 +346,7 @@ func issuePullAccessibleRepoCond(repoIDstr string, userID int64, owner *user_mod } if owner != nil && owner.IsOrganization() { if team != nil { - cond = cond.And(teamUnitsRepoCond(repoIDstr, userID, owner.ID, team.ID, unitType)) // special team member repos + cond = cond.And(teamUnitsRepoReaderCond(repoIDstr, userID, owner.ID, team.ID, unitType)) // special team member repos } else { cond = cond.And( builder.Or( diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 646b261070..1fd9d0d68f 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -626,16 +626,7 @@ func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *u unittype = unit.TypePullRequests } for _, team := range teams { - if team.HasAdminAccess() { - checked = append(checked, team.ID) - resolved[issue.Repo.Owner.LowerName+"/"+team.LowerName] = true - continue - } - has, err := db.Exist[organization.TeamUnit](ctx, builder.Eq{"org_id": issue.Repo.Owner.ID, "team_id": team.ID, "`type`": unittype}) - if err != nil { - return nil, fmt.Errorf("get team units (%d): %w", team.ID, err) - } - if has { + if team.UnitEnabled(ctx, unittype) { checked = append(checked, team.ID) resolved[issue.Repo.Owner.LowerName+"/"+team.LowerName] = true } diff --git a/models/organization/org.go b/models/organization/org.go index deb7d54b79..e668269679 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -284,8 +284,8 @@ func (org *Organization) CustomAvatarRelativePath() string { return org.Avatar } -// UnitPermission returns unit permission -func (org *Organization) UnitPermission(ctx context.Context, doer *user_model.User, unitType unit.Type) perm.AccessMode { +func (org *Organization) AnyRepoUnitPermission(ctx context.Context, doer *user_model.User, unitType unit.Type) perm.AccessMode { + // FIXME: ORG-TEAM-UNIT-MAX-PERMISSION: this function is not right, team can access repo1's code doesn't mean it can access repo2's code if doer != nil { teams, err := GetUserOrgTeams(ctx, org.ID, doer.ID) if err != nil { @@ -299,7 +299,7 @@ func (org *Organization) UnitPermission(ctx context.Context, doer *user_model.Us } if len(teams) > 0 { - return teams.UnitMaxAccess(unitType) + return teams.AnyRepoUnitMaxAccess(ctx, unitType) } } diff --git a/models/organization/team.go b/models/organization/team.go index b419aa3225..06cc5c7e69 100644 --- a/models/organization/team.go +++ b/models/organization/team.go @@ -149,29 +149,14 @@ func (t *Team) LoadUnits(ctx context.Context) (err error) { return err } -// GetUnitNames returns the team units names -func (t *Team) GetUnitNames() (res []string) { - if t.HasAdminAccess() { - return unit.AllUnitKeyNames() - } - - for _, u := range t.Units { - res = append(res, unit.Units[u.Type].NameKey) - } - return res -} - // GetUnitsMap returns the team units permissions func (t *Team) GetUnitsMap() map[string]string { + if len(t.Units) == 0 { + return nil + } m := make(map[string]string) - if t.HasAdminAccess() { - for _, u := range unit.Units { - m[u.NameKey] = t.AccessMode.ToString() - } - } else { - for _, u := range t.Units { - m[u.Unit().NameKey] = u.AccessMode.ToString() - } + for _, u := range t.Units { + m[u.Unit().NameKey] = u.AccessMode.ToString() } return m } @@ -214,16 +199,21 @@ func (t *Team) UnitAccessMode(ctx context.Context, tp unit.Type) perm.AccessMode return accessMode } -func (t *Team) UnitAccessModeEx(ctx context.Context, tp unit.Type) (accessMode perm.AccessMode, exist bool) { +func (t *Team) UnitAccessModeEx(ctx context.Context, tp unit.Type) (mode perm.AccessMode, exist bool) { if err := t.LoadUnits(ctx); err != nil { - log.Warn("Error loading team (ID: %d) units: %s", t.ID, err.Error()) + log.Error("Error loading team (ID: %d) units: %v", t.ID, err) } for _, u := range t.Units { if u.Type == tp { - return u.AccessMode, true + mode, exist = u.AccessMode, true + break } } - return perm.AccessModeNone, false + mode = max(mode, t.AccessMode) + if unitDef, ok := unit.Units[tp]; ok { + mode = min(mode, unitDef.MaxPerm()) + } + return mode, exist || t.AccessMode > perm.AccessModeNone } // IsUsableTeamName tests if a name could be as team name diff --git a/models/organization/team_list.go b/models/organization/team_list.go index bb2f9f224d..4e589c4544 100644 --- a/models/organization/team_list.go +++ b/models/organization/team_list.go @@ -27,20 +27,14 @@ func (t TeamList) LoadUnits(ctx context.Context) error { return nil } -func (t TeamList) UnitMaxAccess(tp unit.Type) perm.AccessMode { +func (t TeamList) AnyRepoUnitMaxAccess(ctx context.Context, tp unit.Type) perm.AccessMode { + // FIXME: ORG-TEAM-UNIT-MAX-PERMISSION: this function is not right, team can access repo1's code doesn't mean it can access repo2's code maxAccess := perm.AccessModeNone for _, team := range t { if team.IsOwnerTeam() { return perm.AccessModeOwner } - for _, teamUnit := range team.Units { - if teamUnit.Type != tp { - continue - } - if teamUnit.AccessMode > maxAccess { - maxAccess = teamUnit.AccessMode - } - } + maxAccess = max(maxAccess, team.UnitAccessMode(ctx, tp)) } return maxAccess } diff --git a/models/organization/team_list_test.go b/models/organization/team_list_test.go index 553f1f1c91..d76f1b6e6b 100644 --- a/models/organization/team_list_test.go +++ b/models/organization/team_list_test.go @@ -7,6 +7,8 @@ import ( "testing" org_model "gitea.dev/models/organization" + "gitea.dev/models/perm" + "gitea.dev/models/unit" "gitea.dev/models/unittest" "github.com/stretchr/testify/assert" @@ -22,3 +24,21 @@ func Test_GetTeamsByIDs(t *testing.T) { assert.Equal(t, "Owners", teams[1].Name) assert.Equal(t, "team1", teams[2].Name) } + +func TestTeamList_UnitMaxAccess(t *testing.T) { + ctx := t.Context() + adminTeam := &org_model.Team{AccessMode: perm.AccessModeAdmin, Units: nil} + writeTeam := &org_model.Team{AccessMode: perm.AccessModeWrite, Units: nil} + granularTeam := &org_model.Team{ + AccessMode: perm.AccessModeNone, + Units: []*org_model.TeamUnit{ + {Type: unit.TypeCode, AccessMode: perm.AccessModeWrite}, + }, + } + + assert.Equal(t, perm.AccessModeAdmin, org_model.TeamList{adminTeam}.AnyRepoUnitMaxAccess(ctx, unit.TypeActions)) + assert.Equal(t, perm.AccessModeWrite, org_model.TeamList{writeTeam}.AnyRepoUnitMaxAccess(ctx, unit.TypeActions)) + assert.Equal(t, perm.AccessModeWrite, org_model.TeamList{granularTeam}.AnyRepoUnitMaxAccess(ctx, unit.TypeCode)) + assert.Equal(t, perm.AccessModeNone, org_model.TeamList{granularTeam}.AnyRepoUnitMaxAccess(ctx, unit.TypeActions)) + assert.Equal(t, perm.AccessModeAdmin, org_model.TeamList{granularTeam, adminTeam}.AnyRepoUnitMaxAccess(ctx, unit.TypeActions)) +} diff --git a/models/organization/team_repo.go b/models/organization/team_repo.go index b497419979..f41f8fbb0d 100644 --- a/models/organization/team_repo.go +++ b/models/organization/team_repo.go @@ -52,7 +52,6 @@ func RemoveTeamRepo(ctx context.Context, teamID, repoID int64) error { // GetTeamsWithAccessToAnyRepoUnit returns all teams in an organization that have given access level to the repository special unit. // This function is only used for finding some teams that can be used as branch protection allowlist or reviewers, it isn't really used for access control. -// FIXME: TEAM-UNIT-PERMISSION this logic is not complete, search the fixme keyword to see more details func GetTeamsWithAccessToAnyRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type, unitTypesMore ...unit.Type) (teams []*Team, err error) { teamIDs, err := getTeamIDsWithAccessToAnyRepoUnit(ctx, orgID, repoID, mode, unitType, unitTypesMore...) if err != nil { diff --git a/models/organization/team_test.go b/models/organization/team_test.go index 30fe6e8229..df63c74658 100644 --- a/models/organization/team_test.go +++ b/models/organization/team_test.go @@ -8,7 +8,9 @@ import ( "gitea.dev/models/db" "gitea.dev/models/organization" + "gitea.dev/models/perm" repo_model "gitea.dev/models/repo" + "gitea.dev/models/unit" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" "gitea.dev/modules/structs" @@ -291,3 +293,28 @@ func TestIsUsableTeamName(t *testing.T) { assert.NoError(t, organization.IsUsableTeamName("usable")) assert.True(t, db.IsErrNameReserved(organization.IsUsableTeamName("new"))) } + +func TestTeam_UnitAccessModeEx(t *testing.T) { + team := &organization.Team{ + AccessMode: perm.AccessModeWrite, Units: []*organization.TeamUnit{ + {Type: unit.TypeIssues, AccessMode: perm.AccessModeRead}, // team mode wins + {Type: unit.TypeWiki, AccessMode: perm.AccessModeAdmin}, // unit mode wins + }, + } + mode, exist := team.UnitAccessModeEx(t.Context(), unit.TypeActions) + assert.True(t, exist) + assert.Equal(t, perm.AccessModeWrite, mode) + assert.Equal(t, perm.AccessModeWrite, team.UnitAccessMode(t.Context(), unit.TypeIssues)) + assert.Equal(t, perm.AccessModeAdmin, team.UnitAccessMode(t.Context(), unit.TypeWiki)) + assert.Equal(t, perm.AccessModeRead, team.UnitAccessMode(t.Context(), unit.TypeExternalWiki)) // limited by unit definition + + team = &organization.Team{AccessMode: perm.AccessModeOwner, Units: []*organization.TeamUnit{}} + mode, exist = team.UnitAccessModeEx(t.Context(), unit.TypePackages) + assert.True(t, exist) + assert.Equal(t, perm.AccessModeAdmin, mode) + + team = &organization.Team{AccessMode: perm.AccessModeNone, Units: []*organization.TeamUnit{}} + mode, exist = team.UnitAccessModeEx(t.Context(), unit.TypeActions) + assert.False(t, exist) + assert.Equal(t, perm.AccessModeNone, mode) +} diff --git a/models/repo/collaboration.go b/models/repo/collaboration.go index 9f7b610072..bfbf7b6388 100644 --- a/models/repo/collaboration.go +++ b/models/repo/collaboration.go @@ -111,16 +111,21 @@ func IsCollaborator(ctx context.Context, repoID, userID int64) (bool, error) { return db.Exist[Collaboration](ctx, builder.Eq{"repo_id": repoID, "user_id": userID}) } -// IsOwnerMemberCollaborator checks if a provided user is the owner, a collaborator or a member of a team in a repository -func IsOwnerMemberCollaborator(ctx context.Context, repo *Repository, userID int64) (bool, error) { +func HasAccessToRepoCodeUnit(ctx context.Context, repo *Repository, userID int64) (bool, error) { if repo.OwnerID == userID { return true, nil } - teamMember, err := db.GetEngine(ctx).Join("INNER", "team_repo", "team_repo.team_id = team_user.team_id"). - Join("INNER", "team_unit", "team_unit.team_id = team_user.team_id"). + teamMember, err := db.GetEngine(ctx).Table("team_user"). + Join("INNER", "team_repo", "team_repo.team_id = team_user.team_id"). + Join("INNER", "team", "team.id = team_user.team_id"). + Join("LEFT", "team_unit", "team_unit.team_id = team_user.team_id AND team_unit.`type` = ?", unit.TypeCode). Where("team_repo.repo_id = ?", repo.ID). - And("team_unit.`type` = ?", unit.TypeCode). - And("team_user.uid = ?", userID).Table("team_user").Exist() + And("team_user.uid = ?", userID). + And(builder.Or( + builder.Gt{"team.authorize": perm.AccessModeNone}, + builder.Gt{"team_unit.access_mode": perm.AccessModeNone}, + )). + Exist() if err != nil { return false, err } diff --git a/models/repo/collaboration_test.go b/models/repo/collaboration_test.go index 766817a8fa..5d2045524a 100644 --- a/models/repo/collaboration_test.go +++ b/models/repo/collaboration_test.go @@ -73,31 +73,31 @@ func TestRepository_IsOwnerMemberCollaborator(t *testing.T) { repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // Organisation owner. - actual, err := repo_model.IsOwnerMemberCollaborator(t.Context(), repo1, 2) + actual, err := repo_model.HasAccessToRepoCodeUnit(t.Context(), repo1, 2) assert.NoError(t, err) assert.True(t, actual) // Team member. - actual, err = repo_model.IsOwnerMemberCollaborator(t.Context(), repo1, 4) + actual, err = repo_model.HasAccessToRepoCodeUnit(t.Context(), repo1, 4) assert.NoError(t, err) assert.True(t, actual) // Normal user. - actual, err = repo_model.IsOwnerMemberCollaborator(t.Context(), repo1, 1) + actual, err = repo_model.HasAccessToRepoCodeUnit(t.Context(), repo1, 1) assert.NoError(t, err) assert.False(t, actual) repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 4}) // Collaborator. - actual, err = repo_model.IsOwnerMemberCollaborator(t.Context(), repo2, 4) + actual, err = repo_model.HasAccessToRepoCodeUnit(t.Context(), repo2, 4) assert.NoError(t, err) assert.True(t, actual) repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 15}) // Repository owner. - actual, err = repo_model.IsOwnerMemberCollaborator(t.Context(), repo3, 2) + actual, err = repo_model.HasAccessToRepoCodeUnit(t.Context(), repo3, 2) assert.NoError(t, err) assert.True(t, actual) } diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index 268723787e..88a49b864c 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -310,15 +310,12 @@ func userOrgTeamRepoBuilder(userID int64) *builder.Builder { } // userOrgTeamUnitRepoBuilder returns repo ids where user's teams can access the special unit. -// A team grants the unit either through an explicit team_unit row (access_mode > none) or by being an -// admin/owner team (team.authorize >= admin), which grants every unit regardless of team_unit rows — -// mirroring the HasAdminAccess() short-circuit in access.GetIndividualUserRepoPermission. func userOrgTeamUnitRepoBuilder(userID int64, unitType unit.Type) *builder.Builder { return userOrgTeamRepoBuilder(userID). Join("INNER", "team", "`team`.id = `team_repo`.team_id"). Join("LEFT", "team_unit", builder.Expr("`team_unit`.team_id = `team_repo`.team_id AND `team_unit`.`type` = ?", unitType)). Where(builder.Or( - builder.Gte{"`team`.authorize": int(perm.AccessModeAdmin)}, + builder.Gt{"`team`.authorize": int(perm.AccessModeNone)}, builder.Gt{"`team_unit`.`access_mode`": int(perm.AccessModeNone)}, )) } diff --git a/models/repo/repo_list_test.go b/models/repo/repo_list_test.go index e3db58af23..742b0d97c7 100644 --- a/models/repo/repo_list_test.go +++ b/models/repo/repo_list_test.go @@ -486,10 +486,7 @@ func TestFindUserActionsAccessibleOwnerRepoIDs(t *testing.T) { assert.Contains(t, publicOnly, int64(32), "a public repo under a public owner stays listed") } -// TestUserOrgUnitRepoCondTeamAuthorize pins the team.authorize behavior of userOrgTeamUnitRepoBuilder -// (exercised through UserOrgUnitRepoCond): an admin/owner team grants every unit even without an explicit -// team_unit row, while a non-admin team only grants a unit it has an explicit row for. This guards both -// directions — hiding repos from admin-team members, and over-broadening a plain team's access. +// TestUserOrgUnitRepoCondTeamAuthorize pins team.authorize vs team_unit.access_mode func TestUserOrgUnitRepoCondTeamAuthorize(t *testing.T) { require.NoError(t, unittest.PrepareTestDatabase()) @@ -500,17 +497,16 @@ func TestUserOrgUnitRepoCondTeamAuthorize(t *testing.T) { return ids } - // Case A: user18 is only on org17's owner team (team5, authorize=owner), linked to the private repo24 - // but with no Actions team_unit row. The owner authorize must still grant it, mirroring the runtime - // HasAdminAccess() short-circuit in access.GetIndividualUserRepoPermission. - assert.Contains(t, accessibleRepoIDs(18, 17, unit.TypeActions), int64(24), - "an owner team grants a unit it has no explicit team_unit row for") + // Owner team5 has no Actions team_unit row but still grants via authorize=owner. + assert.Contains(t, accessibleRepoIDs(18, 17, unit.TypeActions), int64(24)) - // Cases B and C share one subject so the team_unit row is the only difference: user4 is only on org3's - // write team (team2, authorize=write, non-admin), linked to the private repo3. team2 has an explicit - // Projects row but none for Actions. - assert.Contains(t, accessibleRepoIDs(4, 3, unit.TypeProjects), int64(3), - "a non-admin team grants a unit it has an explicit team_unit row for") - assert.NotContains(t, accessibleRepoIDs(4, 3, unit.TypeActions), int64(3), - "a non-admin team must NOT grant a unit it has no team_unit row for") + // team2 is "authorize=write" with Projects team_unit but no Actions row. + assert.Contains(t, accessibleRepoIDs(4, 3, unit.TypeProjects), int64(3)) + assert.Contains(t, accessibleRepoIDs(4, 3, unit.TypeActions), int64(3)) + + // now team2 is "authorize=none", no Actions row. + _, err := db.GetEngine(t.Context()).Exec("UPDATE team SET authorize=0 WHERE id=2") + assert.NoError(t, err) + assert.Contains(t, accessibleRepoIDs(4, 3, unit.TypeProjects), int64(3)) + assert.NotContains(t, accessibleRepoIDs(4, 3, unit.TypeActions), int64(3)) } diff --git a/models/unit/unit.go b/models/unit/unit.go index 9e34fe354e..d8d8a4d420 100644 --- a/models/unit/unit.go +++ b/models/unit/unit.go @@ -33,9 +33,6 @@ const ( TypeProjects // 8 Projects TypePackages // 9 Packages TypeActions // 10 Actions - - // FIXME: TEAM-UNIT-PERMISSION: the team unit "admin" permission's design is not right, when a new unit is added in the future, - // admin team won't inherit the correct admin permission for the new unit, need to have a complete fix before adding any new unit. ) // Value returns integer value for unit type (used by template) diff --git a/models/unittest/unit_tests.go b/models/unittest/unit_tests.go index c989550021..9965ede63e 100644 --- a/models/unittest/unit_tests.go +++ b/models/unittest/unit_tests.go @@ -27,6 +27,7 @@ type TestingT interface { require.TestingT assert.TestingT Context() context.Context + Helper() } type testCond struct { @@ -77,6 +78,7 @@ func GetBean[T any](t TestingT, bean T, conditions ...any) (ret T) { // AssertExistsAndLoadBean assert that a bean exists and load it from the test database func AssertExistsAndLoadBean[T any](t TestingT, bean T, conditions ...any) T { + t.Helper() exists, err := getBeanIfExists(t, bean, conditions...) require.NoError(t, err) require.True(t, exists, @@ -87,6 +89,7 @@ func AssertExistsAndLoadBean[T any](t TestingT, bean T, conditions ...any) T { // AssertExistsAndLoadMap assert that a row exists and load it from the test database func AssertExistsAndLoadMap(t TestingT, table string, conditions ...any) map[string]string { + t.Helper() e := db.GetEngine(t.Context()).Table(table) res, err := whereOrderConditions(e, conditions).Query() assert.NoError(t, err) @@ -123,6 +126,7 @@ func GetCount(t TestingT, bean any, conditions ...any) int { // AssertNotExistsBean assert that a bean does not exist in the test database func AssertNotExistsBean(t TestingT, bean any, conditions ...any) { + t.Helper() exists, err := getBeanIfExists(t, bean, conditions...) assert.NoError(t, err) assert.False(t, exists) @@ -130,17 +134,20 @@ func AssertNotExistsBean(t TestingT, bean any, conditions ...any) { // AssertCount assert the count of a bean func AssertCount(t TestingT, bean, expected any) bool { + t.Helper() return assert.EqualValues(t, expected, GetCount(t, bean)) } // AssertInt64InRange assert value is in range [low, high] -func AssertInt64InRange(t assert.TestingT, low, high, value int64) { +func AssertInt64InRange(t TestingT, low, high, value int64) { + t.Helper() assert.True(t, value >= low && value <= high, "Expected value in range [%d, %d], found %d", low, high, value) } // GetCountByCond get the count of database entries matching bean func GetCountByCond(t TestingT, tableName string, cond builder.Cond) int64 { + t.Helper() e := db.GetEngine(t.Context()) count, err := e.Table(tableName).Where(cond).Count() assert.NoError(t, err) diff --git a/modules/structs/org_team.go b/modules/structs/org_team.go index 02f806aadb..f80a6eebee 100644 --- a/modules/structs/org_team.go +++ b/modules/structs/org_team.go @@ -52,8 +52,9 @@ type CreateTeamOption struct { // The description of the team Description string `json:"description" binding:"MaxSize(255)"` // Whether the team has access to all repositories in the organization - IncludesAllRepositories bool `json:"includes_all_repositories"` - Permission RepoWritePermission `json:"permission"` + IncludesAllRepositories bool `json:"includes_all_repositories"` + // All units have this permission (read/write/admin) + Permission RepoWritePermission `json:"permission"` // example: ["repo.actions","repo.packages","repo.code","repo.issues","repo.ext_issues","repo.wiki","repo.pulls","repo.releases","repo.projects","repo.ext_wiki"] // Deprecated: This variable should be replaced by UnitsMap and will be dropped in later versions. Units []string `json:"units"` @@ -72,8 +73,9 @@ type EditTeamOption struct { // The description of the team Description *string `json:"description" binding:"MaxSize(255)"` // Whether the team has access to all repositories in the organization - IncludesAllRepositories *bool `json:"includes_all_repositories"` - Permission RepoWritePermission `json:"permission"` + IncludesAllRepositories *bool `json:"includes_all_repositories"` + // All units have this permission (read/write/admin) + Permission RepoWritePermission `json:"permission"` // example: ["repo.code","repo.issues","repo.ext_issues","repo.wiki","repo.pulls","repo.releases","repo.projects","repo.ext_wiki"] // Deprecated: This variable should be replaced by UnitsMap and will be dropped in later versions. Units []string `json:"units"` diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index 14a88aedda..11cc9ec519 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -2845,8 +2845,9 @@ "org.teams.read_access_helper": "Members can view and clone team repositories.", "org.teams.write_access": "Write", "org.teams.write_access_helper": "Members can read and push to team repositories.", + "org.teams.write_access_all_helper": "Members get write access for all units to the team repositories.", "org.teams.admin_access": "Administrator Access", - "org.teams.admin_access_helper": "Members can pull and push to team repositories and add collaborators to them.", + "org.teams.admin_access_helper": "Members get administrator access to the team repositories.", "org.teams.no_desc": "This team has no description", "org.teams.settings": "Settings", "org.teams.owners_permission_desc": "Owners have full access to all repositories and have administrator access to the organization.", diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go index 1dee830b49..f15b3b49b5 100644 --- a/routers/api/v1/api.go +++ b/routers/api/v1/api.go @@ -816,7 +816,7 @@ func reqProjectsUnitAccess(accessMode perm.AccessMode) func(ctx *context.APICont } // individual visibility is handled by individualPermsChecker if ctx.ContextUser.IsOrganization() && - organization.OrgFromUser(ctx.ContextUser).UnitPermission(ctx, ctx.Doer, unit.TypeProjects) < accessMode { + organization.OrgFromUser(ctx.ContextUser).AnyRepoUnitPermission(ctx, ctx.Doer, unit.TypeProjects) < accessMode { ctx.APIErrorNotFound() } } diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go index 2c5d91e050..7b8e6937cb 100644 --- a/routers/api/v1/org/team.go +++ b/routers/api/v1/org/team.go @@ -6,6 +6,7 @@ package org import ( "errors" + "maps" "net/http" activities_model "gitea.dev/models/activities" @@ -17,6 +18,7 @@ import ( user_model "gitea.dev/models/user" "gitea.dev/modules/log" api "gitea.dev/modules/structs" + "gitea.dev/modules/util" "gitea.dev/modules/web" "gitea.dev/routers/api/v1/user" "gitea.dev/routers/api/v1/utils" @@ -150,42 +152,36 @@ func GetTeam(ctx *context.APIContext) { ctx.JSON(http.StatusOK, apiTeam) } -func attachTeamUnits(team *organization.Team, defaultAccessMode perm.AccessMode, units []string) { - unitTypes, _ := unit_model.FindUnitTypes(units...) - team.Units = make([]*organization.TeamUnit, 0, len(units)) - for _, tp := range unitTypes { - team.Units = append(team.Units, &organization.TeamUnit{ - OrgID: team.OrgID, - Type: tp, - AccessMode: defaultAccessMode, - }) +// assignTeamPermissionUnits sets authorize + team_unit rows. +func assignTeamPermissionUnits(team *organization.Team, permission string, units []string, unitsMap map[string]string) (changed bool, _ error) { + if len(units) > 0 && len(unitsMap) > 0 { + return false, util.NewInvalidArgumentErrorf("only one of units or units_map can be set") } -} - -func attachTeamUnitsMap(team *organization.Team, unitsMap map[string]string) { - team.Units = make([]*organization.TeamUnit, 0, len(unitsMap)) - for unitKey, p := range unitsMap { - team.Units = append(team.Units, &organization.TeamUnit{ - OrgID: team.OrgID, - Type: unit_model.TypeFromKey(unitKey), - AccessMode: perm.ParseAccessMode(p), - }) - } -} - -func attachAdminTeamUnits(team *organization.Team) { - team.Units = make([]*organization.TeamUnit, 0, len(unit_model.AllRepoUnitTypes)) - for _, ut := range unit_model.AllRepoUnitTypes { - up := perm.AccessModeAdmin - if ut == unit_model.TypeExternalTracker || ut == unit_model.TypeExternalWiki { - up = perm.AccessModeRead + if len(units) > 0 { + unitsMap = map[string]string{} + for _, unit := range units { + unitsMap[unit] = permission } - team.Units = append(team.Units, &organization.TeamUnit{ - OrgID: team.OrgID, - Type: ut, - AccessMode: up, - }) } + + oldAccessMode := team.AccessMode + oldUnitPerms := team.GetUnitsMap() + if len(unitsMap) > 0 { + team.Units = make([]*organization.TeamUnit, 0, len(unitsMap)) + for unitKey, p := range unitsMap { + unitType, unitPerm := unit_model.TypeFromKey(unitKey), perm.ParseAccessMode(p) + team.Units = append(team.Units, &organization.TeamUnit{OrgID: team.OrgID, Type: unitType, AccessMode: unitPerm}) + } + } else { + requested := perm.ParseAccessMode(permission, perm.AccessModeNone, perm.AccessModeRead, perm.AccessModeWrite, perm.AccessModeAdmin) + if requested == perm.AccessModeNone { + return false, util.NewInvalidArgumentErrorf("no permission specified") + } + team.AccessMode, team.Units = requested, nil + } + + changed = oldAccessMode != team.AccessMode || !maps.Equal(oldUnitPerms, team.GetUnitsMap()) + return changed, nil } // CreateTeam api for create a team @@ -215,29 +211,18 @@ func CreateTeam(ctx *context.APIContext) { // "422": // "$ref": "#/responses/validationError" form := web.GetForm[*api.CreateTeamOption](ctx) - teamPermission := perm.ParseAccessMode(string(form.Permission), perm.AccessModeNone, perm.AccessModeAdmin) team := &organization.Team{ OrgID: ctx.Org.Organization.ID, Name: form.Name, Description: form.Description, IncludesAllRepositories: form.IncludesAllRepositories, CanCreateOrgRepo: form.CanCreateOrgRepo, - AccessMode: teamPermission, Visibility: organization.NormalizeTeamVisibility(form.Visibility), } - - if team.AccessMode < perm.AccessModeAdmin { - if len(form.UnitsMap) > 0 { - attachTeamUnitsMap(team, form.UnitsMap) - } else if len(form.Units) > 0 { - unitPerm := perm.ParseAccessMode(string(form.Permission), perm.AccessModeRead, perm.AccessModeWrite) - attachTeamUnits(team, unitPerm, form.Units) - } else { - ctx.APIErrorInternal(errors.New("units permission should not be empty")) - return - } - } else { - attachAdminTeamUnits(team) + _, err := assignTeamPermissionUnits(team, string(form.Permission), form.Units, form.UnitsMap) + if err != nil { + ctx.APIErrorAuto(err) + return } if err := org_service.NewTeam(ctx, team); err != nil { @@ -307,28 +292,19 @@ func EditTeam(ctx *context.APIContext) { isAuthChanged := false isIncludeAllChanged := false - if !team.IsOwnerTeam() && len(form.Permission) != 0 { - teamPermission := perm.ParseAccessMode(string(form.Permission), perm.AccessModeNone, perm.AccessModeAdmin) - if team.AccessMode != teamPermission { - isAuthChanged = true - team.AccessMode = teamPermission - } - - if form.IncludesAllRepositories != nil { - isIncludeAllChanged = true - team.IncludesAllRepositories = *form.IncludesAllRepositories + hasPermFields := form.Permission != "" || len(form.Units) > 0 || len(form.UnitsMap) > 0 + if !team.IsOwnerTeam() && hasPermFields { + var err error + isAuthChanged, err = assignTeamPermissionUnits(team, string(form.Permission), form.Units, form.UnitsMap) + if err != nil { + ctx.APIErrorAuto(err) + return } } - if team.AccessMode < perm.AccessModeAdmin { - if len(form.UnitsMap) > 0 { - attachTeamUnitsMap(team, form.UnitsMap) - } else if len(form.Units) > 0 { - unitPerm := perm.ParseAccessMode(string(form.Permission), perm.AccessModeRead, perm.AccessModeWrite) - attachTeamUnits(team, unitPerm, form.Units) - } - } else { - attachAdminTeamUnits(team) + if !team.IsOwnerTeam() && form.IncludesAllRepositories != nil { + isIncludeAllChanged = true + team.IncludesAllRepositories = *form.IncludesAllRepositories } if err := org_service.UpdateTeam(ctx, team, isAuthChanged, isIncludeAllChanged); err != nil { diff --git a/routers/web/org/projects.go b/routers/web/org/projects.go index fb343b4cdb..b3abb6cafe 100644 --- a/routers/web/org/projects.go +++ b/routers/web/org/projects.go @@ -117,7 +117,7 @@ func Projects(ctx *context.Context) { func canWriteProjects(ctx *context.Context) bool { if ctx.ContextUser.IsOrganization() { - return ctx.Org.CanWriteUnit(ctx, unit.TypeProjects) + return ctx.Org.CanWriteAnyRepoUnit(ctx, unit.TypeProjects) } return ctx.Doer != nil && ctx.ContextUser.ID == ctx.Doer.ID } diff --git a/routers/web/org/teams.go b/routers/web/org/teams.go index e3dcc1ca79..233a8b1907 100644 --- a/routers/web/org/teams.go +++ b/routers/web/org/teams.go @@ -6,7 +6,7 @@ package org import ( "errors" - "fmt" + "maps" "net/http" "net/url" "path" @@ -327,51 +327,25 @@ func NewTeam(ctx *context.Context) { ctx.HTML(http.StatusOK, tplTeamNew) } -// FIXME: TEAM-UNIT-PERMISSION: this design is not right, when a new unit is added in the future, -// The existing teams won't inherit the correct admin permission for the new unit. -// The full history is like this: -// 1. There was only "team", no "team unit", so "team.authorize" was used to determine the team permission. -// 2. Later, "team unit" was introduced, then the usage of "team.authorize" became inconsistent, and causes various bugs. -// - Sometimes, "team.authorize" is used to determine the team permission, e.g. admin, owner -// - Sometimes, "team unit" is used not really used and "team unit" is used. -// - Some functions like `GetTeamsWithAccessToAnyRepoUnit` use both. -// -// 3. After introducing "team unit" and more unclear changes, it becomes difficult to maintain team permissions. -// - Org owner need to click the permission for each unit, but can't just set a common "write" permission for all units. -// -// Ideally, "team.authorize=write" should mean the team has write access to all units including newly (future) added ones. -func getUnitPerms(forms url.Values, teamPermission perm.AccessMode) map[unit_model.Type]perm.AccessMode { - unitPerms := make(map[unit_model.Type]perm.AccessMode) +func paresFormTeamUnits(orgID int64, forms url.Values) (units []*org_model.TeamUnit) { for _, ut := range unit_model.AllRepoUnitTypes { - // Default access mode is none - unitPerms[ut] = perm.AccessModeNone - - v, ok := forms[fmt.Sprintf("unit_%d", ut)] - if ok { - vv, _ := strconv.Atoi(v[0]) - if teamPermission >= perm.AccessModeAdmin { - unitPerms[ut] = teamPermission - // Don't allow `TypeExternal{Tracker,Wiki}` to influence this as they can only be set to READ perms. - if ut == unit_model.TypeExternalTracker || ut == unit_model.TypeExternalWiki { - unitPerms[ut] = perm.AccessModeRead - } - } else { - unitPerms[ut] = perm.AccessMode(vv) - if unitPerms[ut] >= perm.AccessModeAdmin { - unitPerms[ut] = perm.AccessModeWrite - } - } + v, ok := forms["unit_"+strconv.Itoa(ut.Value())] + if !ok { + continue } + vv, _ := strconv.Atoi(v[0]) + mode := perm.AccessMode(vv) + mode = min(mode, perm.AccessModeWrite, unit_model.Units[ut].MaxPerm()) + units = append(units, &org_model.TeamUnit{OrgID: orgID, Type: ut, AccessMode: mode}) } - return unitPerms + return units } // NewTeamPost response for create new team func NewTeamPost(ctx *context.Context) { form := web.GetForm[*forms.CreateTeamForm](ctx) includesAllRepositories := form.RepoAccess == "all" - teamPermission := perm.ParseAccessMode(form.Permission, perm.AccessModeNone, perm.AccessModeAdmin) - unitPerms := getUnitPerms(ctx.Req.Form, teamPermission) + teamPermission := perm.ParseAccessMode(form.Permission, perm.AccessModeNone, perm.AccessModeWrite, perm.AccessModeAdmin) t := &org_model.Team{ OrgID: ctx.Org.Organization.ID, @@ -383,16 +357,6 @@ func NewTeamPost(ctx *context.Context) { Visibility: org_model.NormalizeTeamVisibility(form.Visibility), } - units := make([]*org_model.TeamUnit, 0, len(unitPerms)) - for tp, perm := range unitPerms { - units = append(units, &org_model.TeamUnit{ - OrgID: ctx.Org.Organization.ID, - Type: tp, - AccessMode: perm, - }) - } - t.Units = units - ctx.Data["Title"] = ctx.Org.Organization.FullName ctx.Data["PageIsOrgTeams"] = true ctx.Data["PageIsOrgTeamsNew"] = true @@ -404,9 +368,12 @@ func NewTeamPost(ctx *context.Context) { return } - if t.AccessMode < perm.AccessModeAdmin && len(unitPerms) == 0 { - ctx.RenderWithErrDeprecated(ctx.Tr("form.team_no_units_error"), tplTeamNew, &form) - return + if t.AccessMode == perm.AccessModeNone { + t.Units = paresFormTeamUnits(ctx.Org.Organization.ID, ctx.Req.Form) + if len(t.Units) == 0 { + ctx.RenderWithErrDeprecated(ctx.Tr("form.team_no_units_error"), tplTeamNew, &form) + return + } } if err := org_service.NewTeam(ctx, t); err != nil { @@ -545,10 +512,9 @@ func EditTeam(ctx *context.Context) { // EditTeamPost response for modify team information func EditTeamPost(ctx *context.Context) { form := web.GetForm[*forms.CreateTeamForm](ctx) + t := ctx.Org.Team - teamPermission := perm.ParseAccessMode(form.Permission, perm.AccessModeNone, perm.AccessModeAdmin) - unitPerms := getUnitPerms(ctx.Req.Form, teamPermission) - isAuthChanged := false + teamPermission := perm.ParseAccessMode(form.Permission, perm.AccessModeNone, perm.AccessModeWrite, perm.AccessModeAdmin) isIncludeAllChanged := false includesAllRepositories := form.RepoAccess == "all" @@ -557,13 +523,10 @@ func EditTeamPost(ctx *context.Context) { ctx.Data["Team"] = t ctx.Data["Units"] = unit_model.Units + oldTeamAccessMode := t.AccessMode if !t.IsOwnerTeam() { t.Name = form.TeamName - if t.AccessMode != teamPermission { - isAuthChanged = true - t.AccessMode = teamPermission - } - + t.AccessMode = teamPermission if t.IncludesAllRepositories != includesAllRepositories { isIncludeAllChanged = true t.IncludesAllRepositories = includesAllRepositories @@ -576,28 +539,22 @@ func EditTeamPost(ctx *context.Context) { t.Visibility = structs.VisibleTypeLimited } - t.Description = form.Description - units := make([]*org_model.TeamUnit, 0, len(unitPerms)) - for tp, perm := range unitPerms { - units = append(units, &org_model.TeamUnit{ - OrgID: t.OrgID, - TeamID: t.ID, - Type: tp, - AccessMode: perm, - }) + oldTeamUnitsMap := t.GetUnitsMap() + if t.AccessMode == perm.AccessModeNone { + t.Units = paresFormTeamUnits(ctx.Org.Organization.ID, ctx.Req.Form) + if len(t.Units) == 0 { + ctx.RenderWithErrDeprecated(ctx.Tr("form.team_no_units_error"), tplTeamNew, &form) + return + } } - t.Units = units if ctx.HasError() { ctx.HTML(http.StatusOK, tplTeamNew) return } - if t.AccessMode < perm.AccessModeAdmin && len(unitPerms) == 0 { - ctx.RenderWithErrDeprecated(ctx.Tr("form.team_no_units_error"), tplTeamNew, &form) - return - } - + isAuthChanged := oldTeamAccessMode != t.AccessMode || !maps.Equal(oldTeamUnitsMap, t.GetUnitsMap()) + t.Description = form.Description if err := org_service.UpdateTeam(ctx, t, isAuthChanged, isIncludeAllChanged); err != nil { ctx.Data["Err_TeamName"] = true switch { diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 03bca8c1f2..ecd4bba4d5 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -403,7 +403,7 @@ func Diff(ctx *context.Context) { ctx.Data["DiffNotAvailable"] = diffShortStat.NumFiles == 0 if err := asymkey_model.CalculateTrustStatus(verification, ctx.Repo.Repository.GetTrustModel(), func(user *user_model.User) (bool, error) { - return repo_model.IsOwnerMemberCollaborator(ctx, ctx.Repo.Repository, user.ID) + return repo_model.HasAccessToRepoCodeUnit(ctx, ctx.Repo.Repository, user.ID) }, nil); err != nil { ctx.ServerError("CalculateTrustStatus", err) return diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 0391732dfc..0867263a0b 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -128,7 +128,7 @@ func loadLatestCommitData(ctx *context.Context, latestCommit *git.Commit) bool { verification := asymkey_service.ParseCommitWithSignature(ctx, latestCommit) if err := asymkey_model.CalculateTrustStatus(verification, ctx.Repo.Repository.GetTrustModel(), func(user *user_model.User) (bool, error) { - return repo_model.IsOwnerMemberCollaborator(ctx, ctx.Repo.Repository, user.ID) + return repo_model.HasAccessToRepoCodeUnit(ctx, ctx.Repo.Repository, user.ID) }, nil); err != nil { ctx.ServerError("CalculateTrustStatus", err) return false diff --git a/routers/web/web.go b/routers/web/web.go index 9dff831baa..e0c40b933b 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -453,7 +453,7 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) { } if ctx.ContextUser.IsOrganization() { - if ctx.Org.Organization.UnitPermission(ctx, ctx.Doer, unitType) < accessMode { + if ctx.Org.Organization.AnyRepoUnitPermission(ctx, ctx.Doer, unitType) < accessMode { ctx.NotFound(nil) return } diff --git a/services/context/org.go b/services/context/org.go index 0f6a06e326..310e158c8a 100644 --- a/services/context/org.go +++ b/services/context/org.go @@ -31,12 +31,12 @@ type Organization struct { Teams []*organization.Team } -func (org *Organization) CanWriteUnit(ctx *Context, unitType unit.Type) bool { - return org.Organization.UnitPermission(ctx, ctx.Doer, unitType) >= perm.AccessModeWrite +func (org *Organization) CanWriteAnyRepoUnit(ctx *Context, unitType unit.Type) bool { + return org.Organization.AnyRepoUnitPermission(ctx, ctx.Doer, unitType) >= perm.AccessModeWrite } -func (org *Organization) CanReadUnit(ctx *Context, unitType unit.Type) bool { - return org.Organization.UnitPermission(ctx, ctx.Doer, unitType) >= perm.AccessModeRead +func (org *Organization) CanReadAnyRepoUnit(ctx *Context, unitType unit.Type) bool { + return org.Organization.AnyRepoUnitPermission(ctx, ctx.Doer, unitType) >= perm.AccessModeRead } func GetOrganizationByParams(ctx *Context) { @@ -247,9 +247,9 @@ func OrgAssignment(orgAssignmentOpts OrgAssignmentOptions) func(ctx *Context) { } ctx.Data["ContextUser"] = ctx.ContextUser - ctx.Data["CanReadProjects"] = ctx.Org.CanReadUnit(ctx, unit.TypeProjects) - ctx.Data["CanReadPackages"] = ctx.Org.CanReadUnit(ctx, unit.TypePackages) - ctx.Data["CanReadCode"] = ctx.Org.CanReadUnit(ctx, unit.TypeCode) + ctx.Data["CanReadProjects"] = ctx.Org.CanReadAnyRepoUnit(ctx, unit.TypeProjects) + ctx.Data["CanReadPackages"] = ctx.Org.CanReadAnyRepoUnit(ctx, unit.TypePackages) + ctx.Data["CanReadCode"] = ctx.Org.CanReadAnyRepoUnit(ctx, unit.TypeCode) ctx.Data["IsFollowing"] = ctx.Doer != nil && user_model.IsFollowing(ctx, ctx.Doer.ID, ctx.ContextUser.ID) if len(ctx.ContextUser.Description) != 0 { diff --git a/services/convert/convert.go b/services/convert/convert.go index ba16825db0..3ec8f5ab7a 100644 --- a/services/convert/convert.go +++ b/services/convert/convert.go @@ -9,8 +9,10 @@ import ( "context" "errors" "fmt" + "maps" "net/url" "path" + "slices" "strconv" "time" @@ -894,6 +896,7 @@ func ToTeams(ctx context.Context, teams []*organization.Team, loadOrgs bool) ([] return nil, err } + unitsMap := t.GetUnitsMap() apiTeam := &api.Team{ ID: t.ID, Name: t.Name, @@ -901,7 +904,7 @@ func ToTeams(ctx context.Context, teams []*organization.Team, loadOrgs bool) ([] IncludesAllRepositories: t.IncludesAllRepositories, CanCreateOrgRepo: t.CanCreateOrgRepo, Permission: api.AccessLevelName(t.AccessMode.ToString()), - Units: t.GetUnitNames(), + Units: slices.Collect(maps.Keys(unitsMap)), UnitsMap: t.GetUnitsMap(), Visibility: api.TeamVisibility(t.Visibility.String()), } diff --git a/services/git/commit.go b/services/git/commit.go index 662e050713..414624a2ea 100644 --- a/services/git/commit.go +++ b/services/git/commit.go @@ -41,7 +41,7 @@ func ParseCommitsWithSignature(ctx context.Context, repo *repo_model.Repository, } isOwnerMemberCollaborator := func(user *user_model.User) (bool, error) { - return repo_model.IsOwnerMemberCollaborator(ctx, repo, user.ID) + return repo_model.HasAccessToRepoCodeUnit(ctx, repo, user.ID) } _ = asymkey_model.CalculateTrustStatus(signCommit.Verification, repoTrustModel, isOwnerMemberCollaborator, &keyMap) diff --git a/services/org/team.go b/services/org/team.go index 63242418bc..b3db179f62 100644 --- a/services/org/team.go +++ b/services/org/team.go @@ -114,8 +114,8 @@ func UpdateTeam(ctx context.Context, t *organization.Team, authChanged, includeA return fmt.Errorf("update: %w", err) } - // update units for team - if len(t.Units) > 0 { + if authChanged { + // update units for team for _, unit := range t.Units { unit.TeamID = t.ID } @@ -125,13 +125,13 @@ func UpdateTeam(ctx context.Context, t *organization.Team, authChanged, includeA Delete(new(organization.TeamUnit)); err != nil { return err } - if _, err = sess.Cols("org_id", "team_id", "type", "access_mode").Insert(&t.Units); err != nil { - return err + if len(t.Units) > 0 { + if _, err = sess.Cols("org_id", "team_id", "type", "access_mode").Insert(&t.Units); err != nil { + return err + } } - } - // Update access for team members if needed. - if authChanged { + // Update access for team members if needed. repos, err := repo_model.GetTeamRepositories(ctx, &repo_model.SearchTeamRepoOptions{ TeamID: t.ID, }) diff --git a/services/pull/pull.go b/services/pull/pull.go index f3b5eae3e5..90eb2271dc 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -69,7 +69,7 @@ func NewPullRequest(ctx context.Context, opts *NewPullRequestOptions) error { canCreate := issue.Poster.IsAdmin || pr.Flow == issues_model.PullRequestFlowAGit canAssignProject := canCreate if !canCreate { - canCreate, err := repo_model.IsOwnerMemberCollaborator(ctx, repo, issue.Poster.ID) + canCreate, err := repo_model.HasAccessToRepoCodeUnit(ctx, repo, issue.Poster.ID) if err != nil { return err } diff --git a/services/repository/gitgraph/graph_models.go b/services/repository/gitgraph/graph_models.go index 7e246acd74..dd98f010a1 100644 --- a/services/repository/gitgraph/graph_models.go +++ b/services/repository/gitgraph/graph_models.go @@ -130,7 +130,7 @@ func (graph *Graph) LoadAndProcessCommits(ctx context.Context, repository *repo_ c.Verification = asymkey_service.ParseCommitWithSignature(ctx, c.Commit) _ = asymkey_model.CalculateTrustStatus(c.Verification, repository.GetTrustModel(), func(user *user_model.User) (bool, error) { - return repo_model.IsOwnerMemberCollaborator(ctx, repository, user.ID) + return repo_model.HasAccessToRepoCodeUnit(ctx, repository, user.ID) }, &keyMap) statuses, err := git_model.GetLatestCommitStatus(ctx, repository.ID, c.Commit.ID.String(), db.ListOptionsAll) diff --git a/templates/org/team/new.tmpl b/templates/org/team/new.tmpl index 1aa306a3e9..31f35ef994 100644 --- a/templates/org/team/new.tmpl +++ b/templates/org/team/new.tmpl @@ -95,11 +95,18 @@
- + {{ctx.Locale.Tr "org.teams.general_access_helper"}}
+
+
+ + + {{ctx.Locale.Tr "org.teams.write_access_all_helper"}} +
+
@@ -110,7 +117,7 @@
-
+
diff --git a/templates/org/team/sidebar.tmpl b/templates/org/team/sidebar.tmpl index d2c092c1da..156a92ef14 100644 --- a/templates/org/team/sidebar.tmpl +++ b/templates/org/team/sidebar.tmpl @@ -50,12 +50,10 @@
  • {{ctx.Locale.Tr "org.teams.can_create_org_repo"}}
    • {{end}} - {{/* the AccessMode should be either none or admin/owner, the real permissions are provided by each team unit */}} - {{if false}}{{/*(eq .Team.AccessMode 2)*/}} + {{if (eq .Team.AccessMode 2)}}

    {{ctx.Locale.Tr "org.settings.permission"}}

    {{ctx.Locale.Tr "org.teams.write_permission_desc"}} - {{else if (eq .Team.AccessMode 3)}} - {{/* FIXME: here might not right, see "FIXME: TEAM-UNIT-PERMISSION", new units might not have correct admin permission*/}} + {{else if (ge .Team.AccessMode 3)}}

    {{ctx.Locale.Tr "org.settings.permission"}}

    {{ctx.Locale.Tr "org.teams.admin_permission_desc"}} {{else}} diff --git a/templates/repo/settings/collaboration.tmpl b/templates/repo/settings/collaboration.tmpl index 48e3f3287e..0324de644e 100644 --- a/templates/repo/settings/collaboration.tmpl +++ b/templates/repo/settings/collaboration.tmpl @@ -62,23 +62,20 @@ {{.Name}}
    - {{/*FIXME: TEAM-UNIT-PERMISSION this display is not right, search the fixme keyword to see more details */}} {{svg "octicon-shield-lock"}} - {{if eq .AccessMode 0}} - {{ctx.Locale.Tr "repo.settings.collaboration.per_unit"}} - {{else if eq .AccessMode 1}} - {{ctx.Locale.Tr "repo.settings.collaboration.read"}} + {{if ge .AccessMode 4}} + {{ctx.Locale.Tr "repo.settings.collaboration.owner"}} + {{else if ge .AccessMode 3}} + {{ctx.Locale.Tr "repo.settings.collaboration.admin"}} {{else if eq .AccessMode 2}} {{ctx.Locale.Tr "repo.settings.collaboration.write"}} - {{else if eq .AccessMode 3}} - {{ctx.Locale.Tr "repo.settings.collaboration.admin"}} - {{else if eq .AccessMode 4}} - {{ctx.Locale.Tr "repo.settings.collaboration.owner"}} + {{else if eq .AccessMode 1}} + {{ctx.Locale.Tr "repo.settings.collaboration.read"}} {{else}} - {{ctx.Locale.Tr "repo.settings.collaboration.undefined"}} + {{ctx.Locale.Tr "repo.settings.collaboration.per_unit"}} {{end}}
    - {{if or (eq .AccessMode 0) (eq .AccessMode 1) (eq .AccessMode 2)}} + {{if lt .AccessMode 2}} {{$first := true}}
    Units: diff --git a/templates/swagger/v1-openapi3.generated.json b/templates/swagger/v1-openapi3.generated.json index fe6197465a..d2fad39293 100644 --- a/templates/swagger/v1-openapi3.generated.json +++ b/templates/swagger/v1-openapi3.generated.json @@ -4879,7 +4879,12 @@ "x-go-name": "Name" }, "permission": { - "$ref": "#/components/schemas/RepoWritePermission" + "allOf": [ + { + "$ref": "#/components/schemas/RepoWritePermission" + } + ], + "description": "All units have this permission (read/write/admin)" }, "units": { "deprecated": true, @@ -6129,7 +6134,12 @@ "x-go-name": "Name" }, "permission": { - "$ref": "#/components/schemas/RepoWritePermission" + "allOf": [ + { + "$ref": "#/components/schemas/RepoWritePermission" + } + ], + "description": "All units have this permission (read/write/admin)" }, "units": { "deprecated": true, diff --git a/templates/swagger/v1-swagger.generated.json b/templates/swagger/v1-swagger.generated.json index e3b017977e..9d662680fd 100644 --- a/templates/swagger/v1-swagger.generated.json +++ b/templates/swagger/v1-swagger.generated.json @@ -27790,6 +27790,7 @@ "x-go-name": "Name" }, "permission": { + "description": "All units have this permission (read/write/admin)", "type": "string", "enum": [ "read", @@ -29060,6 +29061,7 @@ "x-go-name": "Name" }, "permission": { + "description": "All units have this permission (read/write/admin)", "type": "string", "enum": [ "read", diff --git a/tests/integration/api_issue_config_test.go b/tests/integration/api_issue_config_test.go index 4227f33237..88dcd53920 100644 --- a/tests/integration/api_issue_config_test.go +++ b/tests/integration/api_issue_config_test.go @@ -183,7 +183,7 @@ func TestAPIRepoIssueConfigRequiresCodeUnit(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 24}) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - token := getUserToken(t, user.Name, auth_model.AccessTokenScopeReadRepository) + token := getUserToken(t, user.Name, auth_model.AccessTokenScopeReadUser) for _, path := range []string{ fmt.Sprintf("/api/v1/repos/%s/issue_config", repo.FullName()), diff --git a/tests/integration/api_issue_templates_test.go b/tests/integration/api_issue_templates_test.go index 5ed4023b72..c5326cd2bf 100644 --- a/tests/integration/api_issue_templates_test.go +++ b/tests/integration/api_issue_templates_test.go @@ -57,7 +57,7 @@ func TestAPIIssueTemplateRequiresCodeUnit(t *testing.T) { repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 24}) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - token := getUserToken(t, user.Name, auth_model.AccessTokenScopeReadRepository) + token := getUserToken(t, user.Name, auth_model.AccessTokenScopeReadUser) issueTemplatesURL := "/api/v1/repos/" + repo.FullName() + "/issue_templates" languagesURL := "/api/v1/repos/" + repo.FullName() + "/languages" diff --git a/tests/integration/api_packages_test.go b/tests/integration/api_packages_test.go index efb32e271f..5585ff0cb5 100644 --- a/tests/integration/api_packages_test.go +++ b/tests/integration/api_packages_test.go @@ -293,7 +293,8 @@ func TestPackageAccess(t *testing.T) { limitedOrgNoMember := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 22}) publicOrgNoMember := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 17}) - uploadPackage := func(doer, owner *user_model.User, filename string, expectedStatus int) { + uploadPackage := func(t *testing.T, doer, owner *user_model.User, filename string, expectedStatus int) { + t.Helper() url := fmt.Sprintf("/api/packages/%s/generic/test-package/1.0/%s.bin", owner.Name, filename) req := NewRequestWithBody(t, "PUT", url, bytes.NewReader([]byte{1})) if doer != nil { @@ -395,8 +396,10 @@ func TestPackageAccess(t *testing.T) { } for _, c := range cases { - for _, t := range c.Targets { - uploadPackage(c.Doer, t.Owner, c.Filename, t.ExpectedStatus) + for _, target := range c.Targets { + t.Run(fmt.Sprintf("%s-%s", c.Filename, target.Owner.Name), func(t *testing.T) { + uploadPackage(t, c.Doer, target.Owner, c.Filename, target.ExpectedStatus) + }) } } }) diff --git a/tests/integration/api_project_test.go b/tests/integration/api_project_test.go index c3851745a4..79df45a6a7 100644 --- a/tests/integration/api_project_test.go +++ b/tests/integration/api_project_test.go @@ -38,9 +38,9 @@ func TestAPIProjects(t *testing.T) { // user2 owns repo1 and is on org3's Owners team, so one token covers all three scopes token := getUserToken(t, "user2", auth_model.AccessTokenScopeWriteIssue, auth_model.AccessTokenScopeWriteOrganization, auth_model.AccessTokenScopeWriteUser, auth_model.AccessTokenScopeWriteRepository) - // user5 is signed in but is neither an org member nor a repo1 collaborator, and is scoped + // user8 is signed in but is neither an org member nor a repo1 collaborator, and is scoped // generously so that permissions rather than token scopes are what denies below - outsider := getUserToken(t, "user5", auth_model.AccessTokenScopeWriteIssue, + outsider := getUserToken(t, "user8", auth_model.AccessTokenScopeWriteIssue, auth_model.AccessTokenScopeWriteOrganization, auth_model.AccessTokenScopeReadUser) for _, scope := range []projectScope{ @@ -262,8 +262,7 @@ func testAPIProjectPermissions(t *testing.T, ownerToken, outsiderToken string) { // fixture project 1 belongs to repo1, so this needs no project of its own const projectURL = "/api/v1/repos/user2/repo1/projects/1" - title := "hijacked" - req := NewRequestWithJSON(t, "PATCH", projectURL, &api.EditProjectOption{Title: &title}).AddTokenAuth(outsiderToken) + req := NewRequestWithJSON(t, "PATCH", projectURL, &api.EditProjectOption{Title: new("hijacked")}).AddTokenAuth(outsiderToken) MakeRequest(t, req, http.StatusForbidden) MakeRequest(t, NewRequest(t, "DELETE", projectURL).AddTokenAuth(outsiderToken), http.StatusForbidden) diff --git a/tests/integration/api_repo_teams_test.go b/tests/integration/api_repo_teams_test.go index 506270e3ac..e88f5bb9cb 100644 --- a/tests/integration/api_repo_teams_test.go +++ b/tests/integration/api_repo_teams_test.go @@ -10,11 +10,9 @@ import ( auth_model "gitea.dev/models/auth" repo_model "gitea.dev/models/repo" - "gitea.dev/models/unit" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" api "gitea.dev/modules/structs" - "gitea.dev/modules/util" "gitea.dev/tests" "github.com/stretchr/testify/assert" @@ -38,13 +36,13 @@ func TestAPIRepoTeams(t *testing.T) { if assert.Len(t, teams, 2) { assert.Equal(t, "Owners", teams[0].Name) assert.True(t, teams[0].CanCreateOrgRepo) - assert.True(t, util.SliceSortedEqual(unit.AllUnitKeyNames(), teams[0].Units), "%v == %v", unit.AllUnitKeyNames(), teams[0].Units) + assert.Equal(t, []string{"repo.issues"}, teams[1].Units) // legacy dirty data, although the team.authorize is "owner", the units are also responded assert.Equal(t, api.AccessLevelNameOwner, teams[0].Permission) assert.Equal(t, "test_team", teams[1].Name) assert.False(t, teams[1].CanCreateOrgRepo) assert.Equal(t, []string{"repo.issues"}, teams[1].Units) - assert.Equal(t, api.AccessLevelNameWrite, teams[1].Permission) + assert.Equal(t, api.AccessLevelNameWrite, teams[1].Permission) // legacy dirty data, although the team.authorize is "write", the units are also responded } // IsTeam diff --git a/tests/integration/api_team_test.go b/tests/integration/api_team_test.go index 3ad28be180..43d47fa18d 100644 --- a/tests/integration/api_team_test.go +++ b/tests/integration/api_team_test.go @@ -5,8 +5,9 @@ package integration import ( "fmt" + "maps" "net/http" - "sort" + "slices" "testing" auth_model "gitea.dev/models/auth" @@ -14,7 +15,6 @@ import ( "gitea.dev/models/organization" "gitea.dev/models/perm" "gitea.dev/models/repo" - "gitea.dev/models/unit" "gitea.dev/models/unittest" user_model "gitea.dev/models/user" "gitea.dev/modules/structs" @@ -70,17 +70,16 @@ func TestAPITeam(t *testing.T) { Name: "team1", Description: "team one", IncludesAllRepositories: true, - Permission: "write", + Permission: "read", Units: []string{"repo.code", "repo.issues"}, } req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/orgs/%s/teams", org.Name), teamToCreate). AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusCreated) apiTeam = DecodeJSON(t, resp, &api.Team{}) - checkTeamResponse(t, "CreateTeam1", apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, - api.AccessLevelNameNone, teamToCreate.Units, nil) - checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, - api.AccessLevelNameNone, teamToCreate.Units, nil) + expectedTeamUnitsMap := map[string]string{"repo.code": "read", "repo.issues": "read"} + checkTeamResponse(t, "CreateTeam1", apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, api.AccessLevelNameNone, expectedTeamUnitsMap) + checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, api.AccessLevelNameNone, expectedTeamUnitsMap) teamID := apiTeam.ID // Edit team. @@ -91,17 +90,14 @@ func TestAPITeam(t *testing.T) { Description: &editDescription, Permission: "admin", IncludesAllRepositories: &editFalse, - Units: []string{"repo.code", "repo.pulls", "repo.releases"}, } req = NewRequestWithJSON(t, "PATCH", fmt.Sprintf("/api/v1/teams/%d", teamID), teamToEdit). AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) apiTeam = DecodeJSON(t, resp, &api.Team{}) - checkTeamResponse(t, "EditTeam1", apiTeam, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories, - api.AccessLevelName(teamToEdit.Permission), unit.AllUnitKeyNames(), nil) - checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories, - api.AccessLevelName(teamToEdit.Permission), unit.AllUnitKeyNames(), nil) + checkTeamResponse(t, "EditTeam1", apiTeam, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories, api.AccessLevelName(teamToEdit.Permission), nil) + checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories, api.AccessLevelName(teamToEdit.Permission), nil) // Edit team Description only editDescription = "first team" @@ -110,10 +106,8 @@ func TestAPITeam(t *testing.T) { AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) apiTeam = DecodeJSON(t, resp, &api.Team{}) - checkTeamResponse(t, "EditTeam1_DescOnly", apiTeam, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories, - api.AccessLevelName(teamToEdit.Permission), unit.AllUnitKeyNames(), nil) - checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories, - api.AccessLevelName(teamToEdit.Permission), unit.AllUnitKeyNames(), nil) + checkTeamResponse(t, "EditTeam1_DescOnly", apiTeam, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories, api.AccessLevelName(teamToEdit.Permission), nil) + checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories, api.AccessLevelName(teamToEdit.Permission), nil) // Read team. teamRead := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: teamID}) @@ -122,8 +116,7 @@ func TestAPITeam(t *testing.T) { AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) apiTeam = DecodeJSON(t, resp, &api.Team{}) - checkTeamResponse(t, "ReadTeam1", apiTeam, teamRead.Name, *teamToEditDesc.Description, teamRead.IncludesAllRepositories, - api.AccessLevelName(teamRead.AccessMode.ToString()), teamRead.GetUnitNames(), teamRead.GetUnitsMap()) + checkTeamResponse(t, "ReadTeam1", apiTeam, teamRead.Name, *teamToEditDesc.Description, teamRead.IncludesAllRepositories, api.AccessLevelName(teamRead.AccessMode.ToString()), teamRead.GetUnitsMap()) // Delete team. req = NewRequestf(t, "DELETE", "/api/v1/teams/%d", teamID). @@ -131,23 +124,19 @@ func TestAPITeam(t *testing.T) { MakeRequest(t, req, http.StatusNoContent) unittest.AssertNotExistsBean(t, &organization.Team{ID: teamID}) - // create team again via UnitsMap - // Create team. + // create team again via UnitsMap (granular: do not send permission=write). teamToCreate = &api.CreateTeamOption{ Name: "team2", Description: "team two", IncludesAllRepositories: true, - Permission: "write", UnitsMap: map[string]string{"repo.code": "read", "repo.issues": "write", "repo.wiki": "none"}, } req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/orgs/%s/teams", org.Name), teamToCreate). AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusCreated) apiTeam = DecodeJSON(t, resp, &api.Team{}) - checkTeamResponse(t, "CreateTeam2", apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, - api.AccessLevelNameNone, nil, teamToCreate.UnitsMap) - checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, - api.AccessLevelNameNone, nil, teamToCreate.UnitsMap) + checkTeamResponse(t, "CreateTeam2", apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, api.AccessLevelNameNone, teamToCreate.UnitsMap) + checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, api.AccessLevelNameNone, teamToCreate.UnitsMap) teamID = apiTeam.ID // Edit team. @@ -156,7 +145,6 @@ func TestAPITeam(t *testing.T) { teamToEdit = &api.EditTeamOption{ Name: "teamtwo", Description: &editDescription, - Permission: "write", IncludesAllRepositories: &editFalse, UnitsMap: map[string]string{"repo.code": "read", "repo.pulls": "read", "repo.releases": "write"}, } @@ -165,10 +153,8 @@ func TestAPITeam(t *testing.T) { AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) apiTeam = DecodeJSON(t, resp, &api.Team{}) - checkTeamResponse(t, "EditTeam2", apiTeam, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories, - api.AccessLevelNameNone, nil, teamToEdit.UnitsMap) - checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories, - api.AccessLevelNameNone, nil, teamToEdit.UnitsMap) + checkTeamResponse(t, "EditTeam2", apiTeam, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories, api.AccessLevelNameNone, teamToEdit.UnitsMap) + checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEdit.Description, *teamToEdit.IncludesAllRepositories, api.AccessLevelNameNone, teamToEdit.UnitsMap) // Edit team Description only editDescription = "second team" @@ -177,10 +163,8 @@ func TestAPITeam(t *testing.T) { AddTokenAuth(token) resp = MakeRequest(t, req, http.StatusOK) apiTeam = DecodeJSON(t, resp, &api.Team{}) - checkTeamResponse(t, "EditTeam2_DescOnly", apiTeam, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories, - api.AccessLevelNameNone, nil, teamToEdit.UnitsMap) - checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories, - api.AccessLevelNameNone, nil, teamToEdit.UnitsMap) + checkTeamResponse(t, "EditTeam2_DescOnly", apiTeam, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories, api.AccessLevelNameNone, teamToEdit.UnitsMap) + checkTeamBean(t, apiTeam.ID, teamToEdit.Name, *teamToEditDesc.Description, *teamToEdit.IncludesAllRepositories, api.AccessLevelNameNone, teamToEdit.UnitsMap) // Read team. teamRead = unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: teamID}) @@ -189,39 +173,7 @@ func TestAPITeam(t *testing.T) { resp = MakeRequest(t, req, http.StatusOK) apiTeam = DecodeJSON(t, resp, &api.Team{}) assert.NoError(t, teamRead.LoadUnits(t.Context())) - checkTeamResponse(t, "ReadTeam2", apiTeam, teamRead.Name, *teamToEditDesc.Description, teamRead.IncludesAllRepositories, - api.AccessLevelName(teamRead.AccessMode.ToString()), teamRead.GetUnitNames(), teamRead.GetUnitsMap()) - - // Delete team. - req = NewRequestf(t, "DELETE", "/api/v1/teams/%d", teamID). - AddTokenAuth(token) - MakeRequest(t, req, http.StatusNoContent) - unittest.AssertNotExistsBean(t, &organization.Team{ID: teamID}) - - // Create admin team - teamToCreate = &api.CreateTeamOption{ - Name: "teamadmin", - Description: "team admin", - IncludesAllRepositories: true, - Permission: "admin", - } - req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/orgs/%s/teams", org.Name), teamToCreate). - AddTokenAuth(token) - resp = MakeRequest(t, req, http.StatusCreated) - apiTeam = DecodeJSON(t, resp, &api.Team{}) - for _, ut := range unit.AllRepoUnitTypes { - up := perm.AccessModeAdmin - if ut == unit.TypeExternalTracker || ut == unit.TypeExternalWiki { - up = perm.AccessModeRead - } - unittest.AssertExistsAndLoadBean(t, &organization.TeamUnit{ - OrgID: org.ID, - TeamID: apiTeam.ID, - Type: ut, - AccessMode: up, - }) - } - teamID = apiTeam.ID + checkTeamResponse(t, "ReadTeam2", apiTeam, teamRead.Name, *teamToEditDesc.Description, teamRead.IncludesAllRepositories, api.AccessLevelName(teamRead.AccessMode.ToString()), teamRead.GetUnitsMap()) // Delete team. req = NewRequestf(t, "DELETE", "/api/v1/teams/%d", teamID). @@ -230,29 +182,23 @@ func TestAPITeam(t *testing.T) { unittest.AssertNotExistsBean(t, &organization.Team{ID: teamID}) } -func checkTeamResponse(t *testing.T, testName string, apiTeam *api.Team, name, description string, includesAllRepositories bool, permission api.AccessLevelName, units []string, unitsMap map[string]string) { +func checkTeamResponse(t *testing.T, testName string, apiTeam *api.Team, name, description string, includesAllRepositories bool, permission api.AccessLevelName, unitsMap map[string]string) { t.Run(testName, func(t *testing.T) { assert.Equal(t, name, apiTeam.Name, "name") assert.Equal(t, description, apiTeam.Description, "description") assert.Equal(t, includesAllRepositories, apiTeam.IncludesAllRepositories, "includesAllRepositories") assert.Equal(t, permission, apiTeam.Permission, "permission") - if units != nil { - sort.StringSlice(units).Sort() - sort.StringSlice(apiTeam.Units).Sort() - assert.Equal(t, units, apiTeam.Units, "units") - } - if unitsMap != nil { - assert.Equal(t, unitsMap, apiTeam.UnitsMap, "unitsMap") - } + assert.ElementsMatch(t, slices.Collect(maps.Keys(unitsMap)), apiTeam.Units, "unitsMap") + assert.Equal(t, unitsMap, apiTeam.UnitsMap, "unitsMap") }) } -func checkTeamBean(t *testing.T, id int64, name, description string, includesAllRepositories bool, permission api.AccessLevelName, units []string, unitsMap map[string]string) { +func checkTeamBean(t *testing.T, id int64, name, description string, includesAllRepositories bool, permission api.AccessLevelName, unitsMap map[string]string) { team := unittest.AssertExistsAndLoadBean(t, &organization.Team{ID: id}) assert.NoError(t, team.LoadUnits(t.Context()), "LoadUnits") apiTeam, err := convert.ToTeam(t.Context(), team) assert.NoError(t, err) - checkTeamResponse(t, fmt.Sprintf("checkTeamBean/%s_%s", name, description), apiTeam, name, description, includesAllRepositories, permission, units, unitsMap) + checkTeamResponse(t, fmt.Sprintf("checkTeamBean/%s_%s", name, description), apiTeam, name, description, includesAllRepositories, permission, unitsMap) } type TeamSearchResults struct { diff --git a/tests/integration/org_project_test.go b/tests/integration/org_project_test.go index c4ec402a3f..c638da4389 100644 --- a/tests/integration/org_project_test.go +++ b/tests/integration/org_project_test.go @@ -6,6 +6,7 @@ package integration import ( "net/http" "slices" + "strconv" "testing" unit_model "gitea.dev/models/unit" @@ -50,7 +51,7 @@ func TestOrgProjectAccess(t *testing.T) { "team_name": "team1", "repo_access": "specific", "permission": "read", - "unit_8": "0", + "unit_" + strconv.Itoa(unit_model.TypeProjects.Value()): "0", }) session.MakeRequest(t, req, http.StatusSeeOther) diff --git a/tests/integration/org_test.go b/tests/integration/org_test.go index 27629d0fa0..bfc6b11613 100644 --- a/tests/integration/org_test.go +++ b/tests/integration/org_test.go @@ -184,10 +184,9 @@ func testOrgRestrictedUser(t *testing.T) { resp := adminSession.MakeRequest(t, req, http.StatusCreated) apiTeam := DecodeJSON(t, resp, &api.Team{}) - checkTeamResponse(t, "CreateTeam_codereader", apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, - "none", teamToCreate.Units, nil) - checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, - "none", teamToCreate.Units, nil) + expectedUnitsMap := map[string]string{"repo.code": "read"} + checkTeamResponse(t, "CreateTeam_codereader", apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, "none", expectedUnitsMap) + checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories, "none", expectedUnitsMap) // teamID := apiTeam.ID // Now we need to add the restricted user to the team diff --git a/web_src/js/features/org-team.ts b/web_src/js/features/org-team.ts index 5ac2fa41ff..b1b9eb7ca1 100644 --- a/web_src/js/features/org-team.ts +++ b/web_src/js/features/org-team.ts @@ -7,7 +7,7 @@ function initOrgTeamSettings() { queryElems(pageContent, 'input[name=permission]', (el) => el.addEventListener('change', () => { // Change team access mode const val = pageContent.querySelector('input[name=permission]:checked')?.value; - toggleElem(pageContent.querySelectorAll('.team-units'), val !== 'admin'); + toggleElem(pageContent.querySelectorAll('.team-units'), val !== 'admin' && val !== 'write'); })); }