[Bug Report] Team Permission Status

[AaronS8]

Gitea Version

1.16 && 1.17.0+dev-75-g7f2530e00

Git Version

2.33

Operating System

Windows 10

How are you running Gitea?

Downloaded from Gitea
Runining as service

Database

PostgreSQL
MSSQL

Can you reproduce the bug on the Gitea demo site?

Yes, SeeOn

Log Gist

No response

Description

Bug

1. Team permission is alwasy readonly or adminstrator
2. Even if you give all units write access
3. Team Members aren't listed on assignes (this is the currrent behaviour Team Members Assignment on Issues #18555 feature requeste )
4. Team Members won't see Repository before added as collabrators on thier organization if the repository is private
5. Team Members have wirte permission on everything but they don't see thier names on assignes even though they can assigne others

Additional Infromation

1. Gitea doesn't have this problem it if it's upgraded from 1.15.10 && you have created the team in 1.15.10
2. found the bug on 1.16 and above

Test Information

1. Create a new team in gitea 1.16 or 1.17 dev and assign it the team all write permmissions

Screenshots

Even when all units are on given on write Permmssion, the Team has Readonly access See Below Screen shoot.

[AaronS8]

problem doesn't appear on team created on gitea version 1.15, then upgraded to v1.17 it works fine, but if the team is created on v.1.17 it seems the team will either readonly or administrator permission

[AaronS8]

@zeripath could you check this out, I think this major problem on the team permmission assignment.

[techknowlogick]

@AaronS8 please don't share credentials for gitea.com, if you'd like to share a replication please use try.Gitea.io. I've deleted your comment, but please change the credentials too

[AaronS8]

Okay @techknowlogick I couldn't creat an organization or teams on try.Gitea.io, the only way was by using the officail gitea website.

[zeripath]

Ugh. There is a real and serious issue here.

The UI on the sidebar would not be difficult to update - (translations mean that we would need to be cautious in the way we fix this to ensure that backports work.)

The real problem is the access checking underneath for assignees (and likely elsewhere too). This relies on checking the access table which has a coarse permission check only.

I suspect this wasn't considered when the fine grained team permissions pr was added.

Ideally the access table would have been changed to include units in some way - which we can't do now on 1.16.

This means that access checking code will need to be significantly refactored to do the correct tests in order to generate the correct assignees list.

But everything that relies on the access table is suspect for not doing the right test now.

[zacheryph]

I wonder if @zeripath has notifications setup for when i'm mentioned 🤣

[AaronS8]

Could it be a simpler problem, I think units assignment works for teams made in pervious version 1.15, could it be just coding problem?

[zeripath]

It is not simple. The following are now wrong:

gitea/models/access.go

Lines 134 to 179 in df44017

	// recalculateTeamAccesses recalculates new accesses for teams of an organization
	// except the team whose ID is given. It is used to assign a team ID when
	// remove repository from that team.
	func recalculateTeamAccesses(ctx context.Context, repo *repo_model.Repository, ignTeamID int64) (err error) {
	accessMap := make(map[int64]*userAccess, 20)
	if err = repo.GetOwner(ctx); err != nil {
	return err
	} else if !repo.Owner.IsOrganization() {
	return fmt.Errorf("owner is not an organization: %d", repo.OwnerID)
	}
	e := db.GetEngine(ctx)
	if err = refreshCollaboratorAccesses(e, repo.ID, accessMap); err != nil {
	return fmt.Errorf("refreshCollaboratorAccesses: %v", err)
	}
	teams, err := OrgFromUser(repo.Owner).loadTeams(e)
	if err != nil {
	return err
	}
	for _, t := range teams {
	if t.ID == ignTeamID {
	continue
	}
	// Owner team gets owner access, and skip for teams that do not
	// have relations with repository.
	if t.IsOwnerTeam() {
	t.AccessMode = perm.AccessModeOwner
	} else if !t.hasRepository(e, repo.ID) {
	continue
	}
	if err = t.getMembers(e); err != nil {
	return fmt.Errorf("getMembers '%d': %v", t.ID, err)
	}
	for _, m := range t.Members {
	updateUserAccess(accessMap, m, t.AccessMode)
	}
	}
	return refreshAccesses(e, repo, accessMap)
	}

gitea/models/repo.go

Lines 147 to 178 in df44017

	func getRepoAssignees(ctx context.Context, repo *repo_model.Repository) (_ []*user_model.User, err error) {
	if err = repo.GetOwner(ctx); err != nil {
	return nil, err
	}
	e := db.GetEngine(ctx)
	accesses := make([]*Access, 0, 10)
	if err = e.
	Where("repo_id = ? AND mode >= ?", repo.ID, perm.AccessModeWrite).
	Find(&accesses); err != nil {
	return nil, err
	}
	// Leave a seat for owner itself to append later, but if owner is an organization
	// and just waste 1 unit is cheaper than re-allocate memory once.
	users := make([]*user_model.User, 0, len(accesses)+1)
	if len(accesses) > 0 {
	userIDs := make([]int64, len(accesses))
	for i := 0; i < len(accesses); i++ {
	userIDs[i] = accesses[i].UserID
	}
	if err = e.In("id", userIDs).Find(&users); err != nil {
	return nil, err
	}
	}
	if !repo.Owner.IsOrganization() {
	users = append(users, repo.Owner)
	}
	return users, nil
	}

Anything that uses the access table as a proxy for write permissions is incorrect - we need to search the entire codebase for other examples.

---

The correct solution is add finergrained permissions checking in to the access table - the access table should have always been a quadruplet:

(Repo, User, Unit, Mode)

Whether that is in Codd normal form as in a row per repo, user, unit, mode or it is in some more compressed form:

(Repo,User,Mode,Unit1, ..., UnitN)

or even:

(Repo, User, Mode, Units)

The problem is that we cannot backport such a change to 1.16. So fixing this is going to be complex.