Skip to content

Commit

Permalink
Prevent deleted users from authenticating
Browse files Browse the repository at this point in the history
Prevent users who've been marked as deleted (but still exist in the DB,
for now) from authenticating with auth clients, cookies,
`HTTP_X_FORWARDED_USER` headers, or bearer tokens.
  • Loading branch information
seanh committed May 23, 2024
1 parent 56d468b commit cbd1733
Show file tree
Hide file tree
Showing 9 changed files with 39 additions and 6 deletions.
2 changes: 1 addition & 1 deletion h/security/policy/_basic_http_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def identity(self, request):
return None

# If you forward a user it must exist and match your authority
if not user or user.authority != auth_client.authority:
if (not user) or user.deleted or (user.authority != auth_client.authority):
return None

return Identity.from_models(auth_client=auth_client, user=user)
Expand Down
2 changes: 1 addition & 1 deletion h/security/policy/_cookie.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def identity(self, request):
self._add_vary_by_cookie(request)

user = request.find_service(AuthCookieService).verify_cookie()
if not user:
if (not user) or user.deleted:
return None

return Identity.from_models(user=user)
Expand Down
2 changes: 1 addition & 1 deletion h/security/policy/_remote_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def identity(self, request):
return None

user = request.find_service(name="user").fetch(user_id)
if user is None:
if user is None or user.deleted:
return None

return Identity.from_models(user=user)
2 changes: 1 addition & 1 deletion h/security/policy/bearer_token.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def _load_identity(self, request):
return None

user = request.find_service(name="user").fetch(token.userid)
if user is None:
if user is None or user.deleted:
return None

return Identity.from_models(user=user)
Expand Down
8 changes: 6 additions & 2 deletions tests/common/fixtures/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ def annotation_metadata_service(mock_service):

@pytest.fixture
def auth_cookie_service(mock_service):
return mock_service(AuthCookieService)
auth_cookie_service = mock_service(AuthCookieService)
auth_cookie_service.verify_cookie.return_value.deleted = False
return auth_cookie_service


@pytest.fixture
Expand Down Expand Up @@ -279,7 +281,9 @@ def user_password_service(mock_service):

@pytest.fixture
def user_service(mock_service):
return mock_service(UserService, name="user")
user_service = mock_service(UserService, name="user")
user_service.fetch.return_value.deleted = False
return user_service


@pytest.fixture
Expand Down
7 changes: 7 additions & 0 deletions tests/unit/h/security/policy/_basic_http_auth_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,13 @@ def test_identify_returns_None_if_forwarded_user_is_not_found(

assert AuthClientPolicy().identity(pyramid_request) is None

def test_identify_returns_None_if_forwarded_user_is_marked_as_deleted(
self, user_service, pyramid_request
):
user_service.fetch.return_value.deleted = True

assert AuthClientPolicy().identity(pyramid_request) is None

def test_identify_returns_None_if_forwarded_userid_is_invalid(
self, user_service, pyramid_request
):
Expand Down
7 changes: 7 additions & 0 deletions tests/unit/h/security/policy/_cookie_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ def test_identity(self, pyramid_request, auth_cookie_service):
user=auth_cookie_service.verify_cookie.return_value
)

def test_identity_when_user_marked_as_deleted(
self, pyramid_request, auth_cookie_service
):
auth_cookie_service.verify_cookie.return_value.deleted = True

assert CookiePolicy().identity(pyramid_request) is None

def test_identity_with_no_cookie(self, pyramid_request, auth_cookie_service):
auth_cookie_service.verify_cookie.return_value = None

Expand Down
8 changes: 8 additions & 0 deletions tests/unit/h/security/policy/_remote_user_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,3 +26,11 @@ def test_identity_returns_None_for_no_user(self, pyramid_request, user_service):
user_service.fetch.return_value = None

assert RemoteUserPolicy().identity(pyramid_request) is None

def test_identity_returns_None_for_user_marked_as_deleted(
self, pyramid_request, user_service
):
pyramid_request.environ["HTTP_X_FORWARDED_USER"] = sentinel.forwarded_user
user_service.fetch.return_value.deleted = True

assert RemoteUserPolicy().identity(pyramid_request) is None
7 changes: 7 additions & 0 deletions tests/unit/h/security/policy/bearer_token_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,10 @@ def test_identity_returns_None_for_invalid_users(
user_service.fetch.return_value = None

assert BearerTokenPolicy().identity(pyramid_request) is None

def test_identity_returns_None_for_user_marked_as_deleted(
self, pyramid_request, user_service
):
user_service.fetch.return_value.deleted = True

assert BearerTokenPolicy().identity(pyramid_request) is None

0 comments on commit cbd1733

Please sign in to comment.