From 11116eca84dafedcdf370b449b0e078437929442 Mon Sep 17 00:00:00 2001 From: MartaOB <151519478+MartaOB@users.noreply.github.com> Date: Thu, 11 Apr 2024 10:46:12 +0200 Subject: [PATCH] feat: add support for Vault secrets (#44) * feat: add `secrets` routes * feat: add secrets tests * fix: use EGI CheckIn prod for testing * feat: allow retrieving secrets from subpath * feat: initial implementation of FL_server+Vault * feat: make vault token available to deployment * feat: remove federated secret from config * feat: only use Vault tokens for FL server * fix: fix Vault token TTL * fix: remove env OIDC_ACCESS_TOKEN from Nomad job * fix: add federated secret * fix: delete federated secret from config files * fix: add leading "/" in `SECRET_PATH` in tests --------- Co-authored-by: Ignacio Heredia --- ai4papi/routers/v1/__init__.py | 3 +- ai4papi/routers/v1/catalog/tools.py | 8 +- ai4papi/routers/v1/deployments/tools.py | 39 ++- ai4papi/routers/v1/secrets.py | 244 +++++++++++++++++++ etc/tools/deep-oc-federated-server/nomad.hcl | 2 +- etc/tools/deep-oc-federated-server/user.yaml | 5 - requirements.txt | 1 + tests/catalog/modules.py | 2 - tests/catalog/tools.py | 2 - tests/deployments/modules.py | 5 +- tests/deployments/tools.py | 5 +- tests/main.py | 4 + tests/test_secrets.py | 60 +++++ 13 files changed, 355 insertions(+), 25 deletions(-) create mode 100644 ai4papi/routers/v1/secrets.py create mode 100644 tests/test_secrets.py diff --git a/ai4papi/routers/v1/__init__.py b/ai4papi/routers/v1/__init__.py index ab85707..6bfcfb7 100644 --- a/ai4papi/routers/v1/__init__.py +++ b/ai4papi/routers/v1/__init__.py @@ -1,10 +1,11 @@ import fastapi -from . import catalog, deployments +from . import catalog, deployments, secrets app = fastapi.APIRouter() app.include_router(catalog.app) app.include_router(deployments.app) +app.include_router(secrets.router) @app.get( diff --git a/ai4papi/routers/v1/catalog/tools.py b/ai4papi/routers/v1/catalog/tools.py index a8654bd..2aafe80 100644 --- a/ai4papi/routers/v1/catalog/tools.py +++ b/ai4papi/routers/v1/catalog/tools.py @@ -5,7 +5,6 @@ from cachetools import cached, TTLCache from fastapi import APIRouter, HTTPException import requests -import secrets from ai4papi import quotas import ai4papi.conf as papiconf @@ -101,12 +100,7 @@ def get_config( item_name=item_name, vo=vo, ) - - # Extra tool-dependent steps - if item_name == 'deep-oc-federated-server': - # Create unique secret for that federated server - conf["general"]["federated_secret"]["value"] = secrets.token_hex() - + return conf diff --git a/ai4papi/routers/v1/deployments/tools.py b/ai4papi/routers/v1/deployments/tools.py index 3b8d722..edb7064 100644 --- a/ai4papi/routers/v1/deployments/tools.py +++ b/ai4papi/routers/v1/deployments/tools.py @@ -1,6 +1,8 @@ from copy import deepcopy import re +import secrets import types +from types import SimpleNamespace from typing import Tuple, Union import uuid @@ -10,6 +12,7 @@ from ai4papi import auth, quotas, utils import ai4papi.conf as papiconf import ai4papi.nomad.common as nomad +from ai4papi.routers.v1 import secrets as ai4secrets router = APIRouter( @@ -206,6 +209,23 @@ def create_deployment( ) utils.check_domain(domain) + # Create a default secret for the Federated Server + _ = ai4secrets.create_secret( + vo=vo, + secret_path=f"deployments/{job_uuid}/federated/default", + secret_data={'token': secrets.token_hex()}, + authorization=SimpleNamespace( + credentials=authorization.credentials, + ), + ) + + # Create a Vault token so that the deployment can access the Federated secret + vault_token = ai4secrets.create_vault_token( + jwt=authorization.credentials, + issuer=auth_info['issuer'], + ttl='365d', # 1 year expiration date + ) + # Replace the Nomad job template nomad_conf = nomad_conf.safe_substitute( { @@ -226,7 +246,7 @@ def create_deployment( 'SHARED_MEMORY': user_conf['hardware']['ram'] * 10**6 * 0.5, # Limit at 50% of RAM memory, in bytes 'JUPYTER_PASSWORD': user_conf['general']['jupyter_password'], - 'FEDERATED_SECRET': user_conf['general']['federated_secret'], + 'VAULT_TOKEN': vault_token, 'FEDERATED_ROUNDS': user_conf['configuration']['rounds'], 'FEDERATED_METRIC': user_conf['configuration']['metric'], 'FEDERATED_MIN_CLIENTS': user_conf['configuration']['min_clients'], @@ -278,4 +298,21 @@ def delete_deployment( owner=auth_info['id'], ) + # Remove Vault secrets belonging to that deployment + r = ai4secrets.get_secrets( + vo=vo, + subpath=f"/deployments/{deployment_uuid}", + authorization=SimpleNamespace( + credentials=authorization.credentials, + ), + ) + for path in r.keys(): + r = ai4secrets.delete_secret( + vo=vo, + secret_path=path, + authorization=SimpleNamespace( + credentials=authorization.credentials, + ), + ) + return r diff --git a/ai4papi/routers/v1/secrets.py b/ai4papi/routers/v1/secrets.py new file mode 100644 index 0000000..06cb6a0 --- /dev/null +++ b/ai4papi/routers/v1/secrets.py @@ -0,0 +1,244 @@ +""" +Manage user secrets with Vault +""" + +import hvac +from fastapi import APIRouter, Depends, HTTPException +from fastapi.security import HTTPBearer + +from ai4papi import auth + + +router = APIRouter( + prefix="/secrets", + tags=["Secrets management"], + responses={404: {"description": "Not found"}}, +) +security = HTTPBearer() + +# For now, we use for everyone the official EGI Vault server. +# We can reconsider this is we start using the IAM in auth. +VAULT_ADDR = "https://vault.services.fedcloud.eu:8200" +VAULT_AUTH_PATH = "jwt" +VAULT_ROLE = "" +VAULT_MOUNT_POINT = "/secrets/" + + +def vault_client(jwt, issuer): + """ + Common init steps of Vault client + """ + # Check we are using EGI Check-In prod + if issuer != 'https://aai.egi.eu/auth/realms/egi': + raise HTTPException( + status_code=400, + detail="Secrets are only compatible with EGI Check-In Production OIDC " \ + "provider.", + ) + + # Init the Vault client + client = hvac.Client( + url=VAULT_ADDR, + ) + client.auth.jwt.jwt_login( + role=VAULT_ROLE, + jwt=jwt, + path=VAULT_AUTH_PATH, + ) + + return client + + +def create_vault_token( + jwt, + issuer, + ttl='1h', + ): + """ + Create a Vault token from a JWT. + + Parameters: + * jwt: JSON web token + * issuer: JWT issuer + * ttl: duration of the token + """ + client = vault_client(jwt, issuer) + + # When creating the client (`jwt_login`) we are already creating a login token with + # default TTL (1h). So any newly created child token (independently of their TTL) + # will be revoked after the login token expires (1h). + # So instead of creating a child token, we have to *extend* login token. + client.auth.token.renew_self(increment=ttl) + + #TODO: for extra security we should only allow reading/listing from a given subpath. + # - Restrict to read/list can be done with user roles + # - Restricting subpaths might not be done because policies are static (and + # deployment paths are dynamic). In addition only admins can create policies) + + return client.token + + +def recursive_path_builder(client, kv_list): + """ + Reference: https://github.com/drewmullen/vault-kv-migrate + """ + change = 0 + + # if any list items end in '/' return 1 + for li in kv_list[:]: + if li[-1] == '/': + r = client.secrets.kv.v1.list_secrets( + path=li, + mount_point=VAULT_MOUNT_POINT + ) + append_list = r['data']['keys'] + for new_item in append_list: + kv_list.append(li + new_item) + # remove list item ending in '/' + kv_list.remove(li) + change = 1 + + # new list items added, rerun search + if change == 1: + recursive_path_builder(client, kv_list) + + return kv_list + + +@router.get("/") +def get_secrets( + vo: str, + subpath: str = '', + authorization=Depends(security), + ): + """ + Returns a list of secrets belonging to a user. + + Parameters: + * **vo**: Virtual Organization where you belong. + * **subpath**: retrieve secrets only from a given subpath. + If not specified, it will retrieve all secrets from the user. \n + Examples: + - `/deployments//federated/` + """ + # Retrieve authenticated user info + auth_info = auth.get_user_info(token=authorization.credentials) + auth.check_vo_membership(vo, auth_info['vos']) + + # Init the Vault client + client = vault_client( + jwt=authorization.credentials, + issuer=auth_info['issuer'], + ) + + # Check subpath syntax + if not subpath.startswith('/'): + subpath = '/' + subpath + if not subpath.endswith('/'): + subpath += '/' + + # Retrieve initial level-0 secrets + user_path = f"users/{auth_info['id']}" + try: + r = client.secrets.kv.v1.list_secrets( + path = user_path + subpath, + mount_point=VAULT_MOUNT_POINT + ) + seed_list = r['data']['keys'] + except hvac.exceptions.InvalidPath: + # InvalidPath is raised when there are no secrets available + return {} + + # Now iterate recursively to retrieve all secrets from child paths + for i, li in enumerate(seed_list): + seed_list[i] = user_path + subpath + li + final_list = recursive_path_builder(client, seed_list) + + # Extract secrets data + out = {} + for secret_path in final_list: + r1 = client.secrets.kv.v1.read_secret( + path=secret_path, + mount_point=VAULT_MOUNT_POINT, + ) + + # Remove user-path prefix and save + secret_path = secret_path.replace(user_path, '') + out[secret_path] = r1['data'] + + return out + + +@router.post("/") +def create_secret( + vo: str, + secret_path: str, + secret_data: dict, + authorization=Depends(security), + ): + """ + Creates a new secret or updates an existing one. + + Parameters: + * **vo**: Virtual Organization where you belong. + * **secret_path**: path of the secret. + Not sensitive to leading/trailing slashes. \n + Examples: + - `/deployments//federated/` + * **secret_data**: data to be saved at the path. \n + Examples: + - `{'token': 515c5d4f5d45fd15df}` + """ + # Retrieve authenticated user info + auth_info = auth.get_user_info(token=authorization.credentials) + auth.check_vo_membership(vo, auth_info['vos']) + + # Init the Vault client + client = vault_client( + jwt=authorization.credentials, + issuer=auth_info['issuer'], + ) + + # Create secret + client.secrets.kv.v1.create_or_update_secret( + path=f"users/{auth_info['id']}/{secret_path}", + mount_point='/secrets/', + secret=secret_data, + ) + + return {'status': 'success'} + + +@router.delete("/") +def delete_secret( + vo: str, + secret_path: str, + authorization=Depends(security), + ): + """ + Delete a secret. + + Parameters: + * **vo**: Virtual Organization where you belong. + * **secret_path**: path of the secret. + Not sensitive to leading/trailing slashes. \n + Examples: + - `deployments//fl-token` + """ + # Retrieve authenticated user info + auth_info = auth.get_user_info(token=authorization.credentials) + auth.check_vo_membership(vo, auth_info['vos']) + + # Init the Vault client + client = vault_client( + jwt=authorization.credentials, + issuer=auth_info['issuer'], + ) + + # Delete secret + client.secrets.kv.v1.delete_secret( + path=f"users/{auth_info['id']}/{secret_path}", + mount_point=VAULT_MOUNT_POINT, + ) + + return {'status': 'success'} diff --git a/etc/tools/deep-oc-federated-server/nomad.hcl b/etc/tools/deep-oc-federated-server/nomad.hcl index d20f112..dc320a0 100644 --- a/etc/tools/deep-oc-federated-server/nomad.hcl +++ b/etc/tools/deep-oc-federated-server/nomad.hcl @@ -110,8 +110,8 @@ job "userjob-${JOB_UUID}" { } env { + VAULT_TOKEN = "${VAULT_TOKEN}" jupyterPASSWORD = "${JUPYTER_PASSWORD}" - FEDERATED_SECRET = "${FEDERATED_SECRET}" FEDERATED_ROUNDS = "${FEDERATED_ROUNDS}" FEDERATED_METRIC = "${FEDERATED_METRIC}" FEDERATED_MIN_CLIENTS = "${FEDERATED_MIN_CLIENTS}" diff --git a/etc/tools/deep-oc-federated-server/user.yaml b/etc/tools/deep-oc-federated-server/user.yaml index 0d9ab54..0397c5c 100644 --- a/etc/tools/deep-oc-federated-server/user.yaml +++ b/etc/tools/deep-oc-federated-server/user.yaml @@ -47,11 +47,6 @@ general: value: '' description: Select a password for your IDE (JupyterLab or VS Code). It should have at least 9 characters. - federated_secret: - name: Secret training token - value: '' - description: This is the federated secret token that your clients should use to connect to the server. - hardware: cpu_num: name: Number of CPUs diff --git a/requirements.txt b/requirements.txt index f508a57..07074cc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,3 +7,4 @@ fastapi >= 0.89.1, < 1.0 uvicorn[standard]>=0.20.0, < 1.0 flaat >= 1.1.8, < 2.0 typer >= 0.7.0, < 1.0 +hvac >= 2.1.0, < 3.0 diff --git a/tests/catalog/modules.py b/tests/catalog/modules.py index ec02934..8bcbb70 100644 --- a/tests/catalog/modules.py +++ b/tests/catalog/modules.py @@ -1,5 +1,3 @@ -#TODO: move to proper testing package - from ai4papi.routers.v1.catalog.modules import Modules diff --git a/tests/catalog/tools.py b/tests/catalog/tools.py index 03be76c..032e2f1 100644 --- a/tests/catalog/tools.py +++ b/tests/catalog/tools.py @@ -1,5 +1,3 @@ -#TODO: move to proper testing package - from ai4papi.routers.v1.catalog.tools import Tools diff --git a/tests/deployments/modules.py b/tests/deployments/modules.py index bf98fb7..9b6a266 100644 --- a/tests/deployments/modules.py +++ b/tests/deployments/modules.py @@ -1,4 +1,3 @@ -#TODO: move to proper testing package import os import time from types import SimpleNamespace @@ -7,14 +6,14 @@ from ai4papi.routers.v1.deployments import tools -# Retrieve EGI token (not generated on the fly in case the are rate limitng issues +# Retrieve EGI token (not generated on the fly in case the are rate limiting issues # if too many queries) token = os.getenv('TMP_EGI_TOKEN') if not token: raise Exception( 'Please remember to set a token as ENV variable before executing \ the tests! \n\n \ - export TMP_EGI_TOKEN="$(oidc-token egi-checkin-demo)" \n\n \ + export TMP_EGI_TOKEN="$(oidc-token egi-checkin)" \n\n \ If running from VScode make sure to launch `code` from that terminal so it can access \ that ENV variable.' ) diff --git a/tests/deployments/tools.py b/tests/deployments/tools.py index 121ab28..be366b3 100644 --- a/tests/deployments/tools.py +++ b/tests/deployments/tools.py @@ -1,4 +1,3 @@ -#TODO: move to proper testing package import os import time from types import SimpleNamespace @@ -7,14 +6,14 @@ from ai4papi.routers.v1.deployments import tools -# Retrieve EGI token (not generated on the fly in case the are rate limitng issues +# Retrieve EGI token (not generated on the fly in case the are rate limiting issues # if too many queries) token = os.getenv('TMP_EGI_TOKEN') if not token: raise Exception( 'Please remember to set a token as ENV variable before executing \ the tests! \n\n \ - export TMP_EGI_TOKEN="$(oidc-token egi-checkin-demo)" \n\n \ + export TMP_EGI_TOKEN="$(oidc-token egi-checkin)" \n\n \ If running from VScode make sure to launch `code` from that terminal so it can access \ that ENV variable.' ) diff --git a/tests/main.py b/tests/main.py index 08f87d3..7be87d8 100644 --- a/tests/main.py +++ b/tests/main.py @@ -7,8 +7,12 @@ Nomad (ie. after launching) """ +#TODO: move to proper testing package +#TODO: rename test script: modules --> test_modules + import catalog.modules import catalog.tools import deployments.modules import deployments.tools import routes +import test_secrets diff --git a/tests/test_secrets.py b/tests/test_secrets.py new file mode 100644 index 0000000..f3ea026 --- /dev/null +++ b/tests/test_secrets.py @@ -0,0 +1,60 @@ +import os +from types import SimpleNamespace + +from ai4papi.routers.v1 import secrets + + +# Retrieve EGI token (not generated on the fly in case the are rate limiting issues +# if too many queries) +token = os.getenv('TMP_EGI_TOKEN') +if not token: + raise Exception( +'Please remember to set a token as ENV variable before executing \ +the tests! \n\n \ + export TMP_EGI_TOKEN="$(oidc-token egi-checkin)" \n\n \ +If running from VScode make sure to launch `code` from that terminal so it can access \ +that ENV variable.' + ) + +SECRET_PATH = '/demo-papi-tests/demo-secret' +SECRET_DATA = {'pwd': 12345} + +# Create secret +r = secrets.create_secret( + vo='vo.ai4eosc.eu', + secret_path=SECRET_PATH, + secret_data=SECRET_DATA, + authorization=SimpleNamespace( + credentials=token + ), +) + +# Check that secret is in list +r = secrets.get_secrets( + vo='vo.ai4eosc.eu', + authorization=SimpleNamespace( + credentials=token + ), +) +assert SECRET_PATH in r.keys() +assert r[SECRET_PATH] == SECRET_DATA + +# Delete +r = secrets.delete_secret( + vo='vo.ai4eosc.eu', + secret_path=SECRET_PATH, + authorization=SimpleNamespace( + credentials=token + ), +) + +# Check that secret is no longer in list +r = secrets.get_secrets( + vo='vo.ai4eosc.eu', + authorization=SimpleNamespace( + credentials=token + ), +) +assert SECRET_PATH not in r.keys() + +print('Secrets tests passed!')