diff --git a/src/ralph/api/auth/__init__.py b/src/ralph/api/auth/__init__.py index f5e80b737..092cc7e01 100644 --- a/src/ralph/api/auth/__init__.py +++ b/src/ralph/api/auth/__init__.py @@ -5,8 +5,7 @@ from ralph.conf import settings # At startup, select the authentication mode that will be used -get_authenticated_user = ( - get_oidc_user - if settings.RUNSERVER_AUTH_BACKEND == settings.AuthBackends.OIDC - else get_basic_user -) +if settings.RUNSERVER_AUTH_BACKEND == settings.AuthBackends.OIDC: + get_authenticated_user = get_oidc_user +else: + get_authenticated_user = get_basic_user diff --git a/src/ralph/api/auth/basic.py b/src/ralph/api/auth/basic.py index ddabb5add..fb8de6850 100644 --- a/src/ralph/api/auth/basic.py +++ b/src/ralph/api/auth/basic.py @@ -9,11 +9,11 @@ import bcrypt from cachetools import TTLCache, cached from fastapi import Depends, HTTPException, status -from fastapi.security import HTTPBasic, HTTPBasicCredentials +from fastapi.security import HTTPBasic, HTTPBasicCredentials, SecurityScopes from pydantic import BaseModel, root_validator from starlette.authentication import AuthenticationError -from ralph.api.auth.user import AuthenticatedUser +from ralph.api.auth.user import AuthenticatedUser, UserScopes from ralph.conf import settings # Unused password used to avoid timing attacks, by comparing passwords supplied @@ -102,15 +102,17 @@ 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: ( + key=lambda credentials, security_scopes: ( credentials.username, credentials.password, + security_scopes.scope_str, ) if credentials is not None else None, ) def get_authenticated_user( credentials: Union[HTTPBasicCredentials, None] = Depends(security), + security_scopes: SecurityScopes = SecurityScopes([]), ) -> AuthenticatedUser: """Checks valid auth parameters. @@ -118,14 +120,11 @@ def get_authenticated_user( against our own list of hashed credentials. Args: + security_scopes: scopes requested for access credentials (iterator): auth parameters from the Authorization header - Return: - AuthenticatedUser (AuthenticatedUser) - Raises: HTTPException - """ if not credentials: logger.error("The basic authentication mode requires a Basic Auth header") @@ -156,6 +155,7 @@ def get_authenticated_user( status_code=status.HTTP_403_FORBIDDEN, detail=str(exc) ) from exc + # Check that password was passed if not hashed_password: # We're doing a bogus password check anyway to avoid timing attacks on # usernames @@ -168,6 +168,7 @@ def get_authenticated_user( headers={"WWW-Authenticate": "Basic"}, ) + # Check password validity if not bcrypt.checkpw( credentials.password.encode(settings.LOCALE_ENCODING), hashed_password.encode(settings.LOCALE_ENCODING), @@ -182,4 +183,16 @@ def get_authenticated_user( headers={"WWW-Authenticate": "Basic"}, ) - return AuthenticatedUser(scopes=user.scopes, agent=user.agent) + user = AuthenticatedUser(scopes=UserScopes(user.scopes), agent=user.agent) + + # Restrict access by scopes + if settings.LRS_RESTRICT_BY_SCOPES: + for requested_scope in security_scopes.scopes: + is_auth = user.scopes.is_authorized(requested_scope) + if not is_auth: + 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 423cfbb5c..c682c5e50 100644 --- a/src/ralph/api/auth/oidc.py +++ b/src/ralph/api/auth/oidc.py @@ -2,16 +2,17 @@ import logging from functools import lru_cache -from typing import Optional, Union +from typing import Optional import requests from fastapi import Depends, HTTPException, status -from fastapi.security import OpenIdConnect +from fastapi.security import OpenIdConnect, SecurityScopes from jose import ExpiredSignatureError, JWTError, jwt from jose.exceptions import JWTClaimsError from pydantic import AnyUrl, BaseModel, Extra +from typing_extensions import Annotated -from ralph.api.auth.user import AuthenticatedUser +from ralph.api.auth.user import AuthenticatedUser, UserScopes from ralph.conf import settings OPENID_CONFIGURATION_PATH = "/.well-known/openid-configuration" @@ -93,7 +94,8 @@ def get_public_keys(jwks_uri: AnyUrl) -> dict: def get_authenticated_user( - auth_header: Union[str, None] = Depends(oauth2_scheme) + auth_header: Annotated[Optional[str], Depends(oauth2_scheme)], + security_scopes: SecurityScopes = SecurityScopes([]), ) -> AuthenticatedUser: """Decode and validate OpenId Connect ID token against issuer in config. @@ -143,7 +145,20 @@ def get_authenticated_user( id_token = IDToken.parse_obj(decoded_token) - return AuthenticatedUser( + user = AuthenticatedUser( agent={"openid": id_token.sub}, - scopes=id_token.scope.split(" ") if id_token.scope else [], + 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: + is_auth = user.scopes.is_authorized(requested_scope) + if not is_auth: + 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/user.py b/src/ralph/api/auth/user.py index 6184a7611..08e61370b 100644 --- a/src/ralph/api/auth/user.py +++ b/src/ralph/api/auth/user.py @@ -1,6 +1,7 @@ """Authenticated user for the Ralph API.""" -from typing import Dict, List, Literal +from functools import lru_cache +from typing import Dict, FrozenSet, Literal from pydantic import BaseModel @@ -18,6 +19,43 @@ ] +class UserScopes(FrozenSet[Scope]): + """Scopes available to users.""" + + @lru_cache() + def is_authorized(self, requested_scope: Scope): + """Check if the requested scope can be accessed based on user scopes.""" + + expanded_scopes = { + "statements/read": {"statements/read/mine", "statements/read"}, + "all/read": { + "statements/read/mine", + "statements/read", + "state/read", + "profile/read", + "all/read", + }, + "all": { + "statements/write", + "statements/read/mine", + "statements/read", + "state/read", + "state/write", + "define", + "profile/read", + "profile/write", + "all/read", + "all", + }, + } + + expanded_user_scopes = set() + for scope in self: + expanded_user_scopes.update(expanded_scopes.get(scope, {scope})) + + return requested_scope in expanded_user_scopes + + class AuthenticatedUser(BaseModel): """Pydantic model for user authentication. @@ -27,4 +65,4 @@ class AuthenticatedUser(BaseModel): """ agent: Dict - scopes: List[Scope] + scopes: UserScopes diff --git a/src/ralph/api/routers/statements.py b/src/ralph/api/routers/statements.py index 9c1d31ad9..e3ac3c332 100644 --- a/src/ralph/api/routers/statements.py +++ b/src/ralph/api/routers/statements.py @@ -15,6 +15,7 @@ Query, Request, Response, + Security, status, ) from fastapi.dependencies.models import Dependant @@ -130,10 +131,12 @@ def strict_query_params(request: Request): @router.get("") @router.get("/") -# pylint: disable=too-many-arguments, too-many-locals async def get( request: Request, - current_user: Annotated[AuthenticatedUser, Depends(get_authenticated_user)], + current_user: Annotated[ + AuthenticatedUser, + Security(get_authenticated_user, scopes=["statements/read/mine"]), + ], ### # Query string parameters defined by the LRS specification ### @@ -163,7 +166,6 @@ async def get( "of the Statement is an Activity with the specified id" ), ), - # pylint: disable=unused-argument registration: Optional[UUID] = Query( None, description=( @@ -171,7 +173,6 @@ async def get( "Filter, only return Statements matching the specified registration id" ), ), - # pylint: disable=unused-argument related_activities: Optional[bool] = Query( False, description=( @@ -182,7 +183,6 @@ async def get( "instead of that parameter's normal behaviour" ), ), - # pylint: disable=unused-argument related_agents: Optional[bool] = Query( False, description=( @@ -214,7 +214,6 @@ async def get( "0 indicates return the maximum the server will allow" ), ), - # pylint: disable=unused-argument, redefined-builtin format: Optional[Literal["ids", "exact", "canonical"]] = Query( "exact", description=( @@ -233,7 +232,6 @@ async def get( 'as in "exact" mode.' ), ), - # pylint: disable=unused-argument attachments: Optional[bool] = Query( False, description=( @@ -279,6 +277,8 @@ async def get( LRS Specification: https://github.com/adlnet/xAPI-Spec/blob/1.0.3/xAPI-Communication.md#213-get-statements """ + # pylint: disable=unused-argument,redefined-builtin,too-many-arguments + # pylint: disable=too-many-locals # Make sure the limit does not go above max from settings limit = min(limit, settings.RUNSERVER_MAX_SEARCH_HITS_COUNT) @@ -327,14 +327,17 @@ async def get( json.loads(query_params["agent"]) ) + # If using authority, always restrict to mine if settings.LRS_RESTRICT_BY_AUTHORITY: - # If using scopes, only restrict results when appropriate - if settings.LRS_RESTRICT_BY_SCOPES: - raise NotImplementedError("Scopes are not yet implemented in Ralph.") - - # Otherwise, enforce mine for all users mine = True + # If using scopes, restrict to "mine" when user does not have + # scopes wider than `statements/read/mine` + if settings.LRS_RESTRICT_BY_SCOPES and current_user.scopes.is_authorized( + "statements/read" + ): + mine = False + if mine: query_params["authority"] = _parse_agent_parameters(current_user.agent) @@ -390,7 +393,10 @@ async def get( @router.put("", responses=POST_PUT_RESPONSES, status_code=status.HTTP_204_NO_CONTENT) # pylint: disable=unused-argument async def put( - current_user: Annotated[AuthenticatedUser, Depends(get_authenticated_user)], + current_user: Annotated[ + AuthenticatedUser, + Security(get_authenticated_user, scopes=["statements/write"]), + ], statement: LaxStatement, background_tasks: BackgroundTasks, statement_id: UUID = Query(alias="statementId"), @@ -459,7 +465,10 @@ async def put( @router.post("/", responses=POST_PUT_RESPONSES) @router.post("", responses=POST_PUT_RESPONSES) async def post( - current_user: Annotated[AuthenticatedUser, Depends(get_authenticated_user)], + current_user: Annotated[ + AuthenticatedUser, + Security(get_authenticated_user, scopes=["statements/write"]), + ], statements: Union[LaxStatement, List[LaxStatement]], background_tasks: BackgroundTasks, response: Response, diff --git a/src/ralph/conf.py b/src/ralph/conf.py index ee6eb94fc..f6036bdee 100644 --- a/src/ralph/conf.py +++ b/src/ralph/conf.py @@ -19,7 +19,17 @@ from unittest.mock import Mock get_app_dir = Mock(return_value=".") -from pydantic import AnyHttpUrl, AnyUrl, BaseModel, BaseSettings, Extra, Field +from pydantic import ( + AnyHttpUrl, + AnyUrl, + BaseModel, + BaseSettings, + Extra, + Field, + root_validator, +) + +from ralph.exceptions import ConfigurationException from .utils import import_string @@ -399,5 +409,18 @@ def LOCALE_ENCODING(self) -> str: # pylint: disable=invalid-name """Returns Ralph's default locale encoding.""" return self._CORE.LOCALE_ENCODING + @root_validator(allow_reuse=True) + @classmethod + def check_restriction_compatibility(cls, values): + """Raise an error if scopes are being used without authority restriction.""" + if values.get("LRS_RESTRICT_BY_SCOPES") and not values.get( + "LRS_RESTRICT_BY_AUTHORITY" + ): + raise ConfigurationException( + "`LRS_RESTRICT_BY_AUTHORITY` must be set to `True` if using " + "`LRS_RESTRICT_BY_SCOPES=True`" + ) + return values + settings = Settings() diff --git a/tests/api/auth/test_basic.py b/tests/api/auth/test_basic.py index ebcbf3aea..7b293ba0d 100644 --- a/tests/api/auth/test_basic.py +++ b/tests/api/auth/test_basic.py @@ -6,7 +6,7 @@ import bcrypt import pytest from fastapi.exceptions import HTTPException -from fastapi.security import HTTPBasicCredentials +from fastapi.security import HTTPBasicCredentials, SecurityScopes from ralph.api.auth.basic import ( ServerUsersCredentials, @@ -14,7 +14,7 @@ get_authenticated_user, get_stored_credentials, ) -from ralph.api.auth.user import AuthenticatedUser +from ralph.api.auth.user import AuthenticatedUser, UserScopes from ralph.conf import Settings, settings STORED_CREDENTIALS = json.dumps( @@ -98,17 +98,20 @@ def test_api_auth_basic_caching_credentials(fs): auth_file_path = settings.APP_DIR / "auth.json" fs.create_file(auth_file_path, contents=STORED_CREDENTIALS) get_authenticated_user.cache_clear() + get_stored_credentials.cache_clear() credentials = HTTPBasicCredentials(username="ralph", password="admin") # Call function as in a first request with these credentials - get_authenticated_user(credentials) + get_authenticated_user( + security_scopes=SecurityScopes(["profile/read"]), credentials=credentials + ) assert get_authenticated_user.cache.popitem() == ( - ("ralph", "admin"), + ("ralph", "admin", "profile/read"), AuthenticatedUser( agent={"mbox": "mailto:ralph@example.com"}, - scopes=["statements/read/mine", "statements/write"], + scopes=UserScopes(["statements/read/mine", "statements/write"]), ), ) @@ -124,7 +127,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_authenticated_user(credentials) + get_authenticated_user(credentials, SecurityScopes(["all"])) def test_api_auth_basic_no_credential_file_found(fs, monkeypatch): @@ -132,12 +135,12 @@ def test_api_auth_basic_no_credential_file_found(fs, monkeypatch): monkeypatch.setenv("RALPH_AUTH_FILE", "other_file") monkeypatch.setattr("ralph.api.auth.basic.settings", Settings()) - get_stored_credentials.cache_clear() + get_authenticated_user.cache_clear() credentials = HTTPBasicCredentials(username="ralph", password="admin") with pytest.raises(HTTPException): - get_authenticated_user(credentials) + get_authenticated_user(credentials, SecurityScopes(["all"])) def test_get_whoami_no_credentials(basic_auth_test_client): @@ -224,7 +227,9 @@ def test_get_whoami_correct_credentials(basic_auth_test_client, fs): ) assert response.status_code == 200 - assert response.json() == { - "agent": {"mbox": "mailto:ralph@example.com"}, - "scopes": ["statements/read/mine", "statements/write"], - } + assert len(response.json().keys()) == 2 + assert response.json()["agent"] == {"mbox": "mailto:ralph@example.com"} + assert sorted(response.json()["scopes"]) == [ + "statements/read/mine", + "statements/write", + ] diff --git a/tests/api/auth/test_oidc.py b/tests/api/auth/test_oidc.py index 0c044bfe6..63cf2b852 100644 --- a/tests/api/auth/test_oidc.py +++ b/tests/api/auth/test_oidc.py @@ -4,44 +4,24 @@ from ralph.api.auth.oidc import discover_provider, get_public_keys -from tests.fixtures.auth import ISSUER_URI +from tests.fixtures.auth import ISSUER_URI, create_mock_oidc_user @responses.activate -def test_api_auth_oidc_valid( - oidc_auth_test_client, mock_discovery_response, mock_oidc_jwks, encoded_token -): +def test_api_auth_oidc_valid(oidc_auth_test_client): """Test a valid OpenId Connect authentication.""" - # Clear LRU cache - discover_provider.cache_clear() - get_public_keys.cache_clear() - - # Mock request to get provider configuration - responses.add( - responses.GET, - f"{ISSUER_URI}/.well-known/openid-configuration", - json=mock_discovery_response, - status=200, - ) - - # Mock request to get keys - responses.add( - responses.GET, - mock_discovery_response["jwks_uri"], - json=mock_oidc_jwks, - status=200, - ) + oidc_token = create_mock_oidc_user(scopes=["all", "profile/read"]) response = oidc_auth_test_client.get( "/whoami", - headers={"Authorization": f"Bearer {encoded_token}"}, + headers={"Authorization": f"Bearer {oidc_token}"}, ) assert response.status_code == 200 - assert response.json() == { - "scopes": ["all", "statements/read"], - "agent": {"openid": "123|oidc"}, - } + + assert len(response.json().keys()) == 2 + assert response.json()["agent"] == {"openid": "123|oidc"} + assert sorted(response.json()["scopes"]) == ["all", "profile/read"] @responses.activate @@ -50,25 +30,7 @@ def test_api_auth_invalid_token( ): """Test API with an invalid audience.""" - # Clear LRU cache - discover_provider.cache_clear() - get_public_keys.cache_clear() - - # Mock request to get provider configuration - responses.add( - responses.GET, - f"{ISSUER_URI}/.well-known/openid-configuration", - json=mock_discovery_response, - status=200, - ) - - # Mock request to get keys - responses.add( - responses.GET, - mock_discovery_response["jwks_uri"], - json=mock_oidc_jwks, - status=200, - ) + create_mock_oidc_user() response = oidc_auth_test_client.get( "/whoami", @@ -143,34 +105,14 @@ def test_api_auth_invalid_keys( @responses.activate -def test_api_auth_invalid_header( - oidc_auth_test_client, mock_discovery_response, mock_oidc_jwks, encoded_token -): +def test_api_auth_invalid_header(oidc_auth_test_client): """Test API with an invalid request header.""" - # Clear LRU cache - discover_provider.cache_clear() - get_public_keys.cache_clear() - - # Mock request to get provider configuration - responses.add( - responses.GET, - f"{ISSUER_URI}/.well-known/openid-configuration", - json=mock_discovery_response, - status=200, - ) - - # Mock request to get keys - responses.add( - responses.GET, - mock_discovery_response["jwks_uri"], - json=mock_oidc_jwks, - status=200, - ) + oidc_token = create_mock_oidc_user() response = oidc_auth_test_client.get( "/whoami", - headers={"Authorization": f"Wrong header {encoded_token}"}, + headers={"Authorization": f"Wrong header {oidc_token}"}, ) assert response.status_code == 401 diff --git a/tests/api/test_statements_get.py b/tests/api/test_statements_get.py index 163b4abde..6348142f3 100644 --- a/tests/api/test_statements_get.py +++ b/tests/api/test_statements_get.py @@ -5,11 +5,14 @@ from urllib.parse import parse_qs, quote_plus, urlparse import pytest +import responses from elasticsearch.helpers import bulk from fastapi.testclient import TestClient from ralph.api import app -from ralph.api.auth.basic import get_authenticated_user +from ralph.api.auth import get_authenticated_user +from ralph.api.auth.basic import get_authenticated_user as get_basic_user +from ralph.api.auth.oidc import get_authenticated_user as get_oidc_user from ralph.backends.database.clickhouse import ClickHouseDatabase from ralph.backends.database.mongo import MongoDatabase from ralph.exceptions import BackendException @@ -27,7 +30,7 @@ get_mongo_test_backend, ) -from ..fixtures.auth import create_user +from ..fixtures.auth import create_mock_basic_auth_user, create_mock_oidc_user from ..helpers import create_mock_activity, create_mock_agent client = TestClient(app) @@ -106,7 +109,7 @@ def _insert_statements_and_monkeypatch_backend(statements): "account_different_home_page", ], ) -def test_api_statements_get_statements_mine( +def test_api_statements_get_mine( monkeypatch, fs, insert_statements_and_monkeypatch_backend, ifi ): """(Security) Test that the get statements API route, given a "mine=True" @@ -137,10 +140,12 @@ def test_api_statements_get_statements_mine( password_1 = "janepwd" scopes = [] - credentials_1_bis = create_user(fs, username_1, password_1, scopes, agent_1_bis) + credentials_1_bis = create_mock_basic_auth_user( + fs, username_1, password_1, scopes, agent_1_bis + ) # Clear cache before each test iteration - get_authenticated_user.cache_clear() + get_basic_user.cache_clear() statements = [ { @@ -219,8 +224,8 @@ def test_api_statements_get_statements_mine( assert response.status_code == 422 -def test_api_statements_get_statements( - insert_statements_and_monkeypatch_backend, auth_credentials +def test_api_statements_get( + insert_statements_and_monkeypatch_backend, basic_auth_credentials ): """Test the get statements API route without any filters set up.""" # pylint: disable=redefined-outer-name @@ -240,15 +245,15 @@ def test_api_statements_get_statements( # Confirm that calling this with and without the trailing slash both work for path in ("/xAPI/statements", "/xAPI/statements/"): response = client.get( - path, headers={"Authorization": f"Basic {auth_credentials}"} + path, headers={"Authorization": f"Basic {basic_auth_credentials}"} ) assert response.status_code == 200 assert response.json() == {"statements": [statements[1], statements[0]]} -def test_api_statements_get_statements_ascending( - insert_statements_and_monkeypatch_backend, auth_credentials +def test_api_statements_get_ascending( + insert_statements_and_monkeypatch_backend, basic_auth_credentials ): """Test the get statements API route, given an "ascending" query parameter, should return statements in ascending order by their timestamp. @@ -269,15 +274,15 @@ def test_api_statements_get_statements_ascending( response = client.get( "/xAPI/statements/?ascending=true", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert response.json() == {"statements": [statements[0], statements[1]]} -def test_api_statements_get_statements_by_statement_id( - insert_statements_and_monkeypatch_backend, auth_credentials +def test_api_statements_get_by_statement_id( + insert_statements_and_monkeypatch_backend, basic_auth_credentials ): """Test the get statements API route, given a "statementId" query parameter, should return a list of statements matching the given statementId. @@ -298,7 +303,7 @@ def test_api_statements_get_statements_by_statement_id( response = client.get( f"/xAPI/statements/?statementId={statements[1]['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 @@ -315,8 +320,8 @@ def test_api_statements_get_statements_by_statement_id( "account_different_home_page", ], ) -def test_api_statements_get_statements_by_agent( - ifi, insert_statements_and_monkeypatch_backend, auth_credentials +def test_api_statements_get_by_agent( + ifi, insert_statements_and_monkeypatch_backend, basic_auth_credentials ): """Test the get statements API route, given an "agent" query parameter, should return a list of statements filtered by the given agent. @@ -352,15 +357,15 @@ def test_api_statements_get_statements_by_agent( response = client.get( f"/xAPI/statements/?agent={quote_plus(json.dumps(agent_1))}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert response.json() == {"statements": [statements[0]]} -def test_api_statements_get_statements_by_verb( - insert_statements_and_monkeypatch_backend, auth_credentials +def test_api_statements_get_by_verb( + insert_statements_and_monkeypatch_backend, basic_auth_credentials ): """Test the get statements API route, given a "verb" query parameter, should return a list of statements filtered by the given verb id. @@ -383,15 +388,15 @@ def test_api_statements_get_statements_by_verb( response = client.get( "/xAPI/statements/?verb=" + quote_plus("http://adlnet.gov/expapi/verbs/played"), - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert response.json() == {"statements": [statements[1]]} -def test_api_statements_get_statements_by_activity( - insert_statements_and_monkeypatch_backend, auth_credentials +def test_api_statements_get_by_activity( + insert_statements_and_monkeypatch_backend, basic_auth_credentials ): """Test the get statements API route, given an "activity" query parameter, should return a list of statements filtered by the given activity id. @@ -417,7 +422,7 @@ def test_api_statements_get_statements_by_activity( response = client.get( f"/xAPI/statements/?activity={activity_1['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 @@ -426,15 +431,15 @@ def test_api_statements_get_statements_by_activity( # Check that badly formated activity returns an error response = client.get( "/xAPI/statements/?activity=INVALID_IRI", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 422 assert response.json()["detail"][0]["msg"] == "'INVALID_IRI' is not a valid 'IRI'." -def test_api_statements_get_statements_since_timestamp( - insert_statements_and_monkeypatch_backend, auth_credentials +def test_api_statements_get_since_timestamp( + insert_statements_and_monkeypatch_backend, basic_auth_credentials ): """Test the get statements API route, given a "since" query parameter, should return a list of statements filtered by the given timestamp. @@ -456,15 +461,15 @@ def test_api_statements_get_statements_since_timestamp( since = (datetime.now() - timedelta(minutes=30)).isoformat() response = client.get( f"/xAPI/statements/?since={since}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert response.json() == {"statements": [statements[1]]} -def test_api_statements_get_statements_until_timestamp( - insert_statements_and_monkeypatch_backend, auth_credentials +def test_api_statements_get_until_timestamp( + insert_statements_and_monkeypatch_backend, basic_auth_credentials ): """Test the get statements API route, given an "until" query parameter, should return a list of statements filtered by the given timestamp. @@ -486,15 +491,15 @@ def test_api_statements_get_statements_until_timestamp( until = (datetime.now() - timedelta(minutes=30)).isoformat() response = client.get( f"/xAPI/statements/?until={until}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert response.json() == {"statements": [statements[0]]} -def test_api_statements_get_statements_with_pagination( - monkeypatch, insert_statements_and_monkeypatch_backend, auth_credentials +def test_api_statements_get_with_pagination( + monkeypatch, insert_statements_and_monkeypatch_backend, basic_auth_credentials ): """Test the get statements API route, given a request leading to more results than can fit on the first page, should return a list of statements non-exceeding the page @@ -533,7 +538,8 @@ def test_api_statements_get_statements_with_pagination( # First response gets the first two results, with a "more" entry as # we have more results to return on a later page. first_response = client.get( - "/xAPI/statements/", headers={"Authorization": f"Basic {auth_credentials}"} + "/xAPI/statements/", + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert first_response.status_code == 200 assert first_response.json()["statements"] == [statements[4], statements[3]] @@ -545,7 +551,7 @@ def test_api_statements_get_statements_with_pagination( # Second response gets the missing result from the first response. second_response = client.get( first_response.json()["more"], - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert second_response.status_code == 200 assert second_response.json()["statements"] == [statements[2], statements[1]] @@ -557,14 +563,14 @@ def test_api_statements_get_statements_with_pagination( # Third response gets the missing result from the first response third_response = client.get( second_response.json()["more"], - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert third_response.status_code == 200 assert third_response.json() == {"statements": [statements[0]]} -def test_api_statements_get_statements_with_pagination_and_query( - monkeypatch, insert_statements_and_monkeypatch_backend, auth_credentials +def test_api_statements_get_with_pagination_and_query( + monkeypatch, insert_statements_and_monkeypatch_backend, basic_auth_credentials ): """Test the get statements API route, given a request with a query parameter leading to more results than can fit on the first page, should return a list @@ -610,7 +616,7 @@ def test_api_statements_get_statements_with_pagination_and_query( first_response = client.get( "/xAPI/statements/?verb=" + quote_plus("https://w3id.org/xapi/video/verbs/played"), - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert first_response.status_code == 200 assert first_response.json()["statements"] == [statements[2], statements[1]] @@ -622,14 +628,14 @@ def test_api_statements_get_statements_with_pagination_and_query( # Second response gets the missing result from the first response. second_response = client.get( first_response.json()["more"], - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert second_response.status_code == 200 assert second_response.json() == {"statements": [statements[0]]} -def test_api_statements_get_statements_with_no_matching_statement( - insert_statements_and_monkeypatch_backend, auth_credentials +def test_api_statements_get_with_no_matching_statement( + insert_statements_and_monkeypatch_backend, basic_auth_credentials ): """Test the get statements API route, given a query yielding no matching statement, should return an empty list. @@ -650,15 +656,15 @@ def test_api_statements_get_statements_with_no_matching_statement( response = client.get( "/xAPI/statements/?statementId=66c81e98-1763-4730-8cfc-f5ab34f1bad5", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert response.json() == {"statements": []} -def test_api_statements_get_statements_with_database_query_failure( - auth_credentials, monkeypatch +def test_api_statements_get_with_database_query_failure( + basic_auth_credentials, monkeypatch ): """Test the get statements API route, given a query raising a BackendException, should return an error response with HTTP code 500. @@ -676,16 +682,14 @@ def mock_query_statements(*_): response = client.get( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 500 assert response.json() == {"detail": "xAPI statements query failed"} @pytest.mark.parametrize("id_param", ["statementId", "voidedStatementId"]) -def test_api_statements_get_statements_invalid_query_parameters( - auth_credentials, id_param -): +def test_api_statements_get_invalid_query_parameters(basic_auth_credentials, id_param): """Test error response for invalid query parameters""" id_1 = "be67b160-d958-4f51-b8b8-1892002dbac6" @@ -694,8 +698,9 @@ def test_api_statements_get_statements_invalid_query_parameters( # Check for 400 status code when unknown parameters are provided response = client.get( "/xAPI/statements/?mamamia=herewegoagain", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) + assert response.status_code == 400 assert response.json() == { "detail": "The following parameter is not allowed: `mamamia`" @@ -704,7 +709,7 @@ def test_api_statements_get_statements_invalid_query_parameters( # Check for 400 status code when both statementId and voidedStatementId are provided response = client.get( f"/xAPI/statements/?statementId={id_1}&voidedStatementId={id_2}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 400 @@ -716,7 +721,7 @@ def test_api_statements_get_statements_invalid_query_parameters( ]: response = client.get( f"/xAPI/statements/?{id_param}={id_1}&{invalid_param}={value}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 400 assert response.json() == { @@ -730,6 +735,168 @@ def test_api_statements_get_statements_invalid_query_parameters( for valid_param, value in [("format", "ids"), ("attachments", "true")]: response = client.get( f"/xAPI/statements/?{id_param}={id_1}&{valid_param}={value}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code != 400 + + +@responses.activate +@pytest.mark.parametrize("auth_method", ["basic", "oidc"]) +@pytest.mark.parametrize( + "scopes,is_authorized", + [ + (["all"], True), + (["all/read"], True), + (["statements/read/mine"], True), + (["statements/read"], True), + (["profile/write", "all/write", "statements/read"], True), + (["statements/write"], False), + (["profile/read"], False), + (["all/write"], False), + ([], False), + ], +) +def test_api_statements_get_scopes( + monkeypatch, fs, es, auth_method, scopes, is_authorized +): + """Test that getting statements behaves properly according to user scopes.""" + # pylint: disable=invalid-name + # pylint: disable=too-many-locals + # pylint: disable=too-many-arguments + + monkeypatch.setattr( + "ralph.api.routers.statements.settings.LRS_RESTRICT_BY_SCOPES", True + ) + monkeypatch.setattr("ralph.api.auth.basic.settings.LRS_RESTRICT_BY_SCOPES", True) + + if auth_method == "basic": + agent = create_mock_agent("mbox", 1) + credentials = create_mock_basic_auth_user(fs, scopes=scopes, agent=agent) + headers = {"Authorization": f"Basic {credentials}"} + + app.dependency_overrides[get_authenticated_user] = get_basic_user + get_basic_user.cache_clear() + + elif auth_method == "oidc": + sub = "123|oidc" + agent = {"openid": sub} + oidc_token = create_mock_oidc_user(sub=sub, scopes=scopes) + headers = {"Authorization": f"Bearer {oidc_token}"} + + monkeypatch.setattr( + "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_ISSUER_URI", + "http://providerHost:8080/auth/realms/real_name", + ) + monkeypatch.setattr( + "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_AUDIENCE", + "http://clientHost:8100", + ) + + app.dependency_overrides[get_authenticated_user] = get_oidc_user + + statements = [ + { + "id": "be67b160-d958-4f51-b8b8-1892002dbac6", + "timestamp": (datetime.now() - timedelta(hours=1)).isoformat(), + "actor": agent, + "authority": agent, + }, + { + "id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2", + "timestamp": datetime.now().isoformat(), + "actor": agent, + "authority": agent, + }, + ] + + # NB: scopes are not linked to statements and backends, we therefore test with ES + database_client_class_path = "ralph.api.routers.statements.DATABASE_CLIENT" + insert_es_statements(es, statements) + monkeypatch.setattr(database_client_class_path, get_es_test_backend()) + + response = client.get( + "/xAPI/statements/", + headers=headers, + ) + + if is_authorized: + assert response.status_code == 200 + assert response.json() == {"statements": [statements[1], statements[0]]} + else: + assert response.status_code == 401 + assert response.json() == { + "detail": 'Access not authorized to scope: "statements/read/mine".' + } + + app.dependency_overrides.pop(get_authenticated_user, None) + + +@pytest.mark.parametrize( + "scopes,read_all_access", + [ + (["all"], True), + (["all/read", "statements/read/mine"], True), + (["statements/read"], True), + (["statements/read/mine"], False), + ], +) +def test_api_statements_get_scopes_with_authority( + monkeypatch, fs, es, scopes, read_all_access +): + """Test that restricting by scope and by authority behaves properly. + + Getting statements should be restricted to mine for users which only have + `statements/read/mine` scope but should not be restricted when the user + has wider scopes. + """ + # pylint: disable=invalid-name + monkeypatch.setattr( + "ralph.api.routers.statements.settings.LRS_RESTRICT_BY_AUTHORITY", True + ) + monkeypatch.setattr( + "ralph.api.routers.statements.settings.LRS_RESTRICT_BY_SCOPES", True + ) + monkeypatch.setattr("ralph.api.auth.basic.settings.LRS_RESTRICT_BY_SCOPES", True) + + agent = create_mock_agent("mbox", 1) + agent_2 = create_mock_agent("mbox", 2) + username = "jane" + password = "janepwd" + credentials = create_mock_basic_auth_user(fs, username, password, scopes, agent) + headers = {"Authorization": f"Basic {credentials}"} + + get_basic_user.cache_clear() + + statements = [ + { + "id": "be67b160-d958-4f51-b8b8-1892002dbac6", + "timestamp": (datetime.now() - timedelta(hours=1)).isoformat(), + "actor": agent, + "authority": agent, + }, + { + "id": "72c81e98-1763-4730-8cfc-f5ab34f1bad2", + "timestamp": datetime.now().isoformat(), + "actor": agent, + "authority": agent_2, + }, + ] + + # NB: scopes are not linked to statements and backends, we therefore test with ES + database_client_class_path = "ralph.api.routers.statements.DATABASE_CLIENT" + insert_es_statements(es, statements) + monkeypatch.setattr(database_client_class_path, get_es_test_backend()) + + response = client.get( + "/xAPI/statements/", + headers=headers, + ) + + assert response.status_code == 200 + + if read_all_access: + 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 5c743ec37..b0d21a47c 100644 --- a/tests/api/test_statements_post.py +++ b/tests/api/test_statements_post.py @@ -4,15 +4,20 @@ from uuid import uuid4 import pytest +import responses from fastapi.testclient import TestClient from httpx import AsyncClient from ralph.api import app +from ralph.api.auth import get_authenticated_user +from ralph.api.auth.basic import get_authenticated_user as get_basic_user +from ralph.api.auth.oidc import get_authenticated_user as get_oidc_user from ralph.backends.database.es import ESDatabase from ralph.backends.database.mongo import MongoDatabase from ralph.conf import XapiForwardingConfigurationSettings from ralph.exceptions import BackendException +from tests.fixtures.auth import create_mock_basic_auth_user, create_mock_oidc_user from tests.fixtures.backends import ( ES_TEST_FORWARDING_INDEX, ES_TEST_HOSTS, @@ -28,6 +33,8 @@ from ..helpers import ( assert_statement_get_responses_are_equivalent, + create_mock_agent, + mock_statement, string_is_date, string_is_uuid, ) @@ -35,29 +42,18 @@ client = TestClient(app) -def test_api_statements_post_invalid_parameters(auth_credentials): +def test_api_statements_post_invalid_parameters(basic_auth_credentials): """Test that using invalid parameters returns the proper status code.""" - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() # Check for 400 status code when unknown parameters are provided response = client.post( "/xAPI/statements/?mamamia=herewegoagain", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) + assert response.status_code == 400 assert response.json() == { "detail": "The following parameter is not allowed: `mamamia`" @@ -70,29 +66,17 @@ def test_api_statements_post_invalid_parameters(auth_credentials): ) # pylint: disable=too-many-arguments def test_api_statements_post_single_statement_directly( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): """Test the post statements API route with one statement.""" # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -102,7 +86,8 @@ def test_api_statements_post_single_statement_directly( es.indices.refresh() response = client.get( - "/xAPI/statements/", headers={"Authorization": f"Basic {auth_credentials}"} + "/xAPI/statements/", + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert_statement_get_responses_are_equivalent( @@ -112,7 +97,7 @@ def test_api_statements_post_single_statement_directly( # pylint: disable=too-many-arguments def test_api_statements_post_enriching_without_existing_values( - monkeypatch, auth_credentials, es + monkeypatch, basic_auth_credentials, es ): """Test that statements are properly enriched when statement provides no values.""" # pylint: disable=invalid-name,unused-argument @@ -134,7 +119,7 @@ def test_api_statements_post_enriching_without_existing_values( response = client.post( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -143,7 +128,8 @@ def test_api_statements_post_enriching_without_existing_values( es.indices.refresh() response = client.get( - "/xAPI/statements/", headers={"Authorization": f"Basic {auth_credentials}"} + "/xAPI/statements/", + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) statement = response.json()["statements"][0] @@ -176,7 +162,7 @@ def test_api_statements_post_enriching_without_existing_values( ) # pylint: disable=too-many-arguments def test_api_statements_post_enriching_with_existing_values( - field, value, status, monkeypatch, auth_credentials, es + field, value, status, monkeypatch, basic_auth_credentials, es ): """Test that statements are properly enriched when values are provided.""" # pylint: disable=invalid-name,unused-argument @@ -184,23 +170,14 @@ def test_api_statements_post_enriching_with_existing_values( monkeypatch.setattr( "ralph.api.routers.statements.DATABASE_CLIENT", get_es_test_backend() ) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "object": {"id": "https://example.com/object-id/1/"}, - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() + # Add the field to be tested statement[field] = value response = client.post( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -210,12 +187,12 @@ def test_api_statements_post_enriching_with_existing_values( if status == 200: es.indices.refresh() response = client.get( - "/xAPI/statements/", headers={"Authorization": f"Basic {auth_credentials}"} + "/xAPI/statements/", + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) statement = response.json()["statements"][0] # Test enriching - assert field in statement if field == "stored": # Check that stored value was overwritten @@ -230,29 +207,17 @@ def test_api_statements_post_enriching_with_existing_values( ) # pylint: disable=too-many-arguments def test_api_statements_post_single_statement_no_trailing_slash( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): """Test that the statements endpoint also works without the trailing slash.""" # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -265,30 +230,18 @@ def test_api_statements_post_single_statement_no_trailing_slash( [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) # pylint: disable=too-many-arguments -def test_api_statements_post_statements_list_of_one( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse +def test_api_statements_post_list_of_one( + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): """Test the post statements API route with one statement in a list.""" # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=[statement], ) @@ -297,7 +250,8 @@ def test_api_statements_post_statements_list_of_one( es.indices.refresh() response = client.get( - "/xAPI/statements/", headers={"Authorization": f"Basic {auth_credentials}"} + "/xAPI/statements/", + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert_statement_get_responses_are_equivalent( @@ -310,45 +264,25 @@ def test_api_statements_post_statements_list_of_one( [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) # pylint: disable=too-many-arguments -def test_api_statements_post_statements_list( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse +def test_api_statements_post_list( + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): """Test the post statements API route with two statements in a list.""" # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) - statements = [ - { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:52Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - }, - { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - # Note the second statement has no preexisting ID - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - }, - ] + + statement_1 = mock_statement(timestamp="2022-03-15T14:07:52Z") + + # Note the second statement has no preexisting ID + statement_2 = mock_statement(timestamp="2022-03-15T14:07:51Z") + statement_2.pop("id") + + statements = [statement_1, statement_2] response = client.post( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statements, ) @@ -360,7 +294,8 @@ def test_api_statements_post_statements_list( es.indices.refresh() get_response = client.get( - "/xAPI/statements/", headers={"Authorization": f"Basic {auth_credentials}"} + "/xAPI/statements/", + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert get_response.status_code == 200 @@ -381,30 +316,18 @@ def test_api_statements_post_statements_list( ], ) # pylint: disable=too-many-arguments -def test_api_statements_post_statements_list_with_duplicates( - backend, monkeypatch, auth_credentials, es_data_stream, mongo, clickhouse +def test_api_statements_post_list_with_duplicates( + backend, monkeypatch, basic_auth_credentials, es_data_stream, mongo, clickhouse ): """Test the post statements API route with duplicate statement IDs should fail.""" # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=[statement, statement], ) @@ -415,7 +338,8 @@ def test_api_statements_post_statements_list_with_duplicates( # The failure should imply no statement insertion. es_data_stream.indices.refresh() response = client.get( - "/xAPI/statements/", headers={"Authorization": f"Basic {auth_credentials}"} + "/xAPI/statements/", + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert response.json() == {"statements": []} @@ -426,8 +350,8 @@ def test_api_statements_post_statements_list_with_duplicates( [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) # pylint: disable=too-many-arguments -def test_api_statements_post_statements_list_with_duplicate_of_existing_statement( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse +def test_api_statements_post_list_with_duplicate_of_existing_statement( + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): """Test the post statements API route, given a statement that already exist in the database (has the same ID), should fail. @@ -437,24 +361,12 @@ def test_api_statements_post_statements_list_with_duplicate_of_existing_statemen monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) statement_uuid = str(uuid4()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": statement_uuid, - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement(id_=statement_uuid) # Post the statement once. response = client.post( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) assert response.status_code == 200 @@ -466,7 +378,7 @@ def test_api_statements_post_statements_list_with_duplicate_of_existing_statemen # include the ID in the response as it wasn't inserted. response = client.post( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) assert response.status_code == 204 @@ -476,7 +388,7 @@ def test_api_statements_post_statements_list_with_duplicate_of_existing_statemen # Post the statement again, trying to change the timestamp which is not allowed. response = client.post( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=[dict(statement, **{"timestamp": "2023-03-15T14:07:51Z"})], ) @@ -487,7 +399,8 @@ def test_api_statements_post_statements_list_with_duplicate_of_existing_statemen } response = client.get( - "/xAPI/statements/", headers={"Authorization": f"Basic {auth_credentials}"} + "/xAPI/statements/", + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert_statement_get_responses_are_equivalent( @@ -499,8 +412,8 @@ def test_api_statements_post_statements_list_with_duplicate_of_existing_statemen "backend", [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) -def test_api_statements_post_statements_with_a_failure_during_storage( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse +def test_api_statements_post_with_a_failure_during_storage( + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): """Test the post statements API route with a failure happening during storage.""" # pylint: disable=invalid-name,unused-argument, too-many-arguments @@ -514,23 +427,11 @@ def put_mock(*args, **kwargs): monkeypatch.setattr( "ralph.api.routers.statements.DATABASE_CLIENT", backend_instance ) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -542,8 +443,8 @@ def put_mock(*args, **kwargs): "backend", [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) -def test_api_statements_post_statements_with_a_failure_during_id_query( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse +def test_api_statements_post_with_a_failure_during_id_query( + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): """Test the post statements API route with a failure during query execution.""" # pylint: disable=invalid-name,unused-argument,too-many-arguments @@ -559,23 +460,11 @@ def query_statements_by_ids_mock(*args, **kwargs): monkeypatch.setattr( "ralph.api.routers.statements.DATABASE_CLIENT", backend_instance ) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -589,7 +478,7 @@ def query_statements_by_ids_mock(*args, **kwargs): ) # pylint: disable=too-many-arguments def test_post_statements_list_without_statement_forwarding( - backend, auth_credentials, monkeypatch, es, mongo, clickhouse + backend, basic_auth_credentials, monkeypatch, es, mongo, clickhouse ): """Test the post statements API route, given an empty forwarding configuration, should not start the forwarding background task. @@ -611,23 +500,11 @@ def spy_mock_forward_xapi_statements(_): ) monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.post( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -654,7 +531,7 @@ async def test_post_statements_list_with_statement_forwarding( receiving_backend, forwarding_backend, monkeypatch, - auth_credentials, + basic_auth_credentials, es, es_forwarding, mongo, @@ -668,19 +545,7 @@ async def test_post_statements_list_with_statement_forwarding( """ # pylint: disable=invalid-name,unused-argument,too-many-arguments,too-many-locals - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() # Set-up receiving LRS client with monkeypatch.context() as receiving_patch: @@ -740,7 +605,7 @@ async def test_post_statements_list_with_statement_forwarding( # The statement should be stored on the forwarding client response = await forwarding_client.get( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert_statement_get_responses_are_equivalent( @@ -751,7 +616,7 @@ async def test_post_statements_list_with_statement_forwarding( async with AsyncClient() as receiving_client: response = await receiving_client.get( f"http://{RUNSERVER_TEST_HOST}:{RUNSERVER_TEST_PORT}/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert_statement_get_responses_are_equivalent( @@ -760,3 +625,74 @@ async def test_post_statements_list_with_statement_forwarding( # Stop receiving LRS client await lrs_context.__aexit__(None, None, None) + + +@responses.activate +@pytest.mark.parametrize("auth_method", ["basic", "oidc"]) +@pytest.mark.parametrize( + "scopes,is_authorized", + [ + (["all"], True), + (["profile/read", "statements/write"], True), + (["all/read"], False), + (["statements/read/mine"], False), + (["profile/write"], False), + ([], False), + ], +) +def test_api_statements_post_scopes( + monkeypatch, fs, es, auth_method, scopes, is_authorized +): + """Test that getting statements behaves properly according to user scopes.""" + # pylint: disable=invalid-name,unused-argument + monkeypatch.setattr( + "ralph.api.routers.statements.settings.LRS_RESTRICT_BY_SCOPES", True + ) + monkeypatch.setattr("ralph.api.auth.basic.settings.LRS_RESTRICT_BY_SCOPES", True) + + if auth_method == "basic": + agent = create_mock_agent("mbox", 1) + credentials = create_mock_basic_auth_user(fs, scopes=scopes, agent=agent) + headers = {"Authorization": f"Basic {credentials}"} + + app.dependency_overrides[get_authenticated_user] = get_basic_user + get_basic_user.cache_clear() + + elif auth_method == "oidc": + sub = "123|oidc" + agent = {"openid": sub} + oidc_token = create_mock_oidc_user(sub=sub, scopes=scopes) + headers = {"Authorization": f"Bearer {oidc_token}"} + + monkeypatch.setattr( + "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_ISSUER_URI", + "http://providerHost:8080/auth/realms/real_name", + ) + monkeypatch.setattr( + "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_AUDIENCE", + "http://clientHost:8100", + ) + + 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 + database_client_class_path = "ralph.api.routers.statements.DATABASE_CLIENT" + monkeypatch.setattr(database_client_class_path, get_es_test_backend()) + + response = client.post( + "/xAPI/statements/", + headers=headers, + json=statement, + ) + + if is_authorized: + assert response.status_code == 200 + else: + assert response.status_code == 401 + 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 4700c38ab..ecedb93ec 100644 --- a/tests/api/test_statements_put.py +++ b/tests/api/test_statements_put.py @@ -1,17 +1,23 @@ """Tests for the PUT statements endpoint of the Ralph API.""" - +from importlib import reload from uuid import uuid4 import pytest +import responses from fastapi.testclient import TestClient from httpx import AsyncClient +from ralph import api from ralph.api import app +from ralph.api.auth import get_authenticated_user +from ralph.api.auth.basic import get_authenticated_user as get_basic_user +from ralph.api.auth.oidc import get_authenticated_user as get_oidc_user from ralph.backends.database.es import ESDatabase from ralph.backends.database.mongo import MongoDatabase from ralph.conf import XapiForwardingConfigurationSettings from ralph.exceptions import BackendException +from tests.fixtures.auth import create_mock_basic_auth_user, create_mock_oidc_user from tests.fixtures.backends import ( ES_TEST_FORWARDING_INDEX, ES_TEST_HOSTS, @@ -25,31 +31,25 @@ get_mongo_test_backend, ) -from ..helpers import assert_statement_get_responses_are_equivalent, string_is_date +from ..helpers import ( + assert_statement_get_responses_are_equivalent, + create_mock_agent, + mock_statement, + string_is_date, +) +reload(api) client = TestClient(app) -def test_api_statements_put_invalid_parameters(auth_credentials): +def test_api_statements_put_invalid_parameters(basic_auth_credentials): """Test that using invalid parameters returns the proper status code.""" - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() # Check for 400 status code when unknown parameters are provided response = client.put( "/xAPI/statements/?mamamia=herewegoagain", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) assert response.status_code == 400 @@ -64,29 +64,17 @@ def test_api_statements_put_invalid_parameters(auth_credentials): ) # pylint: disable=too-many-arguments def test_api_statements_put_single_statement_directly( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): """Test the put statements API route with one statement.""" # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.put( f"/xAPI/statements/?statementId={statement['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -95,7 +83,8 @@ def test_api_statements_put_single_statement_directly( es.indices.refresh() response = client.get( - "/xAPI/statements/", headers={"Authorization": f"Basic {auth_credentials}"} + "/xAPI/statements/", + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert_statement_get_responses_are_equivalent( @@ -105,7 +94,7 @@ def test_api_statements_put_single_statement_directly( # pylint: disable=too-many-arguments def test_api_statements_put_enriching_without_existing_values( - monkeypatch, auth_credentials, es + monkeypatch, basic_auth_credentials, es ): """Test that statements are properly enriched when statement provides no values.""" # pylint: disable=invalid-name,unused-argument @@ -128,7 +117,7 @@ def test_api_statements_put_enriching_without_existing_values( response = client.put( f"/xAPI/statements/?statementId={statement['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) assert response.status_code == 204 @@ -136,7 +125,8 @@ def test_api_statements_put_enriching_without_existing_values( es.indices.refresh() response = client.get( - "/xAPI/statements/", headers={"Authorization": f"Basic {auth_credentials}"} + "/xAPI/statements/", + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) statement = response.json()["statements"][0] @@ -168,7 +158,7 @@ def test_api_statements_put_enriching_without_existing_values( ) # pylint: disable=too-many-arguments def test_api_statements_put_enriching_with_existing_values( - field, value, status, monkeypatch, auth_credentials, es + field, value, status, monkeypatch, basic_auth_credentials, es ): """Test that statements are properly enriched when values are provided.""" # pylint: disable=invalid-name,unused-argument @@ -176,24 +166,13 @@ def test_api_statements_put_enriching_with_existing_values( monkeypatch.setattr( "ralph.api.routers.statements.DATABASE_CLIENT", get_es_test_backend() ) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "object": {"id": "https://example.com/object-id/1/"}, - "verb": {"id": "https://example.com/verb-id/1/"}, - "id": str(uuid4()), - } + statement = mock_statement() # Add the field to be tested statement[field] = value response = client.put( f"/xAPI/statements/?statementId={statement['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -203,7 +182,8 @@ def test_api_statements_put_enriching_with_existing_values( if status == 204: es.indices.refresh() response = client.get( - "/xAPI/statements/", headers={"Authorization": f"Basic {auth_credentials}"} + "/xAPI/statements/", + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) statement = response.json()["statements"][0] @@ -222,29 +202,17 @@ def test_api_statements_put_enriching_with_existing_values( ) # pylint: disable=too-many-arguments def test_api_statements_put_single_statement_no_trailing_slash( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): """Test that the statements endpoint also works without the trailing slash.""" # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.put( f"/xAPI/statements?statementId={statement['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -257,29 +225,17 @@ def test_api_statements_put_single_statement_no_trailing_slash( ) # pylint: disable=too-many-arguments def test_api_statements_put_statement_id_mismatch( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): # pylint: disable=invalid-name,unused-argument """Test the put statements API route when the statementId doesn't match.""" monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement(id_=str(uuid4())) different_statement_id = str(uuid4()) response = client.put( f"/xAPI/statements/?statementId={different_statement_id}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -294,29 +250,17 @@ def test_api_statements_put_statement_id_mismatch( [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) # pylint: disable=too-many-arguments -def test_api_statements_put_statements_list_of_one( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse +def test_api_statements_put_list_of_one( + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): # pylint: disable=invalid-name,unused-argument """Test that we fail on PUTs with a list, even if it's one statement.""" monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.put( f"/xAPI/statements/?statementId={statement['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=[statement], ) @@ -329,7 +273,7 @@ def test_api_statements_put_statements_list_of_one( ) # pylint: disable=too-many-arguments def test_api_statements_put_statement_duplicate_of_existing_statement( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): """Test the put statements API route, given a statement that already exist in the database (has the same ID), should fail. @@ -337,24 +281,12 @@ def test_api_statements_put_statement_duplicate_of_existing_statement( # pylint: disable=invalid-name,unused-argument monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() # Put the statement once. response = client.put( f"/xAPI/statements/?statementId={statement['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) assert response.status_code == 204 @@ -364,7 +296,7 @@ def test_api_statements_put_statement_duplicate_of_existing_statement( # Put the statement twice, trying to change the timestamp, which is not allowed response = client.put( f"/xAPI/statements/?statementId={statement['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=dict(statement, **{"timestamp": "2023-03-15T14:07:51Z"}), ) @@ -375,7 +307,7 @@ def test_api_statements_put_statement_duplicate_of_existing_statement( response = client.get( f"/xAPI/statements/?statementId={statement['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert_statement_get_responses_are_equivalent( @@ -388,7 +320,7 @@ def test_api_statements_put_statement_duplicate_of_existing_statement( [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) def test_api_statement_put_statements_with_a_failure_during_storage( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): """Test the put statements API route with a failure happening during storage.""" # pylint: disable=invalid-name,unused-argument, too-many-arguments @@ -402,23 +334,11 @@ def put_mock(*args, **kwargs): monkeypatch.setattr( "ralph.api.routers.statements.DATABASE_CLIENT", backend_instance ) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.put( f"/xAPI/statements/?statementId={statement['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -431,7 +351,7 @@ def put_mock(*args, **kwargs): [get_es_test_backend, get_clickhouse_test_backend, get_mongo_test_backend], ) def test_api_statements_put_statement_with_a_failure_during_id_query( - backend, monkeypatch, auth_credentials, es, mongo, clickhouse + backend, monkeypatch, basic_auth_credentials, es, mongo, clickhouse ): """Test the put statements API route with a failure during query execution.""" # pylint: disable=invalid-name,unused-argument,too-many-arguments @@ -447,23 +367,11 @@ def query_statements_by_ids_mock(*args, **kwargs): monkeypatch.setattr( "ralph.api.routers.statements.DATABASE_CLIENT", backend_instance ) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.put( f"/xAPI/statements/?statementId={statement['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -477,7 +385,7 @@ def query_statements_by_ids_mock(*args, **kwargs): ) # pylint: disable=too-many-arguments def test_put_statement_without_statement_forwarding( - backend, auth_credentials, monkeypatch, es, mongo, clickhouse + backend, basic_auth_credentials, monkeypatch, es, mongo, clickhouse ): """Test the put statements API route, given an empty forwarding configuration, should not start the forwarding background task. @@ -499,23 +407,11 @@ def spy_mock_forward_xapi_statements(_): ) monkeypatch.setattr("ralph.api.routers.statements.DATABASE_CLIENT", backend()) - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-06-22T08:31:38Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() response = client.put( f"/xAPI/statements/?statementId={statement['id']}", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, json=statement, ) @@ -541,7 +437,7 @@ async def test_put_statement_with_statement_forwarding( receiving_backend, forwarding_backend, monkeypatch, - auth_credentials, + basic_auth_credentials, es, es_forwarding, mongo, @@ -555,19 +451,7 @@ async def test_put_statement_with_statement_forwarding( """ # pylint: disable=invalid-name,unused-argument,too-many-arguments,too-many-locals - statement = { - "actor": { - "account": { - "homePage": "https://example.com/homepage/", - "name": str(uuid4()), - }, - "objectType": "Agent", - }, - "id": str(uuid4()), - "object": {"id": "https://example.com/object-id/1/"}, - "timestamp": "2022-03-15T14:07:51Z", - "verb": {"id": "https://example.com/verb-id/1/"}, - } + statement = mock_statement() # Set-up receiving LRS client with monkeypatch.context() as receiving_patch: @@ -630,7 +514,7 @@ async def test_put_statement_with_statement_forwarding( # The statement should be stored on the forwarding client response = await forwarding_client.get( "/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert_statement_get_responses_are_equivalent( @@ -641,7 +525,7 @@ async def test_put_statement_with_statement_forwarding( async with AsyncClient() as receiving_client: response = await receiving_client.get( f"http://{RUNSERVER_TEST_HOST}:{RUNSERVER_TEST_PORT}/xAPI/statements/", - headers={"Authorization": f"Basic {auth_credentials}"}, + headers={"Authorization": f"Basic {basic_auth_credentials}"}, ) assert response.status_code == 200 assert_statement_get_responses_are_equivalent( @@ -650,3 +534,74 @@ async def test_put_statement_with_statement_forwarding( # Stop receiving LRS client await lrs_context.__aexit__(None, None, None) + + +@responses.activate +@pytest.mark.parametrize("auth_method", ["basic", "oidc"]) +@pytest.mark.parametrize( + "scopes,is_authorized", + [ + (["all"], True), + (["profile/read", "statements/write"], True), + (["all/read"], False), + (["statements/read/mine"], False), + (["profile/write"], False), + ([], False), + ], +) +def test_api_statements_put_scopes( + monkeypatch, fs, es, auth_method, scopes, is_authorized +): + """Test that getting statements behaves properly according to user scopes.""" + # pylint: disable=invalid-name,unused-argument,duplicate-code + monkeypatch.setattr( + "ralph.api.routers.statements.settings.LRS_RESTRICT_BY_SCOPES", True + ) + monkeypatch.setattr("ralph.api.auth.basic.settings.LRS_RESTRICT_BY_SCOPES", True) + + if auth_method == "basic": + agent = create_mock_agent("mbox", 1) + credentials = create_mock_basic_auth_user(fs, scopes=scopes, agent=agent) + headers = {"Authorization": f"Basic {credentials}"} + + app.dependency_overrides[get_authenticated_user] = get_basic_user + get_basic_user.cache_clear() + + elif auth_method == "oidc": + sub = "123|oidc" + agent = {"openid": sub} + oidc_token = create_mock_oidc_user(sub=sub, scopes=scopes) + headers = {"Authorization": f"Bearer {oidc_token}"} + + monkeypatch.setattr( + "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_ISSUER_URI", + "http://providerHost:8080/auth/realms/real_name", + ) + monkeypatch.setattr( + "ralph.api.auth.oidc.settings.RUNSERVER_AUTH_OIDC_AUDIENCE", + "http://clientHost:8100", + ) + + 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 + database_client_class_path = "ralph.api.routers.statements.DATABASE_CLIENT" + monkeypatch.setattr(database_client_class_path, get_es_test_backend()) + + response = client.put( + f"/xAPI/statements/?statementId={statement['id']}", + headers=headers, + json=statement, + ) + + if is_authorized: + assert response.status_code == 204 + else: + assert response.status_code == 401 + assert response.json() == { + "detail": 'Access not authorized to scope: "statements/write".' + } + + app.dependency_overrides.pop(get_authenticated_user, None) diff --git a/tests/backends/http/test_async_lrs.py b/tests/backends/http/test_async_lrs.py index a8706975a..fedc1ec32 100644 --- a/tests/backends/http/test_async_lrs.py +++ b/tests/backends/http/test_async_lrs.py @@ -3,12 +3,9 @@ import asyncio import json import logging -import random import time -from datetime import datetime from functools import partial from urllib.parse import ParseResult, parse_qsl, urlencode, urljoin, urlparse -from uuid import uuid4 import httpx import pytest @@ -22,6 +19,8 @@ from ralph.conf import LRSHeaders, settings from ralph.exceptions import BackendException, BackendParameterException +from ...helpers import mock_statement + lrs_settings = settings.BACKENDS.HTTP.LRS @@ -33,26 +32,6 @@ async def _unpack_async_generator(async_gen): return result -def _gen_statement(id_=None, verb=None, timestamp=None): - """Generate fake statements with random or provided parameters.""" - if id_ is None: - id_ = str(uuid4()) - if verb is None: - verb = {"id": f"https://w3id.org/xapi/video/verbs/{random.random()}"} - elif isinstance(verb, int): - verb = {"id": f"https://w3id.org/xapi/video/verbs/{verb}"} - if timestamp is None: - timestamp = datetime.strftime( - datetime.fromtimestamp(time.time() - random.random()), - "%Y-%m-%dT%H:%M:%S", - ) - elif isinstance(timestamp, int): - timestamp = datetime.strftime( - datetime.fromtimestamp((time.time() - timestamp), "%Y-%m-%dT%H:%M:%S") - ) - return {"id": id_, "verb": verb, "timestamp": timestamp} - - def test_backends_http_lrs_http_instantiation(): """Test the LRS backend instantiation.""" assert AsyncLRSHTTP.name == "async_lrs" @@ -209,11 +188,11 @@ async def test_backends_http_lrs_read_max_statements( chunk_size = 3 statements = { - "statements": [_gen_statement() for _ in range(chunk_size)], + "statements": [mock_statement() for _ in range(chunk_size)], "more": more_target, } more_statements = { - "statements": [_gen_statement() for _ in range(chunk_size)], + "statements": [mock_statement() for _ in range(chunk_size)], } # Mock GET response of HTTPX for target and "more" target without query parameter @@ -279,7 +258,7 @@ async def test_backends_http_lrs_read_without_target( backend = AsyncLRSHTTP(base_url=base_url, username="user", password="pass") - statements = {"statements": [_gen_statement() for _ in range(3)]} + statements = {"statements": [mock_statement() for _ in range(3)]} # Mock HTTPX GET default_params = LRSStatementsQuery(limit=500).dict( @@ -360,9 +339,9 @@ async def test_backends_http_lrs_read_without_pagination( statements = { "statements": [ - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/paused"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/paused"}), ] } @@ -453,19 +432,19 @@ async def test_backends_http_lrs_read_with_pagination(httpx_mock: HTTPXMock): more_target = "/xAPI/statements/?pit_id=fake-pit-id" statements = { "statements": [ - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), - _gen_statement( + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), + mock_statement( verb={"id": "https://w3id.org/xapi/video/verbs/initialized"} ), - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/paused"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/paused"}), ], "more": more_target, } more_statements = { "statements": [ - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/seeked"}), - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), - _gen_statement(verb={"id": "https://w3id.org/xapi/video/verbs/paused"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/seeked"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/played"}), + mock_statement(verb={"id": "https://w3id.org/xapi/video/verbs/paused"}), ] } @@ -608,7 +587,7 @@ async def test_backends_http_lrs_write_without_operation( base_url = "http://fake-lrs.com" target = "/xAPI/statements/" - data = [_gen_statement() for _ in range(6)] + data = [mock_statement() for _ in range(6)] backend = AsyncLRSHTTP(base_url=base_url, username="user", password="pass") @@ -742,7 +721,7 @@ async def test_backends_http_lrs_write_without_target(httpx_mock: HTTPXMock, cap backend = AsyncLRSHTTP(base_url=base_url, username="user", password="pass") - data = [_gen_statement() for _ in range(3)] + data = [mock_statement() for _ in range(3)] # Mock HTTPX POST httpx_mock.add_response( @@ -774,7 +753,7 @@ async def test_backends_http_lrs_write_with_create_or_index_operation( backend = AsyncLRSHTTP(base_url=base_url, username="user", password="pass") - data = [_gen_statement() for _ in range(3)] + data = [mock_statement() for _ in range(3)] # Mock HTTPX POST httpx_mock.add_response(url=urljoin(base_url, target), method="POST", json=data) @@ -803,7 +782,7 @@ async def test_backends_http_lrs_write_backend_exception( backend = AsyncLRSHTTP(base_url=base_url, username="user", password="pass") - data = [_gen_statement()] + data = [mock_statement()] # Mock HTTPX POST httpx_mock.add_response( @@ -866,7 +845,7 @@ async def _simulate_slow_processing(): all_statements = {} for index in range(num_pages): all_statements[index] = { - "statements": [_gen_statement() for _ in range(chunk_size)] + "statements": [mock_statement() for _ in range(chunk_size)] } if index < num_pages - 1: all_statements[index]["more"] = targets[index + 1] @@ -925,7 +904,7 @@ async def test_backends_http_lrs_write_concurrency( base_url = "http://fake-lrs.com" - data = [_gen_statement() for _ in range(6)] + data = [mock_statement() for _ in range(6)] # Changing data length might break tests assert len(data) == 6 diff --git a/tests/conftest.py b/tests/conftest.py index 3e1754b31..b77f68c8e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,7 +5,7 @@ from .fixtures import hypothesis_configuration # noqa: F401 from .fixtures import hypothesis_strategies # noqa: F401 from .fixtures.auth import ( # noqa: F401 - auth_credentials, + basic_auth_credentials, basic_auth_test_client, encoded_token, mock_discovery_response, diff --git a/tests/fixtures/auth.py b/tests/fixtures/auth.py index 23173ed85..6e134ad06 100644 --- a/tests/fixtures/auth.py +++ b/tests/fixtures/auth.py @@ -2,9 +2,11 @@ import base64 import json import os +from typing import Optional import bcrypt import pytest +import responses from cryptography.hazmat.primitives import serialization from fastapi.testclient import TestClient from jose import jwt @@ -12,6 +14,7 @@ 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 from . import private_key, public_key @@ -22,12 +25,12 @@ PUBLIC_KEY_ID = "example-key-id" -def create_user( +def create_mock_basic_auth_user( fs_, - username: str, - password: str, - scopes: list, - agent: dict, + username: str = "jane", + password: str = "pwd", + scopes: Optional[list] = None, + agent: Optional[dict] = None, ): """Create a user using Basic Auth in the (fake) file system. @@ -39,6 +42,12 @@ def create_user( agent (dict): an agent that represents the user and may be used as authority """ + # Default values for `scopes` and `agent` + if scopes is None: + scopes = [] + if agent is None: + agent = {"mbox": "mailto:jane@ralphlrs.com"} + # Basic HTTP auth credential_bytes = base64.b64encode(f"{username}:{password}".encode("utf-8")) credentials = str(credential_bytes, "utf-8") @@ -71,7 +80,7 @@ def create_user( # pylint: disable=invalid-name @pytest.fixture -def auth_credentials(fs, user_scopes=None, agent=None): +def basic_auth_credentials(fs, user_scopes=None, agent=None): """Set up the credentials file for request authentication. Args: @@ -91,7 +100,9 @@ def auth_credentials(fs, user_scopes=None, agent=None): if agent is None: agent = {"mbox": "mailto:test_ralph@example.com"} - credentials = create_user(fs, username, password, user_scopes, agent) + credentials = create_mock_basic_auth_user( + fs, username, password, user_scopes, agent + ) return credentials @@ -129,8 +140,7 @@ def oidc_auth_test_client(monkeypatch): yield test_client -@pytest.fixture -def mock_discovery_response(): +def _mock_discovery_response(): """Return an example discovery response.""" return { "issuer": "http://providerHost", @@ -219,6 +229,12 @@ def mock_discovery_response(): } +@pytest.fixture +def mock_discovery_response(): + """Return an example discovery response (fixture).""" + return _mock_discovery_response() + + def get_jwk(pub_key): """Return a JWK representation of the public key.""" public_numbers = pub_key.public_numbers() @@ -233,23 +249,27 @@ def get_jwk(pub_key): } -@pytest.fixture -def mock_oidc_jwks(): +def _mock_oidc_jwks(): """Mock OpenID Connect keys.""" return {"keys": [get_jwk(public_key)]} @pytest.fixture -def encoded_token(): +def mock_oidc_jwks(): + """Mock OpenID Connect keys (fixture).""" + return _mock_oidc_jwks() + + +def _create_oidc_token(sub, scopes): """Encode token with the private key.""" return jwt.encode( claims={ - "sub": "123|oidc", + "sub": sub, "iss": "https://iss.example.com", "aud": AUDIENCE, "iat": 0, # Issued the 1/1/1970 "exp": 9999999999, # Expiring in 11/20/2286 - "scope": "all statements/read", + "scope": " ".join(scopes), }, key=private_key.private_bytes( serialization.Encoding.PEM, @@ -261,3 +281,39 @@ def encoded_token(): "kid": PUBLIC_KEY_ID, }, ) + + +def create_mock_oidc_user(sub="123|oidc", scopes=None): + """Instantiate mock oidc user and return auth token.""" + # Default value for scope + if scopes is None: + scopes = ["all", "statements/read"] + + # Clear LRU cache + discover_provider.cache_clear() + get_public_keys.cache_clear() + + # Mock request to get provider configuration + responses.add( + responses.GET, + f"{ISSUER_URI}/.well-known/openid-configuration", + json=_mock_discovery_response(), + status=200, + ) + + # Mock request to get keys + responses.add( + responses.GET, + _mock_discovery_response()["jwks_uri"], + json=_mock_oidc_jwks(), + status=200, + ) + + oidc_token = _create_oidc_token(sub=sub, scopes=scopes) + return oidc_token + + +@pytest.fixture +def encoded_token(): + """Encode token with the private key (fixture).""" + return _create_oidc_token(sub="123|oidc", scopes=["all", "statements/read"]) diff --git a/tests/helpers.py b/tests/helpers.py index 6d3fdb223..ad3bb78a8 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -1,8 +1,11 @@ """Utilities for testing Ralph.""" -import datetime import hashlib +import random +import time import uuid -from typing import Optional +from datetime import datetime +from typing import Optional, Union +from uuid import UUID from ralph.utils import statements_are_equivalent @@ -10,7 +13,7 @@ def string_is_date(string: str): """Check if string can be parsed as a date.""" try: - datetime.datetime.fromisoformat(string) + datetime.fromisoformat(string) return True except ValueError: return False @@ -66,8 +69,8 @@ def create_mock_activity(id_: int = 0): def create_mock_agent( - ifi: str, - id_: int, + ifi: str = "mbox", + id_: int = 1, home_page_id: Optional[int] = None, name: Optional[str] = None, use_object_type: bool = True, @@ -121,3 +124,69 @@ def create_mock_agent( return agent raise ValueError("No valid ifi was provided to create_mock_agent") + + +def mock_statement( + id_: Optional[Union[UUID, int]] = None, + actor: Optional[Union[dict, int]] = None, + verb: Optional[Union[dict, int]] = None, + object: Optional[Union[dict, int]] = None, + timestamp=None, +): + """Generate fake statements with random or provided parameters. + + Fields `actor`, `verb`, `object` accept integer values which can be used to + create distinct values identifiable by this integer. + """ + # pylint: disable=redefined-builtin + + # Id + if id_ is None: + id_ = str(uuid.uuid4()) + + # Actor + if actor is None: + actor = create_mock_agent() + elif isinstance(actor, int): + actor = create_mock_agent(id_=actor) + + # Verb + if verb is None: + verb = {"id": f"https://w3id.org/xapi/video/verbs/{random.random()}"} + elif isinstance(verb, int): + verb = {"id": f"https://w3id.org/xapi/video/verbs/{verb}"} + + # Object + if object is None: + object = { + "id": f"http://example.adlnet.gov/xapi/example/activity_{random.random()}" + } + elif isinstance(object, int): + object = {"id": f"http://example.adlnet.gov/xapi/example/activity_{object}"} + + # Timestamp + if timestamp is None: + timestamp = datetime.strftime( + datetime.fromtimestamp(time.time() - random.random()), + "%Y-%m-%dT%H:%M:%SZ", + ) + elif isinstance(timestamp, int): + timestamp = datetime.strftime( + datetime.fromtimestamp(1696236665 + timestamp), "%Y-%m-%dT%H:%M:%SZ" + ) + elif timestamp == "": + return { + "id": id_, + "actor": actor, + "verb": verb, + "object": object, + } + + + return { + "id": id_, + "actor": actor, + "verb": verb, + "object": object, + "timestamp": timestamp, + } diff --git a/tests/test_cli.py b/tests/test_cli.py index 4cc2f1af9..6d48d1149 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -211,7 +211,7 @@ def _assert_matching_basic_auth_credentials( assert "hash" in credentials if hash_: assert credentials["hash"] == hash_ - assert credentials["scopes"] == scopes + assert sorted(credentials["scopes"]) == sorted(scopes) assert "agent" in credentials if agent_name is not None: diff --git a/tests/test_conf.py b/tests/test_conf.py index 846288a14..9471a1564 100644 --- a/tests/test_conf.py +++ b/tests/test_conf.py @@ -9,6 +9,7 @@ from ralph import conf from ralph.conf import CommaSeparatedTuple, Settings, settings +from ralph.exceptions import ConfigurationException from ralph.utils import import_string @@ -182,3 +183,20 @@ def test_conf_core_settings_should_impact_settings_defaults(monkeypatch): # Defaults. assert str(conf.settings.AUTH_FILE) == "/foo/auth.json" assert conf.settings.BACKENDS.STORAGE.FS.PATH == "/foo/archives" + + +def test_conf_forbidden_scopes_without_authority(monkeypatch): + """Test that using RESTRICT_BY_SCOPES without RESTRICT_BY_AUTHORITY raises an + error.""" + + monkeypatch.setenv("RALPH_LRS_RESTRICT_BY_AUTHORITY", False) + monkeypatch.setenv("RALPH_LRS_RESTRICT_BY_SCOPES", True) + + with pytest.raises( + ConfigurationException, + match=( + "`LRS_RESTRICT_BY_AUTHORITY` must be set to `True` if using " + "`LRS_RESTRICT_BY_SCOPES=True`" + ), + ): + reload(conf)