Skip to content

Commit

Permalink
Ensure that restricted users can access repos for which they are memb…
Browse files Browse the repository at this point in the history
…ers (#17460)

There is a small bug in the way that repo access is checked in
repoAssignment: Accessibility is checked by checking if the user has a
marked access to the repository instead of checking if the user has any
team granted access.

This PR changes this permissions check to use HasAccess() which does the
correct test. There is also a fix in the release api ListReleases where
it should return draft releases if the user is a member of a team with
write access to the releases.

The PR also adds a testcase.

Signed-off-by: Andrew Thornton <art27@cantab.net>
  • Loading branch information
zeripath authored Oct 28, 2021
1 parent 2b2eb5d commit 0b4a8be
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 4 deletions.
63 changes: 63 additions & 0 deletions integrations/org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
package integrations

import (
"fmt"
"net/http"
"strings"
"testing"

api "code.gitea.io/gitea/modules/structs"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -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)

}
4 changes: 2 additions & 2 deletions models/fixtures/repository.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion modules/context/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ func repoAssignment(ctx *Context, repo *models.Repository) {
}

// Check access.
if ctx.Repo.Permission.AccessMode == models.AccessModeNone {
if !ctx.Repo.Permission.HasAccess() {
if ctx.FormString("go-get") == "1" {
EarlyResponseForGoGetMeta(ctx)
return
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/repo/release.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func ListReleases(ctx *context.APIContext) {

opts := models.FindReleasesOptions{
ListOptions: listOptions,
IncludeDrafts: ctx.Repo.AccessMode >= models.AccessModeWrite,
IncludeDrafts: ctx.Repo.AccessMode >= models.AccessModeWrite || ctx.Repo.UnitAccessMode(models.UnitTypeReleases) >= models.AccessModeWrite,
IncludeTags: false,
IsDraft: ctx.FormOptionalBool("draft"),
IsPreRelease: ctx.FormOptionalBool("pre-release"),
Expand Down

0 comments on commit 0b4a8be

Please sign in to comment.