Fix minimum team access mode

[kdumontnu]

Team access mode appears to be broken, leading to some suspect behavior. Specifically, we currently ignore "NoAccess" when getting the min access for a team in MinUnitAccessMode.

This means that if I set a team to only have write access for wiki page:

I will see the team has team.AccessMode = write:

But adding any read access:

Moves the team.AccessMode = read:

I ultimately suggest that we completely deprecate team.AccessMode, since it's not clear what that variable is used for now that team units exist. We should add team.IsAdmin and team.IsOwner flags instead, but I don't have the availability to take that on.

[delvh]

Shouldn't it be

Suggested change

	res := perm.AccessModeWrite
	res := perm.AccessModeOwner

and letting the for loop scale the variable down?
Or do we pass an empty map as parameter at any point?

[kdumontnu]

Can we have units with perm.AccessModeOwner?

I see here we return MaxPerm=perm.AccessModeAdmin:

gitea/models/unit/unit.go

Line 212 in 23ae939

return perm.AccessModeAdmin

But I don't know of any units that can have admin/owner access in practice (the UI does not allow it that I'm aware of).

So I can set the default to perm.AccessModeAdmin, but I didn't want to go higher than needed just for additional security.

[wxiaoguang]

TBH, I have difficulty to understand the logic if I didn't read the chat history & PR description carefully.

And the new logic seems counterintuitive , could there be some detailed comments or some tests for this problem?

[kdumontnu]

Yeah, comment and tests will be helpful regardless. I’ll work on adding those this weekend unless someone beats me to it.

Part of my issue is that I think this whole function should be deprecated. I’ll see how painful that will be.

I think I will just always set team.AccessMode=NoAccess whenever team.AccessMode < AdminAccess.

Essentially, every auth middleware should be checking unit permissions for anything less than admin.

[silverwind]

Marking WIP as per #24647 (comment).