fix(perm): keep read access for restricted collaborators on public repos (#40234)

Adding a restricted user as a read collaborator to a public repo gave
them no access when the owner isn't fully public, like a limited org.
Restricted users get no default read there, and `RecalculateUserAccess`
dropped their read access row.

- `RecalculateUserAccess` now keeps the same read rows as
`refreshAccesses`, on org-owned repos and for restricted users.
- Re-saving an unchanged collaborator permission recalculates access,
repairing collaborators affected before this fix.

Fixes https://github.com/go-gitea/gitea/issues/40233

Assisted-by: Claude Code:claude-opus-5-5

---------

Co-authored-by: silverwind <me@silverwind.io>
This commit is contained in:
Lazizbek Ergashev
2026-11-05 23:48:55 +00:00
committed by GitHub
parent b81b50be55
commit 288d54951e
6 changed files with 29 additions and 15 deletions

View File

@@ -224,30 +224,32 @@ func RecalculateTeamAccesses(ctx context.Context, repo *repo_model.Repository, i
// RecalculateUserAccess recalculates new access for a single user
// Usable if we know access only affected one user
func RecalculateUserAccess(ctx context.Context, repo *repo_model.Repository, uid int64) (err error) {
func RecalculateUserAccess(ctx context.Context, repo *repo_model.Repository, user *user_model.User) (err error) {
if err = repo.LoadOwner(ctx); err != nil {
return err
}
minMode := perm.AccessModeRead
if !repo.IsPrivate {
if !repo.IsPrivate && !repo.Owner.IsOrganization() && !user.IsRestricted {
minMode = perm.AccessModeWrite
}
accessMode := perm.AccessModeNone
e := db.GetEngine(ctx)
collaborator, err := repo_model.GetCollaboration(ctx, repo.ID, uid)
collaborator, err := repo_model.GetCollaboration(ctx, repo.ID, user.ID)
if err != nil {
return err
} else if collaborator != nil {
accessMode = collaborator.Mode
}
if err = repo.LoadOwner(ctx); err != nil {
return err
} else if repo.Owner.IsOrganization() {
if repo.Owner.IsOrganization() {
var teams []organization.Team
if err := e.Join("INNER", "team_repo", "team_repo.team_id = team.id").
Join("INNER", "team_user", "team_user.team_id = team.id").
Where("team.org_id = ?", repo.OwnerID).
And("team_repo.repo_id=?", repo.ID).
And("team_user.uid=?", uid).
And("team_user.uid=?", user.ID).
Find(&teams); err != nil {
return err
}
@@ -262,10 +264,10 @@ func RecalculateUserAccess(ctx context.Context, repo *repo_model.Repository, uid
}
// Delete old user accesses and insert new one for repository.
if _, err = e.Delete(&Access{RepoID: repo.ID, UserID: uid}); err != nil {
if _, err = e.Delete(&Access{RepoID: repo.ID, UserID: user.ID}); err != nil {
return fmt.Errorf("delete old user accesses: %w", err)
} else if accessMode >= minMode {
if err = db.Insert(ctx, &Access{RepoID: repo.ID, UserID: uid, Mode: accessMode}); err != nil {
if err = db.Insert(ctx, &Access{RepoID: repo.ID, UserID: user.ID, Mode: accessMode}); err != nil {
return fmt.Errorf("insert new user accesses: %w", err)
}
}

View File

@@ -345,7 +345,7 @@ func removeTeamMember(ctx context.Context, team *organization.Team, user *user_m
// Delete access to team repositories. If any user or repo is missing, we can continue.
for _, repo := range repos {
if err := access_model.RecalculateUserAccess(ctx, repo, user.ID); err != nil && !errors.Is(err, util.ErrNotExist) {
if err := access_model.RecalculateUserAccess(ctx, repo, user); err != nil && !errors.Is(err, util.ErrNotExist) {
return err
}

View File

@@ -94,7 +94,7 @@ func TestIsUserAllowedToUpdate(t *testing.T) {
defer db.DeleteByBean(t.Context(), collaboration)
require.NoError(t, pr2.LoadHeadRepo(t.Context()))
assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), pr2.HeadRepo, user4.ID))
assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), pr2.HeadRepo, user4))
pushAllowed, rebaseAllowed, _, err := checkUserAllowedToUpdate(t.Context(), pr2, user4)
assert.NoError(t, err)
@@ -186,7 +186,7 @@ func TestIsUserAllowedToUpdate(t *testing.T) {
defer db.DeleteByBean(t.Context(), collaboration)
require.NoError(t, pr3.LoadHeadRepo(t.Context()))
assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), pr3.HeadRepo, pr3Poster.ID))
assert.NoError(t, access_model.RecalculateUserAccess(t.Context(), pr3.HeadRepo, pr3Poster))
if testCase.disableRebase {
setRepoAllowRebaseUpdate(t, pr3.BaseRepoID, false)

View File

@@ -46,7 +46,7 @@ func AddOrUpdateCollaborator(ctx context.Context, repo *repo_model.Repository, u
return err
} else if has {
if collaboration.Mode == mode {
return nil
return access_model.RecalculateUserAccess(ctx, repo, u)
}
if _, err = db.GetEngine(ctx).
Where("repo_id=?", repo.ID).
@@ -69,7 +69,7 @@ func AddOrUpdateCollaborator(ctx context.Context, repo *repo_model.Repository, u
added = true
}
return access_model.RecalculateUserAccess(ctx, repo, u.ID)
return access_model.RecalculateUserAccess(ctx, repo, u)
}); err != nil {
return err
}

View File

@@ -32,6 +32,18 @@ func TestRepository_AddCollaborator(t *testing.T) {
assert.Error(t, AddOrUpdateCollaborator(t.Context(), repo1, user4, perm.AccessModeOwner))
assert.NoError(t, AddOrUpdateCollaborator(t.Context(), repo1, user4, perm.AccessModeAdmin))
repo8 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 8})
repo38 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 38})
user29 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 29})
for _, collab := range []struct {
repo *repo_model.Repository
user *user_model.User
}{{repo8, user29}, {repo38, user4}} {
assert.NoError(t, db.Insert(t.Context(), &repo_model.Collaboration{RepoID: collab.repo.ID, UserID: collab.user.ID, Mode: perm.AccessModeRead}))
assert.NoError(t, AddOrUpdateCollaborator(t.Context(), collab.repo, collab.user, perm.AccessModeRead))
unittest.AssertExistsAndLoadBean(t, &access_model.Access{RepoID: collab.repo.ID, UserID: collab.user.ID, Mode: perm.AccessModeRead})
}
}
func TestRepository_DeleteCollaboration(t *testing.T) {

View File

@@ -487,7 +487,7 @@ func StartRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.Use
if err := db.Insert(ctx, &repo_model.Collaboration{RepoID: repo.ID, UserID: newOwner.ID, Mode: perm.AccessModeRead}); err != nil {
return err
}
if err := access_model.RecalculateUserAccess(ctx, repo, newOwner.ID); err != nil {
if err := access_model.RecalculateUserAccess(ctx, repo, newOwner); err != nil {
return err
}
}