mirror of
https://github.com/makeplane/plane.git
synced 2026-06-13 19:19:54 +00:00
fix(api): reject API key auth for deactivated user accounts (#9225)
The custom API key authentication only verified that the APIToken row was active and unexpired; it never checked the owning user's is_active flag. DRF's IsAuthenticated only checks user.is_authenticated (always True for a real User), so a user whose account was deactivated could keep using a previously issued API key indefinitely. Add user__is_active=True to the validate_api_token() lookup so a token tied to a disabled account is treated as invalid (a generic AuthenticationFailed, avoiding account-state disclosure). Applied to both the external API middleware (plane/api) and the identical, currently unused copy in plane/app to prevent the gap from being reintroduced. Adds unit coverage on validate_api_token and an end-to-end contract test proving GET /api/v1/users/me/ is denied once the account is deactivated.
This commit is contained in:
committed by
GitHub
parent
2f7941a17c
commit
fd16d033fc
@@ -32,6 +32,7 @@ class APIKeyAuthentication(authentication.BaseAuthentication):
|
||||
Q(Q(expired_at__gt=timezone.now()) | Q(expired_at__isnull=True)),
|
||||
token=token,
|
||||
is_active=True,
|
||||
user__is_active=True,
|
||||
)
|
||||
except APIToken.DoesNotExist:
|
||||
raise AuthenticationFailed("Given API token is not valid")
|
||||
|
||||
@@ -32,6 +32,7 @@ class APIKeyAuthentication(authentication.BaseAuthentication):
|
||||
Q(Q(expired_at__gt=timezone.now()) | Q(expired_at__isnull=True)),
|
||||
token=token,
|
||||
is_active=True,
|
||||
user__is_active=True,
|
||||
)
|
||||
except APIToken.DoesNotExist:
|
||||
raise AuthenticationFailed("Given API token is not valid")
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
# Copyright (c) 2023-present Plane Software, Inc. and contributors
|
||||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
# See the LICENSE file for details.
|
||||
|
||||
"""
|
||||
Contract tests for external API key authentication.
|
||||
|
||||
End-to-end proof that an API key cannot be used to access the API once the
|
||||
owning user account has been deactivated.
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from rest_framework import status
|
||||
|
||||
|
||||
@pytest.mark.contract
|
||||
class TestAPIKeyAuthenticationContract:
|
||||
"""Test API key authentication behaviour against a live endpoint."""
|
||||
|
||||
USERS_ME_URL = "/api/v1/users/me/"
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_active_user_can_access_with_api_key(self, api_key_client):
|
||||
response = api_key_client.get(self.USERS_ME_URL)
|
||||
|
||||
assert response.status_code == status.HTTP_200_OK
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_deactivated_user_cannot_access_with_api_key(
|
||||
self, api_key_client, create_user
|
||||
):
|
||||
# The account is disabled after the API key was generated.
|
||||
create_user.is_active = False
|
||||
create_user.save()
|
||||
|
||||
response = api_key_client.get(self.USERS_ME_URL)
|
||||
|
||||
# Access is denied once the account is deactivated. APIKeyAuthentication
|
||||
# does not set a WWW-Authenticate header, so DRF surfaces the
|
||||
# AuthenticationFailed as 403 Forbidden rather than 401.
|
||||
assert response.status_code in (
|
||||
status.HTTP_401_UNAUTHORIZED,
|
||||
status.HTTP_403_FORBIDDEN,
|
||||
)
|
||||
@@ -0,0 +1,46 @@
|
||||
# Copyright (c) 2023-present Plane Software, Inc. and contributors
|
||||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
# See the LICENSE file for details.
|
||||
|
||||
"""
|
||||
Unit tests for APIKeyAuthentication.
|
||||
|
||||
Covers the access-control guarantees of the external API key authentication:
|
||||
- a valid token belonging to an active user authenticates successfully
|
||||
- a valid token is rejected once the underlying user account is deactivated
|
||||
(prevents an authentication bypass via a disabled account that still holds
|
||||
a previously generated API key)
|
||||
"""
|
||||
|
||||
import pytest
|
||||
from rest_framework.exceptions import AuthenticationFailed
|
||||
|
||||
from plane.api.middleware.api_authentication import APIKeyAuthentication
|
||||
from plane.db.models import APIToken
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestAPIKeyAuthentication:
|
||||
@pytest.mark.django_db
|
||||
def test_validate_api_token_authenticates_active_user(self, create_user):
|
||||
token = APIToken.objects.create(
|
||||
user=create_user, label="Active Token", token="active-user-token"
|
||||
)
|
||||
|
||||
user, returned_token = APIKeyAuthentication().validate_api_token(token.token)
|
||||
|
||||
assert user == create_user
|
||||
assert returned_token == token.token
|
||||
|
||||
@pytest.mark.django_db
|
||||
def test_validate_api_token_rejects_deactivated_user(self, create_user):
|
||||
token = APIToken.objects.create(
|
||||
user=create_user, label="Stale Token", token="deactivated-user-token"
|
||||
)
|
||||
|
||||
# Account is deactivated by an administrator after the token was issued.
|
||||
create_user.is_active = False
|
||||
create_user.save()
|
||||
|
||||
with pytest.raises(AuthenticationFailed):
|
||||
APIKeyAuthentication().validate_api_token(token.token)
|
||||
Reference in New Issue
Block a user