From 3a56d7d75d03cb6ed48404763b26c9c2dc1bf6c6 Mon Sep 17 00:00:00 2001 From: Marco Donadoni Date: Tue, 30 Apr 2024 12:25:18 +0200 Subject: [PATCH] refactor(secrets): adapt to reana-commons secret-handling changes (#686) Refactor secrets-related endpoints to adapt to performance-related refactor of `reana-commons`. Improve validation of endpoints' parameters. Closes reanahub/reana-commons#455 --- docs/openapi.json | 4 -- reana_server/gitlab_client.py | 10 ++--- reana_server/rest/gitlab.py | 7 +-- reana_server/rest/secrets.py | 80 +++++++++++++++++++++++++++++------ reana_server/utils.py | 1 - tests/test_gitlab_client.py | 40 +++++++++--------- tests/test_views.py | 31 +++++++++----- 7 files changed, 118 insertions(+), 55 deletions(-) diff --git a/docs/openapi.json b/docs/openapi.json index 112d7353..651e57a0 100644 --- a/docs/openapi.json +++ b/docs/openapi.json @@ -962,10 +962,6 @@ "additionalProperties": { "description": "Secret definition.", "properties": { - "name": { - "description": "Secret name", - "type": "string" - }, "type": { "description": "How will be the secret assigned to the jobs, either exported as an environment variable or mounted as a file.", "enum": [ diff --git a/reana_server/gitlab_client.py b/reana_server/gitlab_client.py index b0a334c9..f7e7876c 100644 --- a/reana_server/gitlab_client.py +++ b/reana_server/gitlab_client.py @@ -10,7 +10,7 @@ import requests import yaml -from reana_commons.k8s.secrets import REANAUserSecretsStore +from reana_commons.k8s.secrets import UserSecretsStore from reana_server.config import REANA_GITLAB_HOST @@ -63,11 +63,11 @@ def from_k8s_secret(cls, user_id, **kwargs): :param user_id: User UUID. """ - secrets_store = REANAUserSecretsStore(str(user_id)) - gitlab_token = secrets_store.get_secret_value("gitlab_access_token") - if not gitlab_token: + user_secrets = UserSecretsStore.fetch(user_id) + gitlab_token_secret = user_secrets.get_secret("gitlab_access_token") + if not gitlab_token_secret: raise GitLabClientInvalidToken - return cls(access_token=gitlab_token, **kwargs) + return cls(access_token=gitlab_token_secret.value_str, **kwargs) def __init__( self, diff --git a/reana_server/rest/gitlab.py b/reana_server/rest/gitlab.py index b47bca46..1a73ee94 100644 --- a/reana_server/rest/gitlab.py +++ b/reana_server/rest/gitlab.py @@ -25,7 +25,7 @@ from flask_login.utils import _create_identifier from invenio_oauthclient.utils import get_safe_redirect_target from itsdangerous import BadData, TimedJSONWebSignatureSerializer -from reana_commons.k8s.secrets import REANAUserSecretsStore +from reana_commons.k8s.secrets import UserSecretsStore from werkzeug.local import LocalProxy from webargs import fields, validate from webargs.flaskparser import use_kwargs @@ -183,10 +183,11 @@ def gitlab_oauth(user): # noqa gitlab_user = authenticated_gitlab_client.get_user().json() # store access token inside k8s secrets - secrets_store = REANAUserSecretsStore(str(user.id_)) - secrets_store.add_secrets( + user_secrets = UserSecretsStore.fetch(user.id_) + user_secrets.add_secrets( _format_gitlab_secrets(gitlab_user, access_token), overwrite=True ) + UserSecretsStore.update(user_secrets) return redirect(next_url), 302 else: return jsonify({"message": "OK"}), 200 diff --git a/reana_server/rest/secrets.py b/reana_server/rest/secrets.py index e7473470..70850b65 100644 --- a/reana_server/rest/secrets.py +++ b/reana_server/rest/secrets.py @@ -14,7 +14,10 @@ from flask import Blueprint, jsonify, request from reana_commons.errors import REANASecretAlreadyExists, REANASecretDoesNotExist -from reana_commons.k8s.secrets import REANAUserSecretsStore +from reana_commons.k8s.secrets import UserSecretsStore, Secret +from webargs import fields +from webargs.flaskparser import use_kwargs +from marshmallow import Schema, validate from reana_server.decorators import signin_required @@ -22,9 +25,33 @@ blueprint = Blueprint("secrets", __name__) +class AddSecretsBodySchema(Schema): + """Schema for add_secrets endpoint body.""" + + body = ( + fields.Dict( + keys=fields.Str(), + values=fields.Nested( + { + "value": fields.Str(required=True), + "type": fields.Str( + validate=validate.OneOf(Secret.types), required=True + ), + } + ), + required=True, + ), + ) + + @blueprint.route("/secrets/", methods=["POST"]) @signin_required() -def add_secrets(user): # noqa +@use_kwargs( + { + "overwrite": fields.Bool(missing=False, location="query"), + } +) +def add_secrets(user, overwrite=False): r"""Endpoint to create user secrets. --- @@ -57,9 +84,6 @@ def add_secrets(user): # noqa type: object description: Secret definition. properties: - name: - type: string - description: Secret name value: type: string description: Secret value @@ -126,10 +150,26 @@ def add_secrets(user): # noqa "message": "Internal server error." } """ + json_body = request.json + AddSecretsBodySchema(strict=True).validate({"body": json_body}) + + try: + secrets = [ + Secret.from_base64( + name=secret_name, + value=secret["value"], + type_=secret["type"], + ) + for secret_name, secret in json_body.items() + ] + except ValueError as e: + # value is not correctly base64-encoded + return jsonify({"message": str(e)}), 400 + try: - secrets_store = REANAUserSecretsStore(str(user.id_)) - overwrite = json.loads(request.args.get("overwrite")) - secrets_store.add_secrets(request.json, overwrite=overwrite) + user_secrets = UserSecretsStore.fetch(user.id_) + user_secrets.add_secrets(secrets, overwrite=overwrite) + UserSecretsStore.update(user_secrets) return jsonify({"message": "Secret(s) successfully added."}), 201 except REANASecretAlreadyExists as e: return jsonify({"message": str(e)}), 409 @@ -219,9 +259,12 @@ def get_secrets(user): # noqa } """ try: - secrets_store = REANAUserSecretsStore(str(user.id_)) - user_secrets = secrets_store.get_secrets() - return jsonify(user_secrets), 200 + user_secrets = UserSecretsStore.fetch(user.id_) + user_secrets_json = [ + {"name": secret.name, "type": secret.type_} + for secret in user_secrets.get_secrets() + ] + return jsonify(user_secrets_json), 200 except ValueError: return jsonify({"message": "Token is not valid."}), 403 except Exception as e: @@ -229,6 +272,12 @@ def get_secrets(user): # noqa return jsonify({"message": str(e)}), 500 +class DeleteSecretsBodySchema(Schema): + """Schema for delete_secrets endpoint body.""" + + body = fields.List(fields.Str(), required=True) + + @blueprint.route("/secrets/", methods=["DELETE"]) @signin_required() def delete_secrets(user): # noqa @@ -317,9 +366,14 @@ def delete_secrets(user): # noqa "message": "Internal server error." } """ + json_body = request.json + DeleteSecretsBodySchema(strict=True).validate({"body": json_body}) + secrets = json_body + try: - secrets_store = REANAUserSecretsStore(str(user.id_)) - deleted_secrets_list = secrets_store.delete_secrets(request.json) + user_secrets = UserSecretsStore.fetch(user.id_) + deleted_secrets_list = user_secrets.delete_secrets(secrets) + UserSecretsStore.update(user_secrets) return jsonify(deleted_secrets_list), 200 except REANASecretDoesNotExist as e: return jsonify(e.missing_secrets_list), 404 diff --git a/reana_server/utils.py b/reana_server/utils.py index e3ed2286..6f762dc2 100644 --- a/reana_server/utils.py +++ b/reana_server/utils.py @@ -36,7 +36,6 @@ REANAValidationError, REANAEmailNotificationError, ) -from reana_commons.k8s.secrets import REANAUserSecretsStore from reana_commons.utils import get_quota_resource_usage from reana_commons.yadage import yadage_load_from_workspace from reana_db.database import Session diff --git a/tests/test_gitlab_client.py b/tests/test_gitlab_client.py index 2d70f0f9..7b4f8ecd 100644 --- a/tests/test_gitlab_client.py +++ b/tests/test_gitlab_client.py @@ -11,7 +11,7 @@ from uuid import uuid4 import pytest -from reana_commons.k8s.secrets import REANAUserSecretsStore +from reana_commons.k8s.secrets import UserSecrets, Secret import reana_server.config as config from reana_server.gitlab_client import GitLabClient, GitLabClientInvalidToken @@ -27,32 +27,34 @@ def mock_response(status_code=200, json={}, content=b"", links={}): return response -def test_gitlab_client_from_k8s_secret(monkeypatch): +def test_gitlab_client_from_k8s_secret(): """Test creating authenticated GitLab client from user k8s secret.""" user_id = uuid4() - def get_secret_value(store, secret_name): - assert secret_name == "gitlab_access_token" - return "gitlab_token" - - monkeypatch.setattr(REANAUserSecretsStore, "get_secret_value", get_secret_value) - - gitlab_client = GitLabClient.from_k8s_secret(user_id, host="gitlab.example.org") + mock_fetch = mock.Mock() + mock_fetch.return_value = UserSecrets( + user_id=str(user_id), + k8s_secret_name="gitlab_token", + secrets=[Secret(name="gitlab_access_token", type_="env", value="gitlab_token")], + ) + with mock.patch("reana_commons.k8s.secrets.UserSecretsStore.fetch", mock_fetch): + gitlab_client = GitLabClient.from_k8s_secret(user_id, host="gitlab.example.org") assert gitlab_client.access_token == "gitlab_token" assert gitlab_client.host == "gitlab.example.org" -def test_gitlab_client_from_k8s_secret_invalid_token(monkeypatch): +def test_gitlab_client_from_k8s_secret_invalid_token(): """Test creating authenticated GitLab client when secret is not available.""" - - def get_secret_value(store, secret_name): - assert secret_name == "gitlab_access_token" - return None - - monkeypatch.setattr(REANAUserSecretsStore, "get_secret_value", get_secret_value) - - with pytest.raises(GitLabClientInvalidToken): - GitLabClient.from_k8s_secret(uuid4(), host="gitlab.example.org") + user_id = uuid4() + mock_fetch = mock.Mock() + mock_fetch.return_value = UserSecrets( + user_id=str(user_id), + k8s_secret_name="k8s-secret-name", + secrets=[], + ) + with mock.patch("reana_commons.k8s.secrets.UserSecretsStore.fetch", mock_fetch): + with pytest.raises(GitLabClientInvalidToken): + GitLabClient.from_k8s_secret(user_id) def test_gitlab_client_oauth_token(): diff --git a/tests/test_views.py b/tests/test_views.py index b27709d0..de760b3e 100644 --- a/tests/test_views.py +++ b/tests/test_views.py @@ -19,6 +19,7 @@ from mock import Mock, patch from pytest_reana.test_utils import make_mock_api_client from reana_db.models import User, InteractiveSessionType, RunStatus +from reana_commons.k8s.secrets import UserSecrets, Secret from reana_server.utils import ( _create_and_associate_local_user, @@ -800,11 +801,14 @@ def test_gitlab_projects(app: Flask, default_user): assert res.status_code == 403 # missing GitLab token - mock_get_secret_value = Mock() - mock_get_secret_value.return_value = None + fetch_mock = Mock() + fetch_mock.return_value = UserSecrets( + user_id=str(default_user.id_), + k8s_secret_name="k8s_secret_name", + ) with patch( - "reana_commons.k8s.secrets.REANAUserSecretsStore.get_secret_value", - mock_get_secret_value, + "reana_commons.k8s.secrets.UserSecretsStore.fetch", + fetch_mock, ): res = client.get( "/api/gitlab/projects", @@ -847,12 +851,19 @@ def test_gitlab_projects(app: Flask, default_user): mock_requests_get = Mock() mock_requests_get.side_effect = [mock_response_projects, mock_response_webhook] - mock_get_secret_value = Mock() - mock_get_secret_value.return_value = "gitlab_token" - - with patch("requests.request", mock_requests_get), patch( - "reana_commons.k8s.secrets.REANAUserSecretsStore.get_secret_value", - mock_get_secret_value, + mock_fetch = Mock() + mock_fetch.return_value = UserSecrets( + user_id=str(default_user.id_), + k8s_secret_name="gitlab_token", + secrets=[ + Secret(name="gitlab_access_token", type_="env", value="gitlab_token") + ], + ) + with patch( + "reana_server.gitlab_client.GitLabClient._request", mock_requests_get + ), patch( + "reana_commons.k8s.secrets.UserSecretsStore.fetch", + mock_fetch, ): res = client.get( "/api/gitlab/projects",