Repository navigation
Conversation
The base viewsets such as /policies/all/ return the concrete subclass from get_object(), but permission_required and ObjectPermissions checked the base model's permission, like authentik_policies.view_policy, on that object. guardian raises WrongAppError when the subclass is in another app, so every user without the global permission got a 500 from endpoints like the policy and property mapping test actions, and from deleting through a base viewset. guardian stores object permissions under the subclass's content type and refuses to store the base model's on a subclass instance, so check the subclass's matching default permission instead. That is what the subclass's own viewset requires. A global base permission still takes priority.
✅ Deploy Preview for authentik-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
❌ 1 Tests Failed:
View the top 1 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Contributor
Playwright e2eDownload this run's HTML report and traces, then open the report: gh run download 40998258428 -D playwright-artifacts
npx playwright show-report playwright-artifacts/playwright-reportInstallation instructionsInstructions for docker-composeAdd the following block to your AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-6bcc55949d64620a9dfebddaa39b397be4b3cf1b
# Distroless image: AUTHENTIK_TAG=gh-6bcc55949d64620a9dfebddaa39b397be4b3cf1b-distroless
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)sAfterwards, run the upgrade commands from the latest release notes. Instructions for KubernetesAdd the following block to your authentik:
outposts:
container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
global:
image:
repository: ghcr.io/goauthentik/dev-server
tag: gh-6bcc55949d64620a9dfebddaa39b397be4b3cf1b
# Distroless image: tag: gh-6bcc55949d64620a9dfebddaa39b397be4b3cf1b-distrolessAfterwards, run the upgrade commands from the latest release notes. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Details
What does this PR change?
permission_requiredandObjectPermissions.has_object_permissionnow check a base model's default permission against the subclass the object really is. On anExpressionPolicy,authentik_policies.view_policyis checked asauthentik_policies_expression.view_expressionpolicy, globally or on the object. That's what/policies/expression/<pk>/already requires. A global grant of the base permission still takes priority, and permissions that aren't a parent model's default permission are checked unchanged.The translation lives in one helper,
permission_for_objectinauthentik/rbac/permissions.py, which both checks use.Why is this change needed?
The base viewsets (
/policies/all/,/propertymappings/all/,/sources/all/,/providers/all/,/stages/all/,/outposts/service_connections/all/,/endpoints/connectors/) useselect_subclasses(), soget_object()returns the subclass. Both checks then passed the base model's permission with that object to guardian. guardian raisesWrongAppErrorwhen the permission's app doesn't match the object's, and that surfaced as a 500.POST /api/v3/policies/all/<pk>/test/andPOST /api/v3/propertymappings/all/<pk>/test/returned 500 for every non-superuser, including users with no permissions at all and users with the globalview_policy. The POST makesObjectPermissionscheckadd_policyon the object before the action's ownview_policycheck runs.DELETEthrough a base viewset returned 500 for a user with the global base view permission but no global delete permission.Object grants on these objects can only exist under the subclass's content type, because guardian refuses to store the base model's permission on a subclass instance. That's why the check maps to the subclass permission instead of catching the exception. A missing permission is now a 403.
This doesn't change two existing behaviors:
ObjectFilter).testactions still need the add permission as well as view, becauseObjectPermissionsmaps POST toadd_<model>.How was this tested?
Added API tests, run against Postgres:
ExpressionPolicyreturns 200.ScopeMappinggrants returns 200, and 403 without them.ExpressionPolicythrough/policies/all/returns 403 without a delete grant and 204 with an object-leveldelete_expressionpolicy.All of these failed with
WrongAppErrorbefore the change. Reverting only the decorator half still fails the two success cases. The existingauthentik/rbactests pass.Linked issues
None found. I searched open issues for
WrongAppErrorand for 500s on test endpoints with RBAC.Checklist
make all)make docs)