auth: use invenio session cookie to retrieve user #160
Conversation
leticiawanderley
changed the title
leticiawanderley
force-pushed
the
oauth-use-session-cookie
branch
6 times, most recently
from
5635d8e to
d80298a
Compare
leticiawanderley
force-pushed
the
oauth-use-session-cookie
branch
from
d80298a to
5c4c73b
Compare
reana_server/rest/ping.py
Outdated
| @@ -10,7 +10,7 @@ | |||
|
|
|||
| from flask import Blueprint, jsonify | |||
|
|
|||
| blueprint = Blueprint('ping', __name__) | |||
| blueprint = Blueprint('ping', __name__, url_prefix='/reana-api') | |||
There was a problem hiding this comment.
We can't use /api because of Invenio's /api? If that's the case, we will have to fix the auth on the API application since this is going to break reana-client's calls.
| if current_user.is_authenticated: | ||
| user = _get_user_from_invenio_user(current_user.email) | ||
| else: | ||
| user = get_user_from_token(request.args.get('access_token')) |
There was a problem hiding this comment.
updated
Issue to be created once the PR is merged.
Investigate usage of OAuth2Server to generate tokens so we do not need to generate and maintain our own, see OAuth2Server CLI tokens create
Note: this would interfere with the implementation of the interactive sessions. We are configuring the Notebook to enable token authentication and the token we pass, is the REANA access token. Using token authentication is the most straight-forward implementation, but from now on, we will have a login, and eventually one can generate tokens to access the application from CLI, to be solved in the future...
| @@ -112,7 +112,10 @@ def add_secrets(): # noqa | |||
| } | |||
| """ | |||
| try: | |||
| user = get_user_from_token(request.args.get("access_token")) | |||
| if current_user.is_authenticated: | |||
There was a problem hiding this comment.
Can we use the login_required decorator? Otherwise, we need to return a 403 if the user is not authenticated nor with the session nor with the token (before we were throwing a ValueError on get_user_from_token, catching it and throwing the 403).
tests/test_views.py
Outdated
| ) | ||
| assert res.status_code == 200 | ||
| mock_current_user = Mock(is_authenticated=False) | ||
| with patch('flask_login.utils._get_user', |
There was a problem hiding this comment.
You can use the mock.patch.multiple so it is a bit less indented :) see in RWC.
| @@ -112,7 +112,10 @@ def add_secrets(): # noqa | |||
| } | |||
There was a problem hiding this comment.
Originally, we have set the access_token as a required parameter. This is no longer true, can you please update the docstring? This will be applicable for all endpoints which need authentication :).
tests/test_views.py
Outdated
| ): | ||
| res = client.get( | ||
| url_for("workflows.get_workflows"), | ||
| query_string={"user_id": default_user.id_, |
There was a problem hiding this comment.
This was not introduced by you, but since you are changing the code, could you remove the user_id parameter? It is no longer in the API. Thanks!
tests/test_views.py
Outdated
| make_mock_api_client("reana-workflow-controller")(), | ||
| ): | ||
| # access token needs to be passed instead of user_id | ||
| res = client.post( |
There was a problem hiding this comment.
We can remove this access token needs to be passed instead of user_id part of the test.
* With the used Invenio packages Flask-Menu is not initialised but required by the installed ones (accessing current_menu) so we initialise it in REANA Flask extension.
Addresses reanahub#153 Signed-off-by: Leticia Farias Wanderley <leticia.farias.wanderley@cern.ch>
leticiawanderley
force-pushed
the
oauth-use-session-cookie
branch
6 times, most recently
from
48be29f to
4b15bce
Compare
tests/test_views.py
Outdated
| @@ -24,376 +24,325 @@ | |||
|
|
|||
| from reana_server.utils import _create_and_associate_reana_user | |||
|
|
|||
| GET_USER_MOCK = Mock(return_value=Mock(is_authenticated=False)) | |||
There was a problem hiding this comment.
We could extract this and the mock:
with patch("flask_login.utils._get_user", GET_USER_MOCK):
...To a new fixture in conftest so the test runs inside this context, for example here we do it. I think it will simplify a bit more the changes.
leticiawanderley
force-pushed
the
oauth-use-session-cookie
branch
from
4b15bce to
ac96d9a
Compare
Addresses reanahub#153 Signed-off-by: Leticia Farias Wanderley <leticia.farias.wanderley@cern.ch>
leticiawanderley
force-pushed
the
oauth-use-session-cookie
branch
from
ac96d9a to
b2022f4
Compare
No description provided.