Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

[Backport 5.1] [gitlab] Fix GitLab internal project visibility#55359

Merged
coury-clark merged 1 commit into
5.1from
backport-54426-to-5.1
Aug 1, 2023
Merged

[Backport 5.1] [gitlab] Fix GitLab internal project visibility#55359
coury-clark merged 1 commit into
5.1from
backport-54426-to-5.1

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Original Slack thread here

Before this PR

We use a min_access_level=20 check on GitLab project queries, because access_level 10 is GUEST, and users with GUEST access cannot see repo contents, so we don't want to give access to those repos on Sourcegraph
HOWEVER, for Internal repos on GitLab, users can see the contents of the repo depending on how the repo is configured, regardless of their access level. So this puts us in a conundrum. If we have min_access_level=20, users can't see repos they should be able to see, but if we remove it, users can see repos they shouldn't be able to see

After this PR

Adds a feature flag named "gitLabProjectVisibilityExperimental". When set to true:

@indradhanush and I diffed some responses from the GitLab API, depending on whether or not you are part of the GitLab group or not, whether the project is visible or not, etc. etc. and there is a common field:
default_branch is only visible in the project API response if the user is able to see the contents of the project. If they cannot see the contents of the project, they cannot see the default branch of the project.
So by removing min_access_level=20 and checking for the presence of default_branch on the returned project list, we can determine whether or not the user should be able to see the repo contents or not

Test plan

VCR tests should be updated

<!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles -->

Backport 8cb61f8 from #54426

Original Slack thread
[here](https://sourcegraph.slack.com/archives/C05EMJM2SLR/p1687969384430849)

## Before this PR

We use a `min_access_level=20` check on GitLab project queries, because
`access_level` 10 is GUEST, and users with GUEST access cannot see repo
contents, so we don't want to give access to those repos on Sourcegraph
HOWEVER, for Internal repos on GitLab, users can see the contents of the
repo depending on how the repo is configured, regardless of their access
level. So this puts us in a conundrum. If we have `min_access_level=20`,
users can't see repos they should be able to see, but if we remove it,
users can see repos they shouldn't be able to see

## After this PR

Adds a feature flag named "gitLabProjectVisibilityExperimental". When
set to true:

@indradhanush and I diffed some responses from the GitLab API, depending
on whether or not you are part of the GitLab group or not, whether the
project is visible or not, etc. etc. and there is a common field:
`default_branch` is only visible in the project API response if the user
is able to see the contents of the project. If they cannot see the
contents of the project, they cannot see the default branch of the
project.
So by removing `min_access_level=20` and checking for the presence of
`default_branch` on the returned project list, we can determine whether
or not the user should be able to see the repo contents or not

## Test plan

VCR tests should be updated

<!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
-->

(cherry picked from commit 8cb61f8)
@github-actions github-actions Bot requested a review from pjlast July 27, 2023 13:15
@github-actions github-actions Bot added cla-signed iam backports team/source Tickets under the purview of Source - the one Source to graph it all labels Jul 27, 2023
@sourcegraph-bot

sourcegraph-bot commented Jul 27, 2023

Copy link
Copy Markdown
Contributor

📖 Storybook live preview

@coury-clark coury-clark merged commit 1411be1 into 5.1 Aug 1, 2023
@coury-clark coury-clark deleted the backport-54426-to-5.1 branch August 1, 2023 16:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

backports cla-signed team/source Tickets under the purview of Source - the one Source to graph it all

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants