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.

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

Change default oAuth Host and allow config #126

Merged
merged 2 commits into from
Feb 20, 2020
Merged

Conversation

shaunanoordin
Copy link
Member

@shaunanoordin shaunanoordin commented Jan 31, 2020

PR Overview

This PR changes PJC's oAuth host:

  • The default oAuth host is now www.zooniverse.org instead of panoptes.zooniverse.org, to reflect current changes to the backend. That's right! oAuth users now sign in via our "main domain" instead of the panoptes. sub-domain.
  • The oAuth host can now also be changed either by setting ?oauth-host=... in the browser, or setting OAUTH_HOST in the env build variables. This brings the oAuth host config setting in line with other editable items in the PJC's config.

To Custom Front End developers:

  • This change will allow single sign on across all *.zooniverse.org domains (if you use the oauth.js code)
  • If you implement these changes, you should see no changes to sign-in functionality. The only difference should be that when users login, they see a different Zooniverse.org domain asking for their user/pass.
  • Users who are still logged in when you push the changes, however, might find themselves logged out afterwards.

Tested using a local copy of PJC and localhost Education API Front End, which was used to verify the single sign on.

Status

Ready for review. If all goes well, this might only require a minor version bump since it's not breaking any existing functionality - but let me know if you think otherwise.

@shaunanoordin shaunanoordin requested review from camallen and a team January 31, 2020 15:23
Copy link
Contributor

@camallen camallen left a comment

Choose a reason for hiding this comment

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

LGTM

I think this can be a patch version bump. I can't think of any problems for CFEs auto upgrading to this version of the library. I think most users of this code will want these changes

Copy link
Contributor

@srallen srallen left a comment

Choose a reason for hiding this comment

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

For semantic versioning, though, I think minor version bump is more appropriate since this isn't a bug fix, but introducing an API change to allow single sign on.

@srallen
Copy link
Contributor

srallen commented Feb 20, 2020

@shaunanoordin are you ok with merge, versioning, and publishing of this?

@shaunanoordin
Copy link
Member Author

👍 Can do! I'll get this merged and then bump the version. Uh, let me pull up my notes on how to public PJC though.

@shaunanoordin shaunanoordin merged commit 6254ca7 into master Feb 20, 2020
@shaunanoordin shaunanoordin deleted the config-oauth-host branch January 20, 2023 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants