Skip to content
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

auth: use invenio session cookie to retrieve user #160

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 38 additions & 38 deletions docs/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@
"operationId": "get_secrets",
"parameters": [
{
"description": "Required. Secrets owner access token.",
"description": "Secrets owner access token.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
}
],
Expand Down Expand Up @@ -150,10 +150,10 @@
"operationId": "delete_secrets",
"parameters": [
{
"description": "Required. API key of the admin.",
"description": "API key of the admin.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
},
{
Expand Down Expand Up @@ -233,10 +233,10 @@
"operationId": "add_secrets",
"parameters": [
{
"description": "Required. Secrets owner access token.",
"description": "Secrets owner access token.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
},
{
Expand Down Expand Up @@ -495,10 +495,10 @@
"operationId": "get_workflows",
"parameters": [
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
},
{
Expand Down Expand Up @@ -646,10 +646,10 @@
}
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
}
],
Expand Down Expand Up @@ -740,10 +740,10 @@
"type": "string"
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
}
],
Expand Down Expand Up @@ -842,10 +842,10 @@
"type": "string"
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
}
],
Expand Down Expand Up @@ -924,10 +924,10 @@
"type": "string"
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
}
],
Expand Down Expand Up @@ -988,10 +988,10 @@
"operationId": "get_workflow_disk_usage",
"parameters": [
{
"description": "Required. API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
},
{
Expand Down Expand Up @@ -1099,10 +1099,10 @@
"operationId": "get_workflow_logs",
"parameters": [
{
"description": "Required. API access_token of workflow owner.",
"description": "API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
},
{
Expand Down Expand Up @@ -1192,10 +1192,10 @@
"type": "string"
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
},
{
Expand Down Expand Up @@ -1285,10 +1285,10 @@
"type": "string"
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
}
],
Expand Down Expand Up @@ -1376,10 +1376,10 @@
"type": "string"
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
},
{
Expand Down Expand Up @@ -1488,10 +1488,10 @@
"type": "string"
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
}
],
Expand Down Expand Up @@ -1611,10 +1611,10 @@
"type": "string"
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
},
{
Expand Down Expand Up @@ -1723,10 +1723,10 @@
"type": "string"
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
}
],
Expand Down Expand Up @@ -1813,10 +1813,10 @@
"type": "string"
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
}
],
Expand Down Expand Up @@ -1886,10 +1886,10 @@
"type": "string"
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
}
],
Expand Down Expand Up @@ -1949,10 +1949,10 @@
"type": "string"
},
{
"description": "Required. The API access_token of workflow owner.",
"description": "The API access_token of workflow owner.",
"in": "query",
"name": "access_token",
"required": true,
"required": false,
"type": "string"
}
],
Expand Down
2 changes: 2 additions & 0 deletions reana_server/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ def _(x):

# Accounts
# ========
#: Redis URLL
ACCOUNTS_SESSION_REDIS_URL = 'redis://cache:6379/1'
#: Email address used as sender of account registration emails.
SECURITY_EMAIL_SENDER = SUPPORT_EMAIL
#: Email subject for account registration emails.
Expand Down
31 changes: 21 additions & 10 deletions reana_server/rest/secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@

from bravado.exception import HTTPError
from flask import Blueprint, jsonify, request
from flask_login import current_user

from reana_commons.errors import (REANASecretAlreadyExists,
REANASecretDoesNotExist)
from reana_commons.k8s.secrets import REANAUserSecretsStore
from reana_server.utils import get_user_from_token
from reana_server.utils import get_user_from_token, \
_get_user_from_invenio_user

blueprint = Blueprint('secrets', __name__)

Expand All @@ -38,8 +40,8 @@ def add_secrets(): # noqa
parameters:
- name: access_token
in: query
description: Required. Secrets owner access token.
required: true
description: Secrets owner access token.
required: false
type: string
- name: overwrite
in: query
Expand Down Expand Up @@ -112,7 +114,10 @@ def add_secrets(): # noqa
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :).

"""
try:
user = get_user_from_token(request.args.get("access_token"))
if current_user.is_authenticated:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

user = _get_user_from_invenio_user(current_user.email)
else:
user = get_user_from_token(request.args.get('access_token'))
Copy link
Member

@diegodelemos diegodelemos Jul 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

secrets_store = REANAUserSecretsStore(str(user.id_))
overwrite = json.loads(request.args.get('overwrite'))
secrets_store.add_secrets(request.json, overwrite=overwrite)
Expand Down Expand Up @@ -141,8 +146,8 @@ def get_secrets(): # noqa
parameters:
- name: access_token
in: query
description: Required. Secrets owner access token.
required: true
description: Secrets owner access token.
required: false
type: string
responses:
200:
Expand Down Expand Up @@ -194,7 +199,10 @@ def get_secrets(): # noqa
}
"""
try:
user = get_user_from_token(request.args.get("access_token"))
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'))
secrets_store = REANAUserSecretsStore(str(user.id_))
user_secrets = secrets_store.get_secrets()
return jsonify(user_secrets), 200
Expand All @@ -220,8 +228,8 @@ def delete_secrets(): # noqa
parameters:
- name: access_token
in: query
description: Required. API key of the admin.
required: true
description: API key of the admin.
required: false
type: string
- name: secrets
in: body
Expand Down Expand Up @@ -283,7 +291,10 @@ def delete_secrets(): # noqa
}
"""
try:
user = get_user_from_token(request.args.get("access_token"))
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'))
secrets_store = REANAUserSecretsStore(str(user.id_))
deleted_secrets_list = secrets_store.delete_secrets(request.json)
return jsonify(deleted_secrets_list), 200
Expand Down
Loading