Skip to content

Commit 096d5a9

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent e94f8d5 commit 096d5a9

File tree

12 files changed

+52
-130
lines changed

12 files changed

+52
-130
lines changed

Diff for: ‎models/issues/issue_update.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111

1212
"code.gitea.io/gitea/models/db"
1313
"code.gitea.io/gitea/models/organization"
14-
"code.gitea.io/gitea/models/perm"
1514
access_model "code.gitea.io/gitea/models/perm/access"
1615
project_model "code.gitea.io/gitea/models/project"
1716
repo_model "code.gitea.io/gitea/models/repo"
@@ -612,7 +611,7 @@ func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *u
612611
unittype = unit.TypePullRequests
613612
}
614613
for _, team := range teams {
615-
if team.AccessMode >= perm.AccessModeAdmin {
614+
if team.HasAdminAccess() {
616615
checked = append(checked, team.ID)
617616
resolved[issue.Repo.Owner.LowerName+"/"+team.LowerName] = true
618617
continue

Diff for: ‎models/organization/org_user.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func IsOrganizationAdmin(ctx context.Context, orgID, uid int64) (bool, error) {
7878
return false, err
7979
}
8080
for _, t := range teams {
81-
if t.AccessMode >= perm.AccessModeAdmin {
81+
if t.HasAdminAccess() {
8282
return true, nil
8383
}
8484
}

Diff for: ‎models/organization/team.go

+6-18
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ func (t *Team) LoadUnits(ctx context.Context) (err error) {
113113

114114
// GetUnitNames returns the team units names
115115
func (t *Team) GetUnitNames() (res []string) {
116-
if t.AccessMode >= perm.AccessModeAdmin {
116+
if t.HasAdminAccess() {
117117
return unit.AllUnitKeyNames()
118118
}
119119

@@ -126,7 +126,7 @@ func (t *Team) GetUnitNames() (res []string) {
126126
// GetUnitsMap returns the team units permissions
127127
func (t *Team) GetUnitsMap() map[string]string {
128128
m := make(map[string]string)
129-
if t.AccessMode >= perm.AccessModeAdmin {
129+
if t.HasAdminAccess() {
130130
for _, u := range unit.Units {
131131
m[u.NameKey] = t.AccessMode.ToString()
132132
}
@@ -153,6 +153,10 @@ func (t *Team) IsMember(ctx context.Context, userID int64) bool {
153153
return isMember
154154
}
155155

156+
func (t *Team) HasAdminAccess() bool {
157+
return t.AccessMode >= perm.AccessModeAdmin
158+
}
159+
156160
// LoadMembers returns paginated members in team of organization.
157161
func (t *Team) LoadMembers(ctx context.Context) (err error) {
158162
t.Members, err = GetTeamMembers(ctx, &SearchMembersOptions{
@@ -238,22 +242,6 @@ func GetTeamByID(ctx context.Context, teamID int64) (*Team, error) {
238242
return t, nil
239243
}
240244

241-
// GetTeamNamesByID returns team's lower name from a list of team ids.
242-
func GetTeamNamesByID(ctx context.Context, teamIDs []int64) ([]string, error) {
243-
if len(teamIDs) == 0 {
244-
return []string{}, nil
245-
}
246-
247-
var teamNames []string
248-
err := db.GetEngine(ctx).Table("team").
249-
Select("lower_name").
250-
In("id", teamIDs).
251-
Asc("name").
252-
Find(&teamNames)
253-
254-
return teamNames, err
255-
}
256-
257245
// IncrTeamRepoNum increases the number of repos for the given team by 1
258246
func IncrTeamRepoNum(ctx context.Context, teamID int64) error {
259247
_, err := db.GetEngine(ctx).Incr("num_repos").ID(teamID).Update(new(Team))

Diff for: ‎models/perm/access/repo_permission.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
331331

332332
// if user in an owner team
333333
for _, team := range teams {
334-
if team.AccessMode >= perm_model.AccessModeAdmin {
334+
if team.HasAdminAccess() {
335335
perm.AccessMode = perm_model.AccessModeOwner
336336
perm.unitsMode = nil
337337
return perm, nil
@@ -399,7 +399,7 @@ func IsUserRepoAdmin(ctx context.Context, repo *repo_model.Repository, user *use
399399
}
400400

401401
for _, team := range teams {
402-
if team.AccessMode >= perm_model.AccessModeAdmin {
402+
if team.HasAdminAccess() {
403403
return true, nil
404404
}
405405
}

Diff for: ‎models/unit/unit.go

+15-28
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,21 @@ type Type int
2020

2121
// Enumerate all the unit types
2222
const (
23-
TypeInvalid Type = iota // 0 invalid
24-
TypeCode // 1 code
25-
TypeIssues // 2 issues
26-
TypePullRequests // 3 PRs
27-
TypeReleases // 4 Releases
28-
TypeWiki // 5 Wiki
29-
TypeExternalWiki // 6 ExternalWiki
30-
TypeExternalTracker // 7 ExternalTracker
31-
TypeProjects // 8 Projects
32-
TypePackages // 9 Packages
33-
TypeActions // 10 Actions
23+
TypeInvalid Type = iota // 0 invalid
24+
25+
TypeCode // 1 code
26+
TypeIssues // 2 issues
27+
TypePullRequests // 3 PRs
28+
TypeReleases // 4 Releases
29+
TypeWiki // 5 Wiki
30+
TypeExternalWiki // 6 ExternalWiki
31+
TypeExternalTracker // 7 ExternalTracker
32+
TypeProjects // 8 Projects
33+
TypePackages // 9 Packages
34+
TypeActions // 10 Actions
35+
36+
// FIXME: TEAM-UNIT-PERMISSION: the team unit "admin" permission's design is not right, when a new unit is added in the future,
37+
// admin team won't inherit the correct admin permission for the new unit, need to have a complete fix before adding any new unit.
3438
)
3539

3640
// Value returns integer value for unit type (used by template)
@@ -380,20 +384,3 @@ func AllUnitKeyNames() []string {
380384
}
381385
return res
382386
}
383-
384-
// MinUnitAccessMode returns the minial permission of the permission map
385-
func MinUnitAccessMode(unitsMap map[Type]perm.AccessMode) perm.AccessMode {
386-
res := perm.AccessModeNone
387-
for t, mode := range unitsMap {
388-
// Don't allow `TypeExternal{Tracker,Wiki}` to influence this as they can only be set to READ perms.
389-
if t == TypeExternalTracker || t == TypeExternalWiki {
390-
continue
391-
}
392-
393-
// get the minial permission great than AccessModeNone except all are AccessModeNone
394-
if mode > perm.AccessModeNone && (res == perm.AccessModeNone || mode < res) {
395-
res = mode
396-
}
397-
}
398-
return res
399-
}

Diff for: ‎routers/api/v1/org/team.go

+11-25
Original file line numberDiff line numberDiff line change
@@ -141,26 +141,18 @@ func GetTeam(ctx *context.APIContext) {
141141
ctx.JSON(http.StatusOK, apiTeam)
142142
}
143143

144-
func attachTeamUnits(team *organization.Team, units []string) {
144+
func attachTeamUnits(team *organization.Team, defaultAccessMode perm.AccessMode, units []string) {
145145
unitTypes, _ := unit_model.FindUnitTypes(units...)
146146
team.Units = make([]*organization.TeamUnit, 0, len(units))
147147
for _, tp := range unitTypes {
148148
team.Units = append(team.Units, &organization.TeamUnit{
149149
OrgID: team.OrgID,
150150
Type: tp,
151-
AccessMode: team.AccessMode,
151+
AccessMode: defaultAccessMode,
152152
})
153153
}
154154
}
155155

156-
func convertUnitsMap(unitsMap map[string]string) map[unit_model.Type]perm.AccessMode {
157-
res := make(map[unit_model.Type]perm.AccessMode, len(unitsMap))
158-
for unitKey, p := range unitsMap {
159-
res[unit_model.TypeFromKey(unitKey)] = perm.ParseAccessMode(p)
160-
}
161-
return res
162-
}
163-
164156
func attachTeamUnitsMap(team *organization.Team, unitsMap map[string]string) {
165157
team.Units = make([]*organization.TeamUnit, 0, len(unitsMap))
166158
for unitKey, p := range unitsMap {
@@ -214,24 +206,22 @@ func CreateTeam(ctx *context.APIContext) {
214206
// "422":
215207
// "$ref": "#/responses/validationError"
216208
form := web.GetForm(ctx).(*api.CreateTeamOption)
217-
p := perm.ParseAccessMode(form.Permission)
218-
if p < perm.AccessModeAdmin && len(form.UnitsMap) > 0 {
219-
p = unit_model.MinUnitAccessMode(convertUnitsMap(form.UnitsMap))
220-
}
209+
teamPermission := perm.ParseAccessMode(form.Permission, perm.AccessModeNone, perm.AccessModeAdmin)
221210
team := &organization.Team{
222211
OrgID: ctx.Org.Organization.ID,
223212
Name: form.Name,
224213
Description: form.Description,
225214
IncludesAllRepositories: form.IncludesAllRepositories,
226215
CanCreateOrgRepo: form.CanCreateOrgRepo,
227-
AccessMode: p,
216+
AccessMode: teamPermission,
228217
}
229218

230219
if team.AccessMode < perm.AccessModeAdmin {
231220
if len(form.UnitsMap) > 0 {
232221
attachTeamUnitsMap(team, form.UnitsMap)
233222
} else if len(form.Units) > 0 {
234-
attachTeamUnits(team, form.Units)
223+
unitPerm := perm.ParseAccessMode(form.Permission, perm.AccessModeRead, perm.AccessModeWrite)
224+
attachTeamUnits(team, unitPerm, form.Units)
235225
} else {
236226
ctx.APIErrorInternal(errors.New("units permission should not be empty"))
237227
return
@@ -304,15 +294,10 @@ func EditTeam(ctx *context.APIContext) {
304294
isAuthChanged := false
305295
isIncludeAllChanged := false
306296
if !team.IsOwnerTeam() && len(form.Permission) != 0 {
307-
// Validate permission level.
308-
p := perm.ParseAccessMode(form.Permission)
309-
if p < perm.AccessModeAdmin && len(form.UnitsMap) > 0 {
310-
p = unit_model.MinUnitAccessMode(convertUnitsMap(form.UnitsMap))
311-
}
312-
313-
if team.AccessMode != p {
297+
teamPermission := perm.ParseAccessMode(form.Permission, perm.AccessModeNone, perm.AccessModeAdmin)
298+
if team.AccessMode != teamPermission {
314299
isAuthChanged = true
315-
team.AccessMode = p
300+
team.AccessMode = teamPermission
316301
}
317302

318303
if form.IncludesAllRepositories != nil {
@@ -325,7 +310,8 @@ func EditTeam(ctx *context.APIContext) {
325310
if len(form.UnitsMap) > 0 {
326311
attachTeamUnitsMap(team, form.UnitsMap)
327312
} else if len(form.Units) > 0 {
328-
attachTeamUnits(team, form.Units)
313+
unitPerm := perm.ParseAccessMode(form.Permission, perm.AccessModeRead, perm.AccessModeWrite)
314+
attachTeamUnits(team, unitPerm, form.Units)
329315
}
330316
} else {
331317
attachAdminTeamUnits(team)

Diff for: ‎routers/api/v1/repo/collaborators.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ func AddOrUpdateCollaborator(ctx *context.APIContext) {
181181

182182
p := perm.AccessModeWrite
183183
if form.Permission != nil {
184-
p = perm.ParseAccessMode(*form.Permission)
184+
p = perm.ParseAccessMode(*form.Permission, perm.AccessModeRead, perm.AccessModeWrite)
185185
}
186186

187187
if err := repo_service.AddOrUpdateCollaborator(ctx, ctx.Repo.Repository, collaborator, p); err != nil {

Diff for: ‎routers/web/org/teams.go

+9-17
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,8 @@ func NewTeam(ctx *context.Context) {
284284
ctx.HTML(http.StatusOK, tplTeamNew)
285285
}
286286

287+
// FIXME: TEAM-UNIT-PERMISSION: this design is not right, when a new unit is added in the future,
288+
// admin team won't inherit the correct admin permission for the new unit.
287289
func getUnitPerms(forms url.Values, teamPermission perm.AccessMode) map[unit_model.Type]perm.AccessMode {
288290
unitPerms := make(map[unit_model.Type]perm.AccessMode)
289291
for _, ut := range unit_model.AllRepoUnitTypes {
@@ -314,19 +316,14 @@ func getUnitPerms(forms url.Values, teamPermission perm.AccessMode) map[unit_mod
314316
func NewTeamPost(ctx *context.Context) {
315317
form := web.GetForm(ctx).(*forms.CreateTeamForm)
316318
includesAllRepositories := form.RepoAccess == "all"
317-
p := perm.ParseAccessMode(form.Permission)
318-
unitPerms := getUnitPerms(ctx.Req.Form, p)
319-
if p < perm.AccessModeAdmin {
320-
// if p is less than admin accessmode, then it should be general accessmode,
321-
// so we should calculate the minial accessmode from units accessmodes.
322-
p = unit_model.MinUnitAccessMode(unitPerms)
323-
}
319+
teamPermission := perm.ParseAccessMode(form.Permission, perm.AccessModeNone, perm.AccessModeAdmin)
320+
unitPerms := getUnitPerms(ctx.Req.Form, teamPermission)
324321

325322
t := &org_model.Team{
326323
OrgID: ctx.Org.Organization.ID,
327324
Name: form.TeamName,
328325
Description: form.Description,
329-
AccessMode: p,
326+
AccessMode: teamPermission,
330327
IncludesAllRepositories: includesAllRepositories,
331328
CanCreateOrgRepo: form.CanCreateOrgRepo,
332329
}
@@ -485,13 +482,8 @@ func EditTeam(ctx *context.Context) {
485482
func EditTeamPost(ctx *context.Context) {
486483
form := web.GetForm(ctx).(*forms.CreateTeamForm)
487484
t := ctx.Org.Team
488-
newAccessMode := perm.ParseAccessMode(form.Permission)
489-
unitPerms := getUnitPerms(ctx.Req.Form, newAccessMode)
490-
if newAccessMode < perm.AccessModeAdmin {
491-
// if newAccessMode is less than admin accessmode, then it should be general accessmode,
492-
// so we should calculate the minial accessmode from units accessmodes.
493-
newAccessMode = unit_model.MinUnitAccessMode(unitPerms)
494-
}
485+
teamPermission := perm.ParseAccessMode(form.Permission, perm.AccessModeNone, perm.AccessModeAdmin)
486+
unitPerms := getUnitPerms(ctx.Req.Form, teamPermission)
495487
isAuthChanged := false
496488
isIncludeAllChanged := false
497489
includesAllRepositories := form.RepoAccess == "all"
@@ -503,9 +495,9 @@ func EditTeamPost(ctx *context.Context) {
503495

504496
if !t.IsOwnerTeam() {
505497
t.Name = form.TeamName
506-
if t.AccessMode != newAccessMode {
498+
if t.AccessMode != teamPermission {
507499
isAuthChanged = true
508-
t.AccessMode = newAccessMode
500+
t.AccessMode = teamPermission
509501
}
510502

511503
if t.IncludesAllRepositories != includesAllRepositories {

Diff for: ‎services/context/org.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ func OrgAssignment(opts OrgAssignmentOptions) func(ctx *Context) {
182182
return
183183
}
184184
for _, team := range teams {
185-
if team.IncludesAllRepositories && team.AccessMode >= perm.AccessModeAdmin {
185+
if team.IncludesAllRepositories && team.HasAdminAccess() {
186186
shouldSeeAllTeams = true
187187
break
188188
}
@@ -228,7 +228,7 @@ func OrgAssignment(opts OrgAssignmentOptions) func(ctx *Context) {
228228
return
229229
}
230230

231-
ctx.Org.IsTeamAdmin = ctx.Org.Team.IsOwnerTeam() || ctx.Org.Team.AccessMode >= perm.AccessModeAdmin
231+
ctx.Org.IsTeamAdmin = ctx.Org.Team.IsOwnerTeam() || ctx.Org.Team.HasAdminAccess()
232232
ctx.Data["IsTeamAdmin"] = ctx.Org.IsTeamAdmin
233233
if opts.RequireTeamAdmin && !ctx.Org.IsTeamAdmin {
234234
ctx.NotFound(err)

Diff for: ‎services/org/team.go

-31
Original file line numberDiff line numberDiff line change
@@ -259,37 +259,6 @@ func AddTeamMember(ctx context.Context, team *organization.Team, user *user_mode
259259
}
260260

261261
team.NumMembers++
262-
263-
// Give access to team repositories.
264-
// update exist access if mode become bigger
265-
subQuery := builder.Select("repo_id").From("team_repo").
266-
Where(builder.Eq{"team_id": team.ID})
267-
268-
if _, err := sess.Where("user_id=?", user.ID).
269-
In("repo_id", subQuery).
270-
And("mode < ?", team.AccessMode).
271-
SetExpr("mode", team.AccessMode).
272-
Update(new(access_model.Access)); err != nil {
273-
return fmt.Errorf("update user accesses: %w", err)
274-
}
275-
276-
// for not exist access
277-
var repoIDs []int64
278-
accessSubQuery := builder.Select("repo_id").From("access").Where(builder.Eq{"user_id": user.ID})
279-
if err := sess.SQL(subQuery.And(builder.NotIn("repo_id", accessSubQuery))).Find(&repoIDs); err != nil {
280-
return fmt.Errorf("select id accesses: %w", err)
281-
}
282-
283-
accesses := make([]*access_model.Access, 0, 100)
284-
for i, repoID := range repoIDs {
285-
accesses = append(accesses, &access_model.Access{RepoID: repoID, UserID: user.ID, Mode: team.AccessMode})
286-
if (i%100 == 0 || i == len(repoIDs)-1) && len(accesses) > 0 {
287-
if err = db.Insert(ctx, accesses); err != nil {
288-
return fmt.Errorf("insert new user accesses: %w", err)
289-
}
290-
accesses = accesses[:0]
291-
}
292-
}
293262
return nil
294263
})
295264
if err != nil {

Diff for: ‎templates/org/team/new.tmpl

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@
5656
<br>
5757
<div class="field">
5858
<div class="ui radio checkbox">
59-
<input type="radio" name="permission" value="read" {{if or .PageIsOrgTeamsNew (eq .Team.AccessMode 1) (eq .Team.AccessMode 2)}}checked{{end}}>
59+
<input type="radio" name="permission" value="read" {{if or .PageIsOrgTeamsNew (eq .Team.AccessMode 0) (eq .Team.AccessMode 1) (eq .Team.AccessMode 2)}}checked{{end}}>
6060
<label>{{ctx.Locale.Tr "org.teams.general_access"}}</label>
6161
<span class="help">{{ctx.Locale.Tr "org.teams.general_access_helper"}}</span>
6262
</div>

Diff for: ‎templates/org/team/sidebar.tmpl

+3-2
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,11 @@
4242
<li>{{ctx.Locale.Tr "org.teams.can_create_org_repo"}}</li>
4343
{{end}}
4444
</ul>
45-
{{if (eq .Team.AccessMode 2)}}
45+
{{/* the AccessMode should be either none or admin/owner, the real permissions are provided by each team unit */}}
46+
{{if false /*(eq .Team.AccessMode 2)*/}}
4647
<h3>{{ctx.Locale.Tr "org.settings.permission"}}</h3>
4748
{{ctx.Locale.Tr "org.teams.write_permission_desc"}}
48-
{{else if (eq .Team.AccessMode 3)}}
49+
{{else if false /*(eq .Team.AccessMode 3)*/}}
4950
<h3>{{ctx.Locale.Tr "org.settings.permission"}}</h3>
5051
{{ctx.Locale.Tr "org.teams.admin_permission_desc"}}
5152
{{else}}

0 commit comments

Comments
 (0)