Allow the user to logout with a malformed or expired token [#1257] (#…

…1305)

* Allow the user to logout with a malformed or expired token.

* Fix formatting.

* Fix test comment.

* Update changelog.

* Bump fideslib version to raise a 403 if the supplied token is malformed instead of a 500.

* Allow the root user to logout.

CHANGELOG.md

@@ -66,6 +66,7 @@ The types of changes are:
 * Make admin ui work when volumes are mounted [#1266](https://github.com/ethyca/fidesops/pull/1266)
 * Fixed typo in enum value [#1280](https://github.com/ethyca/fidesops/pull/1280)
 * Remove masking of redis error log [#1288](https://github.com/ethyca/fidesops/pull/1288)
+* Logout with malformed or expired token [#1305](https://github.com/ethyca/fidesops/pull/1305)
 
 ### Security
 

requirements.txt

@@ -10,7 +10,7 @@ fastapi[all]==0.82.0
 fastapi-caching[redis]
 fastapi-pagination[sqlalchemy]~= 0.9.3
 fideslang==1.2.0
-fideslib==3.1.1
+fideslib==3.1.2
 fideslog==1.2.3
 hvac==0.11.2
 Jinja2==3.1.2

src/fidesops/ops/api/v1/endpoints/user_endpoints.py

@@ -1,9 +1,15 @@
+import json
 import logging
+from typing import Optional
 
+import jose.exceptions
 from fastapi import Depends, HTTPException, Security
 from fideslib.cryptography.cryptographic_util import b64_str_to_str
+from fideslib.cryptography.schemas.jwt import JWE_PAYLOAD_CLIENT_ID
+from fideslib.exceptions import AuthenticationError
 from fideslib.models.client import ClientDetail
 from fideslib.models.fides_user import FidesUser
+from fideslib.oauth.oauth_util import extract_payload
 from fideslib.oauth.schemas.user import UserPasswordReset, UserResponse, UserUpdate
 from sqlalchemy.orm import Session
 from starlette.status import (
@@ -14,12 +20,21 @@
 )
 
 from fidesops.ops.api import deps
+from fidesops.ops.api.deps import get_db
 from fidesops.ops.api.v1 import urn_registry as urls
-from fidesops.ops.api.v1.scope_registry import USER_PASSWORD_RESET, USER_UPDATE
+from fidesops.ops.api.v1.scope_registry import (
+    SCOPE_REGISTRY,
+    USER_PASSWORD_RESET,
+    USER_UPDATE,
+)
 from fidesops.ops.api.v1.urn_registry import V1_URL_PREFIX
 from fidesops.ops.core.config import config
 from fidesops.ops.util.api_router import APIRouter
-from fidesops.ops.util.oauth_util import get_current_user, verify_oauth_client
+from fidesops.ops.util.oauth_util import (
+    get_current_user,
+    oauth2_scheme,
+    verify_oauth_client,
+)
 
 logger = logging.getLogger(__name__)
 router = APIRouter(tags=["Users"], prefix=V1_URL_PREFIX)
@@ -98,19 +113,50 @@ def update_user_password(
     return current_user
 
 
+def logout_oauth_client(
+    authorization: str = Security(oauth2_scheme), db: Session = Depends(get_db)
+) -> Optional[ClientDetail]:
+    """
+    Streamlined oauth checks for logout.  Only raises an error if no authorization is supplied.
+    Otherwise, regardless if the token is malformed or expired, still return a 204.
+    Returns a client if we can extract one from the token.
+    """
+    if authorization is None:
+        raise AuthenticationError(detail="Authentication Failure")
+
+    try:
+        token_data = json.loads(
+            extract_payload(authorization, config.security.app_encryption_key)
+        )
+    except jose.exceptions.JWEParseError:
+        return None
+
+    client_id = token_data.get(JWE_PAYLOAD_CLIENT_ID)
+    if (
+        not client_id or client_id == config.security.oauth_root_client_id
+    ):  # The root client is not a persisted object
+        return None
+
+    client = ClientDetail.get(
+        db, object_id=client_id, config=config, scopes=SCOPE_REGISTRY
+    )
+
+    return client
+
+
 @router.post(
     urls.LOGOUT,
     status_code=HTTP_204_NO_CONTENT,
 )
 def user_logout(
     *,
-    client: ClientDetail = Security(
-        verify_oauth_client,
-        scopes=[],
+    client: Optional[ClientDetail] = Security(
+        logout_oauth_client,
     ),
     db: Session = Depends(deps.get_db),
 ) -> None:
-    """Logout the user by deleting its client"""
+    """Logout the user by deleting its client where applicable"""
 
     logger.info("Logging out user.")
-    client.delete(db)
+    if client:
+        client.delete(db)

tests/ops/api/v1/endpoints/test_user_endpoints.py

@@ -1,5 +1,5 @@
 import json
-from datetime import datetime
+from datetime import datetime, timedelta
 from typing import List
 
 import pytest
@@ -29,6 +29,7 @@
 
 from fidesops.ops.api.v1.scope_registry import (
     PRIVACY_REQUEST_READ,
+    SCOPE_REGISTRY,
     STORAGE_READ,
     USER_CREATE,
     USER_DELETE,
@@ -749,6 +750,45 @@ class TestUserLogout:
     def url(self, oauth_client: ClientDetail) -> str:
         return V1_URL_PREFIX + LOGOUT
 
+    def test_malformed_token_ignored(self, db, url, api_client, user):
+        auth_header = {"Authorization": "Bearer invalid"}
+        response = api_client.post(url, headers=auth_header, json={})
+        assert response.status_code == HTTP_204_NO_CONTENT
+
+    def test_user_can_logout_with_expired_token(self, db, url, api_client, user):
+        client_id = user.client.id
+        scopes = user.client.scopes
+
+        payload = {
+            JWE_PAYLOAD_SCOPES: scopes,
+            JWE_PAYLOAD_CLIENT_ID: client_id,
+            JWE_ISSUED_AT: (datetime.now() - timedelta(days=360)).isoformat(),
+        }
+
+        auth_header = {
+            "Authorization": "Bearer "
+            + generate_jwe(json.dumps(payload), config.security.app_encryption_key)
+        }
+        response = api_client.post(url, headers=auth_header, json={})
+        assert response.status_code == HTTP_204_NO_CONTENT
+
+        # Verify client was deleted
+        client_search = ClientDetail.get_by(db, field="id", value=client_id)
+        assert client_search is None
+
+    def test_root_user_logout(self, db, url, api_client):
+        payload = {
+            JWE_PAYLOAD_SCOPES: SCOPE_REGISTRY,
+            JWE_PAYLOAD_CLIENT_ID: config.security.oauth_root_client_id,
+            JWE_ISSUED_AT: datetime.now().isoformat(),
+        }
+        auth_header = {
+            "Authorization": "Bearer "
+            + generate_jwe(json.dumps(payload), config.security.app_encryption_key)
+        }
+        response = api_client.post(url, headers=auth_header, json={})
+        assert response.status_code == HTTP_204_NO_CONTENT
+
     def test_user_not_deleted_on_logout(self, db, url, api_client, user):
         user_id = user.id
         client_id = user.client.id
@@ -784,8 +824,7 @@ def test_user_not_deleted_on_logout(self, db, url, api_client, user):
         # Assert user does not still have client reference
         assert user_search.client is None
 
-        # Ensure that the client token is invalidated after logout
-        # Assert a request with the outdated client token gives a 401
+        # Outdated client token logout gives a 204
         payload = {
             JWE_PAYLOAD_SCOPES: scopes,
             JWE_PAYLOAD_CLIENT_ID: client_id,
@@ -796,7 +835,7 @@ def test_user_not_deleted_on_logout(self, db, url, api_client, user):
             + generate_jwe(json.dumps(payload), config.security.app_encryption_key)
         }
         response = api_client.post(url, headers=auth_header, json={})
-        assert HTTP_403_FORBIDDEN == response.status_code
+        assert HTTP_204_NO_CONTENT == response.status_code
 
     def test_logout(self, db, url, api_client, generate_auth_header, oauth_client):
         oauth_client_id = oauth_client.id
@@ -808,6 +847,6 @@ def test_logout(self, db, url, api_client, generate_auth_header, oauth_client):
         client_search = ClientDetail.get_by(db, field="id", value=oauth_client_id)
         assert client_search is None
 
-        # Gets AuthorizationError - client does not exist, this token can't be used anymore
+        # Even though client doesn't exist, we still return a 204
         response = api_client.post(url, headers=auth_header, json={})
-        assert response.status_code == HTTP_403_FORBIDDEN
+        assert response.status_code == HTTP_204_NO_CONTENT