-
Notifications
You must be signed in to change notification settings - Fork 358
feat: add service account checks in plugin auth #5305
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ | |
from rest_framework.authentication import BaseAuthentication, get_authorization_header | ||
from rest_framework.request import Request | ||
|
||
from apps.auth_token.grafana.grafana_auth_token import setup_organization | ||
from apps.grafana_plugin.helpers.gcom import check_token | ||
from apps.grafana_plugin.sync_data import SyncPermission, SyncUser | ||
from apps.user_management.exceptions import OrganizationDeletedException, OrganizationMovedException | ||
|
@@ -133,6 +134,14 @@ def _get_user(request: Request, organization: Organization) -> User: | |
except KeyError: | ||
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.") | ||
return None | ||
|
||
try: | ||
return organization.users.get(user_id=user_id) | ||
except User.DoesNotExist: | ||
|
@@ -148,6 +157,9 @@ def _get_user(request: Request, organization: Organization) -> User: | |
except (ValueError, TypeError): | ||
raise exceptions.AuthenticationFailed("Grafana context must be JSON dict.") | ||
|
||
if context.get("IsServiceAccount", False): | ||
raise exceptions.AuthenticationFailed("Service accounts requests are not allowed.") | ||
|
||
try: | ||
user_id = context.get("UserId", context.get("UserID")) | ||
if user_id is not None: | ||
|
@@ -347,7 +359,7 @@ def authenticate(self, request): | |
if not auth.startswith(ServiceAccountToken.GRAFANA_SA_PREFIX): | ||
return None | ||
|
||
organization = self.get_organization(request) | ||
organization = self.get_organization(request, auth) | ||
if not organization: | ||
raise exceptions.AuthenticationFailed("Invalid organization.") | ||
if organization.is_moved: | ||
|
@@ -357,12 +369,15 @@ def authenticate(self, request): | |
|
||
return self.authenticate_credentials(organization, auth) | ||
|
||
def get_organization(self, request): | ||
def get_organization(self, request, auth): | ||
grafana_url = request.headers.get(X_GRAFANA_URL) | ||
if grafana_url: | ||
organization = Organization.objects.filter(grafana_url=grafana_url).first() | ||
if not organization: | ||
raise exceptions.AuthenticationFailed("Invalid Grafana URL.") | ||
success = setup_organization(grafana_url, auth) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just out of curiosity, what's the use-case for the new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a use case in which you can bootstrap a Grafana stack from scratch (via Terraform), setting up a service account token in the process with which you could hit our API and OnCall may not know about the organization yet, so this should sync the org if the service account token auth passes and we don't have a record for that org yet. |
||
if not success: | ||
raise exceptions.AuthenticationFailed("Invalid Grafana URL.") | ||
organization = Organization.objects.filter(grafana_url=grafana_url).first() | ||
return organization | ||
|
||
if settings.LICENSE == settings.CLOUD_LICENSE_NAME: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just reminded me we need to clean-up the GrafanaHeadersMixin in mixins.py at some point whenever we remove the old status endpoint. This way of doing it is better in that it is more tolerant of missing/different fields when dealing with plugin rollout difference between objects.