diff --git a/diracx-routers/src/diracx/routers/auth/authorize_code_flow.py b/diracx-routers/src/diracx/routers/auth/authorize_code_flow.py index b8dd156c8..733e922c9 100644 --- a/diracx-routers/src/diracx/routers/auth/authorize_code_flow.py +++ b/diracx-routers/src/diracx/routers/auth/authorize_code_flow.py @@ -90,6 +90,11 @@ async def authorization_flow( status_code=status.HTTP_400_BAD_REQUEST, detail=e.args[0], ) from e + except PermissionError as e: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=e.args[0], + ) from e uuid = await auth_db.insert_authorization_flow( client_id, diff --git a/diracx-routers/src/diracx/routers/auth/device_flow.py b/diracx-routers/src/diracx/routers/auth/device_flow.py index d311715b7..246275175 100644 --- a/diracx-routers/src/diracx/routers/auth/device_flow.py +++ b/diracx-routers/src/diracx/routers/auth/device_flow.py @@ -120,6 +120,11 @@ async def initiate_device_flow( status_code=status.HTTP_400_BAD_REQUEST, detail=e.args[0], ) from e + except PermissionError as e: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=e.args[0], + ) from e user_code, device_code = await auth_db.insert_device_flow( client_id, scope, audience diff --git a/diracx-routers/src/diracx/routers/auth/token.py b/diracx-routers/src/diracx/routers/auth/token.py index ce4161f0e..17c3e94fa 100644 --- a/diracx-routers/src/diracx/routers/auth/token.py +++ b/diracx-routers/src/diracx/routers/auth/token.py @@ -313,6 +313,11 @@ async def legacy_exchange( status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid scope or preferred_username", ) from e + except PermissionError as e: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail=e.args[0], + ) from e return await exchange_token( auth_db, diff --git a/diracx-routers/src/diracx/routers/auth/utils.py b/diracx-routers/src/diracx/routers/auth/utils.py index a23ac9b82..914d491c0 100644 --- a/diracx-routers/src/diracx/routers/auth/utils.py +++ b/diracx-routers/src/diracx/routers/auth/utils.py @@ -297,15 +297,22 @@ def parse_and_validate_scope( if group not in config.Registry[vo].Groups: raise ValueError(f"{group} not in {vo} groups") + allowed_properties = config.Registry[vo].Groups[group].Properties if not properties: # If there are no properties set get the defaults from the CS - properties = [str(p) for p in config.Registry[vo].Groups[group].Properties] + properties = [str(p) for p in allowed_properties] if not set(properties).issubset(available_properties): raise ValueError( f"{set(properties)-set(available_properties)} are not valid properties" ) + if not set(properties).issubset(allowed_properties): + raise PermissionError( + f"Attempted to access properties {set(properties)-set(allowed_properties)} which are not allowed." + f" Allowed properties are: {allowed_properties}" + ) + return { "group": group, "properties": sorted(properties), diff --git a/diracx-routers/tests/auth/test_standard.py b/diracx-routers/tests/auth/test_standard.py index aa72de17c..f48862135 100644 --- a/diracx-routers/tests/auth/test_standard.py +++ b/diracx-routers/tests/auth/test_standard.py @@ -184,7 +184,7 @@ async def test_device_flow(test_client, auth_httpx_mock: HTTPXMock): params={ "client_id": DIRAC_CLIENT_ID, "audience": "Dirac server", - "scope": "vo:lhcb group:lhcb_user property:FileCatalogManagement property:NormalUser", + "scope": "vo:lhcb group:lhcb_user property:NormalUser", }, ) assert r.status_code == 200, r.json() @@ -253,6 +253,92 @@ async def test_device_flow(test_client, auth_httpx_mock: HTTPXMock): assert r.json()["detail"] == "Code was already used" +async def test_flows_with_unallowed_properties(test_client): + """Test the authorization flow and the device flow with unallowed properties.""" + unallowed_property = "FileCatalogManagement" + + # Initiate the authorization flow + code_verifier = secrets.token_hex() + code_challenge = ( + base64.urlsafe_b64encode(hashlib.sha256(code_verifier.encode()).digest()) + .decode() + .replace("=", "") + ) + r = test_client.get( + "/api/auth/authorize", + params={ + "response_type": "code", + "code_challenge": code_challenge, + "code_challenge_method": "S256", + "client_id": DIRAC_CLIENT_ID, + "redirect_uri": "http://diracx.test.invalid:8000/api/docs/oauth2-redirect", + "scope": f"vo:lhcb property:{unallowed_property} property:NormalUser", + "state": "external-state", + }, + follow_redirects=False, + ) + assert r.status_code == 403, r.json() + assert ( + f"Attempted to access properties {{'{unallowed_property}'}} which are not allowed." + in r.json()["detail"] + ) + + # Initiate the device flow + r = test_client.post( + "/api/auth/device", + params={ + "client_id": DIRAC_CLIENT_ID, + "audience": "Dirac server", + "scope": f"vo:lhcb group:lhcb_user property:{unallowed_property} property:NormalUser", + }, + ) + assert r.status_code == 403, r.json() + assert ( + f"Attempted to access properties {{'{unallowed_property}'}} which are not allowed." + in r.json()["detail"] + ) + + +async def test_flows_with_invalid_properties(test_client): + """Test the authorization flow and the device flow with invalid properties.""" + invalid_property = "InvalidAndUnknownProperty" + + # Initiate the authorization flow + code_verifier = secrets.token_hex() + code_challenge = ( + base64.urlsafe_b64encode(hashlib.sha256(code_verifier.encode()).digest()) + .decode() + .replace("=", "") + ) + r = test_client.get( + "/api/auth/authorize", + params={ + "response_type": "code", + "code_challenge": code_challenge, + "code_challenge_method": "S256", + "client_id": DIRAC_CLIENT_ID, + "redirect_uri": "http://diracx.test.invalid:8000/api/docs/oauth2-redirect", + "scope": f"vo:lhcb property:{invalid_property} property:NormalUser", + "state": "external-state", + }, + follow_redirects=False, + ) + assert r.status_code == 400, r.json() + assert f"{{'{invalid_property}'}} are not valid properties" in r.json()["detail"] + + # Initiate the device flow + r = test_client.post( + "/api/auth/device", + params={ + "client_id": DIRAC_CLIENT_ID, + "audience": "Dirac server", + "scope": f"vo:lhcb group:lhcb_user property:{invalid_property} property:NormalUser", + }, + ) + assert r.status_code == 400, r.json() + assert f"{{'{invalid_property}'}} are not valid properties" in r.json()["detail"] + + async def test_refresh_token_rotation(test_client, auth_httpx_mock: HTTPXMock): """Test the refresh token rotation. - initiate a device code flow to get an initial refresh token @@ -576,7 +662,7 @@ def _get_tokens( params={ "client_id": DIRAC_CLIENT_ID, "audience": "Dirac server", - "scope": f"vo:lhcb group:{group} property:FileCatalogManagement property:{property}", + "scope": f"vo:lhcb group:{group} property:{property}", }, ) data = r.json()