Skip to content

Commit

Permalink
Consider gitlab inherited permissions (#3308)
Browse files Browse the repository at this point in the history
The gitlab projects endpoint does not include information about
permissions granted by namespace memberships. To get this information a
separate query to
https://docs.gitlab.com/ee/api/members.html#get-a-member-of-a-group-or-project-including-inherited-and-invited-members
is necessary.
  • Loading branch information
lukashass authored Feb 5, 2024
1 parent c7467b9 commit db4a509
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 46 deletions.
8 changes: 4 additions & 4 deletions server/forge/gitlab/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const (
mergeRefs = "refs/merge-requests/%d/head" // merge request merged with base
)

func (g *GitLab) convertGitLabRepo(_repo *gitlab.Project) (*model.Repo, error) {
func (g *GitLab) convertGitLabRepo(_repo *gitlab.Project, projectMember *gitlab.ProjectMember) (*model.Repo, error) {
parts := strings.Split(_repo.PathWithNamespace, "/")
owner := strings.Join(parts[:len(parts)-1], "/")
name := parts[len(parts)-1]
Expand All @@ -48,9 +48,9 @@ func (g *GitLab) convertGitLabRepo(_repo *gitlab.Project) (*model.Repo, error) {
Visibility: model.RepoVisibility(_repo.Visibility),
IsSCMPrivate: !_repo.Public,
Perm: &model.Perm{
Pull: isRead(_repo),
Push: isWrite(_repo),
Admin: isAdmin(_repo),
Pull: isRead(_repo, projectMember),
Push: isWrite(projectMember),
Admin: isAdmin(projectMember),
},
PREnabled: _repo.MergeRequestsEnabled,
}
Expand Down
37 changes: 35 additions & 2 deletions server/forge/gitlab/gitlab.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,20 @@ func (g *GitLab) getProject(ctx context.Context, client *gitlab.Client, forgeRem
return repo, err
}

func (g *GitLab) getInheritedProjectMember(ctx context.Context, client *gitlab.Client, forgeRemoteID model.ForgeRemoteID, owner, name string, userID int) (*gitlab.ProjectMember, error) {
if forgeRemoteID.IsValid() {
intID, err := strconv.Atoi(string(forgeRemoteID))
if err != nil {
return nil, err
}
projectMember, _, err := client.ProjectMembers.GetInheritedProjectMember(intID, userID, gitlab.WithContext(ctx))
return projectMember, err
}

projectMember, _, err := client.ProjectMembers.GetInheritedProjectMember(fmt.Sprintf("%s/%s", owner, name), userID, gitlab.WithContext(ctx))
return projectMember, err
}

// Repo fetches the repository from the forge.
func (g *GitLab) Repo(ctx context.Context, user *model.User, remoteID model.ForgeRemoteID, owner, name string) (*model.Repo, error) {
client, err := newClient(g.url, user.Token, g.SkipVerify)
Expand All @@ -258,7 +272,17 @@ func (g *GitLab) Repo(ctx context.Context, user *model.User, remoteID model.Forg
return nil, err
}

return g.convertGitLabRepo(_repo)
intUserID, err := strconv.Atoi(string(user.ForgeRemoteID))
if err != nil {
return nil, err
}

projectMember, err := g.getInheritedProjectMember(ctx, client, remoteID, owner, name, intUserID)
if err != nil {
return nil, err
}

return g.convertGitLabRepo(_repo, projectMember)
}

// Repos fetches a list of repos from the forge.
Expand All @@ -276,6 +300,10 @@ func (g *GitLab) Repos(ctx context.Context, user *model.User) ([]*model.Repo, er
if g.HideArchives {
opts.Archived = gitlab.Ptr(false)
}
intUserID, err := strconv.Atoi(string(user.ForgeRemoteID))
if err != nil {
return nil, err
}

for i := 1; true; i++ {
opts.Page = i
Expand All @@ -285,7 +313,12 @@ func (g *GitLab) Repos(ctx context.Context, user *model.User) ([]*model.Repo, er
}

for i := range batch {
repo, err := g.convertGitLabRepo(batch[i])
projectMember, _, err := client.ProjectMembers.GetInheritedProjectMember(batch[i].ID, intUserID, gitlab.WithContext(ctx))
if err != nil {
return nil, err
}

repo, err := g.convertGitLabRepo(batch[i], projectMember)
if err != nil {
return nil, err
}
Expand Down
11 changes: 9 additions & 2 deletions server/forge/gitlab/gitlab_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,9 @@ func Test_GitLab(t *testing.T) {
client := load(env)

user := model.User{
Login: "test_user",
Token: "e3b0c44298fc1c149afbf4c8996fb",
Login: "test_user",
Token: "e3b0c44298fc1c149afbf4c8996fb",
ForgeRemoteID: "3",
}

repo := model.Repo{
Expand Down Expand Up @@ -102,6 +103,12 @@ func Test_GitLab(t *testing.T) {
_, err := client.Repo(ctx, &user, "0", "not-existed", "not-existed")
assert.Error(t, err)
})

g.It("Should return repo with push access, when user inherits membership from namespace", func() {
_repo, err := client.Repo(ctx, &user, "6", "brightbox", "puppet")
assert.NoError(t, err)
assert.True(t, _repo.Perm.Push)
})
})

// Test activate method
Expand Down
44 changes: 6 additions & 38 deletions server/forge/gitlab/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,50 +39,18 @@ func newClient(url, accessToken string, skipVerify bool) (*gitlab.Client, error)

// isRead is a helper function that returns true if the
// user has Read-only access to the repository.
func isRead(proj *gitlab.Project) bool {
user := proj.Permissions.ProjectAccess
group := proj.Permissions.GroupAccess

switch {
case proj.Public:
return true
case user != nil && user.AccessLevel >= 20:
return true
case group != nil && group.AccessLevel >= 20:
return true
default:
return false
}
func isRead(proj *gitlab.Project, projectMember *gitlab.ProjectMember) bool {
return proj.Public || projectMember != nil && projectMember.AccessLevel >= gitlab.ReporterPermissions
}

// isWrite is a helper function that returns true if the
// user has Read-Write access to the repository.
func isWrite(proj *gitlab.Project) bool {
user := proj.Permissions.ProjectAccess
group := proj.Permissions.GroupAccess

switch {
case user != nil && user.AccessLevel >= 30:
return true
case group != nil && group.AccessLevel >= 30:
return true
default:
return false
}
func isWrite(projectMember *gitlab.ProjectMember) bool {
return projectMember != nil && projectMember.AccessLevel >= gitlab.DeveloperPermissions
}

// isAdmin is a helper function that returns true if the
// user has Admin access to the repository.
func isAdmin(proj *gitlab.Project) bool {
user := proj.Permissions.ProjectAccess
group := proj.Permissions.GroupAccess

switch {
case user != nil && user.AccessLevel >= 40:
return true
case group != nil && group.AccessLevel >= 40:
return true
default:
return false
}
func isAdmin(projectMember *gitlab.ProjectMember) bool {
return projectMember != nil && projectMember.AccessLevel >= gitlab.MaintainerPermissions
}
30 changes: 30 additions & 0 deletions server/forge/gitlab/testdata/projects.go
Original file line number Diff line number Diff line change
Expand Up @@ -304,3 +304,33 @@ var project4PayloadHooks = []byte(`
}
]
`)

var project4PayloadMembers = []byte(`
{
"id": 3,
"username": "some_user",
"name": "Diaspora",
"state": "active",
"locked": false,
"avatar_url": "https://example.com/uploads/-/system/user/avatar/3/avatar.png",
"web_url": "https://example.com/some_user",
"access_level": 50,
"created_at": "2024-01-16T12:39:58.912Z",
"expires_at": null
}
`)

var project6PayloadMembers = []byte(`
{
"id": 3,
"username": "some_user",
"name": "Diaspora",
"state": "active",
"locked": false,
"avatar_url": "https://example.com/uploads/-/system/user/avatar/3/avatar.png",
"web_url": "https://example.com/some_user",
"access_level": 30,
"created_at": "2024-01-16T12:39:58.912Z",
"expires_at": null
}
`)
10 changes: 10 additions & 0 deletions server/forge/gitlab/testdata/testdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ func NewServer(t *testing.T) *httptest.Server {
w.Write(project4Payload)
return
case "/api/v4/projects/brightbox/puppet":
case "/api/v4/projects/6":
w.Write(project6Payload)
return
case "/api/v4/projects/4/hooks":
Expand All @@ -60,6 +61,15 @@ func NewServer(t *testing.T) *httptest.Server {
case "/api/v4/projects/4/hooks/10717088":
w.WriteHeader(201)
return
case "/api/v4/projects/4/members/all/3":
w.Write(project4PayloadMembers)
return
case "/api/v4/projects/diaspora/diaspora-client/members/all/3":
w.Write(project4PayloadMembers)
return
case "/api/v4/projects/6/members/all/3":
w.Write(project6PayloadMembers)
return
case "/oauth/token":
w.Write(accessTokenPayload)
return
Expand Down

0 comments on commit db4a509

Please sign in to comment.