Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] Ensure token forwarded to n-APIs does not include an extra scheme string #134

Merged
merged 6 commits into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/api/routers/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

from .. import crud, security
from ..models import CombinedQueryResponse, QueryModel
from ..security import verify_token
from ..security import verify_and_extract_token

# from fastapi.security import open_id_connect_url

Expand Down Expand Up @@ -61,7 +61,7 @@ async def get_query(
status_code=status.HTTP_403_FORBIDDEN,
detail="Not authenticated",
)
verify_token(token)
token = verify_and_extract_token(token)

response_dict = await crud.get(
query.min_age,
Expand Down
12 changes: 8 additions & 4 deletions app/api/security.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,23 @@ def check_client_id():
)


def verify_token(token: str):
"""Verify the Google ID token. Raise an HTTPException if the token is invalid."""
def verify_and_extract_token(token: str) -> str:
"""
Verify and return the Google ID token with the authorization scheme stripped.
Raise an HTTPException if the token is invalid.
"""
# Adapted from https://developers.google.com/identity/gsi/web/guides/verify-google-id-token#python
try:
# Extract the token from the "Bearer" scheme
# (See https://github.com/tiangolo/fastapi/blob/master/fastapi/security/oauth2.py#L473-L485)
# TODO: Check also if scheme of token is "Bearer"?
_, param = get_authorization_scheme_param(token)
_, extracted_token = get_authorization_scheme_param(token)
id_info = id_token.verify_oauth2_token(
param, requests.Request(), CLIENT_ID
extracted_token, requests.Request(), CLIENT_ID
)
# TODO: Remove print statement or turn into logging
print("Token verified: ", id_info)
return extracted_token
except (GoogleAuthError, ValueError) as exc:
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
Expand Down
13 changes: 8 additions & 5 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,23 @@ def disable_auth(monkeypatch):


@pytest.fixture()
def mock_verify_token():
def mock_verify_and_extract_token():
"""Mock a successful token verification that does not raise any exceptions."""

def _verify_token(token):
def _verify_and_extract_token(token):
return None

return _verify_token
return _verify_and_extract_token


@pytest.fixture()
def set_mock_verify_token(monkeypatch, mock_verify_token):
def set_mock_verify_and_extract_token(
monkeypatch, mock_verify_and_extract_token
):
"""Set the verify_token function to a mock that does not raise any exceptions."""
monkeypatch.setattr(
"app.api.routers.query.verify_token", mock_verify_token
"app.api.routers.query.verify_and_extract_token",
mock_verify_and_extract_token,
)


Expand Down
8 changes: 4 additions & 4 deletions tests/test_query.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def test_partial_node_failure_responses_handled_gracefully(
set_valid_test_federation_nodes,
mocked_single_matching_dataset_result,
mock_token,
set_mock_verify_token,
set_mock_verify_and_extract_token,
caplog,
):
"""
Expand Down Expand Up @@ -101,7 +101,7 @@ def test_partial_node_request_failures_handled_gracefully(
set_valid_test_federation_nodes,
mocked_single_matching_dataset_result,
mock_token,
set_mock_verify_token,
set_mock_verify_and_extract_token,
error_to_raise,
expected_node_message,
caplog,
Expand Down Expand Up @@ -155,7 +155,7 @@ def test_all_nodes_failure_handled_gracefully(
test_app,
mock_failed_connection_httpx_get,
mock_token,
set_mock_verify_token,
set_mock_verify_and_extract_token,
set_valid_test_federation_nodes,
caplog,
):
Expand Down Expand Up @@ -193,7 +193,7 @@ def test_all_nodes_success_handled_gracefully(
set_valid_test_federation_nodes,
mocked_single_matching_dataset_result,
mock_token,
set_mock_verify_token,
set_mock_verify_and_extract_token,
):
"""
Test that when queries sent to all nodes succeed, the federation API response includes an overall success status and no errors.
Expand Down
39 changes: 36 additions & 3 deletions tests/test_security.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import pytest
from fastapi import HTTPException
from google.oauth2 import id_token

from app.api.security import verify_token
from app.api.security import verify_and_extract_token


def test_missing_client_id_raises_error_when_auth_enabled(
Expand Down Expand Up @@ -40,7 +41,7 @@ def test_missing_client_id_ignored_when_auth_disabled(monkeypatch, test_app):
def test_invalid_token_raises_error(invalid_token):
"""Test that an invalid token raises an error from the verification process."""
with pytest.raises(HTTPException) as exc_info:
verify_token(invalid_token)
verify_and_extract_token(invalid_token)

assert exc_info.value.status_code == 401
assert "Invalid token" in exc_info.value.detail
Expand All @@ -52,7 +53,7 @@ def test_invalid_token_raises_error(invalid_token):
)
def test_query_with_malformed_auth_header_fails(
test_app,
set_mock_verify_token,
set_mock_verify_and_extract_token,
enable_auth,
invalid_auth_header,
monkeypatch,
Expand All @@ -69,3 +70,35 @@ def test_query_with_malformed_auth_header_fails(
)

assert response.status_code == 403


def test_verified_token_returned_without_auth_scheme(monkeypatch, enable_auth):
"""
Test that when a token is valid, verify_token correctly returns the token with the authorization scheme stripped.
"""
mock_valid_token = "Bearer foo"
mock_id_info = {
"iss": "https://accounts.google.com",
"azp": "123abc.apps.googleusercontent.com",
"aud": "123abc.apps.googleusercontent.com",
"sub": "1234567890",
"email": "[email protected]",
"email_verified": True,
"nbf": 1730476622,
"name": "Jane Doe",
"picture": "https://lh3.googleusercontent.com/a/example1234567890",
"given_name": "Jane",
"family_name": "Doe",
"iat": 1730476922,
"exp": 1730480522,
"jti": "123e4567-e89b",
}

def mock_oauth2_verify_token(param, request, client_id, **kwargs):
return mock_id_info

monkeypatch.setattr(
id_token, "verify_oauth2_token", mock_oauth2_verify_token
)

assert verify_and_extract_token(mock_valid_token) == "foo"