404 Repo Inaccessible for Users in Team w/ All Repo Access

[SoulSeekkor]

• Gitea version (or commit ref): 1.15.2 built with GNU Make 4.1, go1.16.7 : bindata, sqlite, sqlite_unlock_notify
• Git version: 2.33.0.2
• Operating system: Windows Server 2012 R2
  Running using your pre-built Win x64 binary. This is being started/stopped using a Windows service created per your installation instructions running as a local user with 'full control' of relevant folders.
• Database (use [x]):
  • PostgreSQL
  • MySQL
  • MSSQL
  • SQLite
• Can you reproduce the bug at https://try.gitea.io:
  • Yes (provide example URL)
  • No (Couldn't register or email was taking a super long time to come. May have to try this later.)

Description

1. Create new organization as admin user and make it private.
2. Create a new empty repo named 'test'.
3. Create new team named 'read-only' with all repo access, read, with everything toggled OFF except code.
4. Add a regular (and restricted) test user to the read-only team.
5. Attempt for that user to access the repo they should have access to will fail with a 404.

Note that creating the team and THEN creating the repo works fine, just not in reverse. The retroactively applied permissions for a team created after the repo was created is botched.

[zeripath]

We need logs.

[SoulSeekkor]

Relevant access.log entry:
x.x.x.x:64329 - testuser [15/Sep/2021:14:33:06 -0500] "GET /GiteaTest/test HTTP/2.0" 404 18396 "" "Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/93.0.4577.63 Safari/537.36 Edg/93.0.961.47"

Entries from the gitea.log:
2021/09/15 14:33:04 ...ices/auth/session.go:73:SessionUser() [T] Session Authorization: Logged in user 9:testuser
2021/09/15 14:33:04 models/repo.go:349:getUnits() [T] repo.Units: [0xc0093e98c0 0xc0093e9920 0xc0093e9980 0xc0093e99e0 0xc0093e9a40 0xc0093e9aa0]
2021/09/15 14:33:04 models/repo.go:349:getUnits() [T] repo.Units: [0xc009682c90 0xc009682cf0 0xc009682d50 0xc009682db0 0xc009682e10 0xc009682e70]
2021/09/15 14:33:04 ...s/repo_permission.go:154:func1() [T] Permission Loaded for 9:testuser in 106:GiteaTest/test:
Permissions: AccessMode: 0:none, 1 Units, 1 UnitsMode(s): [
Units[0]: ID: 772 RepoID: 106 Type: 1:UnitTypeCode Config: {}
UnitMode[1:UnitTypeCode]: 1:read ]
2021/09/15 14:33:04 ...s/repo_permission.go:154:func1() [T] Permission Loaded for 9:testuser in 106:GiteaTest/test:
Permissions: AccessMode: 0:none, 1 Units, 1 UnitsMode(s): [
Units[0]: ID: 772 RepoID: 106 Type: 1:UnitTypeCode Config: {}
UnitMode[1:UnitTypeCode]: 1:read ]
2021/09/15 14:33:04 ...s/context/context.go:740:1() [D] Session ID: f5d659a998deecb9
2021/09/15 14:33:04 ...s/context/context.go:741:1() [D] CSRF Token: 1997u8NBnd-DzYyjfxV0DVPlXyk6MTYzMTczMjg5OTMzODU1NjAwMA
2021/09/15 14:33:04 ...ices/auth/session.go:57:SessionUser() [T] Session Authorization: Found user[9]
2021/09/15 14:33:04 ...ices/auth/session.go:73:SessionUser() [T] Session Authorization: Logged in user 9:testuser
2021/09/15 14:33:06 ...s/context/context.go:740:1() [D] Session ID: f5d659a998deecb9
2021/09/15 14:33:06 ...s/context/context.go:741:1() [D] CSRF Token: 1997u8NBnd-DzYyjfxV0DVPlXyk6MTYzMTczMjg5OTMzODU1NjAwMA
2021/09/15 14:33:06 ...ices/auth/session.go:57:SessionUser() [T] Session Authorization: Found user[9]
2021/09/15 14:33:06 ...ices/auth/session.go:73:SessionUser() [T] Session Authorization: Logged in user 9:testuser
2021/09/15 14:33:06 models/repo.go:349:getUnits() [T] repo.Units: [0xc00924eb10 0xc00924eb70 0xc00924ebd0 0xc00924ec30 0xc00924ec90 0xc00924ecf0]
2021/09/15 14:33:06 ...s/repo_permission.go:154:func1() [T] Permission Loaded for 9:testuser in 106:GiteaTest/test:
Permissions: AccessMode: 0:none, 1 Units, 1 UnitsMode(s): [
Units[0]: ID: 772 RepoID: 106 Type: 1:UnitTypeCode Config: {}
UnitMode[1:UnitTypeCode]: 1:read ]
2021/09/15 14:33:06 ...s/context/context.go:185:HTML() [D] Template: status/404

[zeripath]

OK it looks like this works on 1.16/main right now.

[zeripath]

Just pulled docker 1.15.2 as above:

But was not able to reproduce the problem.

[SoulSeekkor]

Hmm odd, I was able to reliably perform this again to cause the problem. Just to confirm you created the repo first in the private org, created the team (with all repo access for read, code only), added the regular and restricted user to the team and were able to access the repo successfully?

[zeripath]

I didn't test the restricted user yet. But the regular one works.

[SoulSeekkor]

Okay, on Monday I'll test it with a regular user to see if that one works to confirm if it's specific to the restricted user or not.

[SoulSeekkor]

So I confirmed just now that with a regular (non-restricted) user I get the page you posted with "does not contain any content". So it appears to be something specific to a restricted user.

[noerw]

Can confirm with 1.15.3 + restricted user added after a repo is added.
The user can access the repo, once I toggle the private flag for a repo once (independent from the initial private state)
If I remember correctly, this was not an issue on 1.14.x

Note that the repos are listed on the explore page even if they can't be accessed, but that's a different issue I guess, as I discovered that already on 1.14 branch: #16281

[zeripath]

OK I've managed to make a testcase for this:

PATCH

diff --git a/integrations/org_test.go b/integrations/org_test.go
index ee61aae6f..ac234de65 100644
--- a/integrations/org_test.go
+++ b/integrations/org_test.go
@@ -5,10 +5,12 @@
 package integrations
 
 import (
+	"fmt"
 	"net/http"
 	"strings"
 	"testing"
 
+	api "code.gitea.io/gitea/modules/structs"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -110,3 +112,64 @@ func TestPrivateOrg(t *testing.T) {
 	req = NewRequest(t, "GET", "/privated_org/private_repo_on_private_org")
 	session.MakeRequest(t, req, http.StatusOK)
 }
+
+func TestOrgRestrictedUser(t *testing.T) {
+	defer prepareTestEnv(t)()
+
+	// privated_org is a private org who has id 23
+	orgName := "privated_org"
+
+	// public_repo_on_private_org is a public repo on privated_org
+	repoName := "public_repo_on_private_org"
+
+	// user29 is a restricted user who is not a member of the organization
+	restrictedUser := "user29"
+
+	// #17003 reports a bug whereby adding a restricted user to a read-only team doesn't work
+
+	// assert restrictedUser cannot see the org or the public repo
+	restrictedSession := loginUser(t, restrictedUser)
+	req := NewRequest(t, "GET", fmt.Sprintf("/%s", orgName))
+	restrictedSession.MakeRequest(t, req, http.StatusNotFound)
+
+	req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s", orgName, repoName))
+	restrictedSession.MakeRequest(t, req, http.StatusNotFound)
+
+	// Therefore create a read-only team
+	adminSession := loginUser(t, "user1")
+	token := getTokenForLoggedInUser(t, adminSession)
+
+	teamToCreate := &api.CreateTeamOption{
+		Name:                    "codereader",
+		Description:             "Code Reader",
+		IncludesAllRepositories: true,
+		Permission:              "read",
+		Units:                   []string{"repo.code"},
+	}
+
+	req = NewRequestWithJSON(t, "POST",
+		fmt.Sprintf("/api/v1/orgs/%s/teams?token=%s", orgName, token), teamToCreate)
+
+	var apiTeam api.Team
+
+	resp := adminSession.MakeRequest(t, req, http.StatusCreated)
+	DecodeJSON(t, resp, &apiTeam)
+	checkTeamResponse(t, &apiTeam, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
+		teamToCreate.Permission, teamToCreate.Units)
+	checkTeamBean(t, apiTeam.ID, teamToCreate.Name, teamToCreate.Description, teamToCreate.IncludesAllRepositories,
+		teamToCreate.Permission, teamToCreate.Units)
+	//teamID := apiTeam.ID
+
+	// Now we need to add the restricted user to the team
+	req = NewRequest(t, "PUT",
+		fmt.Sprintf("/api/v1/teams/%d/members/%s?token=%s", apiTeam.ID, restrictedUser, token))
+	_ = adminSession.MakeRequest(t, req, http.StatusNoContent)
+
+	// Now we need to check if the restrictedUser can access the repo
+	req = NewRequest(t, "GET", fmt.Sprintf("/%s", orgName))
+	restrictedSession.MakeRequest(t, req, http.StatusOK)
+
+	req = NewRequest(t, "GET", fmt.Sprintf("/%s/%s", orgName, repoName))
+	restrictedSession.MakeRequest(t, req, http.StatusOK)
+
+}
diff --git a/models/fixtures/repository.yml b/models/fixtures/repository.yml
index 6860d00b3..dc8ebecfc 100644
--- a/models/fixtures/repository.yml
+++ b/models/fixtures/repository.yml
@@ -568,7 +568,7 @@
 -
   id: 40
   owner_id: 23
-  owner_name: limited_org
+  owner_name: privated_org
   lower_name: public_repo_on_private_org
   name: public_repo_on_private_org
   is_private: false
@@ -581,7 +581,7 @@
 -
   id: 41
   owner_id: 23
-  owner_name: limited_org
+  owner_name: privated_org
   lower_name: private_repo_on_private_org
   name: private_repo_on_private_org
   is_private: true

[zeripath]

Sorry it's taken me so long to look at this. I think the related PR should solve this issue.