HDFGroup / hsds

Cloud-native, service based access to HDF data
https://www.hdfgroup.org/solutions/hdf-kita/
Apache License 2.0
126 stars 52 forks source link

Add support for Auth0 based on Keycloak implementation #111

Open mkatgert opened 2 years ago

mkatgert commented 2 years ago

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.

bilalshaikh42 commented 2 years ago

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 unique_name refers to a user id). I am not that familiar with Keycloack, but if it also supports OIDC then it should too have a 'sub' claim available. Using that claim would allow for a more vendor-neutral approach.

I would also be interested in seeing how the identity option can be expanded. Here is some information about the OIDC standard. https://openid.net/connect/faq/ \

jreadey commented 2 years ago

Sounds like it wouldn't take much for Auth0 to be supported. There's an "openid_claims" in config.yml. Can that be override to add 'unique_name'?

Regarding the bug, seems like if username cannot be found the server should return a 401 error.

mkatgert commented 2 years ago

@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.

jreadey commented 2 years ago

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.

bilalshaikh42 commented 2 years ago

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 https://{oauth-provider-hostname}/.well-known/openid-configuration This would allow for any auth provider to be used, not just Auth0.

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.