Skip to content

Commit

Permalink
Fix system admin cannot fork or get private fork with API (go-gitea#3…
Browse files Browse the repository at this point in the history
…3401) (go-gitea#33417)

Backport go-gitea#33401 by @lunny

Fix go-gitea#33368

Co-authored-by: Lunny Xiao <[email protected]>
  • Loading branch information
GiteaBot and lunny authored Jan 27, 2025
1 parent 2674d27 commit b6fd874
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 13 deletions.
16 changes: 9 additions & 7 deletions routers/api/v1/repo/fork.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,13 +132,15 @@ func CreateFork(ctx *context.APIContext) {
}
return
}
isMember, err := org.IsOrgMember(ctx, ctx.Doer.ID)
if err != nil {
ctx.Error(http.StatusInternalServerError, "IsOrgMember", err)
return
} else if !isMember {
ctx.Error(http.StatusForbidden, "isMemberNot", fmt.Sprintf("User is no Member of Organisation '%s'", org.Name))
return
if !ctx.Doer.IsAdmin {
isMember, err := org.IsOrgMember(ctx, ctx.Doer.ID)
if err != nil {
ctx.Error(http.StatusInternalServerError, "IsOrgMember", err)
return
} else if !isMember {
ctx.Error(http.StatusForbidden, "isMemberNot", fmt.Sprintf("User is no Member of Organisation '%s'", org.Name))
return
}
}
forker = org.AsUser()
}
Expand Down
8 changes: 5 additions & 3 deletions services/repository/fork.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,9 +258,11 @@ type findForksOptions struct {
}

func (opts findForksOptions) ToConds() builder.Cond {
return builder.Eq{"fork_id": opts.RepoID}.And(
repo_model.AccessibleRepositoryCondition(opts.Doer, unit.TypeInvalid),
)
cond := builder.Eq{"fork_id": opts.RepoID}
if opts.Doer != nil && opts.Doer.IsAdmin {
return cond
}
return cond.And(repo_model.AccessibleRepositoryCondition(opts.Doer, unit.TypeInvalid))
}

// FindForks returns all the forks of the repository
Expand Down
33 changes: 31 additions & 2 deletions tests/integration/api_fork_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
auth_model "code.gitea.io/gitea/models/auth"
"code.gitea.io/gitea/models/db"
org_model "code.gitea.io/gitea/models/organization"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
api "code.gitea.io/gitea/modules/structs"
Expand Down Expand Up @@ -81,8 +82,8 @@ func TestAPIForkListLimitedAndPrivateRepos(t *testing.T) {
var forks []*api.Repository
DecodeJSON(t, resp, &forks)

assert.Len(t, forks, 1)
assert.EqualValues(t, "1", resp.Header().Get("X-Total-Count"))
assert.Len(t, forks, 2)
assert.EqualValues(t, "2", resp.Header().Get("X-Total-Count"))

assert.NoError(t, org_service.AddTeamMember(db.DefaultContext, ownerTeam2, user1))

Expand All @@ -96,3 +97,31 @@ func TestAPIForkListLimitedAndPrivateRepos(t *testing.T) {
assert.EqualValues(t, "2", resp.Header().Get("X-Total-Count"))
})
}

func TestGetPrivateReposForks(t *testing.T) {
defer tests.PrepareTestEnv(t)()

user1Sess := loginUser(t, "user1")
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) // private repository
privateOrg := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 23})
user1Token := getTokenForLoggedInUser(t, user1Sess, auth_model.AccessTokenScopeWriteRepository)

forkedRepoName := "forked-repo"
// create fork from a private repository
req := NewRequestWithJSON(t, "POST", "/api/v1/repos/"+repo2.FullName()+"/forks", &api.CreateForkOption{
Organization: &privateOrg.Name,
Name: &forkedRepoName,
}).AddTokenAuth(user1Token)
MakeRequest(t, req, http.StatusAccepted)

// test get a private fork without clear permissions
req = NewRequest(t, "GET", "/api/v1/repos/"+repo2.FullName()+"/forks").AddTokenAuth(user1Token)
resp := MakeRequest(t, req, http.StatusOK)

forks := []*api.Repository{}
DecodeJSON(t, resp, &forks)
assert.Len(t, forks, 1)
assert.EqualValues(t, "1", resp.Header().Get("X-Total-Count"))
assert.EqualValues(t, "forked-repo", forks[0].Name)
assert.EqualValues(t, privateOrg.Name, forks[0].Owner.UserName)
}
3 changes: 2 additions & 1 deletion tests/integration/repo_fork_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ func TestForkListLimitedAndPrivateRepos(t *testing.T) {
req := NewRequest(t, "GET", "/user2/repo1/forks")
resp := user1Sess.MakeRequest(t, req, http.StatusOK)
htmlDoc := NewHTMLParser(t, resp.Body)
assert.EqualValues(t, 1, htmlDoc.Find(forkItemSelector).Length())
// since user1 is an admin, he can get both of the forked repositories
assert.EqualValues(t, 2, htmlDoc.Find(forkItemSelector).Length())

assert.NoError(t, org_service.AddTeamMember(db.DefaultContext, ownerTeam2, user1))
resp = user1Sess.MakeRequest(t, req, http.StatusOK)
Expand Down

0 comments on commit b6fd874

Please sign in to comment.