From 6f63702649aec4ead10858a19c77e7b6e349cbbf Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Wed, 27 Sep 2023 16:10:35 +0200 Subject: [PATCH 1/6] Add /auth/legacy-exchange --- src/diracx/core/config/schema.py | 10 ++ src/diracx/routers/auth.py | 61 ++++++++++ tests/routers/auth/__init__.py | 0 tests/routers/auth/test_legacy_exchange.py | 104 ++++++++++++++++++ .../{test_auth.py => auth/test_standard.py} | 2 +- 5 files changed, 176 insertions(+), 1 deletion(-) create mode 100644 tests/routers/auth/__init__.py create mode 100644 tests/routers/auth/test_legacy_exchange.py rename tests/routers/{test_auth.py => auth/test_standard.py} (99%) diff --git a/src/diracx/core/config/schema.py b/src/diracx/core/config/schema.py index 8f3470e05..b06dc296d 100644 --- a/src/diracx/core/config/schema.py +++ b/src/diracx/core/config/schema.py @@ -76,6 +76,16 @@ class RegistryConfig(BaseModel): Users: dict[str, UserConfig] Groups: dict[str, GroupConfig] + def sub_from_preferred_username(self, preferred_username: str) -> str: + """Get the user sub from the preferred username. + + TODO: This could easily be cached or optimised + """ + for sub, user in self.Users.items(): + if user.PreferedUsername == preferred_username: + return sub + raise KeyError(f"User {preferred_username} not found in registry") + class DIRACConfig(BaseModel): pass diff --git a/src/diracx/routers/auth.py b/src/diracx/routers/auth.py index b8f991bb6..be590783b 100644 --- a/src/diracx/routers/auth.py +++ b/src/diracx/routers/auth.py @@ -3,6 +3,7 @@ import base64 import hashlib import json +import os import re import secrets from datetime import timedelta @@ -18,6 +19,7 @@ from fastapi import ( Depends, Form, + Header, HTTPException, Request, Response, @@ -1015,3 +1017,62 @@ async def userinfo( "properties": user_info.properties, "preferred_username": user_info.preferred_username, } + + +BASE_64_URL_SAFE_PATTERN = ( + r"(?:[A-Za-z0-9\-_]{4})*(?:[A-Za-z0-9\-_]{2}==|[A-Za-z0-9\-_]{3}=)?" +) +LEGACY_EXCHANGE_PATTERN = rf"Bearer diracx:legacy:({BASE_64_URL_SAFE_PATTERN})" + + +@router.get("/legacy-exchange", include_in_schema=False) +async def legacy_exchange( + preferred_username: str, + scope: str, + authorization: Annotated[str, Header()], + auth_db: AuthDB, + available_properties: AvailableSecurityProperties, + settings: AuthSettings, + config: Config, +): + """Endpoint used by legacy DIRAC to mint tokens for proxy -> token exchange.""" + if not ( + expected_api_key := os.environ.get("DIRACX_LEGACY_EXCHANGE_HASHED_API_KEY") + ): + raise HTTPException( + status_code=status.HTTP_503_SERVICE_UNAVAILABLE, + detail="Legacy exchange is not enabled", + ) + + if match := re.fullmatch(LEGACY_EXCHANGE_PATTERN, authorization): + raw_token = base64.urlsafe_b64decode(match.group(1)) + else: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Invalid authorization header", + ) + + if hashlib.sha256(raw_token).hexdigest() != expected_api_key: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Invalid credentials", + ) + + try: + parsed_scope = parse_and_validate_scope(scope, config, available_properties) + vo_users = config.Registry[parsed_scope["vo"]] + sub = vo_users.sub_from_preferred_username(preferred_username) + except (KeyError, ValueError) as e: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Invalid scope or preferred_username", + ) from e + + return await exchange_token( + auth_db, + scope, + {"sub": sub, "preferred_username": preferred_username}, + config, + settings, + available_properties, + ) diff --git a/tests/routers/auth/__init__.py b/tests/routers/auth/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/routers/auth/test_legacy_exchange.py b/tests/routers/auth/test_legacy_exchange.py new file mode 100644 index 000000000..611f4edf9 --- /dev/null +++ b/tests/routers/auth/test_legacy_exchange.py @@ -0,0 +1,104 @@ +import base64 +import hashlib +import secrets + +import pytest + + +@pytest.fixture +def legacy_credentials(monkeypatch): + secret = secrets.token_bytes() + valid_token = f"diracx:legacy:{base64.urlsafe_b64encode(secret).decode()}" + monkeypatch.setenv( + "DIRACX_LEGACY_EXCHANGE_HASHED_API_KEY", hashlib.sha256(secret).hexdigest() + ) + yield {"Authorization": f"Bearer {valid_token}"} + + +async def test_valid(test_client, legacy_credentials): + r = test_client.get( + "/auth/legacy-exchange", + params={"preferred_username": "chaen", "scope": "vo:lhcb group:lhcb_user"}, + headers=legacy_credentials, + ) + assert r.status_code == 200 + access_token = r.json()["access_token"] + + r = test_client.get( + "/auth/userinfo", headers={"Authorization": f"Bearer {access_token}"} + ) + assert r.status_code == 200 + user_info = r.json() + assert user_info["sub"] == "lhcb:b824d4dc-1f9d-4ee8-8df5-c0ae55d46041" + assert user_info["vo"] == "lhcb" + assert user_info["dirac_group"] == "lhcb_user" + assert user_info["properties"] == ["NormalUser", "PrivateLimitedDelegation"] + + +async def test_disabled(test_client): + r = test_client.get( + "/auth/legacy-exchange", + params={"preferred_username": "chaen", "scope": "vo:lhcb group:lhcb_user"}, + headers={"Authorization": "Bearer diracx:legacy:ChangeME"}, + ) + assert r.status_code == 503 + + +async def test_no_credentials(test_client, legacy_credentials): + r = test_client.get( + "/auth/legacy-exchange", + params={"preferred_username": "chaen", "scope": "vo:lhcb group:lhcb_user"}, + headers={"Authorization": "Bearer invalid"}, + ) + assert r.status_code == 400 + assert r.json()["detail"] == "Invalid authorization header" + + +async def test_invalid_credentials(test_client, legacy_credentials): + r = test_client.get( + "/auth/legacy-exchange", + params={"preferred_username": "chaen", "scope": "vo:lhcb group:lhcb_user"}, + headers={"Authorization": "Bearer invalid"}, + ) + assert r.status_code == 400 + assert r.json()["detail"] == "Invalid authorization header" + + +async def test_wrong_credentials(test_client, legacy_credentials): + r = test_client.get( + "/auth/legacy-exchange", + params={"preferred_username": "chaen", "scope": "vo:lhcb group:lhcb_user"}, + headers={"Authorization": "Bearer diracx:legacy:ChangeME"}, + ) + assert r.status_code == 403 + assert r.json()["detail"] == "Invalid credentials" + + +async def test_unknown_vo(test_client, legacy_credentials): + r = test_client.get( + "/auth/legacy-exchange", + params={"preferred_username": "chaen", "scope": "vo:unknown group:lhcb_user"}, + headers=legacy_credentials, + ) + assert r.status_code == 400 + assert r.json()["detail"] == "Invalid scope or preferred_username" + + +async def test_unknown_group(test_client, legacy_credentials): + r = test_client.get( + "/auth/legacy-exchange", + params={"preferred_username": "chaen", "scope": "vo:lhcb group:unknown"}, + headers=legacy_credentials, + ) + assert r.status_code == 400 + assert r.json()["detail"] == "Invalid scope or preferred_username" + + +async def test_unknown_user(test_client, legacy_credentials): + r = test_client.get( + "/auth/legacy-exchange", + params={"preferred_username": "unknown", "scope": "vo:lhcb group:lhcb_user"}, + headers=legacy_credentials, + ) + assert r.status_code == 400 + assert r.json()["detail"] == "Invalid scope or preferred_username" diff --git a/tests/routers/test_auth.py b/tests/routers/auth/test_standard.py similarity index 99% rename from tests/routers/test_auth.py rename to tests/routers/auth/test_standard.py index 20ef148c6..be7f6a526 100644 --- a/tests/routers/test_auth.py +++ b/tests/routers/auth/test_standard.py @@ -32,7 +32,7 @@ def non_mocked_hosts(test_client) -> list[str]: @pytest.fixture async def auth_httpx_mock(httpx_mock: HTTPXMock, monkeypatch): - data_dir = Path(__file__).parent.parent / "data" + data_dir = Path(__file__).parent.parent.parent / "data" path = "lhcb-auth.web.cern.ch/.well-known/openid-configuration" httpx_mock.add_response(url=f"https://{path}", text=(data_dir / path).read_text()) From 39e6f4175080c6d843314a13df0d271f71b118b1 Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Wed, 27 Sep 2023 17:51:56 +0200 Subject: [PATCH 2/6] HACK: Test against upstream branch --- .github/workflows/integration.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 9bee2d0a3..2d1b20abb 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -30,7 +30,7 @@ jobs: - name: Prepare environment run: | pip install typer pyyaml gitpython packaging - git clone https://github.com/DIRACGrid/DIRAC.git -b "${{ matrix.dirac-branch }}" /tmp/DIRACRepo + git clone https://github.com/chaen/DIRAC.git -b "diracx_sandbox" /tmp/DIRACRepo # We need to cd in the directory for the integration_tests.py to work - name: Prepare environment run: cd /tmp/DIRACRepo && ./integration_tests.py prepare-environment "TEST_DIRACX=Yes" --extra-module "diracx=${GITHUB_WORKSPACE}" From c6248df9da586e72d0c55e0920c8c9ca80f52dd9 Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Wed, 27 Sep 2023 18:03:46 +0200 Subject: [PATCH 3/6] Improve logs of failed integration tests --- .github/workflows/integration.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 2d1b20abb..22549ea37 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -43,7 +43,11 @@ jobs: - name: Client tests run: cd /tmp/DIRACRepo && ./integration_tests.py test-client || touch client-tests-failed - name: diracx logs - run: docker logs diracx + if: ${{ failure() }} + run: sleep 15 && docker logs diracx + - name: DIRAC logs + if: ${{ failure() }} + run: cd /tmp/DIRACRepo && ./integration_tests.py logs --no-follow --lines 1000 - name: Check test status run: | has_error=0 From ed3e29bfb865e8e67186d9de93f847d2374e58f1 Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Wed, 27 Sep 2023 22:06:26 +0200 Subject: [PATCH 4/6] DEBUGGING --- .github/workflows/integration.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 22549ea37..dd7102f88 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -55,3 +55,8 @@ jobs: if [ -f server-tests-failed ]; then has_error=0; echo "Server tests failed"; fi if [ -f client-tests-failed ]; then has_error=0; echo "Client tests failed"; fi if [ ${has_error} = 1 ]; then exit 1; fi + - name: Setup tmate session + if: ${{ failure() }} + uses: mxschmitt/action-tmate@v3 + with: + limit-access-to-actor: true From 88760df6b9cd1b7a959247c21ae76d706bc3c5c6 Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Thu, 28 Sep 2023 05:04:09 +0200 Subject: [PATCH 5/6] Try to fix log uploading --- .github/workflows/integration.yml | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index dd7102f88..9c56f977d 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -42,12 +42,6 @@ jobs: run: cd /tmp/DIRACRepo && ./integration_tests.py test-server || touch server-tests-failed - name: Client tests run: cd /tmp/DIRACRepo && ./integration_tests.py test-client || touch client-tests-failed - - name: diracx logs - if: ${{ failure() }} - run: sleep 15 && docker logs diracx - - name: DIRAC logs - if: ${{ failure() }} - run: cd /tmp/DIRACRepo && ./integration_tests.py logs --no-follow --lines 1000 - name: Check test status run: | has_error=0 @@ -55,8 +49,15 @@ jobs: if [ -f server-tests-failed ]; then has_error=0; echo "Server tests failed"; fi if [ -f client-tests-failed ]; then has_error=0; echo "Client tests failed"; fi if [ ${has_error} = 1 ]; then exit 1; fi - - name: Setup tmate session + - name: diracx logs + if: ${{ failure() }} + run: | + mkdir -p /tmp/service-logs + docker logs diracx > /tmp/service-logs/diracx.log + cd /tmp/DIRACRepo + ./integration_tests.py logs --no-follow --lines 1000 > /tmp/service-logs/dirac.log + - uses: actions/upload-artifact@v3 if: ${{ failure() }} - uses: mxschmitt/action-tmate@v3 with: - limit-access-to-actor: true + name: serivce-logs + path: /tmp/service-logs/ From 80799d12169ae8bbc619305ef20e34336ca1b928 Mon Sep 17 00:00:00 2001 From: Chris Burr Date: Thu, 28 Sep 2023 05:20:25 +0200 Subject: [PATCH 6/6] DEBUGGING --- .github/workflows/integration.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index 9c56f977d..2be887a73 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -53,11 +53,16 @@ jobs: if: ${{ failure() }} run: | mkdir -p /tmp/service-logs - docker logs diracx > /tmp/service-logs/diracx.log + docker logs diracx 2>&1 | tee /tmp/service-logs/diracx.log cd /tmp/DIRACRepo - ./integration_tests.py logs --no-follow --lines 1000 > /tmp/service-logs/dirac.log + ./integration_tests.py logs --no-follow --lines 1000 2>&1 | tee /tmp/service-logs/dirac.log - uses: actions/upload-artifact@v3 if: ${{ failure() }} with: name: serivce-logs path: /tmp/service-logs/ + - name: Setup tmate session + if: ${{ failure() }} + uses: mxschmitt/action-tmate@v3 + with: + limit-access-to-actor: true