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

[gitlab] Disable repo permissions syncing if auth type is oauth#51452

Merged
pjlast merged 8 commits into
mainfrom
pjlast/gitlab-disable-repo-perms-syncs-if-oauth
May 4, 2023
Merged

[gitlab] Disable repo permissions syncing if auth type is oauth#51452
pjlast merged 8 commits into
mainfrom
pjlast/gitlab-disable-repo-perms-syncs-if-oauth

Conversation

@pjlast

@pjlast pjlast commented May 4, 2023

Copy link
Copy Markdown
Contributor

Disables repo-based permissions syncing if the auth provider type for a GitLab code host connection is set as oauth. Repo-based permissions syncing has a lot of gotchas when it comes to token permissions, and they are very hard to debug.

In particular, when GitLab Groups are involved, which users a token can and cannot see become a bit unpredictable.

Test plan

Updated unit tests and verified locally by doing the following:

Set up a GitLab Project
Invite a group that the owner of the GitLab Project is not a part of
Set up a code host connection using the owner of the project's Personal Access Token

Before this PR:
Users of the invited group would lose access to the project whenever the project's repository permissions are synced, and they would regain access when their own permissions are synced.

After this PR:
User access is consistent, as access is only managed by their own token.

@cla-bot cla-bot Bot added the cla-signed label May 4, 2023
@pjlast pjlast requested a review from a team May 4, 2023 08:24
Comment thread enterprise/internal/authz/gitlab/oauth.go
@sourcegraph-bot

sourcegraph-bot commented May 4, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff d18f9de...5cd5710.

Notify File(s)
@sourcegraph/delivery doc/admin/external_service/gitlab.md
@unknwon enterprise/internal/authz/gitlab/BUILD.bazel
enterprise/internal/authz/gitlab/oauth.go
enterprise/internal/authz/gitlab/oauth_test.go
enterprise/internal/authz/gitlab/sudo_test.go

@kopancek kopancek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested some wording changes

Comment thread CHANGELOG.md Outdated
Comment thread doc/admin/external_service/gitlab.md Outdated
pjlast and others added 2 commits May 4, 2023 11:40
Co-authored-by: Milan Freml <kopancek@users.noreply.github.com>
Co-authored-by: Milan Freml <kopancek@users.noreply.github.com>
@pjlast pjlast merged commit f20f1ed into main May 4, 2023
@pjlast pjlast deleted the pjlast/gitlab-disable-repo-perms-syncs-if-oauth branch May 4, 2023 11:44
github-actions Bot pushed a commit that referenced this pull request May 4, 2023
unknwon pushed a commit that referenced this pull request May 4, 2023
… is oauth (#51460)

Disables repo-based permissions syncing if the auth provider type for a
GitLab code host connection is set as oauth. Repo-based permissions
syncing has a lot of gotchas when it comes to token permissions, and
they are _very_ hard to debug and fundamentally flawed.

## Test plan

Updated unit tests

&lt;!-- All pull requests REQUIRE a test plan:
https://docs.sourcegraph.com/dev/background-information/testing_principles
--&gt;
 <br> Backport f20f1ed from #51452

Co-authored-by: Petri-Johan Last <petri.last@sourcegraph.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants