Implement OpenID Connect authentication #48
Conversation
Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>
Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>
Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>
Signed-off-by: Patrick Uiterwijk <puiterwijk@redhat.com>
|
Awesome work, thanks so much for the contribution! After a cursory review, this looks pretty good, but I'll give it a closer look soon. |
| if current_user.is_authenticated: | ||
| return | ||
|
|
||
| from modern_paste import oidc |
There was a problem hiding this comment.
Move this import to the top of the file?
| @@ -70,3 +73,45 @@ def auth_status(): | |||
| 'user_id': getattr(current_user, 'user_id', None), | |||
| }, | |||
| }), constants.api.SUCCESS_CODE | |||
|
|
|||
|
|
|||
| def oidc_request_loader(): | |||
There was a problem hiding this comment.
This function is not an API handler and doesn't belong in this file. Maybe create a new file app/util/oidc.py and move this there?
| # An OAuth2 Bearer Token was sent | ||
| token = flask.request.headers['Authorization'].split(' ', 1)[1].strip() | ||
| valid = oidc.validate_token(token, config.AUTH_OIDC_SCOPE) | ||
| if valid is True: |
| except UserDoesNotExistException: | ||
| info = oidc.user_getinfo(['sub', 'name', 'email'], token) | ||
| else: | ||
| return flask.jsonify(constants.api.AUTH_FAILURE), constants.api.AUTH_FAILURE_CODE |
There was a problem hiding this comment.
This function isn't an API handler, what is the meaning of returning an API response here?
| @@ -39,7 +39,10 @@ def view_function(): | |||
| """ | |||
| @wraps(func) | |||
| def decorated_view(*args, **kwargs): | |||
| template, context = func(*args, **kwargs) | |||
| response = func(*args, **kwargs) | |||
There was a problem hiding this comment.
This change seems to be unrelated to this PR. What is the motivation? @view_function is a deliberate abstraction requiring view functions to always return a tuple of (template name, context object) and this logic overrides that purpose.
| import models | ||
| from views import * | ||
|
|
||
| def setup_oidc(): | ||
| from flask_oidc import OpenIDConnect |
There was a problem hiding this comment.
Move these imports to the top of the file?
| @@ -113,6 +117,7 @@ | |||
| </div> | |||
| </div> | |||
|
|
|||
| {% if auth_method == 'local' %} | |||
| @@ -10,6 +10,20 @@ | |||
|
|
|||
|
|
|||
| @app.context_processor | |||
| def add_config(): | |||
There was a problem hiding this comment.
I'm not sure if this is necessary; there is already an abstraction in place to provide the config to the template during server-side rendering in api.decorators.context_config()
| return 'user/login.html', {} | ||
| auth_method = config.AUTH_METHOD | ||
| if auth_method == 'oidc': | ||
| from modern_paste import oidc |
| @@ -0,0 +1 @@ | |||
| {"web": {"redirect_uris": ["https://paste/oidc_callback"], "token_uri": "https://idp/openidc/Token", "auth_uri": "https://idp/openidc/Authorization", "client_id": "myclient", "client_secret": "thesecret", "userinfo_uri": "https://idp/openidc/UserInfo", "token_introspection_uri": "https://idp/openidc/TokenInfo", "issuer": "https://idp/openidc/"}} | |||
There was a problem hiding this comment.
Doesn't seem like this secrets file should be versioned. Can you delete this file and instead document the shape of the secrets object in the README?
No description provided.