This repository was archived by the owner on Sep 30, 2024. It is now read-only.
Add Bitbucket Cloud as an auth provider#46309
Merged
Merged
Conversation
Bundle size report 📦
Look at the Statoscope report for a full comparison between the commits 24b5c73 and 57f3ed1 or learn more. Open explanation
|
Contributor
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 57f3ed1...24b5c73.
|
kopancek
reviewed
Jan 11, 2023
Comment on lines
+100
to
+106
| // If allowSignup is true, we will create an account using the first verified | ||
| // email address from Bitbucket which we expect to be their primary address. Note | ||
| // that the order of attempts is important. If we manage to connect with an | ||
| // existing account we return early and don't attempt to create a new account. | ||
| if s.allowSignup { | ||
| attempts = append(attempts, attemptConfig{ | ||
| email: emails[0].Email, |
Contributor
There was a problem hiding this comment.
I'm confused here. The comment says we try the first verified email address, but as far as I can see, we try with the first email address, which might or might not be verified.
Also, should we try with all verified email addresses instead of just the first one?
Contributor
There was a problem hiding this comment.
+1 here. Why not use verifiedEmails?
Contributor
Author
There was a problem hiding this comment.
Yep that should be verifiedEmails[0].
I'm mimmicing the way we do the GitHub provider here, which only tries the first verified email. I'm guessing the logic is that, if the account does not exist, this should succeed.
Although I'm curious now how we handle the GitHub scenario if signup is restricted to certain addresses and the verified address that does not match the restriction is not the first address. Will have to check
| "default": "account,email,repository", | ||
| "enum": ["account", "email", "repository:read"] | ||
| }, | ||
| "allowSignup": { |
Contributor
There was a problem hiding this comment.
We should think about how to filter users for signup, similar to how we allow filtering users who can signup via SAML or Github auth providers.
Contributor
Author
There was a problem hiding this comment.
Bitbucket has the concept of "Workspaces" which we could probably leverage for this. However, I do think I'd want to add that as a follow-up feature instead of adding more complexity here.
mrnugget
approved these changes
Jan 11, 2023
Comment on lines
+100
to
+106
| // If allowSignup is true, we will create an account using the first verified | ||
| // email address from Bitbucket which we expect to be their primary address. Note | ||
| // that the order of attempts is important. If we manage to connect with an | ||
| // existing account we return early and don't attempt to create a new account. | ||
| if s.allowSignup { | ||
| attempts = append(attempts, attemptConfig{ | ||
| email: emails[0].Email, |
Contributor
There was a problem hiding this comment.
+1 here. Why not use verifiedEmails?
239425f to
88f7b57
Compare
Contributor
|
❌ Problem: the label |
0c9834e to
4a2747c
Compare
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.
5 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.
This PR adds Bitbucket Cloud as an Auth Provider option.
This allows users to sign into Sourcegraph using Bitbucket Cloud.
This is for sign-in and account creation only. It does not enable permissions syncing for Bitbucket Cloud.
Loom: https://www.loom.com/share/035a812559fb4e0b9629e74f9772b6b4
Test plan
Unit tests added, as well as a manual test (see Loom)