This repository was archived by the owner on Sep 30, 2024. It is now read-only.
[permissions] Revert default behaviour for explicit permissions#54419
Merged
Conversation
…behaviours for explicit permissions
Contributor
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 9e78e1c...c3c33c9.
|
indradhanush
left a comment
Contributor
There was a problem hiding this comment.
I think this looks good to me, but this is tricky and another pair of eyes for confirmation would be nice.
mrnugget
approved these changes
Jun 29, 2023
Co-authored-by: Thorsten Ball <mrnugget@gmail.com>
mrnugget
reviewed
Jun 29, 2023
indradhanush
approved these changes
Jun 29, 2023
github-actions Bot
pushed a commit
that referenced
this pull request
Jun 29, 2023
(cherry picked from commit 5ad2474)
coury-clark
reviewed
Jun 29, 2023
|
|
||
| if tc.explicitPermsEnabled { | ||
| globals.SetPermissionsUserMapping(&schema.PermissionsUserMapping{ | ||
| BindID: "email", |
Contributor
There was a problem hiding this comment.
Can we get a test for all the bind types? As I understand the offending type was username binding.
Contributor
Author
There was a problem hiding this comment.
Bind types wouldn't affect anything here, as they're just used to determine what goes into the table.
The issue is what happens once things are already in the table, which is that the table is not being applied at all because the code host connection itself is marked as Unrestricted.
coury-clark
pushed a commit
that referenced
this pull request
Jun 29, 2023
…rmissions (#54439) With the switch to the `user_repo_permissions` table, the default behaviour for explicit permissions changed. Previously, if explicit permissions API was enabled, then all repositories would be treated as restricted, regardless of code host connection configurations. The `user_repo_permissions` table allows us to run permissions syncs and explicit permissions in parallel, but the default behaviour was to have code host connections be marked as unrestricted, unless authorization is enabled. However, not all code hosts support authorization, leading to connections that are always left unrestricted, even if explicit permissions are being used. This PR reverts the default behaviour back to treating a repository as restricted if explicit permissions are enabled. ## Test plan Added unit tests for the new Unrestricted logic. Also tested locally with an "Other Git code host" connection. <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles --> <br> Backport 5ad2474 from #54419 Co-authored-by: Petri-Johan Last <petri.last@sourcegraph.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With the switch to the
user_repo_permissionstable, the default behaviour for explicit permissions changed. Previously, if explicit permissions API was enabled, then all repositories would be treated as restricted, regardless of code host connection configurations.The
user_repo_permissionstable allows us to run permissions syncs and explicit permissions in parallel, but the default behaviour was to have code host connections be marked as unrestricted, unless authorization is enabled. However, not all code hosts support authorization, leading to connections that are always left unrestricted, even if explicit permissions are being used.This PR reverts the default behaviour back to treating a repository as restricted if explicit permissions are enabled.
Test plan
Added unit tests for the new Unrestricted logic. Also tested locally with an "Other Git code host" connection.