Completed
Push — master ( 65f314...a56b5d )
by Paweł
08:26
created

ExternalOauthAuthenticator::getOauthClient()   A

Complexity

Conditions 1
Paths 1

Size

Total Lines 4

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 4
rs 10
c 0
b 0
f 0
cc 1
nc 1
nop 0
1
<?php
2
3
declare(strict_types=1);
4
5
namespace SWP\Bundle\CoreBundle\Security\Authenticator;
6
7
use Symfony\Component\Security\Core\User\UserInterface;
8
use KnpU\OAuth2ClientBundle\Security\Authenticator\SocialAuthenticator;
9
use KnpU\OAuth2ClientBundle\Client\ClientRegistry;
10
use KnpU\OAuth2ClientBundle\Client\OAuth2Client;
11
use Symfony\Component\HttpFoundation\Request;
12
use Symfony\Component\HttpFoundation\Response;
13
use Symfony\Component\HttpFoundation\RedirectResponse;
14
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
15
use Symfony\Component\Security\Core\Exception\AuthenticationException;
16
use Symfony\Component\Security\Core\User\UserProviderInterface;
17
use FOS\UserBundle\Model\UserManagerInterface;
18
use Symfony\Component\Security\Core\Security;
19
use League\OAuth2\Client\Token\AccessToken;
20
use function uniqid;
21
22
class ExternalOauthAuthenticator extends SocialAuthenticator
23
{
24
    protected $clientRegistry;
25
26
    protected $um;
27
28
    protected $security;
29
30
    public function __construct(
31
        ClientRegistry $clientRegistry,
32
        UserManagerInterface $um,
33
        Security $security
34
    ) {
35
        $this->clientRegistry = $clientRegistry;
36
        $this->um = $um;
37
        $this->security = $security;
38
    }
39
40
    public function supports(Request $request): bool
41
    {
42
        if (!$this->security->getUser() && ($request->query->get('code') && $request->get('state'))) {
43
            return true;
44
        }
45
46
        return false;
47
    }
48
49
    public function getCredentials(Request $request): AccessToken
50
    {
51
        return $this->fetchAccessToken($this->getOauthClient());
52
    }
53
54
    /**
55
     * Get the user given the access token. If the user exists as a local user,
56
     * fetch that one, if it does not, create a new user using the OAuth user
57
     * fetched from the resource server using the access token.
58
     *
59
     * @inehritdoc
60
     */
61
    public function getUser($credentials, UserProviderInterface $userProvider): UserInterface
62
    {
63
        // Fetch the user from the resource server
64
        $oauthUser = $this->getOauthClient()->fetchUserFromToken($credentials);
65
        $oauthEmail = $oauthUser->getEmail();
66
        $oauthId = $oauthUser->getId();
67
68
        if (!$oauthUser) {
69
            return null;
70
        }
71
72
        // Is there an existing user with the same oauth id?
73
        /** @var \SWP\Bundle\CoreBundle\Model\UserInterface $user */
74
        $user = $userProvider->findOneByExternalId($oauthId);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Securi...r\UserProviderInterface as the method findOneByExternalId() does only exist in the following implementations of said interface: SWP\Bundle\CoreBundle\Se...y\Provider\UserProvider.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
75
        if ($user) {
76
            if ($user->getEmail() !== $oauthEmail) {
77
                // If the email has changed for the user, update it here as well
78
                $user->setEmail($oauthEmail);
79
                $user->setUsername($oauthEmail);
80
                $this->um->updateUser($user);
81
            }
82
83
            return $user;
84
        }
85
86
        // Is there an existing user with the same email address?
87
        $user = $userProvider->findOneByEmail($oauthEmail);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Securi...r\UserProviderInterface as the method findOneByEmail() does only exist in the following implementations of said interface: SWP\Bundle\CoreBundle\Se...y\Provider\UserProvider.

Let’s take a look at an example:

interface User
{
    /** @return string */
    public function getPassword();
}

class MyUser implements User
{
    public function getPassword()
    {
        // return something
    }

    public function getDisplayName()
    {
        // return some name.
    }
}

class AuthSystem
{
    public function authenticate(User $user)
    {
        $this->logger->info(sprintf('Authenticating %s.', $user->getDisplayName()));
        // do something.
    }
}

In the above example, the authenticate() method works fine as long as you just pass instances of MyUser. However, if you now also want to pass a different implementation of User which does not have a getDisplayName() method, the code will break.

Available Fixes

  1. Change the type-hint for the parameter:

    class AuthSystem
    {
        public function authenticate(MyUser $user) { /* ... */ }
    }
    
  2. Add an additional type-check:

    class AuthSystem
    {
        public function authenticate(User $user)
        {
            if ($user instanceof MyUser) {
                $this->logger->info(/** ... */);
            }
    
            // or alternatively
            if ( ! $user instanceof MyUser) {
                throw new \LogicException(
                    '$user must be an instance of MyUser, '
                   .'other instances are not supported.'
                );
            }
    
        }
    }
    
Note: PHP Analyzer uses reverse abstract interpretation to narrow down the types inside the if block in such a case.
  1. Add the method to the interface:

    interface User
    {
        /** @return string */
        public function getPassword();
    
        /** @return string */
        public function getDisplayName();
    }
    
Loading history...
88
        if ($user) {
89
            return $user;
90
        }
91
92
        // No user found, create one using the user info provided by resource server
93
        $user = $this->um->createUser();
94
        $user->setEmail($oauthEmail);
95
        $user->setUsername($oauthEmail);
96
        $user->setExternalId($oauthId);
97
        $user->setPassword(uniqid('', true));
98
        $user->setEnabled(true);
99
        $user->setSuperAdmin(false);
100
101
        $this->um->updateUser($user);
102
103
        return $user;
104
    }
105
106
    public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey): void
107
    {
108
    }
109
110
    public function onAuthenticationFailure(Request $request, AuthenticationException $exception): void
111
    {
112
    }
113
114
    public function start(Request $request, AuthenticationException $authException = null): RedirectResponse
115
    {
116
        return new RedirectResponse(
117
            '/connect/oauth/',
118
            Response::HTTP_TEMPORARY_REDIRECT
119
        );
120
    }
121
122
    private function getOauthClient(): OAuth2Client
123
    {
124
        return $this->clientRegistry->getClient('external_oauth');
125
    }
126
}
127