From 0561b8d5788e61f09e6bdda4ebcbdea8fa231b28 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Wed, 20 Mar 2024 13:49:05 +0100 Subject: [PATCH] make it work Signed-off-by: Jens Langhammer --- authentik/api/authentication.py | 12 ++++ authentik/core/expression/evaluator.py | 2 +- authentik/lib/expression/evaluator.py | 71 ++++++++++++++----- authentik/policies/expression/api.py | 4 +- authentik/policies/expression/apps.py | 5 +- authentik/policies/expression/evaluator.py | 7 +- .../0005_expressionpolicy_execution_user.py | 26 +++++++ authentik/policies/expression/models.py | 11 ++- authentik/policies/expression/signals.py | 13 ++++ blueprints/schema.json | 4 ++ schema.yml | 13 ++++ .../expression/ExpressionPolicyForm.ts | 41 ++++++++++- web/src/common/global.ts | 2 +- 13 files changed, 179 insertions(+), 32 deletions(-) create mode 100644 authentik/policies/expression/migrations/0005_expressionpolicy_execution_user.py create mode 100644 authentik/policies/expression/signals.py diff --git a/authentik/api/authentication.py b/authentik/api/authentication.py index ab4b67d731..d12f745648 100644 --- a/authentik/api/authentication.py +++ b/authentik/api/authentication.py @@ -73,6 +73,11 @@ def auth_user_lookup(raw_header: bytes) -> User | None: if user: CTX_AUTH_VIA.set("secret_key") return user + # then try to auth via expression JWT + user = token_expression_jwt(auth_credentials) + if user: + CTX_AUTH_VIA.set("expression_jwt") + return user raise AuthenticationFailed("Token invalid/expired") @@ -90,6 +95,13 @@ def token_secret_key(value: str) -> User | None: return outpost.user +def token_expression_jwt(value: str) -> User | None: + """Authenticate API call made by Expressions""" + from authentik.lib.expression.evaluator import authenticate_token + + return authenticate_token(value) + + class TokenAuthentication(BaseAuthentication): """Token-based authentication using HTTP Bearer authentication""" diff --git a/authentik/core/expression/evaluator.py b/authentik/core/expression/evaluator.py index a8d365a1c0..e6035378eb 100644 --- a/authentik/core/expression/evaluator.py +++ b/authentik/core/expression/evaluator.py @@ -36,7 +36,7 @@ class PropertyMappingEvaluator(BaseEvaluator): _filename = model.name else: _filename = str(model) - super().__init__(filename=_filename) + super().__init__(user=user, filename=_filename) req = PolicyRequest(user=User()) req.obj = model if user: diff --git a/authentik/lib/expression/evaluator.py b/authentik/lib/expression/evaluator.py index 02b89faf9f..ada3197df6 100644 --- a/authentik/lib/expression/evaluator.py +++ b/authentik/lib/expression/evaluator.py @@ -3,6 +3,8 @@ import re import socket from collections.abc import Iterable +from datetime import timedelta +from functools import lru_cache from ipaddress import ip_address, ip_network from pathlib import Path from tempfile import gettempdir @@ -34,7 +36,9 @@ from authentik_client.configuration import Configuration from cachetools import TLRUCache, cached from django.conf import settings from django.core.exceptions import FieldError +from django.utils.timezone import now from guardian.shortcuts import get_anonymous_user +from jwt import PyJWTError, decode, encode from rest_framework.serializers import ValidationError from RestrictedPython import compile_restricted, limited_builtins, safe_builtins, utility_builtins from sentry_sdk.hub import Hub @@ -42,14 +46,12 @@ from sentry_sdk.tracing import Span from structlog.stdlib import get_logger from authentik.core.models import ( - USER_ATTRIBUTE_CHANGE_EMAIL, - USER_ATTRIBUTE_CHANGE_NAME, - USER_ATTRIBUTE_CHANGE_USERNAME, User, ) from authentik.events.models import Event from authentik.lib.config import CONFIG -from authentik.lib.utils.http import get_http_session +from authentik.lib.utils.errors import exception_to_string +from authentik.lib.utils.http import authentik_user_agent, get_http_session from authentik.lib.utils.reflection import get_apps from authentik.policies.models import Policy, PolicyBinding from authentik.policies.process import PolicyProcess @@ -82,6 +84,23 @@ API_CLIENTS = { "TenantsApi": TenantsApi, } +JWT_AUD = "goauthentik.io/api/expression" + + +@lru_cache +def get_api_token_secret(): + return "foo" + + +def authenticate_token(raw_value: str): + """Authenticate API call from evaluator token""" + try: + jwt = decode(raw_value, get_api_token_secret(), ["HS256"], audience=JWT_AUD) + return User.objects.filter(uuid=jwt["sub"]).first() + except PyJWTError as exc: + LOGGER.debug("failed to auth", exc=exc) + return None + class BaseEvaluator: """Validate and evaluate python-based expressions""" @@ -94,8 +113,14 @@ class BaseEvaluator: # Filename used for exec _filename: str - def __init__(self, filename: str | None = None): + _user: User + + # Timeout in seconds, used for the expiration of the API key + timeout = 30 + + def __init__(self, user: User, filename: str | None = None): self._filename = filename if filename else "BaseEvaluator" + self._user = user # update website/docs/expressions/_objects.md # update website/docs/expressions/_functions.md self._globals = { @@ -113,12 +138,6 @@ class BaseEvaluator: "requests": get_http_session(), "resolve_dns": BaseEvaluator.expr_resolve_dns, "reverse_dns": BaseEvaluator.expr_reverse_dns, - # Temporary addition of config until #7590 is through and this is not needed anymore - "CONFIG": CONFIG, - "USER_ATTRIBUTE_CHANGE_EMAIL": USER_ATTRIBUTE_CHANGE_EMAIL, - "USER_ATTRIBUTE_CHANGE_NAME": USER_ATTRIBUTE_CHANGE_NAME, - "USER_ATTRIBUTE_CHANGE_USERNAME": USER_ATTRIBUTE_CHANGE_USERNAME, - "api": self.get_api_client(), } for app in get_apps(): # Load models from each app @@ -127,18 +146,34 @@ class BaseEvaluator: self._globals.update(API_CLIENTS) self._context = {} + def get_token(self) -> str: + """Generate API token to be used by the API Client""" + _now = now() + return encode( + { + "aud": JWT_AUD, + "iss": f"goauthentik.io/expression/{self._filename}", + "sub": str(self._user.uuid), + "iat": int(_now.timestamp()), + "exp": int((_now + timedelta(seconds=self.timeout)).timestamp()), + }, + get_api_token_secret(), + ) + def get_api_client(self): - token = "" + token = self.get_token() config = Configuration( - f"unix://{str(_tmp.joinpath('authentik-core.sock'))}", + f"unix://{str(_tmp.joinpath('authentik-core.sock'))}/api/v3", api_key={ "authentik": token, }, - api_key_prefix={"authentik": "Bearer "}, + api_key_prefix={"authentik": "Bearer"}, ) if settings.DEBUG: - config.host = "http://localhost:8000" - return ApiClient(config) + config.host = "http://localhost:8000/api/v3" + client = ApiClient(config) + client.user_agent = authentik_user_agent() + return client @cached(cache=TLRUCache(maxsize=32, ttu=lambda key, value, now: now + 180)) @staticmethod @@ -296,6 +331,9 @@ class BaseEvaluator: **utility_builtins, } _locals = self._context + # We need to create the API Client later so that the token is valid + # from when the execution starts + self._globals["api"] = self.get_api_client() # Yes this is an exec, yes it is potentially bad. Since we limit what variables are # available here, and these policies can only be edited by admins, this is a risk # we're willing to take. @@ -303,6 +341,7 @@ class BaseEvaluator: exec(ast_obj, self._globals, _locals) # nosec # noqa result = _locals["result"] except Exception as exc: + print(exception_to_string(exc)) # So, this is a bit questionable. Essentially, we are edit the stacktrace # so the user only sees information relevant to them # and none of our surrounding error handling diff --git a/authentik/policies/expression/api.py b/authentik/policies/expression/api.py index 6e970eb39d..b932aaaa38 100644 --- a/authentik/policies/expression/api.py +++ b/authentik/policies/expression/api.py @@ -14,12 +14,12 @@ class ExpressionPolicySerializer(PolicySerializer): def validate_expression(self, expr: str) -> str: """validate the syntax of the expression""" name = "temp-policy" if not self.instance else self.instance.name - PolicyEvaluator(name).validate(expr) + PolicyEvaluator(self.context["request"].user, name).validate(expr) return expr class Meta: model = ExpressionPolicy - fields = PolicySerializer.Meta.fields + ["expression"] + fields = PolicySerializer.Meta.fields + ["expression", "execution_user"] class ExpressionPolicyViewSet(UsedByMixin, ModelViewSet): diff --git a/authentik/policies/expression/apps.py b/authentik/policies/expression/apps.py index de7df61f90..44a614841a 100644 --- a/authentik/policies/expression/apps.py +++ b/authentik/policies/expression/apps.py @@ -1,11 +1,12 @@ """Authentik policy_expression app config""" -from django.apps import AppConfig +from authentik.blueprints.apps import ManagedAppConfig -class AuthentikPolicyExpressionConfig(AppConfig): +class AuthentikPolicyExpressionConfig(ManagedAppConfig): """Authentik policy_expression app config""" name = "authentik.policies.expression" label = "authentik_policies_expression" verbose_name = "authentik Policies.Expression" + default = True diff --git a/authentik/policies/expression/evaluator.py b/authentik/policies/expression/evaluator.py index 536b2634d8..96c4c64352 100644 --- a/authentik/policies/expression/evaluator.py +++ b/authentik/policies/expression/evaluator.py @@ -6,6 +6,7 @@ from typing import TYPE_CHECKING, Optional from django.http import HttpRequest from structlog.stdlib import get_logger +from authentik.core.models import User from authentik.flows.planner import PLAN_CONTEXT_SSO from authentik.lib.expression.evaluator import BaseEvaluator from authentik.policies.exceptions import PolicyException @@ -24,8 +25,8 @@ class PolicyEvaluator(BaseEvaluator): policy: Optional["ExpressionPolicy"] = None - def __init__(self, policy_name: str | None = None): - super().__init__(policy_name or "PolicyEvaluator") + def __init__(self, user: User, policy_name: str | None = None): + super().__init__(user, policy_name or "PolicyEvaluator") self._messages = [] # update website/docs/expressions/_objects.md # update website/docs/expressions/_functions.md @@ -44,6 +45,8 @@ class PolicyEvaluator(BaseEvaluator): if request.http_request: self.set_http_request(request.http_request) self._context["request"] = request + if not self._user: + self._user = request.user self._context["context"] = request.context def set_http_request(self, request: HttpRequest): diff --git a/authentik/policies/expression/migrations/0005_expressionpolicy_execution_user.py b/authentik/policies/expression/migrations/0005_expressionpolicy_execution_user.py new file mode 100644 index 0000000000..60d52bb6e1 --- /dev/null +++ b/authentik/policies/expression/migrations/0005_expressionpolicy_execution_user.py @@ -0,0 +1,26 @@ +# Generated by Django 5.0.3 on 2024-03-20 12:14 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("authentik_policies_expression", "0004_expressionpolicy_authentik_p_policy__fb6feb_idx"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AddField( + model_name="expressionpolicy", + name="execution_user", + field=models.ForeignKey( + default=None, + null=True, + on_delete=django.db.models.deletion.SET_DEFAULT, + to=settings.AUTH_USER_MODEL, + ), + ), + ] diff --git a/authentik/policies/expression/models.py b/authentik/policies/expression/models.py index e70f780562..b5c844f549 100644 --- a/authentik/policies/expression/models.py +++ b/authentik/policies/expression/models.py @@ -12,6 +12,9 @@ from authentik.policies.types import PolicyRequest, PolicyResult class ExpressionPolicy(Policy): """Execute arbitrary Python code to implement custom checks and validation.""" + execution_user = models.ForeignKey( + "authentik_core.User", default=None, null=True, on_delete=models.SET_DEFAULT + ) expression = models.TextField() @property @@ -26,17 +29,11 @@ class ExpressionPolicy(Policy): def passes(self, request: PolicyRequest) -> PolicyResult: """Evaluate and render expression. Returns PolicyResult(false) on error.""" - evaluator = PolicyEvaluator(self.name) + evaluator = PolicyEvaluator(self.execution_user, self.name) evaluator.policy = self evaluator.set_policy_request(request) return evaluator.evaluate(self.expression) - def save(self, *args, **kwargs): - evaluator = PolicyEvaluator(self.name) - evaluator.policy = self - evaluator.validate(self.expression) - return super().save(*args, **kwargs) - class Meta(Policy.PolicyMeta): verbose_name = _("Expression Policy") verbose_name_plural = _("Expression Policies") diff --git a/authentik/policies/expression/signals.py b/authentik/policies/expression/signals.py new file mode 100644 index 0000000000..8401824edd --- /dev/null +++ b/authentik/policies/expression/signals.py @@ -0,0 +1,13 @@ +from django.db.models.signals import pre_save +from django.dispatch import receiver + +from authentik.policies.expression.evaluator import PolicyEvaluator +from authentik.policies.expression.models import ExpressionPolicy + + +@receiver(pre_save, sender=ExpressionPolicy) +def pre_save_expression_policy(sender: type[ExpressionPolicy], instance: ExpressionPolicy, **_): + """Ensure policy is valid before saving""" + evaluator = PolicyEvaluator(instance.execution_user, instance.name) + evaluator.policy = instance + evaluator.validate(instance.expression) diff --git a/blueprints/schema.json b/blueprints/schema.json index 3b16f142a4..2727671ea0 100644 --- a/blueprints/schema.json +++ b/blueprints/schema.json @@ -3477,6 +3477,10 @@ "type": "string", "minLength": 1, "title": "Expression" + }, + "execution_user": { + "type": "integer", + "title": "Execution user" } }, "required": [] diff --git a/schema.yml b/schema.yml index 50a6c85ca1..82100a92fb 100644 --- a/schema.yml +++ b/schema.yml @@ -11852,6 +11852,10 @@ paths: name: execution_logging schema: type: boolean + - in: query + name: execution_user + schema: + type: integer - in: query name: expression schema: @@ -33565,6 +33569,9 @@ components: readOnly: true expression: type: string + execution_user: + type: integer + nullable: true required: - bound_to - component @@ -33588,6 +33595,9 @@ components: expression: type: string minLength: 1 + execution_user: + type: integer + nullable: true required: - expression - name @@ -38841,6 +38851,9 @@ components: expression: type: string minLength: 1 + execution_user: + type: integer + nullable: true PatchedFlowRequest: type: object description: Flow Serializer diff --git a/web/src/admin/policies/expression/ExpressionPolicyForm.ts b/web/src/admin/policies/expression/ExpressionPolicyForm.ts index 01c6039147..29cfb58865 100644 --- a/web/src/admin/policies/expression/ExpressionPolicyForm.ts +++ b/web/src/admin/policies/expression/ExpressionPolicyForm.ts @@ -12,7 +12,13 @@ import { TemplateResult, html } from "lit"; import { customElement } from "lit/decorators.js"; import { ifDefined } from "lit/directives/if-defined.js"; -import { ExpressionPolicy, PoliciesApi } from "@goauthentik/api"; +import { + CoreApi, + CoreUsersListRequest, + ExpressionPolicy, + PoliciesApi, + User, +} from "@goauthentik/api"; @customElement("ak-policy-expression-form") export class ExpressionPolicyForm extends BasePolicyForm { @@ -92,6 +98,39 @@ export class ExpressionPolicyForm extends BasePolicyForm {

+ + => { + const args: CoreUsersListRequest = { + ordering: "username", + }; + if (query !== undefined) { + args.search = query; + } + const users = await new CoreApi(DEFAULT_CONFIG).coreUsersList(args); + return users.results; + }} + .renderElement=${(user: User): string => { + return user.username; + }} + .renderDescription=${(user: User): TemplateResult => { + return html`${user.name}`; + }} + .value=${(user: User | undefined): number | undefined => { + return user?.pk; + }} + .selected=${(user: User): boolean => { + return this.instance?.executionUser === user.pk; + }} + blankable + > + +

+ ${msg( + "Configure which user the bundled API client authenticates as. When left empty, the API client will inherit the permissions of the user triggering the policy execution.", + )} +

+ `; } diff --git a/web/src/common/global.ts b/web/src/common/global.ts index 990303df0d..b0395943c4 100644 --- a/web/src/common/global.ts +++ b/web/src/common/global.ts @@ -44,7 +44,7 @@ export function docLink(path: string): string { const ak = globalAK(); // Default case or beta build which should always point to latest if (!ak || ak.build !== "") { - return `https://goauthentik.io${path}`; + return `https://docs.goauthentik.io${path}`; } return `https://${ak.versionSubdomain}.goauthentik.io${path}`; }