Skip to content

Commit

Permalink
fix: remove service account role check on sync requests (#5355)
Browse files Browse the repository at this point in the history
External service accounts do not have a role set so async triggered
periodic org syncs were rejected. Improving role/perm check in a later
PR instead (restriction wasn't originally there).
  • Loading branch information
matiasb authored Dec 11, 2024
1 parent ec87444 commit b8dc7af
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 35 deletions.
6 changes: 2 additions & 4 deletions engine/apps/auth_token/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,9 @@ def _get_user(request: Request, organization: Organization) -> User:
user_id = context["UserID"]

if context.get("IsServiceAccount", False):
# no user involved in service account requests
logger.info(f"serviceaccount request - id={user_id}")
service_account_role = context.get("Role", "None")
if service_account_role.lower() != "admin":
raise exceptions.AuthenticationFailed("Service account requests must have Admin or Editor role.")
# no user involved in service account requests
logger.info(f"serviceaccount request - id={user_id} - role={service_account_role}")
return None

try:
Expand Down
32 changes: 1 addition & 31 deletions engine/apps/auth_token/tests/test_plugin_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from rest_framework.exceptions import AuthenticationFailed
from rest_framework.test import APIRequestFactory

from apps.auth_token.auth import BasePluginAuthentication, PluginAuthentication
from apps.auth_token.auth import PluginAuthentication

INSTANCE_CONTEXT = '{"stack_id": 42, "org_id": 24, "grafana_token": "abc"}'

Expand Down Expand Up @@ -176,33 +176,3 @@ def test_plugin_authentication_self_hosted_setup_new_user(make_organization, mak
assert ret_user.user_id == 12
assert ret_token.organization == organization
assert organization.users.count() == 1


@pytest.mark.django_db
@pytest.mark.parametrize(
"role,expected_raises", [("Admin", False), ("Editor", True), ("Viewer", True), ("Other", True)]
)
def test_plugin_authentication_service_account(make_organization, role, expected_raises):
# Setting gcom_token_org_last_time_synced to now, so it doesn't try to sync with gcom
organization = make_organization(
stack_id=42, org_id=24, gcom_token="123", api_token="abc", gcom_token_org_last_time_synced=timezone.now()
)

headers = {
"HTTP_AUTHORIZATION": "gcom:123",
"HTTP_X-Instance-Context": INSTANCE_CONTEXT,
"HTTP_X-Grafana-Context": json.dumps({"UserId": 12, "Role": role, "IsServiceAccount": True}),
}
request = APIRequestFactory().get("/", **headers)

if expected_raises:
with pytest.raises(AuthenticationFailed):
BasePluginAuthentication().authenticate(request)
else:
ret_user, ret_token = BasePluginAuthentication().authenticate(request)
assert ret_user is None
assert ret_token.organization == organization

# PluginAuthentication should always raise an exception if the request comes from a service account
with pytest.raises(AuthenticationFailed):
PluginAuthentication().authenticate(request)

0 comments on commit b8dc7af

Please sign in to comment.