Skip to content

Commit

Permalink
refactor(secrets): adapt to reana-commons secret-handling changes (re…
Browse files Browse the repository at this point in the history
…anahub#686)

Refactor secrets-related endpoints to adapt to performance-related
refactor of `reana-commons`. Improve validation of endpoints'
parameters.

Closes reanahub/reana-commons#455
  • Loading branch information
mdonadoni authored and tiborsimko committed Aug 21, 2024
1 parent 703af19 commit 3596ee0
Show file tree
Hide file tree
Showing 8 changed files with 119 additions and 56 deletions.
4 changes: 0 additions & 4 deletions docs/openapi.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
10 changes: 5 additions & 5 deletions reana_server/gitlab_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down
7 changes: 4 additions & 3 deletions reana_server/rest/gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
80 changes: 67 additions & 13 deletions reana_server/rest/secrets.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
---
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -219,16 +259,25 @@ 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:
logging.error(traceback.format_exc())
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
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion reana_server/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
"Flask>=2.1.1,<2.3.0", # same upper pin as invenio-base
"gitpython>=3.1",
"marshmallow>2.13.0,<3.0.0",
"reana-commons[kubernetes,yadage,snakemake,cwl]>=0.95.0a3,<0.96.0",
"reana-commons[kubernetes,yadage,snakemake,cwl] @ git+https://github.com/reanahub/reana-commons.git@0.95.0a4",
"reana-db>=0.95.0a3,<0.96.0",
"requests>=2.25.0",
"tablib>=0.12.1",
Expand Down
40 changes: 21 additions & 19 deletions tests/test_gitlab_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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():
Expand Down
31 changes: 21 additions & 10 deletions tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 3596ee0

Please sign in to comment.