flows: fix flow executor authentication with jwt #16646
Open
Conversation
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
✅ Deploy Preview for authentik-docs canceled.
|
✅ Deploy Preview for authentik-storybook canceled.
|
✅ Deploy Preview for authentik-integrations canceled.
|
lemoer
force-pushed
the
pr_hotfix_flow_executor_jwt
branch
2 times, most recently
from
ac784cc to
377e77c
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16646 +/- ##
==========================================
- Coverage 92.78% 92.73% -0.06%
==========================================
Files 838 838
Lines 45290 45401 +111
==========================================
+ Hits 42024 42101 +77
- Misses 3266 3300 +34
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
lemoer
force-pushed
the
pr_hotfix_flow_executor_jwt
branch
2 times, most recently
from
3f19d31 to
3fbd5ab
Compare
With this commit, we move the session authentication logic from
the dispatch() method to perform_authentication() method, which
is now also overridden from the APIView parent class.
The major difference here is the order in which things are called
now.
The most important point is that the django request object is
wrapped by the django rest framework in APIView.dispatch() by
calling APIView.initialize_request(). This method returns a
wrapped version of the request (of type rest_framework.Request).
This wrapper class implicitly performs the authentication
from the authentication_classes, when request.user is accessed.
Before this commit, the original django request object
was passed to FlowPlanner._check_authentication() before
it was wrapped into a rest_framework.Request, which led
to the authentication_classes not been recognized.
The call chain was:
FlowExecutorView.dispatch()
FlowExecutorView._initiate_plan()
FlowPlanner.plan()
FlowPlanner._check_authentication()
APIView.dispatch()
APIView.initialize_request()
With this commit, the flow session logic is moved to
FlowExecutorView.perform_authentication() such that
the wrapped request instance is properly passed down
to FlowPlanner._check_authentication()
The new call chain is then:
FlowExecutorView.dispatch()
APIView.dispatch()
APIView.initialize_request()
APIView.initial()
FlowExecutorView.perform_authentication()
FlowExecutorView._initiate_plan()
FlowPlanner.plan()
FlowPlanner._check_authentication()
Since we can not return HttpResponses from
FlowExecutorView.perform_authentication() some exception
handlers remain in the dispatch() method. Additionally,
two new exceptions are introduced which are raised
in perform_authentication() and caught in dispatch() to
be transformed into HttpResponses.
Fixes goauthentik#15747
But otherwise, I got random errors that request doesn't have the property request_id. Somebody with more insight should figure out why this happens and how to handle it properly.
lemoer
force-pushed
the
pr_hotfix_flow_executor_jwt
branch
from
3fbd5ab to
65801b0
Compare
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
As described in issue #15747, the authentication with a JWT for the flow executor was broken. This PR fixes that. Furthermore, a unit test is added that catches this error.
I am happy to receive some feedback from the authentik devs. This mostly a proof of concept to show a way how this issue COULD be solved. If another solution is preferred, we could also go for that.
Checklist
ak test authentik/)make lint-fix)