[Generic] Extra Parameter during runtime

[kreuzert]

Hey everyone,

we're using the GenericOAuthenticator (connected to Unity-IdM) with this small additional extra. This allows us to offer different Login views for different user groups.

Usage: Simply add "extra_param_key=value" argument to your oauth_login path.

localhost:8000/hub/oauth_login?extra_param_uy_select_authn=specificAuthenticator

This will add uy_select_authn=specificAuthenticator as an argument to the authorize web entry URL.

This offers more possibilities than the more static authenticator.extra_authorize_params. Furthermore, it will override the value in authenticator.extra_authorize_params. This way you can use authenticator.extra_authorize_params as a default and add the extra_param_key=value arguments for specific users.

Best regards, Tim Kreuzer

[welcome[bot]]

Thanks for submitting your first pull request! You are awesome! :hugs:
If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly. You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! :wave:
Welcome to the Jupyter community! :tada:

[manics]

Any there any potential security issues with passing through arbitrary parameters unchecked to the oauth provider?

[kreuzert]

Any there any potential security issues with passing through arbitrary parameters unchecked to the oauth provider?

That's a valid point. I've added a parameter to define allowed keys and values.

[minrk]

Apologies for not reviewing more promptly. It seems like this functionality might belong more directly on the generic.py instead of the base class. I can't see it being used elsewhere. Additionally, I think it might be simpler to allow GenericOAuthenticator.extra_params to be a callable that returns an arbitrary dict, which would let more generic logic take over. There's some pretty specific transforms here that I'm not sure make sense in general, though they should be at least feasible via configuration or subclassing.

[kreuzert]

No problem. I've used your suggestions for an update. That's based on the 14.0.0 tag. Should I close this PR and open a new one?

https://github.com/kreuzert/oauthenticator/tree/14.0.0

[manics]

@kreuzert I think it's fine to force push to this branch.

[kreuzert]

Tested with this configuration snippet:

jupyterhub_config.py:

from oauthenticator.generic import GenericOAuthenticator
c.JupyterHub.authenticator_class = GenericOAuthenticator

def foo():
    ret = {
        "key1": ["value1", "value2"]
    }
    return ret

c.GenericOAuthenticator.extra_params_allowed_runtime = foo

c.GenericOAuthenticator.extra_authorize_params = {
    "key1": "value1"
}

#c.GenericOAuthenticator.extra_params_allowed_runtime = {
#    "key1": ["value1", "value2"]
#}

[consideRatio]

I'm working to cut a release and making a quicker review pass of various PRs. I found that this can't be merged in its current state and I'm a bit low on time to make a in depth review, but some quick technical points:

• A new class was defined, but it was never used
• No documentation or tests were part of PR