-
-
Notifications
You must be signed in to change notification settings - Fork 53
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
Add support for Auth0 based on Keycloak implementation #111
Comments
Regarding issue 1, each auth0 token contains a 'sub' claim for the unique id. That would ideally be the approach to take rather than a custom claim since it would remain OIDC conformant. (Please correct me if im wrong, I am assuming the I would also be interested in seeing how the identity option can be expanded. Here is some information about the OIDC standard. |
Sounds like it wouldn't take much for Auth0 to be supported. Regarding the bug, seems like if username cannot be found the server should return a 401 error. |
@Jready If I understand the code correctly, this 'openid_claims' is used for figuring out which claims should be looked at, but the claim containing the username is hardcoded, being 'unique_name'. See here. It would need a bit of restructuring of the code, or another key in the config that indicates the claim containing the username. But then that claim name should also be in the openid_claims config key, otherwise the code will not loop over it. @bilalshaikh42 I agree it would be better to conform to the OpenID standards. It only requires you to first figure out the userID from auth0 before you can allow access to a resource on the HSDS server, but that might not be that big of an issue. |
I've checked in a one-liner to raise a 401 if the username can not be found. @mkatgert - could you submit a PR to support Auth0 authentication? I'm not really setup to test Auth0 myself. |
It would take some work, but https://github.com/HDFGroup/hsds/blob/master/hsds/util/jwtUtil.py could be replaced with a standard library that can load the auth provider from the config. Rather than a list of providers, the config could have a field for the discovery endpoint in the form of Here are some libraries that could work: https://oauth.net/code/python/ I can try to take a shot at it after my semester wraps up in late December. We can implement it as a separate file at first to prevent breaking changes until it is ready. @mkatgert If you are able to work on it earlier, I'd be happy to help. |
I've been experimenting with getting an external authentication service to work with the HSDS server. Keycloak was an option but I was trying out if other identity providers would also work.
I managed to get it to work for Auth0, but it required some meddling with the source code of the server.
Basically, the JWT token that is supplied by Auth0 is approximately the same as the token supplied by Keycloak, and the verification of the token signing is actually the same, it just requires using the correct Auth0 verification endpoint. Once the token is read in correctly, I ran into two issues with getting the claim from the token containing the username.
The Keycloak implementation requires a claim called 'unique_name', which is not present in a Auth0 token. Actually, the username is not present in the token at all, but it is possible to add 'custom' claims in the token within Auth0. I just pointed the code to the right (custom) claim, and everything else worked fine.
So, issue 1 is this: currently it is impossible (other than changing the code) to make the name of the claim containing the username variable. Doing so would allow for support for other identity providers.
Issue 2 is a bug: if the username can not be found in the token, the server responds with a 500. This is because there is a
return None
in the code if the username is not found (see here) whereas the code calling this function expects three parameters returned from the function.I'd like to hear your opinion on implementing other identity providers than Keycloak.
The text was updated successfully, but these errors were encountered: