- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 6.2k
Correctly override user unitmodes #35501
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
          
     Merged
      
      
            lunny
  merged 3 commits into
  go-gitea:main
from
lynxplay:bugfix/team-invalidation-of-unit-modes
  
      
      
   
  Sep 17, 2025 
      
    
                
     Merged
            
            Correctly override user unitmodes #35501
                    lunny
  merged 3 commits into
  go-gitea:main
from
lynxplay:bugfix/team-invalidation-of-unit-modes
  
      
      
   
  Sep 17, 2025 
              
            Conversation
  
    
      This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
      Learn more about bidirectional Unicode characters
    
  
  
    
    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.
| For further description of the faulty code, imagine the following run of a collaborator through that method // Collaborators on organization
// EXAMPLE
// The user is a collaborator, e.g. with write permissions.
// We hence give them write permissions for every unit in the repo.
// The map is now filled with AccessWrite.
if isCollaborator {
	for _, u := range repo.Units {
		perm.unitsMode[u.Type] = perm.AccessMode
	}
}
// if user in an owner team
// EXAMPLE
// This is a complete noop in this case, the user does not have admin access on a team.
for _, team := range teams {
	if team.HasAdminAccess() {
		perm.AccessMode = perm_model.AccessModeOwner
		perm.unitsMode = nil
		return perm, nil
	}
}
for _, u := range repo.Units {
	for _, team := range teams {
              // EXAMPLE
	       // We iterate the users team now. We find a team and query it for e.g. the Code unit.
	       // The team we are about to query however does not have access modes configured for the code unit
		unitAccessMode := minAccessMode
		// THIS if now does not run. unitAccessMode is minAccessMode which itself is None or Read at best. Definitely not Write like the collaborator should have
	        if teamMode, exist := team.UnitAccessModeEx(ctx, u.Type); exist {
		        unitAccessMode = max(perm.unitsMode[u.Type], unitAccessMode, teamMode)
	        }
	        perm.unitsMode[u.Type] = unitAccessMode // We completely override the previous unitMode without any regard for its value. We set it to minAccessMode and move on. This is makes this logic faulty in general but also dependent on the order the teams are iterated in as later teams might again bump the unit mode or fall it back down to min access level.
	}
} | 
| Could you add some tests for that? | 
A small set of unit tests to cover merging behaviour between collaborator and teams.
| I pushed some tests, I am very inexperienced with gitea's unittest layout, specifically the split into fixtures and in-code definitions. Would appreciate feedback. | 
              
                    wxiaoguang
  
              
              approved these changes
              
                  
                    Sep 17, 2025 
                  
              
              
            
            
              
                    lunny
  
              
              approved these changes
              
                  
                    Sep 17, 2025 
                  
              
              
            
            
    
  zjjhot 
      added a commit
        to zjjhot/gitea
      that referenced
      this pull request
    
      Sep 18, 2025 
    
    
      
  
    
      
    
  
* giteaofficial/main: Clean up npm dependencies (go-gitea#35508) Correctly override user unitmodes (go-gitea#35501) [skip ci] Updated translations via Crowdin Enable more markdown paste features in textarea editor (go-gitea#35494) Move git command to git/gitcmd (go-gitea#35483) Exposing TimeEstimate field in the API (go-gitea#35475) Clean up npm dependencies (go-gitea#35484)
    
  GiteaBot 
      pushed a commit
        to GiteaBot/gitea
      that referenced
      this pull request
    
      Oct 14, 2025 
    
    
      
  
    
      
    
  
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.
    
  wxiaoguang 
      pushed a commit
      that referenced
      this pull request
    
      Oct 15, 2025 
    
    
  
  
    Sign up for free
    to join this conversation on GitHub.
    Already have an account?
    Sign in to comment
  
      Labels
      
    backport/done
  All backports for this PR have been created 
  
    backport/v1.24
  This PR should be backported to Gitea 1.24 
  
    lgtm/done
  This PR has enough approvals to get merged. There are no important open reservations anymore. 
  
    modifies/go
  Pull requests that update Go code 
  
    skip-changelog
  This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. 
  
    type/bug
  Add this suggestion to a batch that can be applied as a single commit.
  This suggestion is invalid because no changes were made to the code.
  Suggestions cannot be applied while the pull request is closed.
  Suggestions cannot be applied while viewing a subset of changes.
  Only one suggestion per line can be applied in a batch.
  Add this suggestion to a batch that can be applied as a single commit.
  Applying suggestions on deleted lines is not supported.
  You must change the existing code in this line in order to create a valid suggestion.
  Outdated suggestions cannot be applied.
  This suggestion has been applied or marked resolved.
  Suggestions cannot be applied from pending reviews.
  Suggestions cannot be applied on multi-line comments.
  Suggestions cannot be applied while the pull request is queued to merge.
  Suggestion cannot be applied right now. Please check back later.
  
    
  
    
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.