Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cody: change auth endpoint to handle SAMS redirect #2957

Merged
merged 2 commits into from
Feb 1, 2024
Merged

cody: change auth endpoint to handle SAMS redirect #2957

merged 2 commits into from
Feb 1, 2024

Conversation

kalanchan
Copy link
Contributor

@kalanchan kalanchan commented Jan 30, 2024 •
edited by unknwon
Loading

Part of the accounts migration from sourcegraph.com to accounts.sourcegraph.com (SAMS). Using this new auth url that enables us to redirect to SAMS and back to dotcom.

I wasn't able to test the GitLab login properly as my account is too new, could someone help with this?

For more context, see Joe's explanation

Test plan

Pull the branch, make sure you are logged out of sourcegraph.com AND accounts.sourcegraph.com. Head to the editor, log out of Cody, confirm that each login has the right url and try logging in.

See each scenario below:

GitHub

login-github-sams.mp4

GitLab

login-gitlab-sams.mp4

Google

login-google-sams.mp4

@kalanchan kalanchan requested review from unknwon and a team January 30, 2024 22:55
@kalanchan kalanchan marked this pull request as draft January 31, 2024 01:22
@kalanchan kalanchan marked this pull request as ready for review January 31, 2024 01:23
unknwon
unknwon approved these changes Jan 31, 2024
View reviewed changes
Copy link
Member

@unknwon unknwon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks! Nice work, and world-class test plan!

But don't merge just yet! We need to coordinate some changes together on the server-side.

abeatrix
abeatrix approved these changes Jan 31, 2024
View reviewed changes
Copy link
Contributor

@abeatrix abeatrix left a comment •
edited
Loading

Choose a reason for hiding this comment

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

🚀 🚀 🚀

Edit: Haven't set up an instance to test, will wait for the server side change deployed to test

@unknwon unknwon marked this pull request as draft February 1, 2024 03:27
@unknwon
Copy link
Member

unknwon commented Feb 1, 2024

Converted to draft to prevent mis-merge.

@unknwon
Copy link
Member

unknwon commented Feb 1, 2024

All server-side changes should be ready now!

@unknwon unknwon mentioned this pull request Feb 1, 2024
Merged
@kalanchan kalanchan mentioned this pull request Feb 1, 2024
Merged
@unknwon unknwon marked this pull request as ready for review February 1, 2024 20:14
@kalanchan
Copy link
Contributor Author

@unknwon @abeatrix I'M MERGING GOGOG

@kalanchan kalanchan merged commit 739b497 into main Feb 1, 2024
30 checks passed
@kalanchan kalanchan deleted the kalan+joe/change-auth-URLs branch February 1, 2024 23:51
pkukielka pushed a commit to sourcegraph/jetbrains that referenced this pull request Feb 2, 2024
@unknwon
auth: change endpoints to handle SAMS redirect (#464)
1e7b01d 
Sister PR of sourcegraph/cody#2957, please that
that PR for full detail.

## Test plan

Locally tested with sourcegraph.com

![CleanShot 2024-02-01 at 14 22
25@2x](https://github.com/sourcegraph/jetbrains/assets/2946214/062753ff-e8c8-41ab-8836-a6c0e0b82426)
This was referenced Feb 5, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@unknwon unknwon unknwon approved these changes

@abeatrix abeatrix abeatrix approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kalanchan @unknwon @abeatrix