refactor: allow auth_methods on route configurations#20096
Open
miketheman wants to merge 8 commits into
Open
Conversation
As a precursor to refactoring more permissions logic, extract from inline into a lookup table. Signed-off-by: Mike Fiedler <miketheman@gmail.com>
Create a way for routes to declare which authentication methods they expect to receive. Signed-off-by: Mike Fiedler <miketheman@gmail.com>
Declare the authentication methods each route accepts: - basic-auth and macaroon for file uploads - macaroon for the danger-api endpoints Signed-off-by: Mike Fiedler <miketheman@gmail.com>
Replace the hardcoded route-name checks in the session and basic-auth identity() methods with a lookup against the route's auth_methods predicate, via a new auth_methods_for_route() helper. Signed-off-by: Mike Fiedler <miketheman@gmail.com>
Reserve an AuthenticationMethod for the dedicated API auth surface that will replace danger-api's macaroon abuse. No policy implements it yet. Signed-off-by: Mike Fiedler <miketheman@gmail.com>
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
1 participant
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.
See #13854 for historical approach to this idea, but this approach uses route predicates instead.
auth_methodsis a route predicate, not a view predicate.The security policies read this in their
identity()methods (SessionSecurityPolicyandBasicAuthSecurityPolicy).identity()runs at authentication time, right after Pyramid matches the route.The matched route is sitting there to inspect via
request.matched_route.predicates.The view isn't - it hasn't been resolved yet, and there's no request attribute that exposes view predicates anyway. Put this on a view predicate and the policy is blind to it at the one moment it needs the answer.
It's also where the concept belongs. Which credentials an endpoint accepts is a property of the route, not of one view callable. A route can fan out to several views (different methods, accept headers), and all of them should answer to the same auth contract - set once, where the route is defined. This is different than permissions, which may vary based on the HTTP verbs (GET vs POST) - so they can be combined without having to recall the auth_method on each
view_config(and it's "too late" anyhow).The predicate is storage-only:
__call__always returnsTrue,so it never influences routing or view selection.
We already do this with the
domainroute predicate,which is the same kind of thing: route-level metadata that infrastructure reads.
Fixes #7266