Correctly override user unitmodes (#35501)

Commit 6a97ab0 reworked team permission
application. The introduced logic overrode the unitModes for *every*
team a user is in, max(...) the current value and the team value
together.

The logic completely fails in case the team does not have a unit for the
specific unit type defined, in which case the logic inserted the
minimumVisibility, overriding any previous aggregation of access modes
for the unit.

This is resolved by simply always merging the unit access mode of the
team as it will simply default to None in case the team does not have a
permission defined for the unit, which will be swallowed by the max(..)
call in favour of the previous aggregated permission.

Author: lynxplay

models/perm/access/repo_permission.go

@@ -348,10 +348,8 @@ func GetUserRepoPermission(ctx context.Context, repo *repo_model.Repository, use
 
 	for _, u := range repo.Units {
 		for _, team := range teams {
-			unitAccessMode := minAccessMode
-			if teamMode, exist := team.UnitAccessModeEx(ctx, u.Type); exist {
-				unitAccessMode = max(perm.unitsMode[u.Type], unitAccessMode, teamMode)
-			}
+			teamMode, _ := team.UnitAccessModeEx(ctx, u.Type)
+			unitAccessMode := max(perm.unitsMode[u.Type], minAccessMode, teamMode)
 			perm.unitsMode[u.Type] = unitAccessMode
 		}
 	}

models/perm/access/repo_permission_test.go

@@ -197,4 +197,37 @@ func TestGetUserRepoPermission(t *testing.T) {
 		assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeCode])
 		assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeIssues])
 	})
+
+	repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // org private repo, same org as repo 32
+	require.NoError(t, repo3.LoadOwner(ctx))
+	require.True(t, repo3.Owner.IsOrganization())
+	require.NoError(t, db.TruncateBeans(ctx, &organization.TeamUnit{}, &Access{})) // The user has access set of that repo, remove it, it is useless for our test
+	require.NoError(t, db.Insert(ctx, &organization.TeamRepo{OrgID: org.ID, TeamID: team.ID, RepoID: repo3.ID}))
+	t.Run("DoerWithNoopTeamOnPrivateRepo", func(t *testing.T) {
+		perm, err := GetUserRepoPermission(ctx, repo3, user)
+		require.NoError(t, err)
+		assert.Equal(t, perm_model.AccessModeNone, perm.AccessMode)
+		assert.Equal(t, perm_model.AccessModeNone, perm.unitsMode[unit.TypeCode])
+		assert.Equal(t, perm_model.AccessModeNone, perm.unitsMode[unit.TypeIssues])
+	})
+
+	require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeCode, AccessMode: perm_model.AccessModeNone}))
+	require.NoError(t, db.Insert(ctx, &organization.TeamUnit{OrgID: org.ID, TeamID: team.ID, Type: unit.TypeIssues, AccessMode: perm_model.AccessModeRead}))
+	t.Run("DoerWithReadIssueTeamOnPrivateRepo", func(t *testing.T) {
+		perm, err := GetUserRepoPermission(ctx, repo3, user)
+		require.NoError(t, err)
+		assert.Equal(t, perm_model.AccessModeNone, perm.AccessMode)
+		assert.Equal(t, perm_model.AccessModeNone, perm.unitsMode[unit.TypeCode])
+		assert.Equal(t, perm_model.AccessModeRead, perm.unitsMode[unit.TypeIssues])
+	})
+
+	require.NoError(t, db.Insert(ctx, repo_model.Collaboration{RepoID: repo3.ID, UserID: user.ID, Mode: perm_model.AccessModeWrite}))
+	require.NoError(t, db.Insert(ctx, Access{RepoID: repo3.ID, UserID: user.ID, Mode: perm_model.AccessModeWrite}))
+	t.Run("DoerWithReadIssueTeamAndWriteCollaboratorOnPrivateRepo", func(t *testing.T) {
+		perm, err := GetUserRepoPermission(ctx, repo3, user)
+		require.NoError(t, err)
+		assert.Equal(t, perm_model.AccessModeWrite, perm.AccessMode)
+		assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeCode])
+		assert.Equal(t, perm_model.AccessModeWrite, perm.unitsMode[unit.TypeIssues])
+	})
 }