From d49c8b8fb48faf7fce33bc7b8872f4d751e5e267 Mon Sep 17 00:00:00 2001 From: lleeoo Date: Tue, 14 Nov 2023 18:38:31 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(api)=20allow=20multiple=20authenticat?= =?UTF-8?q?ion=20backends=20simultaneously?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, Ralph allowed Basic or OIDC authentication, but not simultaneously. This PR allows to ralph to handle both at once, answer a use case where machine users connect through Basic auth, while human users use OIDC (for example). --- .env.dist | 2 +- CHANGELOG.md | 2 + docs/api.md | 5 +- docs/backends.md | 6 +-- src/ralph/api/auth/__init__.py | 54 ++++++++++++++++--- src/ralph/api/auth/basic.py | 24 ++------- src/ralph/api/auth/oidc.py | 26 +++------ src/ralph/backends/data/es.py | 2 +- src/ralph/cli.py | 8 +-- src/ralph/conf.py | 54 +++++++++++++++---- tests/api/auth/test_basic.py | 89 ++++++++++++++++++------------- tests/api/auth/test_oidc.py | 68 ++++++++++++++++++----- tests/api/test_statements_get.py | 44 ++++++++++----- tests/api/test_statements_post.py | 24 +++++---- tests/api/test_statements_put.py | 24 +++++---- tests/conftest.py | 2 - tests/fixtures/auth.py | 35 ------------ tests/helpers.py | 23 ++++++++ tests/test_conf.py | 33 +++++++++++- 19 files changed, 331 insertions(+), 194 deletions(-) diff --git a/.env.dist b/.env.dist index 383096847..cdcefb259 100644 --- a/.env.dist +++ b/.env.dist @@ -153,7 +153,7 @@ RALPH_BACKENDS__HTTP__LRS__STATEMENTS_ENDPOINT=/xAPI/statements # LRS API -RALPH_RUNSERVER_AUTH_BACKEND=basic +RALPH_RUNSERVER_AUTH_BACKENDS=basic RALPH_RUNSERVER_AUTH_OIDC_AUDIENCE=http://localhost:8100 RALPH_RUNSERVER_AUTH_OIDC_ISSUER_URI=http://learning-analytics-playground_keycloak_1:8080/auth/realms/fun-mooc RALPH_RUNSERVER_BACKEND=es diff --git a/CHANGELOG.md b/CHANGELOG.md index 05ca625d3..55cf7d747 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,8 @@ have an authority field matching that of the user - Backends: Replace reference to a JSON column in ClickHouse with function calls on the String column [BC] - API: enhance 'limit' query parameter's validation +- API: Variable `RUNSERVER_AUTH_BACKEND` becomes `RUNSERVER_AUTH_BACKENDS`, and + multiple authentication methods are supported simultaneously ### Fixed diff --git a/docs/api.md b/docs/api.md index 3e62c6a77..81d052f6f 100644 --- a/docs/api.md +++ b/docs/api.md @@ -108,9 +108,10 @@ $ curl --user john.doe@example.com:PASSWORD http://localhost:8100/whoami Ralph LRS API server supports OpenID Connect (OIDC) on top of OAuth 2.0 for authentication and authorization. -To enable OIDC auth, you should set the `RALPH_RUNSERVER_AUTH_BACKEND` environment variable as follows: + +To enable OIDC auth, you should modify the `RALPH_RUNSERVER_AUTH_BACKENDS` environment variable by adding (or replacing by) `oidc`: ```bash -RALPH_RUNSERVER_AUTH_BACKEND=oidc +RALPH_RUNSERVER_AUTH_BACKENDS=basic,oidc ``` and you should define the `RALPH_RUNSERVER_AUTH_OIDC_ISSUER_URI` environment variable with your identity provider's Issuer Identifier URI as follows: ```bash diff --git a/docs/backends.md b/docs/backends.md index 0d078d62c..4f1406dac 100644 --- a/docs/backends.md +++ b/docs/backends.md @@ -159,7 +159,7 @@ Elasticsearch backend parameters required to connect to a cluster are: - `hosts`: a list of cluster hosts to connect to (_e.g._ `["http://elasticsearch-node:9200"]`) - `index`: the elasticsearch index where to get/put documents -- `client_options`: a comma separated key=value list of Elasticsearch client options +- `client_options`: a comma-separated key=value list of Elasticsearch client options The Elasticsearch client options supported in Ralph are: - `ca_certs`: the path to the CA certificate file. @@ -177,7 +177,7 @@ MongoDB backend parameters required to connect to a cluster are: - `connection_uri`: the connection URI to connect to (_e.g._ `["mongodb://mongo:27017/"]`) - `database`: the database to connect to - `collection`: the collection to get/put objects to -- `client_options`: a comma separated key=value list of MongoDB client options +- `client_options`: a comma-separated key=value list of MongoDB client options The MongoDB client options supported in Ralph are: - `document_class`: default class to use for documents returned from queries @@ -196,7 +196,7 @@ ClickHouse parameters required to connect are: - `port`: the port to the ClickHouse HTTPS interface (_e.g._ `8123`) - `database`: the name of the database to connect to - `event_table_name`: the name of the table to write statements to -- `client_options`: a comma separated key=value list of ClickHouse client options +- `client_options`: a comma-separated key=value list of ClickHouse client options Secondary parameters are needed if not using the default ClickHouse user: diff --git a/src/ralph/api/auth/__init__.py b/src/ralph/api/auth/__init__.py index 80aa52fff..a4b5e3613 100644 --- a/src/ralph/api/auth/__init__.py +++ b/src/ralph/api/auth/__init__.py @@ -1,11 +1,51 @@ """Main module for Ralph's LRS API authentication.""" +from typing import Optional -from ralph.api.auth.basic import get_basic_auth_user +from fastapi import Depends, HTTPException, status +from fastapi.security import SecurityScopes + +from ralph.api.auth.basic import AuthenticatedUser, get_basic_auth_user from ralph.api.auth.oidc import get_oidc_user -from ralph.conf import settings +from ralph.conf import AuthBackend, settings + + +def get_authenticated_user( + security_scopes: SecurityScopes = SecurityScopes([]), + basic_auth_user: Optional[AuthenticatedUser] = Depends(get_basic_auth_user), + oidc_auth_user: Optional[AuthenticatedUser] = Depends(get_oidc_user), +) -> AuthenticatedUser: + """Authenticate user with any allowed method, using credentials in the header.""" + if AuthBackend.BASIC not in settings.RUNSERVER_AUTH_BACKENDS: + basic_auth_user = None + if AuthBackend.OIDC not in settings.RUNSERVER_AUTH_BACKENDS: + oidc_auth_user = None + + if basic_auth_user: + user = basic_auth_user + auth_header = "Basic" + elif oidc_auth_user: + user = oidc_auth_user + auth_header = "Bearer" + else: + auth_header = ",".join( + [ + {"basic": "Basic", "oidc": "Bearer"}[backend.value] + for backend in settings.RUNSERVER_AUTH_BACKENDS + ] + ) + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Invalid authentication credentials", + headers={"WWW-Authenticate": auth_header}, + ) -# At startup, select the authentication mode that will be used -if settings.RUNSERVER_AUTH_BACKEND == settings.AuthBackends.OIDC: - get_authenticated_user = get_oidc_user -else: - get_authenticated_user = get_basic_auth_user + # Restrict access by scopes + if settings.LRS_RESTRICT_BY_SCOPES: + for requested_scope in security_scopes.scopes: + if not user.scopes.is_authorized(requested_scope): + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail=f'Access not authorized to scope: "{requested_scope}".', + headers={"WWW-Authenticate": auth_header}, + ) + return user diff --git a/src/ralph/api/auth/basic.py b/src/ralph/api/auth/basic.py index f309ba371..759e83054 100644 --- a/src/ralph/api/auth/basic.py +++ b/src/ralph/api/auth/basic.py @@ -9,7 +9,7 @@ import bcrypt from cachetools import TTLCache, cached from fastapi import Depends, HTTPException, status -from fastapi.security import HTTPBasic, HTTPBasicCredentials, SecurityScopes +from fastapi.security import HTTPBasic, HTTPBasicCredentials from pydantic import BaseModel, root_validator from starlette.authentication import AuthenticationError @@ -102,17 +102,15 @@ def get_stored_credentials(auth_file: Path) -> ServerUsersCredentials: @cached( TTLCache(maxsize=settings.AUTH_CACHE_MAX_SIZE, ttl=settings.AUTH_CACHE_TTL), lock=Lock(), - key=lambda credentials, security_scopes: ( + key=lambda credentials: ( credentials.username, credentials.password, - security_scopes.scope_str, ) if credentials is not None else None, ) def get_basic_auth_user( credentials: Optional[HTTPBasicCredentials] = Depends(security), - security_scopes: SecurityScopes = SecurityScopes([]), ) -> AuthenticatedUser: """Check valid auth parameters. @@ -121,18 +119,13 @@ def get_basic_auth_user( Args: credentials (iterator): auth parameters from the Authorization header - security_scopes: scopes requested for access Raises: HTTPException """ if not credentials: - logger.error("The basic authentication mode requires a Basic Auth header") - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Could not validate credentials", - headers={"WWW-Authenticate": "Basic"}, - ) + logger.debug("No credentials were found for Basic auth") + return None try: user = next( @@ -185,13 +178,4 @@ def get_basic_auth_user( user = AuthenticatedUser(scopes=user.scopes, agent=dict(user.agent)) - # Restrict access by scopes - if settings.LRS_RESTRICT_BY_SCOPES: - for requested_scope in security_scopes.scopes: - if not user.scopes.is_authorized(requested_scope): - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail=f'Access not authorized to scope: "{requested_scope}".', - headers={"WWW-Authenticate": "Basic"}, - ) return user diff --git a/src/ralph/api/auth/oidc.py b/src/ralph/api/auth/oidc.py index e548e7a5c..1edb6b420 100644 --- a/src/ralph/api/auth/oidc.py +++ b/src/ralph/api/auth/oidc.py @@ -6,7 +6,7 @@ import requests from fastapi import Depends, HTTPException, status -from fastapi.security import OpenIdConnect, SecurityScopes +from fastapi.security import HTTPBearer, OpenIdConnect from jose import ExpiredSignatureError, JWTError, jwt from jose.exceptions import JWTClaimsError from pydantic import AnyUrl, BaseModel, Extra @@ -94,8 +94,7 @@ def get_public_keys(jwks_uri: AnyUrl) -> Dict: def get_oidc_user( - auth_header: Annotated[Optional[str], Depends(oauth2_scheme)], - security_scopes: SecurityScopes = SecurityScopes([]), + auth_header: Annotated[Optional[HTTPBearer], Depends(oauth2_scheme)], ) -> AuthenticatedUser: """Decode and validate OpenId Connect ID token against issuer in config. @@ -110,13 +109,12 @@ def get_oidc_user( Raises: HTTPException """ - if auth_header is None or "Bearer" not in auth_header: - logger.error("The OpenID Connect authentication mode requires a Bearer token") - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Could not validate credentials", - headers={"WWW-Authenticate": "Bearer"}, + if auth_header is None or "bearer" not in auth_header.lower(): + logger.debug( + "Not using OIDC auth. The OpenID Connect authentication mode requires a " + "Bearer token" ) + return None id_token = auth_header.split(" ")[-1] provider_config = discover_provider(settings.RUNSERVER_AUTH_OIDC_ISSUER_URI) @@ -151,14 +149,4 @@ def get_oidc_user( scopes=UserScopes(id_token.scope.split(" ") if id_token.scope else []), ) - # Restrict access by scopes - if settings.LRS_RESTRICT_BY_SCOPES: - for requested_scope in security_scopes.scopes: - if not user.scopes.is_authorized(requested_scope): - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail=f'Access not authorized to scope: "{requested_scope}".', - headers={"WWW-Authenticate": "Basic"}, - ) - return user diff --git a/src/ralph/backends/data/es.py b/src/ralph/backends/data/es.py index cc127223b..03bb96b13 100644 --- a/src/ralph/backends/data/es.py +++ b/src/ralph/backends/data/es.py @@ -45,7 +45,7 @@ class ESDataBackendSettings(BaseDataBackendSettings): DEFAULT_CHUNK_SIZE (int): The default chunk size for reading batches of documents. DEFAULT_INDEX (str): The default index to use for querying Elasticsearch. - HOSTS (str or tuple): The comma separated list of Elasticsearch nodes to + HOSTS (str or tuple): The comma-separated list of Elasticsearch nodes to connect to. LOCALE_ENCODING (str): The encoding used for reading/writing documents. POINT_IN_TIME_KEEP_ALIVE (str): The duration for which Elasticsearch should diff --git a/src/ralph/cli.py b/src/ralph/cli.py index de483e90e..7e9f5a17d 100644 --- a/src/ralph/cli.py +++ b/src/ralph/cli.py @@ -61,7 +61,7 @@ class CommaSeparatedTupleParamType(click.ParamType): - """Comma separated tuple parameter type.""" + """Comma-separated tuple parameter type.""" name = "value1,value2,value3" @@ -81,7 +81,7 @@ def convert(self, value, param, ctx): class CommaSeparatedKeyValueParamType(click.ParamType): - """Comma separated key=value parameter type.""" + """Comma-separated key=value parameter type.""" name = "key=value,key=value" @@ -123,7 +123,7 @@ def convert(self, value, param, ctx): class ClientOptionsParamType(CommaSeparatedKeyValueParamType): - """Comma separated key=value parameter type for client options.""" + """Comma-separated key=value parameter type for client options.""" def __init__(self, client_options_type: Any) -> None: """Instantiate ClientOptionsParamType for a client_options_type. @@ -145,7 +145,7 @@ def convert(self, value, param, ctx): class HeadersParametersParamType(CommaSeparatedKeyValueParamType): - """Comma separated key=value parameter type for headers parameters.""" + """Comma-separated key=value parameter type for headers parameters.""" def __init__(self, headers_parameters_type: Any) -> None: """Instantiate HeadersParametersParamType for a headers_parameters_type. diff --git a/src/ralph/conf.py b/src/ralph/conf.py index 3ee6427cc..c44dceccc 100644 --- a/src/ralph/conf.py +++ b/src/ralph/conf.py @@ -4,7 +4,7 @@ import sys from enum import Enum from pathlib import Path -from typing import List, Sequence, Union +from typing import List, Sequence, Tuple, Union from pydantic import AnyHttpUrl, AnyUrl, BaseModel, BaseSettings, Extra, root_validator @@ -53,19 +53,19 @@ class Config(BaseSettingsConfig): class CommaSeparatedTuple(str): - """Pydantic field type validating comma separated strings or lists/tuples.""" + """Pydantic field type validating comma-separated strings or lists/tuples.""" @classmethod def __get_validators__(cls): # noqa: D105 def validate(value: Union[str, Sequence[str]]) -> Sequence[str]: - """Check whether the value is a comma separated string or a list/tuple.""" + """Check whether the value is a comma-separated string or a list/tuple.""" if isinstance(value, (tuple, list)): return tuple(value) if isinstance(value, str): return tuple(value.split(",")) - raise TypeError("Invalid comma separated list") + raise TypeError("Invalid comma-separated list") yield validate @@ -133,6 +133,44 @@ class Config: # noqa: D106 timeout: float +class AuthBackend(str, Enum): + """Model for valid authentication methods.""" + + BASIC = "basic" + OIDC = "oidc" + + +class AuthBackends(Tuple[AuthBackend]): + """Model representing a tuple of authentication backends.""" + + @classmethod + def __get_validators__(cls): + """Check whether the value is a comma-separated string or a tuple representing + an AuthBackend. + """ # noqa: D205 + + def validate( + auth_backends: Union[ + str, AuthBackend, Tuple[AuthBackend], List[AuthBackend] + ] + ) -> Tuple[AuthBackend]: + """Check whether the value is a comma-separated string or a list/tuple.""" + if isinstance(auth_backends, str): + return tuple( + AuthBackend(value.lower()) for value in auth_backends.split(",") + ) + + if isinstance(auth_backends, AuthBackend): + return (auth_backends,) + + if isinstance(auth_backends, (tuple, list)): + return tuple(auth_backends) + + raise TypeError("Invalid comma-separated list") + + yield validate + + class Settings(BaseSettings): """Pydantic model for Ralph's global environment & configuration settings.""" @@ -142,12 +180,6 @@ class Config(BaseSettingsConfig): env_file = ".env" env_file_encoding = core_settings.LOCALE_ENCODING - class AuthBackends(Enum): - """Enum of the authentication backends.""" - - BASIC = "basic" - OIDC = "oidc" - _CORE: CoreSettings = core_settings AUTH_FILE: Path = _CORE.APP_DIR / "auth.json" AUTH_CACHE_MAX_SIZE = 100 @@ -188,7 +220,7 @@ class AuthBackends(Enum): }, } PARSERS: ParserSettings = ParserSettings() - RUNSERVER_AUTH_BACKEND: AuthBackends = AuthBackends.BASIC + RUNSERVER_AUTH_BACKENDS: AuthBackends = AuthBackends("basic") RUNSERVER_AUTH_OIDC_AUDIENCE: str = None RUNSERVER_AUTH_OIDC_ISSUER_URI: AnyHttpUrl = None RUNSERVER_BACKEND: Literal[ diff --git a/tests/api/auth/test_basic.py b/tests/api/auth/test_basic.py index b761701df..5ee1f99e4 100644 --- a/tests/api/auth/test_basic.py +++ b/tests/api/auth/test_basic.py @@ -6,8 +6,10 @@ import bcrypt import pytest from fastapi.exceptions import HTTPException -from fastapi.security import HTTPBasicCredentials, SecurityScopes +from fastapi.security import HTTPBasicCredentials +from fastapi.testclient import TestClient +from ralph.api import app from ralph.api.auth.basic import ( ServerUsersCredentials, UserCredentials, @@ -15,7 +17,9 @@ get_stored_credentials, ) from ralph.api.auth.user import AuthenticatedUser, UserScopes -from ralph.conf import Settings, settings +from ralph.conf import AuthBackend, Settings, settings + +from tests.helpers import configure_env_for_mock_oidc_auth STORED_CREDENTIALS = json.dumps( [ @@ -29,6 +33,9 @@ ) +client = TestClient(app) + + def test_api_auth_basic_model_serveruserscredentials(): """Test api.auth ServerUsersCredentials model.""" @@ -103,12 +110,10 @@ def test_api_auth_basic_caching_credentials(fs): credentials = HTTPBasicCredentials(username="ralph", password="admin") # Call function as in a first request with these credentials - get_basic_auth_user( - security_scopes=SecurityScopes(["profile/read"]), credentials=credentials - ) + get_basic_auth_user(credentials=credentials) assert get_basic_auth_user.cache.popitem() == ( - ("ralph", "admin", "profile/read"), + ("ralph", "admin"), AuthenticatedUser( agent={"mbox": "mailto:ralph@example.com"}, scopes=UserScopes(["statements/read/mine", "statements/write"]), @@ -127,7 +132,7 @@ def test_api_auth_basic_with_wrong_password(fs): # Call function as in a first request with these credentials with pytest.raises(HTTPException): - get_basic_auth_user(credentials, SecurityScopes(["all"])) + get_basic_auth_user(credentials) def test_api_auth_basic_no_credential_file_found(fs, monkeypatch): @@ -140,38 +145,26 @@ def test_api_auth_basic_no_credential_file_found(fs, monkeypatch): credentials = HTTPBasicCredentials(username="ralph", password="admin") with pytest.raises(HTTPException): - get_basic_auth_user(credentials, SecurityScopes(["all"])) + get_basic_auth_user(credentials) -def test_get_whoami_no_credentials(basic_auth_test_client): +def test_api_auth_basic_get_whoami_no_credentials(): """Whoami route returns a 401 error when no credentials are sent.""" - response = basic_auth_test_client.get("/whoami") - assert response.status_code == 401 - assert response.headers["www-authenticate"] == "Basic" - assert response.json() == {"detail": "Could not validate credentials"} - - -def test_get_whoami_credentials_wrong_scheme(basic_auth_test_client): - """Whoami route returns a 401 error when wrong scheme is used for authorization.""" - response = basic_auth_test_client.get( - "/whoami", headers={"Authorization": "Bearer sometoken"} - ) + response = client.get("/whoami") assert response.status_code == 401 assert response.headers["www-authenticate"] == "Basic" - assert response.json() == {"detail": "Could not validate credentials"} + assert response.json() == {"detail": "Invalid authentication credentials"} -def test_get_whoami_credentials_encoding_error(basic_auth_test_client): +def test_api_auth_basic_get_whoami_credentials_encoding_error(): """Whoami route returns a 401 error when the credentials encoding is broken.""" - response = basic_auth_test_client.get( - "/whoami", headers={"Authorization": "Basic not-base64"} - ) + response = client.get("/whoami", headers={"Authorization": "Basic not-base64"}) assert response.status_code == 401 assert response.headers["www-authenticate"] == "Basic" assert response.json() == {"detail": "Invalid authentication credentials"} -def test_get_whoami_username_not_found(basic_auth_test_client, fs): +def test_api_auth_basic_get_whoami_username_not_found(fs): """Whoami route returns a 401 error when the username cannot be found.""" credential_bytes = base64.b64encode("john:admin".encode("utf-8")) credentials = str(credential_bytes, "utf-8") @@ -180,16 +173,14 @@ def test_get_whoami_username_not_found(basic_auth_test_client, fs): auth_file_path = settings.APP_DIR / "auth.json" fs.create_file(auth_file_path, contents=STORED_CREDENTIALS) - response = basic_auth_test_client.get( - "/whoami", headers={"Authorization": f"Basic {credentials}"} - ) + response = client.get("/whoami", headers={"Authorization": f"Basic {credentials}"}) assert response.status_code == 401 assert response.headers["www-authenticate"] == "Basic" assert response.json() == {"detail": "Invalid authentication credentials"} -def test_get_whoami_wrong_password(basic_auth_test_client, fs): +def test_api_auth_basic_get_whoami_wrong_password(fs): """Whoami route returns a 401 error when the password is wrong.""" credential_bytes = base64.b64encode("john:not-admin".encode("utf-8")) credentials = str(credential_bytes, "utf-8") @@ -198,20 +189,25 @@ def test_get_whoami_wrong_password(basic_auth_test_client, fs): fs.create_file(auth_file_path, contents=STORED_CREDENTIALS) get_basic_auth_user.cache_clear() - response = basic_auth_test_client.get( - "/whoami", headers={"Authorization": f"Basic {credentials}"} - ) + response = client.get("/whoami", headers={"Authorization": f"Basic {credentials}"}) assert response.status_code == 401 - assert response.headers["www-authenticate"] == "Basic" assert response.json() == {"detail": "Invalid authentication credentials"} -def test_get_whoami_correct_credentials(basic_auth_test_client, fs): +@pytest.mark.parametrize( + "runserver_auth_backends", + [[AuthBackend.BASIC, AuthBackend.OIDC], [AuthBackend.BASIC]], +) +def test_api_auth_basic_get_whoami_correct_credentials( + fs, monkeypatch, runserver_auth_backends +): """Whoami returns a 200 response when the credentials are correct. Return the username and associated scopes. """ + configure_env_for_mock_oidc_auth(monkeypatch, runserver_auth_backends) + credential_bytes = base64.b64encode("ralph:admin".encode("utf-8")) credentials = str(credential_bytes, "utf-8") @@ -219,9 +215,7 @@ def test_get_whoami_correct_credentials(basic_auth_test_client, fs): fs.create_file(auth_file_path, contents=STORED_CREDENTIALS) get_basic_auth_user.cache_clear() - response = basic_auth_test_client.get( - "/whoami", headers={"Authorization": f"Basic {credentials}"} - ) + response = client.get("/whoami", headers={"Authorization": f"Basic {credentials}"}) assert response.status_code == 200 @@ -231,3 +225,22 @@ def test_get_whoami_correct_credentials(basic_auth_test_client, fs): "statements/read/mine", "statements/write", ] + + +def test_api_auth_basic_get_whoami_invalid_backend(fs, monkeypatch): + """Check for an exception when providing valid credentials when Basic + authentication is not supported. + """ + configure_env_for_mock_oidc_auth(monkeypatch, [AuthBackend.OIDC]) + + credential_bytes = base64.b64encode("ralph:admin".encode("utf-8")) + credentials = str(credential_bytes, "utf-8") + + auth_file_path = settings.APP_DIR / "auth.json" + fs.create_file(auth_file_path, contents=STORED_CREDENTIALS) + get_basic_auth_user.cache_clear() + + response = client.get("/whoami", headers={"Authorization": f"Basic {credentials}"}) + + assert response.status_code == 401 + assert response.json() == {"detail": "Invalid authentication credentials"} diff --git a/tests/api/auth/test_oidc.py b/tests/api/auth/test_oidc.py index a0b621f01..5dd45e61c 100644 --- a/tests/api/auth/test_oidc.py +++ b/tests/api/auth/test_oidc.py @@ -1,23 +1,36 @@ """Tests for the api.auth.oidc module.""" - +import pytest import responses +from fastapi.testclient import TestClient from pydantic import parse_obj_as +from ralph.api import app from ralph.api.auth.oidc import discover_provider, get_public_keys +from ralph.conf import AuthBackend from ralph.models.xapi.base.agents import BaseXapiAgentWithOpenId from tests.fixtures.auth import ISSUER_URI, mock_oidc_user +from tests.helpers import configure_env_for_mock_oidc_auth + +client = TestClient(app) +@pytest.mark.parametrize( + "runserver_auth_backends", + [[AuthBackend.BASIC, AuthBackend.OIDC], [AuthBackend.OIDC]], +) @responses.activate -def test_api_auth_oidc_valid(oidc_auth_test_client): +def test_api_auth_oidc_get_whoami_valid(monkeypatch, runserver_auth_backends): """Test a valid OpenId Connect authentication.""" + configure_env_for_mock_oidc_auth(monkeypatch, runserver_auth_backends) + oidc_token = mock_oidc_user(scopes=["all", "profile/read"]) - response = oidc_auth_test_client.get( + headers = {"Authorization": f"Bearer {oidc_token}"} + response = client.get( "/whoami", - headers={"Authorization": f"Bearer {oidc_token}"}, + headers=headers, ) assert response.status_code == 200 assert len(response.json().keys()) == 2 @@ -27,14 +40,16 @@ def test_api_auth_oidc_valid(oidc_auth_test_client): @responses.activate -def test_api_auth_invalid_token( - oidc_auth_test_client, mock_discovery_response, mock_oidc_jwks +def test_api_auth_oidc_get_whoami_invalid_token( + monkeypatch, mock_discovery_response, mock_oidc_jwks ): """Test API with an invalid audience.""" + configure_env_for_mock_oidc_auth(monkeypatch) + mock_oidc_user() - response = oidc_auth_test_client.get( + response = client.get( "/whoami", headers={"Authorization": "Bearer wrong_token"}, ) @@ -45,9 +60,11 @@ def test_api_auth_invalid_token( @responses.activate -def test_api_auth_invalid_discovery(oidc_auth_test_client, encoded_token): +def test_api_auth_oidc_get_whoami_invalid_discovery(monkeypatch, encoded_token): """Test API with an invalid provider discovery.""" + configure_env_for_mock_oidc_auth(monkeypatch) + # Clear LRU cache discover_provider.cache_clear() get_public_keys.cache_clear() @@ -60,7 +77,7 @@ def test_api_auth_invalid_discovery(oidc_auth_test_client, encoded_token): status=500, ) - response = oidc_auth_test_client.get( + response = client.get( "/whoami", headers={"Authorization": f"Bearer {encoded_token}"}, ) @@ -71,11 +88,13 @@ def test_api_auth_invalid_discovery(oidc_auth_test_client, encoded_token): @responses.activate -def test_api_auth_invalid_keys( - oidc_auth_test_client, mock_discovery_response, mock_oidc_jwks, encoded_token +def test_api_auth_oidc_get_whoami_invalid_keys( + monkeypatch, mock_discovery_response, mock_oidc_jwks, encoded_token ): """Test API with an invalid request for keys.""" + configure_env_for_mock_oidc_auth(monkeypatch) + # Clear LRU cache discover_provider.cache_clear() get_public_keys.cache_clear() @@ -96,7 +115,7 @@ def test_api_auth_invalid_keys( status=500, ) - response = oidc_auth_test_client.get( + response = client.get( "/whoami", headers={"Authorization": f"Bearer {encoded_token}"}, ) @@ -107,16 +126,37 @@ def test_api_auth_invalid_keys( @responses.activate -def test_api_auth_invalid_header(oidc_auth_test_client): +def test_api_auth_oidc_get_whoami_invalid_header(monkeypatch): """Test API with an invalid request header.""" + configure_env_for_mock_oidc_auth(monkeypatch) + oidc_token = mock_oidc_user() - response = oidc_auth_test_client.get( + response = client.get( "/whoami", headers={"Authorization": f"Wrong header {oidc_token}"}, ) assert response.status_code == 401 assert response.headers["www-authenticate"] == "Bearer" + assert response.json() == {"detail": "Invalid authentication credentials"} + + +def test_api_auth_oidc_get_whoami_invalid_backend(fs, monkeypatch): + """Check for an exception when providing valid OIDC credentials while + OIDC authentication is not supported. + """ + + configure_env_for_mock_oidc_auth(monkeypatch, [AuthBackend.BASIC]) + + oidc_token = mock_oidc_user(scopes=["all", "profile/read"]) + + headers = {"Authorization": f"Bearer {oidc_token}"} + response = client.get( + "/whoami", + headers=headers, + ) + + assert response.status_code == 401 assert response.json() == {"detail": "Could not validate credentials"} diff --git a/tests/api/test_statements_get.py b/tests/api/test_statements_get.py index 449183f30..39d7f4468 100644 --- a/tests/api/test_statements_get.py +++ b/tests/api/test_statements_get.py @@ -8,13 +8,11 @@ import responses from elasticsearch.helpers import bulk -from ralph.api import app -from ralph.api.auth import get_authenticated_user from ralph.api.auth.basic import get_basic_auth_user -from ralph.api.auth.oidc import get_oidc_user from ralph.backends.data.base import BaseOperationType from ralph.backends.data.clickhouse import ClickHouseDataBackend from ralph.backends.data.mongo import MongoDataBackend +from ralph.conf import AuthBackend from ralph.exceptions import BackendException from tests.fixtures.backends import ( @@ -32,7 +30,7 @@ get_mongo_test_backend, ) -from ..fixtures.auth import mock_basic_auth_user, mock_oidc_user +from ..fixtures.auth import AUDIENCE, ISSUER_URI, mock_basic_auth_user, mock_oidc_user from ..helpers import mock_activity, mock_agent @@ -799,20 +797,43 @@ async def test_api_statements_get_scopes( # noqa: PLR0913 ): """Test that getting statements behaves properly according to user scopes.""" + # Scopes settings monkeypatch.setattr( "ralph.api.routers.statements.settings.LRS_RESTRICT_BY_SCOPES", True ) - monkeypatch.setattr("ralph.api.auth.basic.settings.LRS_RESTRICT_BY_SCOPES", True) + monkeypatch.setattr( + f"ralph.api.auth.{auth_method}.settings.LRS_RESTRICT_BY_SCOPES", True + ) + # Authority settings + monkeypatch.setattr( + "ralph.api.routers.statements.settings.LRS_RESTRICT_BY_AUTHORITY", True + ) + monkeypatch.setattr( + f"ralph.api.auth.{auth_method}.settings.LRS_RESTRICT_BY_AUTHORITY", True + ) + + # Set up the authentication method if auth_method == "basic": agent = mock_agent("mbox", 1) credentials = mock_basic_auth_user(fs, scopes=scopes, agent=agent) headers = {"Authorization": f"Basic {credentials}"} - - app.dependency_overrides[get_authenticated_user] = get_basic_auth_user get_basic_auth_user.cache_clear() elif auth_method == "oidc": + monkeypatch.setenv("RUNSERVER_AUTH_BACKENDS", [AuthBackend.OIDC]) + monkeypatch.setattr( + "ralph.api.auth.settings.RUNSERVER_AUTH_BACKENDS", [AuthBackend.OIDC] + ) + monkeypatch.setattr( + "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_ISSUER_URI", + ISSUER_URI, + ) + monkeypatch.setattr( + "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_AUDIENCE", + AUDIENCE, + ) + sub = "123|oidc" iss = "https://iss.example.com" agent = {"openid": f"{iss}/{sub}"} @@ -828,8 +849,7 @@ async def test_api_statements_get_scopes( # noqa: PLR0913 "http://clientHost:8100", ) - app.dependency_overrides[get_authenticated_user] = get_oidc_user - + # Mock statements statements = [ { "id": "be67b160-d958-4f51-b8b8-1892002dbac6", @@ -850,6 +870,7 @@ async def test_api_statements_get_scopes( # noqa: PLR0913 insert_es_statements(es, statements) monkeypatch.setattr(backend_client_class_path, get_es_test_backend()) + # Fetch Statements response = await client.get( "/xAPI/statements/", headers=headers, @@ -864,8 +885,6 @@ async def test_api_statements_get_scopes( # noqa: PLR0913 "detail": 'Access not authorized to scope: "statements/read/mine".' } - app.dependency_overrides.pop(get_authenticated_user, None) - @pytest.mark.anyio @pytest.mark.parametrize( @@ -893,6 +912,7 @@ async def test_api_statements_get_scopes_with_authority( # noqa: PLR0913 "ralph.api.routers.statements.settings.LRS_RESTRICT_BY_SCOPES", True ) monkeypatch.setattr("ralph.api.auth.basic.settings.LRS_RESTRICT_BY_SCOPES", True) + monkeypatch.setattr("ralph.api.auth.oidc.settings.LRS_RESTRICT_BY_SCOPES", True) agent = mock_agent("mbox", 1) agent_2 = mock_agent("mbox", 2) @@ -934,5 +954,3 @@ async def test_api_statements_get_scopes_with_authority( # noqa: PLR0913 assert response.json() == {"statements": [statements[1], statements[0]]} else: assert response.json() == {"statements": [statements[0]]} - - app.dependency_overrides.pop(get_authenticated_user, None) diff --git a/tests/api/test_statements_post.py b/tests/api/test_statements_post.py index b8396342f..92b2ce636 100644 --- a/tests/api/test_statements_post.py +++ b/tests/api/test_statements_post.py @@ -8,15 +8,18 @@ from httpx import AsyncClient from ralph.api import app -from ralph.api.auth import get_authenticated_user from ralph.api.auth.basic import get_basic_auth_user -from ralph.api.auth.oidc import get_oidc_user from ralph.backends.lrs.es import ESLRSBackend from ralph.backends.lrs.mongo import MongoLRSBackend -from ralph.conf import XapiForwardingConfigurationSettings +from ralph.conf import AuthBackend, XapiForwardingConfigurationSettings from ralph.exceptions import BackendException -from tests.fixtures.auth import mock_basic_auth_user, mock_oidc_user +from tests.fixtures.auth import ( + AUDIENCE, + ISSUER_URI, + mock_basic_auth_user, + mock_oidc_user, +) from tests.fixtures.backends import ( ES_TEST_FORWARDING_INDEX, ES_TEST_HOSTS, @@ -709,7 +712,6 @@ async def test_api_statements_post_scopes( # noqa: PLR0913 credentials = mock_basic_auth_user(fs, scopes=scopes, agent=agent) headers = {"Authorization": f"Basic {credentials}"} - app.dependency_overrides[get_authenticated_user] = get_basic_auth_user get_basic_auth_user.cache_clear() elif auth_method == "oidc": @@ -718,17 +720,19 @@ async def test_api_statements_post_scopes( # noqa: PLR0913 oidc_token = mock_oidc_user(sub=sub, scopes=scopes) headers = {"Authorization": f"Bearer {oidc_token}"} + monkeypatch.setenv("RUNSERVER_AUTH_BACKENDS", [AuthBackend.OIDC]) + monkeypatch.setattr( + "ralph.api.auth.settings.RUNSERVER_AUTH_BACKENDS", [AuthBackend.OIDC] + ) monkeypatch.setattr( "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_ISSUER_URI", - "http://providerHost:8080/auth/realms/real_name", + ISSUER_URI, ) monkeypatch.setattr( "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_AUDIENCE", - "http://clientHost:8100", + AUDIENCE, ) - app.dependency_overrides[get_authenticated_user] = get_oidc_user - statement = mock_statement() # NB: scopes are not linked to statements and backends, we therefore test with ES @@ -748,5 +752,3 @@ async def test_api_statements_post_scopes( # noqa: PLR0913 assert response.json() == { "detail": 'Access not authorized to scope: "statements/write".' } - - app.dependency_overrides.pop(get_authenticated_user, None) diff --git a/tests/api/test_statements_put.py b/tests/api/test_statements_put.py index 41db55db6..79ff01282 100644 --- a/tests/api/test_statements_put.py +++ b/tests/api/test_statements_put.py @@ -6,15 +6,18 @@ from httpx import AsyncClient from ralph.api import app -from ralph.api.auth import get_authenticated_user from ralph.api.auth.basic import get_basic_auth_user -from ralph.api.auth.oidc import get_oidc_user from ralph.backends.lrs.es import ESLRSBackend from ralph.backends.lrs.mongo import MongoLRSBackend -from ralph.conf import XapiForwardingConfigurationSettings +from ralph.conf import AuthBackend, XapiForwardingConfigurationSettings from ralph.exceptions import BackendException -from tests.fixtures.auth import mock_basic_auth_user, mock_oidc_user +from tests.fixtures.auth import ( + AUDIENCE, + ISSUER_URI, + mock_basic_auth_user, + mock_oidc_user, +) from tests.fixtures.backends import ( ES_TEST_FORWARDING_INDEX, ES_TEST_HOSTS, @@ -597,7 +600,6 @@ async def test_api_statements_put_scopes( # noqa: PLR0913 credentials = mock_basic_auth_user(fs, scopes=scopes, agent=agent) headers = {"Authorization": f"Basic {credentials}"} - app.dependency_overrides[get_authenticated_user] = get_basic_auth_user get_basic_auth_user.cache_clear() elif auth_method == "oidc": @@ -606,17 +608,19 @@ async def test_api_statements_put_scopes( # noqa: PLR0913 oidc_token = mock_oidc_user(sub=sub, scopes=scopes) headers = {"Authorization": f"Bearer {oidc_token}"} + monkeypatch.setenv("RUNSERVER_AUTH_BACKENDS", [AuthBackend.OIDC]) + monkeypatch.setattr( + "ralph.api.auth.settings.RUNSERVER_AUTH_BACKENDS", [AuthBackend.OIDC] + ) monkeypatch.setattr( "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_ISSUER_URI", - "http://providerHost:8080/auth/realms/real_name", + ISSUER_URI, ) monkeypatch.setattr( "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_AUDIENCE", - "http://clientHost:8100", + AUDIENCE, ) - app.dependency_overrides[get_authenticated_user] = get_oidc_user - statement = mock_statement() # NB: scopes are not linked to statements and backends, we therefore test with ES @@ -636,5 +640,3 @@ async def test_api_statements_put_scopes( # noqa: PLR0913 assert response.json() == { "detail": 'Access not authorized to scope: "statements/write".' } - - app.dependency_overrides.pop(get_authenticated_user, None) diff --git a/tests/conftest.py b/tests/conftest.py index 47411907d..6b1050e95 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -8,11 +8,9 @@ from .fixtures.api import client # noqa: F401 from .fixtures.auth import ( # noqa: F401 basic_auth_credentials, - basic_auth_test_client, encoded_token, mock_discovery_response, mock_oidc_jwks, - oidc_auth_test_client, ) from .fixtures.backends import ( # noqa: F401 anyio_backend, diff --git a/tests/fixtures/auth.py b/tests/fixtures/auth.py index 4ba182a0b..b0a0e13c3 100644 --- a/tests/fixtures/auth.py +++ b/tests/fixtures/auth.py @@ -8,11 +8,9 @@ import pytest import responses from cryptography.hazmat.primitives import serialization -from fastapi.testclient import TestClient from jose import jwt from jose.utils import long_to_base64 -from ralph.api import app, get_authenticated_user from ralph.api.auth.basic import get_stored_credentials from ralph.api.auth.oidc import discover_provider, get_public_keys from ralph.conf import settings @@ -103,39 +101,6 @@ def basic_auth_credentials(fs, user_scopes=None, agent=None): return credentials -@pytest.fixture -def basic_auth_test_client(): - """Return a TestClient with HTTP basic authentication mode.""" - - from ralph.api.auth.basic import ( - get_basic_auth_user, - ) - - app.dependency_overrides[get_authenticated_user] = get_basic_auth_user - - with TestClient(app) as test_client: - yield test_client - - -@pytest.fixture -def oidc_auth_test_client(monkeypatch): - """Return a TestClient with OpenId Connect authentication mode.""" - - monkeypatch.setattr( - "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_ISSUER_URI", - ISSUER_URI, - ) - monkeypatch.setattr( - "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_AUDIENCE", - AUDIENCE, - ) - from ralph.api.auth.oidc import get_oidc_user - - app.dependency_overrides[get_authenticated_user] = get_oidc_user - with TestClient(app) as test_client: - yield test_client - - def _mock_discovery_response(): """Return an example discovery response.""" return { diff --git a/tests/helpers.py b/tests/helpers.py index 337f7628e..a513168a2 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -7,8 +7,11 @@ from typing import Optional, Union from uuid import UUID +from ralph.api.auth import AuthBackend from ralph.utils import statements_are_equivalent +from tests.fixtures.auth import AUDIENCE, ISSUER_URI + def string_is_date(string: str): """Check if string can be parsed as a date.""" @@ -196,3 +199,23 @@ def mock_statement( "object": object, "timestamp": timestamp, } + + +def configure_env_for_mock_oidc_auth(monkeypatch, runserver_auth_backends=None): + """Configure environment variables to simulate OIDC use.""" + + if runserver_auth_backends is None: + runserver_auth_backends = [AuthBackend.OIDC] + + monkeypatch.setenv("RUNSERVER_AUTH_BACKENDS", runserver_auth_backends) + monkeypatch.setattr( + "ralph.api.auth.settings.RUNSERVER_AUTH_BACKENDS", runserver_auth_backends + ) + monkeypatch.setattr( + "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_ISSUER_URI", + ISSUER_URI, + ) + monkeypatch.setattr( + "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_AUDIENCE", + AUDIENCE, + ) diff --git a/tests/test_conf.py b/tests/test_conf.py index b1427797e..9d30f4e8e 100644 --- a/tests/test_conf.py +++ b/tests/test_conf.py @@ -6,7 +6,13 @@ from ralph import conf from ralph.backends.data.es import ESDataBackend -from ralph.conf import CommaSeparatedTuple, Settings, settings +from ralph.conf import ( + AuthBackend, + AuthBackends, + CommaSeparatedTuple, + Settings, + settings, +) from ralph.exceptions import ConfigurationException @@ -54,10 +60,33 @@ def test_conf_comma_separated_list_with_valid_values(value, expected, monkeypatc @pytest.mark.parametrize("value", [{}, None]) def test_conf_comma_separated_list_with_invalid_values(value): """Test the CommaSeparatedTuple pydantic data type with invalid values.""" - with pytest.raises(TypeError, match="Invalid comma separated list"): + with pytest.raises(TypeError, match="Invalid comma-separated list"): next(CommaSeparatedTuple.__get_validators__())(value) +@pytest.mark.parametrize( + "value,is_valid,expected", + [ + ("oidc", True, (AuthBackend.OIDC,)), + ("basic", True, (AuthBackend.BASIC,)), + ("bASIc", True, (AuthBackend.BASIC,)), + ("oidc,basic", True, (AuthBackend.OIDC, AuthBackend.BASIC)), + ("notvalid", False, None), + ("basic,notvalid", False, None), + ], +) +def test_conf_auth_backend(value, is_valid, expected, monkeypatch): + """Test the AuthBackends data type with valid and invalid values.""" + if is_valid: + assert next(AuthBackends.__get_validators__())(value) == expected + monkeypatch.setenv("RALPH_RUNSERVER_AUTH_BACKENDS", "".join(value)) + reload(conf) + assert conf.settings.RUNSERVER_AUTH_BACKENDS == expected + else: + with pytest.raises(ValueError, match="'notvalid' is not a valid AuthBackend"): + next(AuthBackends.__get_validators__())(value) + + def test_conf_core_settings_should_impact_settings_defaults(monkeypatch): """Test that core settings update application settings values.""" monkeypatch.setenv("RALPH_APP_DIR", "/foo")