-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Security] Add ability for authenticators to explain why they didn’t support a request #60538
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
base: 7.4
Are you sure you want to change the base?
Conversation
a175f93
to
b0cdff7
Compare
@@ -11,6 +11,7 @@ CHANGELOG | |||
* Add support for closures in `#[IsGranted]` | |||
* Add `OAuth2TokenHandler` with OAuth2 Token Introspection support for `AccessTokenAuthenticator` | |||
* Add discovery support to `OidcTokenHandler` and `OidcUserInfoTokenHandler` | |||
* Add ability for authenticators to explain why they support the request or not |
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.
* Add ability for authenticators to explain why they support the request or not | |
* Add ability for authenticators to explain why they do not support the request |
If they support it, no reason is set, right?
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.
s/does/do
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.
Updated, thanks
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.
If they support it, no reason is set, right?
I was not sure if the userland could benefit from it, but since it is only displayed when the authenticator doesn’t support the request let’s not advertise it indeed.
12f9679
to
cc4097b
Compare
You may want to update your PR description to target 7.4, as well as create a new UPGRADE file to target 7.4 as well |
Yep I’ll do it while rebasing on the 7.4 branch when it’ll be created. |
cc4097b
to
18bdbc1
Compare
…support a request
18bdbc1
to
fa94c26
Compare
Inspired by #59771 this PR allows authenticators to advertise why they didn’t support a request by passing a new
RequestSupport
argument to theirsupports
method. Calling itsaddReason
method will cause the profiler to display them:If this is accepted, this will pave the way for firewall listeners to expose informations about their calls to
supports
(this is why theRequestSupport
class also holds$result
and$lazy
properties) andauthenticate
, thus closing #36668.