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

feat: allow to request specific properties when logging in #300

Merged
merged 4 commits into from
Oct 15, 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
28 changes: 22 additions & 6 deletions diracx-routers/src/diracx/routers/auth/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

from ..dependencies import AuthDB, AvailableSecurityProperties, Config
from ..fastapi_classes import DiracxRouter
from ..utils.users import AuthSettings
from ..utils.users import AuthSettings, get_allowed_user_properties
from .utils import (
parse_and_validate_scope,
verify_dirac_refresh_token,
Expand Down Expand Up @@ -98,7 +98,6 @@
raise NotImplementedError(f"Grant type not implemented {grant_type}")

# Get a TokenResponse to return to the user

return await exchange_token(
auth_db,
scope,
Expand Down Expand Up @@ -360,9 +359,16 @@
) -> TokenResponse:
"""Method called to exchange the OIDC token for a DIRAC generated access token."""
# Extract dirac attributes from the OIDC scope
parsed_scope = parse_and_validate_scope(scope, config, available_properties)
try:
parsed_scope = parse_and_validate_scope(scope, config, available_properties)
except ValueError as e:
raise HTTPException(

Check warning on line 365 in diracx-routers/src/diracx/routers/auth/token.py

View check run for this annotation

Codecov / codecov/patch

diracx-routers/src/diracx/routers/auth/token.py#L364-L365

Added lines #L364 - L365 were not covered by tests
status_code=status.HTTP_400_BAD_REQUEST,
detail=e.args[0],
) from e
vo = parsed_scope["vo"]
dirac_group = parsed_scope["group"]
properties = parsed_scope["properties"]

# Extract attributes from the OIDC token details
sub = oidc_token_info["sub"]
Expand All @@ -379,8 +385,18 @@

# Check that the subject is part of the dirac users
if sub not in config.Registry[vo].Groups[dirac_group].Users:
raise ValueError(
f"User is not a member of the requested group ({preferred_username}, {dirac_group})"
raise HTTPException(

Check warning on line 388 in diracx-routers/src/diracx/routers/auth/token.py

View check run for this annotation

Codecov / codecov/patch

diracx-routers/src/diracx/routers/auth/token.py#L388

Added line #L388 was not covered by tests
status_code=status.HTTP_403_FORBIDDEN,
detail=f"User is not a member of the requested group ({preferred_username}, {dirac_group})",
)

# Check that the user properties are valid
allowed_user_properties = get_allowed_user_properties(config, sub, vo)
if not properties.issubset(allowed_user_properties):
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail=f"{' '.join(properties - allowed_user_properties)} are not valid properties "
f"for user {preferred_username}, available values: {' '.join(allowed_user_properties)}",
)

# Merge the VO with the subject to get a unique DIRAC sub
Expand Down Expand Up @@ -412,7 +428,7 @@
"sub": sub,
"vo": vo,
"iss": issuer,
"dirac_properties": parsed_scope["properties"],
"dirac_properties": list(properties),
"jti": str(uuid4()),
"preferred_username": preferred_username,
"dirac_group": dirac_group,
Expand Down
14 changes: 3 additions & 11 deletions diracx-routers/src/diracx/routers/auth/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class GrantType(StrEnum):

class ScopeInfoDict(TypedDict):
group: str
properties: list[str]
properties: set[str]
vo: str


Expand Down Expand Up @@ -217,24 +217,16 @@ def parse_and_validate_scope(
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 allowed_properties]
properties.extend([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),
"properties": set(sorted(properties)),
"vo": vo,
}

Expand Down
11 changes: 10 additions & 1 deletion diracx-routers/src/diracx/routers/utils/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from diracx.core.models import UserInfo
from diracx.core.properties import SecurityProperty
from diracx.core.settings import FernetKey, ServiceSettingsBase, TokenSigningKey
from diracx.routers.dependencies import add_settings_annotation
from diracx.routers.dependencies import Config, add_settings_annotation

# auto_error=False is used to avoid raising the wrong exception when the token is missing
# The error is handled in the verify_dirac_access_token function
Expand Down Expand Up @@ -117,3 +117,12 @@ async def verify_dirac_access_token(
vo=token["vo"],
policies=token.get("dirac_policies", {}),
)


def get_allowed_user_properties(config: Config, sub, vo: str) -> set[SecurityProperty]:
"""Retrieve all properties of groups a user is registered in."""
allowed_user_properties = set()
for group in config.Registry[vo].Groups:
if sub in config.Registry[vo].Groups[group].Users:
allowed_user_properties.update(config.Registry[vo].Groups[group].Properties)
return allowed_user_properties
4 changes: 3 additions & 1 deletion diracx-routers/tests/auth/test_legacy_exchange.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ async def test_valid(test_client, legacy_credentials, expires_seconds):
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"]
assert sorted(user_info["properties"]) == sorted(
["PrivateLimitedDelegation", "NormalUser"]
)


async def test_refresh_token(test_client, legacy_credentials):
Expand Down
Loading