Skip to content

[SECUR-104] fix: Arbitrary Modification of API Token Rate Limits #8612

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.

Sign up for GitHub

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

Open
sangeethailango wants to merge 1 commit into
base: preview
Choose a base branch
from
+67 −14
Open

[SECUR-104] fix: Arbitrary Modification of API Token Rate Limits #8612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions apps/api/plane/app/serializers/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ class Meta:
"updated_at",
"workspace",
"user",
"is_active",
"last_used",
"user_type",
]


Expand Down
78 changes: 64 additions & 14 deletions apps/api/plane/tests/contract/app/test_api_token.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def test_get_specific_api_token(self, session_client, create_user, create_api_to
"""Test retrieving a specific API token"""
# Arrange
session_client.force_authenticate(user=create_user)
url = reverse("api-tokens", kwargs={"pk": create_api_token_for_user.pk})
url = reverse("api-tokens-details", kwargs={"pk": create_api_token_for_user.pk})

# Act
response = session_client.get(url)
Expand All @@ -159,7 +159,7 @@ def test_get_nonexistent_api_token(self, session_client, create_user):
# Arrange
session_client.force_authenticate(user=create_user)
fake_pk = uuid4()
url = reverse("api-tokens", kwargs={"pk": fake_pk})
url = reverse("api-tokens-details", kwargs={"pk": fake_pk})

# Act
response = session_client.get(url)
Expand All @@ -178,7 +178,7 @@ def test_get_other_users_api_token(self, session_client, create_user, db):
other_user = User.objects.create(email=unique_email, username=unique_username)
other_token = APIToken.objects.create(label="Other Token", user=other_user, user_type=0)
session_client.force_authenticate(user=create_user)
url = reverse("api-tokens", kwargs={"pk": other_token.pk})
url = reverse("api-tokens-details", kwargs={"pk": other_token.pk})

# Act
response = session_client.get(url)
Expand All @@ -192,7 +192,7 @@ def test_delete_api_token_success(self, session_client, create_user, create_api_
"""Test successful API token deletion"""
# Arrange
session_client.force_authenticate(user=create_user)
url = reverse("api-tokens", kwargs={"pk": create_api_token_for_user.pk})
url = reverse("api-tokens-details", kwargs={"pk": create_api_token_for_user.pk})

# Act
response = session_client.delete(url)
Expand All @@ -207,7 +207,7 @@ def test_delete_nonexistent_api_token(self, session_client, create_user):
# Arrange
session_client.force_authenticate(user=create_user)
fake_pk = uuid4()
url = reverse("api-tokens", kwargs={"pk": fake_pk})
url = reverse("api-tokens-details", kwargs={"pk": fake_pk})

# Act
response = session_client.delete(url)
Expand All @@ -226,7 +226,7 @@ def test_delete_other_users_api_token(self, session_client, create_user, db):
other_user = User.objects.create(email=unique_email, username=unique_username)
other_token = APIToken.objects.create(label="Other Token", user=other_user, user_type=0)
session_client.force_authenticate(user=create_user)
url = reverse("api-tokens", kwargs={"pk": other_token.pk})
url = reverse("api-tokens-details", kwargs={"pk": other_token.pk})

# Act
response = session_client.delete(url)
Expand All @@ -242,7 +242,7 @@ def test_delete_service_api_token_forbidden(self, session_client, create_user):
# Arrange
service_token = APIToken.objects.create(label="Service Token", user=create_user, user_type=0, is_service=True)
session_client.force_authenticate(user=create_user)
url = reverse("api-tokens", kwargs={"pk": service_token.pk})
url = reverse("api-tokens-details", kwargs={"pk": service_token.pk})

# Act
response = session_client.delete(url)
Expand All @@ -258,7 +258,7 @@ def test_patch_api_token_success(self, session_client, create_user, create_api_t
"""Test successful API token update"""
# Arrange
session_client.force_authenticate(user=create_user)
url = reverse("api-tokens", kwargs={"pk": create_api_token_for_user.pk})
url = reverse("api-tokens-details", kwargs={"pk": create_api_token_for_user.pk})
update_data = {
"label": "Updated Token Label",
"description": "Updated description",
Expand All @@ -282,7 +282,7 @@ def test_patch_api_token_partial_update(self, session_client, create_user, creat
"""Test partial API token update"""
# Arrange
session_client.force_authenticate(user=create_user)
url = reverse("api-tokens", kwargs={"pk": create_api_token_for_user.pk})
url = reverse("api-tokens-details", kwargs={"pk": create_api_token_for_user.pk})
original_description = create_api_token_for_user.description
update_data = {"label": "Only Label Updated"}

Expand All @@ -300,7 +300,7 @@ def test_patch_nonexistent_api_token(self, session_client, create_user):
# Arrange
session_client.force_authenticate(user=create_user)
fake_pk = uuid4()
url = reverse("api-tokens", kwargs={"pk": fake_pk})
url = reverse("api-tokens-details", kwargs={"pk": fake_pk})
update_data = {"label": "New Label"}

# Act
Expand All @@ -320,7 +320,7 @@ def test_patch_other_users_api_token(self, session_client, create_user, db):
other_user = User.objects.create(email=unique_email, username=unique_username)
other_token = APIToken.objects.create(label="Other Token", user=other_user, user_type=0)
session_client.force_authenticate(user=create_user)
url = reverse("api-tokens", kwargs={"pk": other_token.pk})
url = reverse("api-tokens-details", kwargs={"pk": other_token.pk})
update_data = {"label": "Hacked Label"}

# Act
Expand All @@ -333,6 +333,56 @@ def test_patch_other_users_api_token(self, session_client, create_user, db):
other_token.refresh_from_db()
assert other_token.label == "Other Token"

@pytest.mark.django_db
def test_patch_cannot_modify_token(self, session_client, create_user, create_api_token_for_user):
"""Test that token value cannot be modified via PATCH"""
# Arrange
session_client.force_authenticate(user=create_user)
url = reverse("api-tokens-details", kwargs={"pk": create_api_token_for_user.pk})
original_token = create_api_token_for_user.token
update_data = {"token": "plane_api_malicious_token_value"}

# Act
response = session_client.patch(url, update_data, format="json")

# Assert
assert response.status_code == status.HTTP_200_OK
create_api_token_for_user.refresh_from_db()
assert create_api_token_for_user.token == original_token

@pytest.mark.django_db
def test_patch_cannot_modify_user_type(self, session_client, create_user, create_api_token_for_user):
"""Test that user_type cannot be modified via PATCH"""
# Arrange
session_client.force_authenticate(user=create_user)
url = reverse("api-tokens-details", kwargs={"pk": create_api_token_for_user.pk})
update_data = {"user_type": 1}

# Act
response = session_client.patch(url, update_data, format="json")

# Assert
assert response.status_code == status.HTTP_200_OK
create_api_token_for_user.refresh_from_db()
assert create_api_token_for_user.user_type == 0

@pytest.mark.django_db
def test_patch_cannot_modify_service_token(self, session_client, create_user):
"""Test that service tokens cannot be modified through user token endpoint"""
# Arrange
service_token = APIToken.objects.create(label="Service Token", user=create_user, user_type=0, is_service=True)
session_client.force_authenticate(user=create_user)
url = reverse("api-tokens-details", kwargs={"pk": service_token.pk})
update_data = {"label": "Hacked Service Token"}

# Act
response = session_client.patch(url, update_data, format="json")

# Assert
assert response.status_code == status.HTTP_404_NOT_FOUND
service_token.refresh_from_db()
assert service_token.label == "Service Token"

# Authentication tests
@pytest.mark.django_db
def test_all_endpoints_require_authentication(self, api_client):
Expand All @@ -341,9 +391,9 @@ def test_all_endpoints_require_authentication(self, api_client):
endpoints = [
(reverse("api-tokens"), "get"),
(reverse("api-tokens"), "post"),
(reverse("api-tokens", kwargs={"pk": uuid4()}), "get"),
(reverse("api-tokens", kwargs={"pk": uuid4()}), "patch"),
(reverse("api-tokens", kwargs={"pk": uuid4()}), "delete"),
(reverse("api-tokens-details", kwargs={"pk": uuid4()}), "get"),
(reverse("api-tokens-details", kwargs={"pk": uuid4()}), "patch"),
(reverse("api-tokens-details", kwargs={"pk": uuid4()}), "delete"),
]

# Act & Assert
Expand Down
Loading