Clarify permission "HasAccess" behavior (#30585)

Follow #30495

"HasAccess" behavior wasn't clear, to make it clear:

* Use a new name `HasAnyUnitAccess`, it will be easier to review related
code and permission problems.
* Separate everyone access mode to a separate field, then all calls to
HasAccess are reverted to old behavior before #30495.
* Add new tests.

---------

Co-authored-by: Giteabot <teabot@gitea.io>

4 files changed, 84 insertions, 28 deletions

diff --git a/models/org_team.go b/models/org_team.go
index aecf0d80fd..b6908478c7 100644
--- a/models/org_team.go
+++ b/models/org_team.go
@@ -118,7 +118,7 @@ func removeAllRepositories(ctx context.Context, t *organization.Team) (err error
 
 		// Remove watches from all users and now unaccessible repos
 		for _, user := range t.Members {
-			has, err := access_model.HasAccess(ctx, user.ID, repo)
+			has, err := access_model.HasAnyUnitAccess(ctx, user.ID, repo)
 			if err != nil {
 				return err
 			} else if has {
@@ -544,7 +544,7 @@ func ReconsiderRepoIssuesAssignee(ctx context.Context, repo *repo_model.Reposito
 }
 
 func ReconsiderWatches(ctx context.Context, repo *repo_model.Repository, user *user_model.User) error {
-	if has, err := access_model.HasAccess(ctx, user.ID, repo); err != nil || has {
+	if has, err := access_model.HasAnyUnitAccess(ctx, user.ID, repo); err != nil || has {
 		return err
 	}
 	if err := repo_model.WatchRepo(ctx, user, repo, false); err != nil {
diff --git a/models/perm/access/access_test.go b/models/perm/access/access_test.go
index 79b131fe89..51d625707c 100644
--- a/models/perm/access/access_test.go
+++ b/models/perm/access/access_test.go
@@ -79,17 +79,17 @@ func TestHasAccess(t *testing.T) {
 	repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3})
 	assert.True(t, repo2.IsPrivate)
 
-	has, err := access_model.HasAccess(db.DefaultContext, user1.ID, repo1)
+	has, err := access_model.HasAnyUnitAccess(db.DefaultContext, user1.ID, repo1)
 	assert.NoError(t, err)
 	assert.True(t, has)
 
-	_, err = access_model.HasAccess(db.DefaultContext, user1.ID, repo2)
+	_, err = access_model.HasAnyUnitAccess(db.DefaultContext, user1.ID, repo2)
 	assert.NoError(t, err)
 
-	_, err = access_model.HasAccess(db.DefaultContext, user2.ID, repo1)
+	_, err = access_model.HasAnyUnitAccess(db.DefaultContext, user2.ID, repo1)
 	assert.NoError(t, err)
 
-	_, err = access_model.HasAccess(db.DefaultContext, user2.ID, repo2)
+	_, err = access_model.HasAnyUnitAccess(db.DefaultContext, user2.ID, repo2)
 	assert.NoError(t, err)
 }
 
diff --git a/models/perm/access/repo_permission.go b/models/perm/access/repo_permission.go
index 9cce95b776..0ed116a132 100644
--- a/models/perm/access/repo_permission.go
+++ b/models/perm/access/repo_permission.go
@@ -24,6 +24,8 @@ type Permission struct {
 
 	units     []*repo_model.RepoUnit
 	unitsMode map[unit.Type]perm_model.AccessMode
+
+	everyoneAccessMode map[unit.Type]perm_model.AccessMode
 }
 
 // IsOwner returns true if current user is the owner of repository.
@@ -36,9 +38,24 @@ func (p *Permission) IsAdmin() bool {
 	return p.AccessMode >= perm_model.AccessModeAdmin
 }
 
-// HasAccess returns true if the current user might have at least read access to any unit of this repository
-func (p *Permission) HasAccess() bool {
-	return len(p.unitsMode) > 0 || p.AccessMode >= perm_model.AccessModeRead
+// HasAnyUnitAccess returns true if the user might have at least one access mode to any unit of this repository.
+// It doesn't count the "everyone access mode".
+func (p *Permission) HasAnyUnitAccess() bool {
+	for _, v := range p.unitsMode {
+		if v >= perm_model.AccessModeRead {
+			return true
+		}
+	}
+	return p.AccessMode >= perm_model.AccessModeRead
+}
+
+func (p *Permission) HasAnyUnitAccessOrEveryoneAccess() bool {
+	for _, v := range p.everyoneAccessMode {
+		if v >= perm_model.AccessModeRead {
+			return true
+		}
+	}
+	return p.HasAnyUnitAccess()
 }
 
 // HasUnits returns true if the permission contains attached units
@@ -56,16 +73,16 @@ func (p *Permission) GetFirstUnitRepoID() int64 {
 }
 
 // UnitAccessMode returns current user access mode to the specify unit of the repository
+// It also considers "everyone access mode"
 func (p *Permission) UnitAccessMode(unitType unit.Type) perm_model.AccessMode {
-	if p.unitsMode != nil {
-		// if the units map contains the access mode, use it, but admin/owner mode could override it
-		if m, ok := p.unitsMode[unitType]; ok {
-			return util.Iif(p.AccessMode >= perm_model.AccessModeAdmin, p.AccessMode, m)
-		}
+	// if the units map contains the access mode, use it, but admin/owner mode could override it
+	if m, ok := p.unitsMode[unitType]; ok {
+		return util.Iif(p.AccessMode >= perm_model.AccessModeAdmin, p.AccessMode, m)
 	}
 	// if the units map does not contain the access mode, return the default access mode if the unit exists
+	unitDefaultAccessMode := max(p.AccessMode, p.everyoneAccessMode[unitType])
 	hasUnit := slices.ContainsFunc(p.units, func(u *repo_model.RepoUnit) bool { return u.Type == unitType })
-	return util.Iif(hasUnit, p.AccessMode, perm_model.AccessModeNone)
+	return util.Iif(hasUnit, unitDefaultAccessMode, perm_model.AccessModeNone)
 }
 
 func (p *Permission) SetUnitsWithDefaultAccessMode(units []*repo_model.RepoUnit, mode perm_model.AccessMode) {
@@ -159,14 +176,15 @@ func (p *Permission) LogString() string {
 }
 
 func applyEveryoneRepoPermission(user *user_model.User, perm *Permission) {
-	if user != nil && user.ID > 0 {
-		for _, u := range perm.units {
-			if perm.unitsMode == nil {
-				perm.unitsMode = make(map[unit.Type]perm_model.AccessMode)
-			}
-			if u.EveryoneAccessMode >= perm_model.AccessModeRead && u.EveryoneAccessMode > perm.unitsMode[u.Type] {
-				perm.unitsMode[u.Type] = u.EveryoneAccessMode
+	if user == nil || user.ID <= 0 {
+		return
+	}
+	for _, u := range perm.units {
+		if u.EveryoneAccessMode >= perm_model.AccessModeRead && u.EveryoneAccessMode > perm.everyoneAccessMode[u.Type] {
+			if perm.everyoneAccessMode == nil {
+				perm.everyoneAccessMode = make(map[unit.Type]perm_model.AccessMode)
 			}
+			perm.everyoneAccessMode[u.Type] = u.EveryoneAccessMode
 		}
 	}
 }
@@ -373,8 +391,8 @@ func CanBeAssigned(ctx context.Context, user *user_model.User, repo *repo_model.
 		perm.CanAccessAny(perm_model.AccessModeRead, unit.TypePullRequests), nil
 }
 
-// HasAccess returns true if user has access to repo
-func HasAccess(ctx context.Context, userID int64, repo *repo_model.Repository) (bool, error) {
+// HasAnyUnitAccess see the comment of "perm.HasAnyUnitAccess"
+func HasAnyUnitAccess(ctx context.Context, userID int64, repo *repo_model.Repository) (bool, error) {
 	var user *user_model.User
 	var err error
 	if userID > 0 {
@@ -387,7 +405,7 @@ func HasAccess(ctx context.Context, userID int64, repo *repo_model.Repository) (
 	if err != nil {
 		return false, err
 	}
-	return perm.HasAccess(), nil
+	return perm.HasAnyUnitAccess(), nil
 }
 
 // getUsersWithAccessMode returns users that have at least given access mode to the repository.
diff --git a/models/perm/access/repo_permission_test.go b/models/perm/access/repo_permission_test.go
index aaa53bb24f..50070c4368 100644
--- a/models/perm/access/repo_permission_test.go
+++ b/models/perm/access/repo_permission_test.go
@@ -14,11 +14,40 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+func TestHasAnyUnitAccess(t *testing.T) {
+	perm := Permission{}
+	assert.False(t, perm.HasAnyUnitAccess())
+
+	perm = Permission{
+		units: []*repo_model.RepoUnit{{Type: unit.TypeWiki}},
+	}
+	assert.False(t, perm.HasAnyUnitAccess())
+	assert.False(t, perm.HasAnyUnitAccessOrEveryoneAccess())
+
+	perm = Permission{
+		units:              []*repo_model.RepoUnit{{Type: unit.TypeWiki}},
+		everyoneAccessMode: map[unit.Type]perm_model.AccessMode{unit.TypeIssues: perm_model.AccessModeRead},
+	}
+	assert.False(t, perm.HasAnyUnitAccess())
+	assert.True(t, perm.HasAnyUnitAccessOrEveryoneAccess())
+
+	perm = Permission{
+		AccessMode: perm_model.AccessModeRead,
+		units:      []*repo_model.RepoUnit{{Type: unit.TypeWiki}},
+	}
+	assert.True(t, perm.HasAnyUnitAccess())
+
+	perm = Permission{
+		unitsMode: map[unit.Type]perm_model.AccessMode{unit.TypeWiki: perm_model.AccessModeRead},
+	}
+	assert.True(t, perm.HasAnyUnitAccess())
+}
+
 func TestApplyEveryoneRepoPermission(t *testing.T) {
 	perm := Permission{
 		AccessMode: perm_model.AccessModeNone,
 		units: []*repo_model.RepoUnit{
-			{Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeNone},
+			{Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead},
 		},
 	}
 	applyEveryoneRepoPermission(nil, &perm)
@@ -30,6 +59,15 @@ func TestApplyEveryoneRepoPermission(t *testing.T) {
 			{Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead},
 		},
 	}
+	applyEveryoneRepoPermission(&user_model.User{ID: 0}, &perm)
+	assert.False(t, perm.CanRead(unit.TypeWiki))
+
+	perm = Permission{
+		AccessMode: perm_model.AccessModeNone,
+		units: []*repo_model.RepoUnit{
+			{Type: unit.TypeWiki, EveryoneAccessMode: perm_model.AccessModeRead},
+		},
+	}
 	applyEveryoneRepoPermission(&user_model.User{ID: 1}, &perm)
 	assert.True(t, perm.CanRead(unit.TypeWiki))
 
@@ -40,8 +78,8 @@ func TestApplyEveryoneRepoPermission(t *testing.T) {
 		},
 	}
 	applyEveryoneRepoPermission(&user_model.User{ID: 1}, &perm)
-	assert.True(t, perm.CanRead(unit.TypeWiki))
-	assert.False(t, perm.CanWrite(unit.TypeWiki)) // because there is no unit mode, so the everyone-mode is used as the unit's access mode
+	// it should work the same as "EveryoneAccessMode: none" because the default AccessMode should be applied to units
+	assert.True(t, perm.CanWrite(unit.TypeWiki))
 
 	perm = Permission{
 		units: []*repo_model.RepoUnit{