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] Add Guard authenticator <supports> method #16835

Merged
merged 2 commits into from Oct 5, 2017

Conversation

@Amo
Contributor

Amo commented Dec 4, 2015

This method will be called before starting an authentication against a guard authenticator.
The authentication will be tried only if the supports method returned true
This improves understanding of code, increase consistency and removes responsability for getCredentials method to decide if the current request should be supported or not.

Q A
Bug fix? no
New feature? yes
BC breaks? yes
Deprecations? yes
Tests pass? yes
Fixed tickets none
License MIT
Doc PR
@Amo

This comment has been minimized.

Show comment
Hide comment
@Amo

Amo Dec 4, 2015

Contributor

Note: as being not familiar with the recent deprecation mechanisms, i'm not sure i did it well in src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php, line 109

Contributor

Amo commented Dec 4, 2015

Note: as being not familiar with the recent deprecation mechanisms, i'm not sure i did it well in src/Symfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php, line 109

@Amo

This comment has been minimized.

Show comment
Hide comment
@Amo

Amo Dec 4, 2015

Contributor

About tests, i get, i'm not sure how is this wrong or ok.

$ ./phpunit --filter GuardAuthenticationListenerTest
PHPUnit 4.8.19 by Sebastian Bergmann and contributors.

Testing Symfony Test Suite
.......

Time: 4.1 seconds, Memory: 173.00Mb

OK (7 tests, 22 assertions)

Remaining deprecation notices (6)

Class Mock_GuardAuthenticatorInterface_a5f0cf1d should provide a method <supports(Request $request)>. see GuardAuthenticatorSupportInterface: 6x
    2x in GuardAuthenticationListenerTest::testReturnNullToSkipAuth from Symfony\Component\Security\Guard\Tests\Firewall
    1x in GuardAuthenticationListenerTest::testHandleCatchesAuthenticationException from Symfony\Component\Security\Guard\Tests\Firewall
    1x in GuardAuthenticationListenerTest::testHandleSuccessStopsAfterResponseIsSet from Symfony\Component\Security\Guard\Tests\Firewall
    1x in GuardAuthenticationListenerTest::testHandleSuccess from Symfony\Component\Security\Guard\Tests\Firewall
    1x in GuardAuthenticationListenerTest::testHandleSuccessWithRememberMe from Symfony\Component\Security\Guard\Tests\Firewall

KO symfony
Contributor

Amo commented Dec 4, 2015

About tests, i get, i'm not sure how is this wrong or ok.

$ ./phpunit --filter GuardAuthenticationListenerTest
PHPUnit 4.8.19 by Sebastian Bergmann and contributors.

Testing Symfony Test Suite
.......

Time: 4.1 seconds, Memory: 173.00Mb

OK (7 tests, 22 assertions)

Remaining deprecation notices (6)

Class Mock_GuardAuthenticatorInterface_a5f0cf1d should provide a method <supports(Request $request)>. see GuardAuthenticatorSupportInterface: 6x
    2x in GuardAuthenticationListenerTest::testReturnNullToSkipAuth from Symfony\Component\Security\Guard\Tests\Firewall
    1x in GuardAuthenticationListenerTest::testHandleCatchesAuthenticationException from Symfony\Component\Security\Guard\Tests\Firewall
    1x in GuardAuthenticationListenerTest::testHandleSuccessStopsAfterResponseIsSet from Symfony\Component\Security\Guard\Tests\Firewall
    1x in GuardAuthenticationListenerTest::testHandleSuccess from Symfony\Component\Security\Guard\Tests\Firewall
    1x in GuardAuthenticationListenerTest::testHandleSuccessWithRememberMe from Symfony\Component\Security\Guard\Tests\Firewall

KO symfony
@iltar

View changes

Show outdated Hide outdated ...ymfony/Component/Security/Guard/Firewall/GuardAuthenticationListener.php
@Amo

This comment has been minimized.

Show comment
Hide comment
@Amo

Amo Dec 4, 2015

Contributor

by the way @weaverryan , that's what i was talking with you this morning.

Contributor

Amo commented Dec 4, 2015

by the way @weaverryan , that's what i was talking with you this morning.

@Amo Amo changed the title from Add Guard authenticator <supports> method to [Security] Add Guard authenticator <supports> method Dec 4, 2015

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Dec 5, 2015

Member

I'm not sure we need this. It would probably have to duplicate some of the work done in getCredentials in many time.

@weaverryan what do you think ?

Member

stof commented Dec 5, 2015

I'm not sure we need this. It would probably have to duplicate some of the work done in getCredentials in many time.

@weaverryan what do you think ?

@stof

This comment has been minimized.

Show comment
Hide comment
@stof

stof Dec 5, 2015

Member

@Amo tests are failing because you are using the deprecated way in a non-legacy test.

Member

stof commented Dec 5, 2015

@Amo tests are failing because you are using the deprecated way in a non-legacy test.

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Dec 7, 2015

Member

It's "clearer" in some ways because supports() can return a clean true/false value and then getCredentials() can cause an AuthException if it returns null, which would make it more consistent with getUser and checkCredentials.

I discussed this at the conference - I'm mixed on it - the only negative I see is exactly what Stof mentions, but that is a practical concern. For an API token auth, I'll either have to duplicate code that gets/checks for the API token value in supports() and getUser() or just always return true from supports, which defeats the purpose.

@Amo Can you convince us that this will be a better user experience?

Thx!

Member

weaverryan commented Dec 7, 2015

It's "clearer" in some ways because supports() can return a clean true/false value and then getCredentials() can cause an AuthException if it returns null, which would make it more consistent with getUser and checkCredentials.

I discussed this at the conference - I'm mixed on it - the only negative I see is exactly what Stof mentions, but that is a practical concern. For an API token auth, I'll either have to duplicate code that gets/checks for the API token value in supports() and getUser() or just always return true from supports, which defeats the purpose.

@Amo Can you convince us that this will be a better user experience?

Thx!

@Amo

This comment has been minimized.

Show comment
Hide comment
@Amo

Amo Dec 7, 2015

Contributor

The idea is to separate the responsabilities of

  • Identifying if the authentication context is elligible/supported
  • Extracting the credentials from the context

and doing so, improve the readability, support of variations and testability of an authentication method.

Supports would be responsible to detect a specific context, guaranteeing that when getCredentials is called, this context has been met.

As @stof said, it could end-up in duplicating the same checks in supports and getCredentials. But, imho, that would occurs if the authentication system supports many context and is poorly designed.

Let's take an example :

Access to /api requires a X-API-Token header.
An authenticator is created :

    // ...

    /**
     * Ensure the given request provides an API token in the headers
     * 
     * @param  Request $request
     * 
     * @return null|mixed
     */
    public function supports(Request $request)
    {
        if (false === $request->headers->has('X-API-Token')) {
            return false;
        }

        return true;
    }

    /**
     * Extract the credentials from the request.
     * 
     * @param  Request $request
     * 
     * @return string
     */
    public function getCredentials(Request $request)
    {
        return $request->headers->get('X-API-Token'));
    }

If additional credentials retrieval methods are required (qs, payload ...) it is likely that the getCredentials method will get many if and else, which would make the code harder to read, maintain, test ...

This separation supports vs getCredentials will spur the developer to create dedicated guard authenticator for each specificities. A common abstract class or a trait could be used to share the common stages of the authentication. But concrete final classes will explicitly indicates with use-cases they manage :

Security
└── Authenticator
    ├── ApiBaseAuthenticator.php
    ├── ApiHeaderAuthenticator.php
    ├── ApiPayloadAuthenticator.php
    └── ApiQueryAuthenticator.php

Where ApiBaseAuthenticator.php handles the common logic for

getUser
checkCredentials
createAuthenticatedToken
onAuthenticationFailure
onAuthenticationSuccess
supportsRememberMe

And each dedicated authenticator provides the dedicated logic to retrieve credentials from header, query string or payload

Contributor

Amo commented Dec 7, 2015

The idea is to separate the responsabilities of

  • Identifying if the authentication context is elligible/supported
  • Extracting the credentials from the context

and doing so, improve the readability, support of variations and testability of an authentication method.

Supports would be responsible to detect a specific context, guaranteeing that when getCredentials is called, this context has been met.

As @stof said, it could end-up in duplicating the same checks in supports and getCredentials. But, imho, that would occurs if the authentication system supports many context and is poorly designed.

Let's take an example :

Access to /api requires a X-API-Token header.
An authenticator is created :

    // ...

    /**
     * Ensure the given request provides an API token in the headers
     * 
     * @param  Request $request
     * 
     * @return null|mixed
     */
    public function supports(Request $request)
    {
        if (false === $request->headers->has('X-API-Token')) {
            return false;
        }

        return true;
    }

    /**
     * Extract the credentials from the request.
     * 
     * @param  Request $request
     * 
     * @return string
     */
    public function getCredentials(Request $request)
    {
        return $request->headers->get('X-API-Token'));
    }

If additional credentials retrieval methods are required (qs, payload ...) it is likely that the getCredentials method will get many if and else, which would make the code harder to read, maintain, test ...

This separation supports vs getCredentials will spur the developer to create dedicated guard authenticator for each specificities. A common abstract class or a trait could be used to share the common stages of the authentication. But concrete final classes will explicitly indicates with use-cases they manage :

Security
└── Authenticator
    ├── ApiBaseAuthenticator.php
    ├── ApiHeaderAuthenticator.php
    ├── ApiPayloadAuthenticator.php
    └── ApiQueryAuthenticator.php

Where ApiBaseAuthenticator.php handles the common logic for

getUser
checkCredentials
createAuthenticatedToken
onAuthenticationFailure
onAuthenticationSuccess
supportsRememberMe

And each dedicated authenticator provides the dedicated logic to retrieve credentials from header, query string or payload

@Amo

This comment has been minimized.

Show comment
Hide comment
@Amo

Amo Dec 7, 2015

Contributor

@Amo tests are failing because you are using the deprecated way in a non-legacy test.

How can i avoid it ? googling about Symfony 3 and deprecation conventions didn't give any results.

Contributor

Amo commented Dec 7, 2015

@Amo tests are failing because you are using the deprecated way in a non-legacy test.

How can i avoid it ? googling about Symfony 3 and deprecation conventions didn't give any results.

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Dec 8, 2015

Member

@Amo I think your arguments are quite good. So, let's see if we can complete this PR :).

Forgetting about BC, how would you want the interface to look in 4.0? You added a new GuardAuthenticatorSupportsInterface, but I imagine that you'd really want supports() to be in the GuardAuthenticatorInterface, correct? There will be some work that will need to be done to create a new interface (that has the new method) and deprecate the old interface.

Member

weaverryan commented Dec 8, 2015

@Amo I think your arguments are quite good. So, let's see if we can complete this PR :).

Forgetting about BC, how would you want the interface to look in 4.0? You added a new GuardAuthenticatorSupportsInterface, but I imagine that you'd really want supports() to be in the GuardAuthenticatorInterface, correct? There will be some work that will need to be done to create a new interface (that has the new method) and deprecate the old interface.

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Dec 8, 2015

Member

@Amo about the tests, the GuardAuthenticationListenerTest is mocking the GuardAuthenticatorInterface, which then triggers your deprecation warning. This mock should be changed to GuardAuthenticatorSupportsInterface so that the deprecated functionality isn't triggered (or changed to whatever final interface we decide on that has the new supports() method). There should also be a test that verifies that if you use the original GuardAuthenticatorInterface, then the old behavior is used. Put an @legacy above tests that specifically test how legacy functionality works.

Member

weaverryan commented Dec 8, 2015

@Amo about the tests, the GuardAuthenticationListenerTest is mocking the GuardAuthenticatorInterface, which then triggers your deprecation warning. This mock should be changed to GuardAuthenticatorSupportsInterface so that the deprecated functionality isn't triggered (or changed to whatever final interface we decide on that has the new supports() method). There should also be a test that verifies that if you use the original GuardAuthenticatorInterface, then the old behavior is used. Put an @legacy above tests that specifically test how legacy functionality works.

@Amo

This comment has been minimized.

Show comment
Hide comment
@Amo

Amo Dec 8, 2015

Contributor

ok thanks, i'm on it

Contributor

Amo commented Dec 8, 2015

ok thanks, i'm on it

@Amo

This comment has been minimized.

Show comment
Hide comment
@Amo

Amo Dec 8, 2015

Contributor

Tests are still running.
But, (take a deep breath), i moved the interface into the Authenticator sub namespace, Symfony\Component\Security\Guard\Authenticator\GuardAuthenticatorInterface which makes sense to me.
This interface adds the supports method and keep the same the signature as Symfony\Component\Security\Guard\GuardAuthenticatorInterface. It extends Symfony\Component\Security\Guard\GuardAuthenticatorInterfaceas GuardAuthenticatorLegacyInterface to avoid BC break. the Symfony\Component\Security\Guard\GuardAuthenticatorInterface has now a @deprecated annotation and is still used as type hint in Symfony\Component\Security\Guard\Firewall\GuardAuthenticationListener alongside the new Symfony\Component\Security\Guard\Authenticator\GuardAuthenticatorInterface interface.

Contributor

Amo commented Dec 8, 2015

Tests are still running.
But, (take a deep breath), i moved the interface into the Authenticator sub namespace, Symfony\Component\Security\Guard\Authenticator\GuardAuthenticatorInterface which makes sense to me.
This interface adds the supports method and keep the same the signature as Symfony\Component\Security\Guard\GuardAuthenticatorInterface. It extends Symfony\Component\Security\Guard\GuardAuthenticatorInterfaceas GuardAuthenticatorLegacyInterface to avoid BC break. the Symfony\Component\Security\Guard\GuardAuthenticatorInterface has now a @deprecated annotation and is still used as type hint in Symfony\Component\Security\Guard\Firewall\GuardAuthenticationListener alongside the new Symfony\Component\Security\Guard\Authenticator\GuardAuthenticatorInterface interface.

@Amo

This comment has been minimized.

Show comment
Hide comment
@Amo

Amo Dec 8, 2015

Contributor

Should i fixup and git push --force all the changes on the Amo:symfony branch ?

Contributor

Amo commented Dec 8, 2015

Should i fixup and git push --force all the changes on the Amo:symfony branch ?

@iltar

This comment has been minimized.

Show comment
Hide comment
@iltar

iltar Dec 9, 2015

Contributor

I notice the handle()/executeGuardAuthenticator() are becoming big_(ger with those changes)_. I think the flow and changes are good, but this will become harder and harder to maintain. Some suggestions I have for this method which could be done (after this PR is merged):

Using a null logger instead of a PSR logger
It will improve readability and reduce complexity and would remove 8 if statements in just those 2 methods

Move code out of the try catch which is unrelated
This try catch is pretty much just a sort of "failsafe" at the moment. However, things like supports() should not throw an AuthenticationException. A list of method calls:

  • $this->logger->debug('...');
  • $guardAuthenticator->supports($request)
  • $guardAuthenticator->getCredentials($request)
  • $this->logger->debug('...');
  • $this->authenticationManager->authenticate($token)
  • $this->logger->info('...');
  • $this->guardHandler->authenticateWithToken($token, $request);

Out of all available methods in the authenticator, only getUser() and checkCredentials() throw an exception and they are not being called here. In fact, the only method that throws an exception is $this->authenticationManager->authenticate($token). It's actually making it possible for people to throw all kinds of AuthenticationExceptions which can cause unwanted behavior or break when someone decides this try/catch can be narrowed down in say 3.4.

Make GuardAuthenticationListener final
We've created a very nice extension point which people will abuse in the future. My suggestion is to make this class final as it makes it easier for people to fix things here. Behavioral changes should still be avoided as much as possible though.

After this the code will still be quite massive but it will at least be easier to read and understand what it does.

Contributor

iltar commented Dec 9, 2015

I notice the handle()/executeGuardAuthenticator() are becoming big_(ger with those changes)_. I think the flow and changes are good, but this will become harder and harder to maintain. Some suggestions I have for this method which could be done (after this PR is merged):

Using a null logger instead of a PSR logger
It will improve readability and reduce complexity and would remove 8 if statements in just those 2 methods

Move code out of the try catch which is unrelated
This try catch is pretty much just a sort of "failsafe" at the moment. However, things like supports() should not throw an AuthenticationException. A list of method calls:

  • $this->logger->debug('...');
  • $guardAuthenticator->supports($request)
  • $guardAuthenticator->getCredentials($request)
  • $this->logger->debug('...');
  • $this->authenticationManager->authenticate($token)
  • $this->logger->info('...');
  • $this->guardHandler->authenticateWithToken($token, $request);

Out of all available methods in the authenticator, only getUser() and checkCredentials() throw an exception and they are not being called here. In fact, the only method that throws an exception is $this->authenticationManager->authenticate($token). It's actually making it possible for people to throw all kinds of AuthenticationExceptions which can cause unwanted behavior or break when someone decides this try/catch can be narrowed down in say 3.4.

Make GuardAuthenticationListener final
We've created a very nice extension point which people will abuse in the future. My suggestion is to make this class final as it makes it easier for people to fix things here. Behavioral changes should still be avoided as much as possible though.

After this the code will still be quite massive but it will at least be easier to read and understand what it does.

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Jan 25, 2016

Member

@weaverryan What's the status of this one?

Member

fabpot commented Jan 25, 2016

@weaverryan What's the status of this one?

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Feb 18, 2016

Member

@weaverryan I'd like to move this one forward. Can you have a look at it? Thanks.

Member

fabpot commented Feb 18, 2016

@weaverryan I'd like to move this one forward. Can you have a look at it? Thanks.

@Amo

This comment has been minimized.

Show comment
Hide comment
@Amo

Amo Feb 18, 2016

Contributor

Updated.

Contributor

Amo commented Feb 18, 2016

Updated.

@weaverryan

This comment has been minimized.

Show comment
Hide comment
@weaverryan

weaverryan Feb 18, 2016

Member

Thanks for the quick update @Amo - and sorry for the extra comments - I had to pause in the middle of reviewing :). Btw, when I talked about Guard at the last conference, someone else suggested this exact change afterwards.

Member

weaverryan commented Feb 18, 2016

Thanks for the quick update @Amo - and sorry for the extra comments - I had to pause in the middle of reviewing :). Btw, when I talked about Guard at the last conference, someone else suggested this exact change afterwards.

@Amo

This comment has been minimized.

Show comment
Hide comment
@Amo

Amo Feb 23, 2016

Contributor

I've been updating the code according to the discussion about getUserFromRequest.
Unit tests need to be updated.

Contributor

Amo commented Feb 23, 2016

I've been updating the code according to the discussion about getUserFromRequest.
Unit tests need to be updated.

@chalasr chalasr changed the base branch from master to 3.4 Oct 4, 2017

@chalasr

chalasr approved these changes Oct 4, 2017

👍

@chalasr

This comment has been minimized.

Show comment
Hide comment
@chalasr

chalasr Oct 4, 2017

Member

@nicolas-grekas comments addressed, thanks

Member

chalasr commented Oct 4, 2017

@nicolas-grekas comments addressed, thanks

@nicolas-grekas

(but there are more FQCN strings that could be replaced by ...::class)

@chalasr

This comment has been minimized.

Show comment
Hide comment
@chalasr

chalasr Oct 5, 2017

Member

Remaining FQCNs fixed

Member

chalasr commented Oct 5, 2017

Remaining FQCNs fixed

@chalasr chalasr added the Ready label Oct 5, 2017

@xabbuh

xabbuh approved these changes Oct 5, 2017

@fabpot

fabpot approved these changes Oct 5, 2017

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Oct 5, 2017

Member

Thank you @Amo.

Member

fabpot commented Oct 5, 2017

Thank you @Amo.

@fabpot fabpot merged commit 78eecba into symfony:3.4 Oct 5, 2017

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 Oct 5, 2017

feature #16835 [Security] Add Guard authenticator <supports> method (…
…Amo, chalasr)

This PR was merged into the 3.4 branch.

Discussion
----------

[Security] Add Guard authenticator <supports> method

This method will be called before starting an authentication against a guard authenticator.
The authentication will be tried only if the supports method returned `true`
This improves understanding of code, increase consistency and removes responsability for `getCredentials` method to decide if the current request should be supported or not.

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

Commits
-------

78eecba Fix BC layer
a7a6f8a [Security] Add Guard authenticator <supports> method
@Amo

This comment has been minimized.

Show comment
Hide comment
@Amo

Amo Oct 5, 2017

Contributor

Thank you ! Now i'm a proud contributor of Symfony

Contributor

Amo commented Oct 5, 2017

Thank you ! Now i'm a proud contributor of Symfony

@chalasr

This comment has been minimized.

Show comment
Hide comment
@chalasr

chalasr Oct 5, 2017

Member

@Amo Thanks for this and congrats for your first PR on github! You did it right, in the right repo :)

Member

chalasr commented Oct 5, 2017

@Amo Thanks for this and congrats for your first PR on github! You did it right, in the right repo :)

fabpot added a commit that referenced this pull request Oct 5, 2017

feature #24446 [Security] Remove GuardAuthenticatorInterface (chalasr)
This PR was merged into the 4.0-dev branch.

Discussion
----------

[Security] Remove GuardAuthenticatorInterface

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | symfony/symfony-docs#8485

Removes BC layers for #16835.

Commits
-------

3408152 [Security][Guard] Remove GuardAuthenticatorInterface

This was referenced Oct 18, 2017

@kbond

This comment has been minimized.

Show comment
Hide comment
@kbond

kbond Oct 24, 2017

Contributor

I am getting what I think is a false positive deprecation notice in 3.4:

User Deprecated: The "Symfony\Component\Security\Guard\AbstractGuardAuthenticator::supports()" 
Method is deprecated since version 3.4, to be removed in 4.0. You should not extend it from 
"...\MyAuthenticator".

I have added MyAuthenticator::supports() but am still getting this notice. Did I not upgrade correctly or is this a bug?

Contributor

kbond commented Oct 24, 2017

I am getting what I think is a false positive deprecation notice in 3.4:

User Deprecated: The "Symfony\Component\Security\Guard\AbstractGuardAuthenticator::supports()" 
Method is deprecated since version 3.4, to be removed in 4.0. You should not extend it from 
"...\MyAuthenticator".

I have added MyAuthenticator::supports() but am still getting this notice. Did I not upgrade correctly or is this a bug?

{
/**
* {@inheritdoc}
*
* @deprecated since version 3.4, to be removed in 4.0

This comment has been minimized.

@kbond

kbond Oct 24, 2017

Contributor

If I remove this line, the false positive deprecation notice is removed. Should I open a PR?

@kbond

kbond Oct 24, 2017

Contributor

If I remove this line, the false positive deprecation notice is removed. Should I open a PR?

This comment has been minimized.

@chalasr

chalasr Oct 24, 2017

Member

Please do. This method must be implemented by concrete subclasses and it is unlikely to be called directly.

@chalasr

chalasr Oct 24, 2017

Member

Please do. This method must be implemented by concrete subclasses and it is unlikely to be called directly.

fabpot added a commit that referenced this pull request Oct 25, 2017

bug #24684 [Security] remove invalid deprecation notice on AbstractGu…
…ardAuthenticator::supports() (kbond)

This PR was merged into the 3.4 branch.

Discussion
----------

[Security] remove invalid deprecation notice on AbstractGuardAuthenticator::supports()

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #16835 (comment)
| License       | MIT
| Doc PR        | n/a

This deprecation flag causes a false positive.

Commits
-------

5fb44e7 [Guard] remove invalid deprecation notice
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment