Skip to content

Commit

Permalink
RHCLOUD-30838 | refactor: move the token validation to the middleware (…
Browse files Browse the repository at this point in the history 
…RedHatInsights#1021)

* refactor: move the token validation to the middleware

Instead of validating the token that comes in the requests made by the
service accounts in every view, it makes more sense to validate it when
we are building our internal user object, and to leave it there for easy
access for the rest of the code.

RHCLOUD-30838

* refactor: apply review suggestions

RHCLOUD-30838

* refactor: exception handling closer to the function that raises it

RHCLOUD-30838
  • Loading branch information
@MikelAlejoBR
MikelAlejoBR authored Feb 14, 2024
1 parent 961b85e commit 34781a6
Show file tree
Hide file tree
Showing 8 changed files with 300 additions and 169 deletions.
1 change: 1 addition & 0 deletions rbac/api/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,6 @@ class User:
is_active = True
org_id = None
# Service account properties.
bearer_token: str = ""
client_id: str = ""
is_service_account: bool = False
101 changes: 6 additions & 95 deletions rbac/management/group/view.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,12 @@
from rest_framework import mixins, serializers, status, viewsets
from rest_framework.decorators import action
from rest_framework.filters import OrderingFilter
from rest_framework.request import Request
from rest_framework.response import Response

from api.models import Tenant, User
from .insufficient_privileges import InsufficientPrivilegesError
from .service_account_not_found_error import ServiceAccountNotFoundError
from ..authorization.token_validator import ITSSOTokenValidator, InvalidTokenError, MissingAuthorizationError
from ..authorization.token_validator import UnableMeetPrerequisitesError
from ..principal.unexpected_status_code_from_it import UnexpectedStatusCodeFromITError

USERNAMES_KEY = "usernames"
Expand Down Expand Up @@ -397,7 +396,6 @@ def add_service_accounts(
self,
user: User,
group: Group,
bearer_token: str,
service_accounts: Iterable[dict],
account_name: str = "",
org_id: str = "",
Expand All @@ -407,7 +405,7 @@ def add_service_accounts(
# want to skip calling IT
it_service = ITService()
if not settings.IT_BYPASS_IT_CALLS:
it_service_accounts = it_service.request_service_accounts(bearer_token=bearer_token)
it_service_accounts = it_service.request_service_accounts(bearer_token=user.bearer_token)

# Organize them by their client ID.
it_service_accounts_by_client_ids: dict[str, dict] = {}
Expand Down Expand Up @@ -502,7 +500,7 @@ def remove_principals(self, group, principals, account=None, org_id=None):
return group

@action(detail=True, methods=["get", "post", "delete"])
def principals(self, request, uuid=None):
def principals(self, request: Request, uuid=None):
"""Get, add or remove principals from a group."""
"""
@api {get} /api/v1/groups/:uuid/principals/ Get principals for a group
Expand Down Expand Up @@ -612,56 +610,11 @@ def principals(self, request, uuid=None):

# Process the service accounts and add them to the group.
if len(service_accounts) > 0:
try:
# Attempt validating the JWT token.
token_validator = ITSSOTokenValidator()
bearer_token = token_validator.validate_token(request=request)
except MissingAuthorizationError:
return Response(
status=status.HTTP_401_UNAUTHORIZED,
data={
"errors": [
{
"detail": "The authorization header is required for fetching service accounts.",
"source": "groups",
"status": str(status.HTTP_401_UNAUTHORIZED),
}
]
},
)
except InvalidTokenError:
return Response(
status=status.HTTP_401_UNAUTHORIZED,
data={
"errors": [
{
"detail": "Invalid token provided.",
"source": "groups",
"status": str(status.HTTP_401_UNAUTHORIZED),
}
]
},
)
except UnableMeetPrerequisitesError:
return Response(
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
data={
"errors": [
{
"detail": "Unable to validate token.",
"source": "groups",
"status": str(status.HTTP_500_INTERNAL_SERVER_ERROR),
}
]
},
)

try:
resp = self.add_service_accounts(
user=request.user,
group=group,
service_accounts=service_accounts,
bearer_token=bearer_token,
account_name=account,
org_id=org_id,
)
Expand Down Expand Up @@ -727,61 +680,19 @@ def principals(self, request, uuid=None):

# Make sure we return early for service accounts.
if principalType == "service-account":
try:
# Attempt validating the JWT token.
token_validator = ITSSOTokenValidator()
bearer_token = token_validator.validate_token(request=request)
except MissingAuthorizationError:
return Response(
status=status.HTTP_401_UNAUTHORIZED,
data={
"errors": [
{
"detail": "The authorization header is required for fetching service accounts.",
"source": "groups",
"status": str(status.HTTP_401_UNAUTHORIZED),
}
]
},
)
except InvalidTokenError:
return Response(
status=status.HTTP_401_UNAUTHORIZED,
data={
"errors": [
{
"detail": "Invalid token provided.",
"source": "groups",
"status": str(status.HTTP_401_UNAUTHORIZED),
}
]
},
)
except UnableMeetPrerequisitesError:
return Response(
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
data={
"errors": [
{
"detail": "Unable to validate token.",
"source": "groups",
"status": str(status.HTTP_500_INTERNAL_SERVER_ERROR),
}
]
},
)

# Get the service account's description and name filters, and the principal's username filter too.
# Finally, get the limit and offset parameters.
options[SERVICE_ACCOUNT_DESCRIPTION_KEY] = request.query_params.get(SERVICE_ACCOUNT_DESCRIPTION_KEY)
options[SERVICE_ACCOUNT_NAME_KEY] = request.query_params.get(SERVICE_ACCOUNT_NAME_KEY)

# Get the "principal username" parameter.
options[PRINCIPAL_USERNAME_KEY] = request.query_params.get(PRINCIPAL_USERNAME_KEY)

# Fetch the group's service accounts.
it_service = ITService()
try:
service_accounts = it_service.get_service_accounts_group(
group=group, bearer_token=bearer_token, options=options
group=group, user=request.user, options=options
)
except (requests.exceptions.ConnectionError, UnexpectedStatusCodeFromITError):
return Response(
Expand Down
8 changes: 4 additions & 4 deletions rbac/management/principal/it_service.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,12 @@ def request_service_accounts(self, bearer_token: str) -> list[dict]:

return service_accounts

def get_service_accounts(self, user: User, bearer_token: str, options: dict = {}) -> Tuple[list[dict], int]:
def get_service_accounts(self, user: User, options: dict = {}) -> Tuple[list[dict], int]:
"""Request and returns the service accounts for the given tenant."""
# We might want to bypass calls to the IT service on ephemeral or test environments.
it_service_accounts: list[dict] = []
if not settings.IT_BYPASS_IT_CALLS:
it_service_accounts = self.request_service_accounts(bearer_token=bearer_token)
it_service_accounts = self.request_service_accounts(bearer_token=user.bearer_token)

# Get the service accounts from the database. The weird filter is to fetch the service accounts depending on
# the account number or the organization ID the user gave.
Expand Down Expand Up @@ -238,12 +238,12 @@ def get_service_accounts(self, user: User, bearer_token: str, options: dict = {}

return service_accounts, count

def get_service_accounts_group(self, group: Group, bearer_token: str, options: dict = {}) -> list[dict]:
def get_service_accounts_group(self, group: Group, user: User, options: dict = {}) -> list[dict]:
"""Get the service accounts for the given group."""
# We might want to bypass calls to the IT service on ephemeral or test environments.
it_service_accounts: list[dict] = []
if not settings.IT_BYPASS_IT_CALLS:
it_service_accounts = self.request_service_accounts(bearer_token=bearer_token)
it_service_accounts = self.request_service_accounts(bearer_token=user.bearer_token)

# Fetch the service accounts from the group.
group_service_account_principals = group.principals.filter(type=TYPE_SERVICE_ACCOUNT)
Expand Down
50 changes: 1 addition & 49 deletions rbac/management/principal/view.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@
from .it_service import ITService
from .proxy import PrincipalProxy
from .unexpected_status_code_from_it import UnexpectedStatusCodeFromITError
from ..authorization.token_validator import ITSSOTokenValidator, InvalidTokenError, MissingAuthorizationError
from ..authorization.token_validator import UnableMeetPrerequisitesError
from ..permissions.principal_access import PrincipalAccessPermission

USERNAMES_KEY = "usernames"
Expand Down Expand Up @@ -133,50 +131,6 @@ def get(self, request):

# Get either service accounts or user principals, depending on what the user specified.
if principal_type == "service-account":
try:
# Attempt validating the JWT token.
token_validator = ITSSOTokenValidator()
bearer_token = token_validator.validate_token(request=request)
except MissingAuthorizationError:
return Response(
status=status.HTTP_401_UNAUTHORIZED,
data={
"errors": [
{
"detail": "The authorization header is required for fetching service accounts.",
"source": "principals",
"status": str(status.HTTP_401_UNAUTHORIZED),
}
]
},
)
except InvalidTokenError:
return Response(
status=status.HTTP_401_UNAUTHORIZED,
data={
"errors": [
{
"detail": "Invalid token provided.",
"source": "principals",
"status": str(status.HTTP_401_UNAUTHORIZED),
}
]
},
)
except UnableMeetPrerequisitesError:
return Response(
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
data={
"errors": [
{
"detail": "Unable to validate token.",
"source": "principals",
"status": str(status.HTTP_500_INTERNAL_SERVER_ERROR),
}
]
},
)

options["email"] = query_params.get(EMAIL_KEY)
options["match_criteria"] = validate_and_get_key(
query_params, MATCH_CRITERIA_KEY, VALID_MATCH_VALUE, required=False
Expand All @@ -189,9 +143,7 @@ def get(self, request):
# Fetch the service accounts from IT.
try:
it_service = ITService()
service_accounts, sa_count = it_service.get_service_accounts(
user=user, bearer_token=bearer_token, options=options
)
service_accounts, sa_count = it_service.get_service_accounts(user=user, options=options)
except (requests.exceptions.ConnectionError, UnexpectedStatusCodeFromITError):
return Response(
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
Expand Down
55 changes: 55 additions & 0 deletions rbac/rbac/middleware.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
from django.http import Http404, HttpResponse, QueryDict
from django.urls import resolve
from django.utils.deprecation import MiddlewareMixin
from management.authorization.token_validator import ITSSOTokenValidator, InvalidTokenError, MissingAuthorizationError
from management.authorization.token_validator import UnableMeetPrerequisitesError
from management.cache import TenantCache
from management.models import Principal
from management.utils import APPLICATION_KEY, access_for_principal, validate_psk
Expand Down Expand Up @@ -232,6 +234,59 @@ def process_request(self, request): # pylint: disable=R1710
user.user_id = None
user.system = False

# The requests made by service accounts are expected to come with an Authorization header which
# contains a Bearer token. Therefore, we will attempt to extract it and validate it, and also store it
# in case we need to use it to contact IT with it.
token_validator = ITSSOTokenValidator()
try:
user.bearer_token = token_validator.validate_token(request=request)
except InvalidTokenError:
return HttpResponse(
content=json.dumps(
{
"errors": [
{
"detail": "Invalid token provided.",
"status": str(status.HTTP_401_UNAUTHORIZED),
}
]
}
),
content_type="application/json",
status=status.HTTP_401_UNAUTHORIZED,
)
except MissingAuthorizationError:
return HttpResponse(
content=json.dumps(
{
"errors": [
{
"detail": "A Bearer token in an authorization header is required when"
" contacting RBAC with a service account.",
"status": str(status.HTTP_401_UNAUTHORIZED),
}
]
}
),
content_type="application/json",
status=status.HTTP_401_UNAUTHORIZED,
)
except UnableMeetPrerequisitesError:
return HttpResponse(
content=json.dumps(
{
"errors": [
{
"detail": "Unable to validate the provided token.",
"status": str(status.HTTP_500_INTERNAL_SERVER_ERROR),
}
]
}
),
content_type="application/json",
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
)

# If we did not get the user information or service account information from the "x-rh-identity" header,
# then the request is directly unauthorized.
if not user_info and not service_account:
Expand Down
Loading

0 comments on commit 34781a6

Please sign in to comment.