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

@@ -966,10 +966,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": [

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
@@ -170,10 +170,11 @@ def gitlab_oauth(user):  # noqa
             gitlab_response = requests.post(
                 REANA_GITLAB_URL + "/oauth/token", data=params
             ).content
-            secrets_store = REANAUserSecretsStore(str(user.id_))
+            secrets_store = UserSecretsStore.fetch(user.id_)
             secrets_store.add_secrets(
                 _format_gitlab_secrets(gitlab_response), overwrite=True
             )
+            UserSecretsStore.update(secrets_store)
             return redirect(next_url), 302
         else:
             return jsonify({"message": "OK"}), 200
@@ -291,12 +292,14 @@ def gitlab_projects(
               }
     """
     try:
-        secrets_store = REANAUserSecretsStore(str(user.id_))
-        gitlab_token = secrets_store.get_secret_value("gitlab_access_token")
+        secrets_store = UserSecretsStore.fetch(user.id_)
+        gitlab_token_secret = secrets_store.get_secret("gitlab_access_token")
 
-        if not gitlab_token:
+        if not gitlab_token_secret:
             return jsonify({"message": "Missing GitLab access token."}), 401
 
+        gitlab_token = gitlab_token_secret.value_str
+
         gitlab_url = urljoin(REANA_GITLAB_URL, "/api/v4/projects")
         params = {
             "access_token": gitlab_token,
@@ -465,8 +468,9 @@ def gitlab_webhook(user):  # noqa
     """
 
     try:
-        secrets_store = REANAUserSecretsStore(str(user.id_))
-        gitlab_token = secrets_store.get_secret_value("gitlab_access_token")
+        secrets_store = UserSecretsStore.fetch(user.id_)
+        # TODO: properly handle missing secret, this will be fixed by a refactor of GitLab API requests
+        gitlab_token = secrets_store.get_secret("gitlab_access_token").value_str
         parameters = request.json
         if request.method == "POST":
             gitlab_url = (

reana_server/rest/secrets.py

@@ -14,17 +14,44 @@
 
 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
 
 
 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)
+        secrets_store = UserSecretsStore.fetch(user.id_)
+        secrets_store.add_secrets(secrets, overwrite=overwrite)
+        UserSecretsStore.update(secrets_store)
         return jsonify({"message": "Secret(s) successfully added."}), 201
     except REANASecretAlreadyExists as e:
         return jsonify({"message": str(e)}), 409
@@ -219,8 +259,11 @@ def get_secrets(user):  # noqa
               }
     """
     try:
-        secrets_store = REANAUserSecretsStore(str(user.id_))
-        user_secrets = secrets_store.get_secrets()
+        secrets_store = UserSecretsStore.fetch(user.id_)
+        user_secrets = [
+            {"name": secret.name, "type": secret.type_}
+            for secret in secrets_store.get_secrets()
+        ]
         return jsonify(user_secrets), 200
     except ValueError:
         return jsonify({"message": "Token is not valid."}), 403
@@ -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)
+        secrets_store = UserSecretsStore.fetch(user.id_)
+        deleted_secrets_list = secrets_store.delete_secrets(secrets)
+        UserSecretsStore.update(secrets_store)
         return jsonify(deleted_secrets_list), 200
     except REANASecretDoesNotExist as e:
         return jsonify(e.missing_secrets_list), 404

reana_server/utils.py

@@ -36,7 +36,7 @@
     REANAValidationError,
     REANAEmailNotificationError,
 )
-from reana_commons.k8s.secrets import REANAUserSecretsStore
+from reana_commons.k8s.secrets import Secret, UserSecretsStore
 from reana_commons.utils import get_quota_resource_usage
 from reana_commons.yadage import yadage_load_from_workspace
 from reana_db.database import Session
@@ -444,8 +444,9 @@ def _get_reana_yaml_from_gitlab(webhook_data, user_id):
     elif webhook_data["object_kind"] == "merge_request":
         branch = webhook_data["object_attributes"]["source_branch"]
         commit_sha = webhook_data["object_attributes"]["last_commit"]["id"]
-    secrets_store = REANAUserSecretsStore(str(user_id))
-    gitlab_token = secrets_store.get_secret_value("gitlab_access_token")
+    secrets_store = UserSecretsStore.fetch(user_id)
+    # TODO: properly handle missing secret, this will be fixed by a refactor of GitLab API requests
+    gitlab_token = secrets_store.get_secret("gitlab_access_token").value_str
     project_id = webhook_data["project"]["id"]
     yaml_file = requests.get(
         gitlab_api.format(project_id, reana_yaml, branch, gitlab_token)
@@ -471,14 +472,29 @@ def _fail_gitlab_commit_build_status(
     git_repo = urlparse.quote_plus(git_repo)
     description = urlparse.quote_plus(description)
 
-    secret_store = REANAUserSecretsStore(user.id_)
-    gitlab_access_token = secret_store.get_secret_value("gitlab_access_token")
+    secret_store = UserSecretsStore.fetch(user.id_)
+    gitlab_access_token_secret = secret_store.get_secret("gitlab_access_token")
+    if not gitlab_access_token_secret:
+        # only logging, this function never returns errors
+        logging.error(
+            f"Skipping setting commit status to {state} for {git_repo}@{git_ref} of user "
+            f"{user.id_}: GitLab access token not found"
+        )
+        return
+    gitlab_access_token = gitlab_access_token_secret.value_str
+
     commit_status_url = (
         f"{REANA_GITLAB_URL}/api/v4/projects/{git_repo}/statuses/"
         f"{git_ref}?access_token={gitlab_access_token}&state={state}"
         f"&description={description}&name={system_name}"
     )
-    requests.post(commit_status_url)
+
+    res = requests.post(commit_status_url)
+    if res.status_code >= 400:
+        logging.error(
+            f"Failed to set commit status to {state} for {git_repo}@{git_ref} of user "
+            f"{user.id_}: status code {res.status_code}, content {res.text}"
+        )
 
 
 def _format_gitlab_secrets(gitlab_response):
@@ -488,16 +504,10 @@ def _format_gitlab_secrets(gitlab_response):
             REANA_GITLAB_URL + "/api/v4/user?access_token={0}".format(access_token)
         ).content
     )
-    return {
-        "gitlab_access_token": {
-            "value": base64.b64encode(access_token.encode("utf-8")).decode("utf-8"),
-            "type": "env",
-        },
-        "gitlab_user": {
-            "value": base64.b64encode(user["username"].encode("utf-8")).decode("utf-8"),
-            "type": "env",
-        },
-    }
+    return [
+        Secret("gitlab_access_token", type_="env", value=access_token),
+        Secret("gitlab_user", type_="env", value=user["username"]),
+    ]
 
 
 def _get_gitlab_hook_id(project_id, gitlab_token):

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",
@@ -846,12 +850,17 @@ 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"
-
+        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("requests.get", mock_requests_get), patch(
-            "reana_commons.k8s.secrets.REANAUserSecretsStore.get_secret_value",
-            mock_get_secret_value,
+            "reana_commons.k8s.secrets.UserSecretsStore.fetch",
+            mock_fetch,
         ):
             res = client.get(
                 "/api/gitlab/projects",