Skip to content
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

Fix error when trying to decode token using new authenticator system #922

Merged
merged 1 commit into from
Sep 16, 2021

Conversation

fd6130
Copy link

@fd6130 fd6130 commented Sep 16, 2021

If you are using new authenticator system, you cannot decode the token in a service or controller using $jwtManager->decode($token->getToken()); because the Token Class getCredentials() return empty array instead of token.

Now i'm not sure about the public function createToken(Passport $passport, string $firewallName): TokenInterface in JwtAuthenticator. Do i need to adjust that one as well?

Anyways, this Fix #921.

@fd6130 fd6130 changed the title Fix error when trying to decode token using new authenticator system [WIP] Fix error when trying to decode token using new authenticator system Sep 16, 2021
@fd6130
Copy link
Author

fd6130 commented Sep 16, 2021

The test got issue, i don't know how can i make it pass. Hope someone can help me to solve it.

I tried this PR along with custom authenticator in my fresh project, having no issue so far.

@fd6130 fd6130 changed the title [WIP] Fix error when trying to decode token using new authenticator system Fix error when trying to decode token using new authenticator system Sep 16, 2021
@chalasr
Copy link
Collaborator

chalasr commented Sep 16, 2021

Thanks for the PR!
Here is a patch that fixes the failing test:

diff --git a/Security/Authenticator/JWTAuthenticator.php b/Security/Authenticator/JWTAuthenticator.php
index 7d7ffc7..f27b4e3 100644
--- a/Security/Authenticator/JWTAuthenticator.php
+++ b/Security/Authenticator/JWTAuthenticator.php
@@ -248,7 +248,7 @@ class JWTAuthenticator extends AbstractAuthenticator implements AuthenticationEn
 
     public function createToken(Passport $passport, string $firewallName): TokenInterface
     {
-        $token = parent::createToken($passport, $firewallName);
+        $token = new JWTPostAuthenticationToken($passport->getUser(), $firewallName, $passport->getUser()->getRoles(), $passport->getAttribute('token'));
 
         $this->eventDispatcher->dispatch(new JWTAuthenticatedEvent($passport->getAttribute('payload'), $token), Events::JWT_AUTHENTICATED);
 
diff --git a/Tests/Security/Authenticator/JWTAuthenticatorTest.php b/Tests/Security/Authenticator/JWTAuthenticatorTest.php
index 348d66b..1a16874 100644
--- a/Tests/Security/Authenticator/JWTAuthenticatorTest.php
+++ b/Tests/Security/Authenticator/JWTAuthenticatorTest.php
@@ -14,6 +14,7 @@ use Lexik\Bundle\JWTAuthenticationBundle\Exception\JWTDecodeFailureException;
 use Lexik\Bundle\JWTAuthenticationBundle\Exception\MissingTokenException;
 use Lexik\Bundle\JWTAuthenticationBundle\Response\JWTAuthenticationFailureResponse;
 use Lexik\Bundle\JWTAuthenticationBundle\Security\Authenticator\JWTAuthenticator;
+use Lexik\Bundle\JWTAuthenticationBundle\Security\Authenticator\Token\JWTPostAuthenticationToken;
 use Lexik\Bundle\JWTAuthenticationBundle\Security\User\PayloadAwareUserProviderInterface;
 use Lexik\Bundle\JWTAuthenticationBundle\Services\JWTTokenManagerInterface;
 use Lexik\Bundle\JWTAuthenticationBundle\Tests\Stubs\User as AdvancedUserStub;
@@ -214,7 +215,7 @@ class JWTAuthenticatorTest extends TestCase
         $user->method('getRoles')->willReturn(['ROLE_USER']);
 
         $dispatcher = $this->getEventDispatcherMock();
-        $dispatcher->expects($this->once())->method('dispatch')->with($this->equalTo(new JWTAuthenticatedEvent(['claim' => 'val'], new PostAuthenticationToken($user, 'dummy', ['ROLE_USER']))), Events::JWT_AUTHENTICATED);
+        $dispatcher->expects($this->once())->method('dispatch')->with($this->equalTo(new JWTAuthenticatedEvent(['claim' => 'val'], new JWTPostAuthenticationToken($user, 'dummy', ['ROLE_USER'], 'dummytoken'))), Events::JWT_AUTHENTICATED);
 
         $authenticator = new JWTAuthenticator(
             $this->getJWTManagerMock(),
@@ -225,13 +226,18 @@ class JWTAuthenticatorTest extends TestCase
 
         $passport = $this->createMock(SelfValidatingPassport::class);
         $passport->method('getUser')->willReturn($user);
-        $passport->method('getAttribute')->with('payload')->willReturn(['claim' => 'val']);
+        $passport->method('getAttribute')
+            ->withConsecutive(['token', null], ['payload', null])
+            ->willReturnOnConsecutiveCalls('dummytoken', ['claim' => 'val']);
 
         if (method_exists(FormLoginAuthenticator::class, 'createToken')) {
-            $authenticator->createToken($passport, 'dummy');
+            $token = $authenticator->createToken($passport, 'dummy');
         } else {
-            $authenticator->createAuthenticatedToken($passport, 'dummy');
+            $token = $authenticator->createAuthenticatedToken($passport, 'dummy');
         }
+
+        $this->assertInstanceOf(JWTPostAuthenticationToken::class, $token);
+        $this->assertSame('dummytoken', $token->getCredentials());
     }
 
     private function getJWTManagerMock($userIdentityField = null, $userIdClaim = null)

Could you apply it?

@fd6130
Copy link
Author

fd6130 commented Sep 16, 2021

Alright, done for the test.

update

But the 5.4@dev test fail, not sure why.

@chalasr
Copy link
Collaborator

chalasr commented Sep 16, 2021

Woops, sorry! We need to patch createToken() as well:

diff --git a/Security/Authenticator/JWTAuthenticator.php b/Security/Authenticator/JWTAuthenticator.php
index 7d7ffc7..f27b4e3 100644
--- a/Security/Authenticator/JWTAuthenticator.php
+++ b/Security/Authenticator/JWTAuthenticator.php
@@ -248,7 +248,7 @@ class JWTAuthenticator extends AbstractAuthenticator implements AuthenticationEn
 
     public function createToken(Passport $passport, string $firewallName): TokenInterface
     {
-        $token = parent::createToken($passport, $firewallName);
+        $token = new JWTPostAuthenticationToken($passport->getUser(), $firewallName, $passport->getUser()->getRoles(), $passport->getAttribute('token'));
 
         $this->eventDispatcher->dispatch(new JWTAuthenticatedEvent($passport->getAttribute('payload'), $token), Events::JWT_AUTHENTICATED);
 

(FYI createToken() is the replacement for createAuthenticatedToken(), the latter is deprecated in 5.4)

@fd6130
Copy link
Author

fd6130 commented Sep 16, 2021

Alright, all green.

@chalasr
Copy link
Collaborator

chalasr commented Sep 16, 2021

Good catch, thanks @fd6130.

@chalasr chalasr merged commit 3f30e12 into lexik:2.x Sep 16, 2021
@fd6130 fd6130 deleted the jwt_authenticator branch September 16, 2021 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Invalid JWT Token" when using decode from JWTTokenManagerInterface
2 participants