-
Notifications
You must be signed in to change notification settings - Fork 27
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
Enabling OAuth for the origin and cache, adding support for Globus as another auth server #963
Conversation
@bbockelm any chance that I could get a review in soon? since this is a critical item for 7.7 release and Cannon's login UI change also depends on this PR to be merged, or do you want to hand this off to someone else? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very minor changes requested. Some of the formatting nitpicks I fixed up already in a pushed commit.
@CannonLock as I was going over the code to address the code review comments, I realized that I didn't use |
Hey @bbockelm, could you take another look at this PR? This one is blocking the release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revisions LGTM.
Closes #928
This PR also added API support for OAuth login for cache.
To accommodate the second OAuth provider, the login and callback urls for OAuth have breaking changes. This time we can barely do a backward compatible change due to the fact that the redirect URL as part of the shared OAuth configuration can't be changed in between different gin router handler (i.e.
/auth/cilogon/login
VS/auth/oauth/login
).Now the endpoints are renamed from
/auth/cilogon/login
and/auth/cilogon/callback
to/auth/oauth/login
and/auth/oauth/callback
respectively.This breaking change is documented in the Swagger and should be highlighted in the release notes too.
Another deprecate action is the
Registry.AdminUsers
, which is renamed toOIDC.AdminUsers
. This is still backward-compatible.This PR also addressed the first issue listed in #631 where if we have more than one server running, we direct user to login even if server(s) with a public-view are enabled (such as registry/director)
This PR also enforced the permission check on web pages, where the public can only access
/view/director
,/view/registry
, and/
page. For other pages, attempting to access without admin privilege will result in a 403 error with a string"You don't have the permission to view this page. If you think this is wrong, please contact your server admin."
.