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

[Security] defer log message in guard authenticator #29323

Merged
merged 1 commit into from Dec 10, 2018

Conversation

Projects
None yet
6 participants
@eschultz-magix
Copy link
Contributor

eschultz-magix commented Nov 26, 2018

prevent an unnecessary log message if the guard authenticator does not support the current request

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

This PR defers log messages about "getCredentials()" method calls if more than one guard authentication provider is used. The method is only called if the provider supports the request. The log message may be confusing during development.

@eschultz-magix eschultz-magix changed the title defer log message in guard authenticator [Security] defer log message in guard authenticator Nov 26, 2018

if (null !== $this->logger) {
$this->logger->debug('Calling getCredentials() on guard authenticator.', array('firewall_key' => $this->providerKey, 'authenticator' => \get_class($guardAuthenticator)));
}
// abort the execution of the authenticator if it doesn't support the request
if (!$guardAuthenticator->supports($request)) {

This comment has been minimized.

@ro0NL

ro0NL Nov 26, 2018

Contributor

would it be useful to add a new log in this block then?

This comment has been minimized.

@eschultz-magix

eschultz-magix Nov 26, 2018

Contributor

Maybe it helps debugging. I have added two more log messages.

@ro0NL

ro0NL approved these changes Nov 26, 2018

[Security] defer log message in guard authenticator
prevent an unneccessary log message if the guard authenticator does not support the current request

@chalasr chalasr added this to the 3.4 milestone Dec 9, 2018

@chalasr chalasr changed the base branch from master to 3.4 Dec 9, 2018

@chalasr chalasr force-pushed the eschultz-magix:defer_guard_log_message branch from f67a317 to 21c3030 Dec 9, 2018

@chalasr

chalasr approved these changes Dec 9, 2018

@fabpot

fabpot approved these changes Dec 10, 2018

@fabpot

This comment has been minimized.

Copy link
Member

fabpot commented Dec 10, 2018

Thank you @eschultz-magix.

@fabpot fabpot merged commit 21c3030 into symfony:3.4 Dec 10, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Dec 10, 2018

bug #29323 [Security] defer log message in guard authenticator (eschu…
…ltz-magix)

This PR was merged into the 3.4 branch.

Discussion
----------

[Security] defer log message in guard authenticator

prevent an unnecessary log message if the guard authenticator does not support the current request

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

This PR defers log messages about "getCredentials()" method calls if more than one guard authentication provider is used. The method is only called if the provider supports the request. The log message may be confusing during development.

Commits
-------

21c3030 [Security] defer log message in guard authenticator

This was referenced Jan 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment