Completed
Push — develop ( 784583...9a84aa )
by Michiel
02:05 queued 10s
created

SamlProvider::authenticate()   C

Complexity

Conditions 10
Paths 26

Size

Total Lines 78

Duplication

Lines 0
Ratio 0 %

Importance

Changes 0
Metric Value
dl 0
loc 78
rs 6.6133
c 0
b 0
f 0
cc 10
nc 26
nop 1

How to fix   Long Method    Complexity   

Long Method

Small methods make your code easier to understand, in particular if combined with a good name. Besides, if your method is small, finding a good name is usually much easier.

For example, if you find yourself adding comments to a method's body, this is usually a good sign to extract the commented part to a new method, and use the comment as a starting point when coming up with a good name for this new method.

Commonly applied refactorings include:

1
<?php
2
3
/**
4
 * Copyright 2014 SURFnet bv
5
 *
6
 * Licensed under the Apache License, Version 2.0 (the "License");
7
 * you may not use this file except in compliance with the License.
8
 * You may obtain a copy of the License at
9
 *
10
 *     http://www.apache.org/licenses/LICENSE-2.0
11
 *
12
 * Unless required by applicable law or agreed to in writing, software
13
 * distributed under the License is distributed on an "AS IS" BASIS,
14
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15
 * See the License for the specific language governing permissions and
16
 * limitations under the License.
17
 */
18
19
namespace Surfnet\StepupRa\RaBundle\Security\Authentication\Provider;
20
21
use Psr\Log\LoggerInterface;
22
use Surfnet\SamlBundle\SAML2\Attribute\AttributeDictionary;
23
use Surfnet\SamlBundle\SAML2\Response\AssertionAdapter;
24
use Surfnet\StepupRa\RaBundle\Exception\InconsistentStateException;
25
use Surfnet\StepupRa\RaBundle\Exception\MissingRequiredAttributeException;
26
use Surfnet\StepupRa\RaBundle\Exception\UserNotRaException;
27
use Surfnet\StepupRa\RaBundle\Security\Authentication\Token\SamlToken;
28
use Surfnet\StepupRa\RaBundle\Service\IdentityService;
29
use Surfnet\StepupRa\RaBundle\Service\InstitutionConfigurationOptionsService;
30
use Surfnet\StepupRa\RaBundle\Service\RaListingService;
31
use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface;
32
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
33
use Symfony\Component\Security\Core\Exception\AuthenticationException;
34
use Symfony\Component\Security\Core\Exception\BadCredentialsException;
35
36
/**
37
 * @SuppressWarnings(PHPMD.CouplingBetweenObjects) - The SamlProvider needs to test several authorizations in order
38
 *  to determine the user may, or may not log in. This causes the coupling.
39
 */
40
class SamlProvider implements AuthenticationProviderInterface
41
{
42
    /**
43
     * @var \Surfnet\StepupRa\RaBundle\Service\IdentityService
44
     */
45
    private $identityService;
46
47
    /**
48
     * @var \Surfnet\SamlBundle\SAML2\Attribute\AttributeDictionary
49
     */
50
    private $attributeDictionary;
51
52
    /**
53
     * @var InstitutionConfigurationOptionsService
54
     */
55
    private $institutionConfigurationOptionsService;
56
57
    /**
58
     * @var RaListingService
59
     */
60
    private $raListingService;
61
62
    /**
63
     * @var LoggerInterface
64
     */
65
    private $logger;
66
67
    public function __construct(
68
        IdentityService $identityService,
69
        AttributeDictionary $attributeDictionary,
70
        InstitutionConfigurationOptionsService $institutionConfigurationOptionsService,
71
        RaListingService $raListingService,
72
        LoggerInterface $logger
73
    ) {
74
        $this->identityService                        = $identityService;
75
        $this->attributeDictionary                    = $attributeDictionary;
76
        $this->institutionConfigurationOptionsService = $institutionConfigurationOptionsService;
77
        $this->raListingService                       = $raListingService;
78
        $this->logger                                 = $logger;
79
    }
80
81
    /**
82
     * @SuppressWarnings(PHPMD.CyclomaticComplexity) - The authorization tests cause the complexity to raise, could and
83
     *  might be changed by introducing additional utility classes. Consider rebuilding this in the future.
84
     *
85
     * @param SamlToken|TokenInterface $token
86
     * @return TokenInterface|void
87
     */
88
    public function authenticate(TokenInterface $token)
89
    {
90
        $translatedAssertion = $this->attributeDictionary->translate($token->assertion);
0 ignored issues
show
Bug introduced by
Accessing assertion on the interface Symfony\Component\Securi...on\Token\TokenInterface suggest that you code against a concrete implementation. How about adding an instanceof check?

If you access a property on an interface, you most likely code against a concrete implementation of the interface.

Available Fixes

  1. Adding an additional type check:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeInterface $object) {
        if ($object instanceof SomeClass) {
            $a = $object->a;
        }
    }
    
  2. Changing the type hint:

    interface SomeInterface { }
    class SomeClass implements SomeInterface {
        public $a;
    }
    
    function someFunction(SomeClass $object) {
        $a = $object->a;
    }
    
Loading history...
91
92
        $nameId      = $translatedAssertion->getNameID();
93
        $institution = $this->getSingleStringValue('schacHomeOrganization', $translatedAssertion);
94
95
        $identity = $this->identityService->findByNameIdAndInstitution($nameId, $institution);
96
97
        // if no identity can be found, we're done.
98
        if ($identity === null) {
99
            throw new BadCredentialsException(
100
                'Unable to find Identity matching the criteria. Has the identity been registered before?'
101
            );
102
        }
103
104
        $raCredentials = $this->identityService->getRaCredentials($identity);
105
106
        // if no credentials can be found, we're done.
107
        if (!$raCredentials) {
108
            throw new UserNotRaException(
109
                'The Identity is not registered as (S)RA(A) and therefor does not have access to this application'
110
            );
111
        }
112
113
        // determine the role based on the credentials given
114
        $roles = [];
115
        if ($raCredentials->isSraa) {
116
            $roles[] = 'ROLE_SRAA';
117
        }
118
119
        if ($raCredentials->isRaa) {
120
            $roles[] = 'ROLE_RAA';
121
        } else {
122
            $roles[] = 'ROLE_RA';
123
        }
124
125
        $institutionConfigurationOptions = $this->institutionConfigurationOptionsService
126
            ->getInstitutionConfigurationOptionsFor($identity->institution);
127
128
        if ($institutionConfigurationOptions === null) {
129
            throw new InconsistentStateException(
130
                sprintf(
131
                    'InstitutionConfigurationOptions for institution "%s" '
132
                    . 'must exist but cannot be found after authenticating Identity "%s"',
133
                    $identity->institution,
134
                    $identity->id
135
                )
136
            );
137
        }
138
139
        // set the token
140
        $authenticatedToken = new SamlToken($token->getLoa(), $roles, $institutionConfigurationOptions);
0 ignored issues
show
Bug introduced by
It seems like you code against a concrete implementation and not the interface Symfony\Component\Securi...on\Token\TokenInterface as the method getLoa() does only exist in the following implementations of said interface: Surfnet\StepupRa\RaBundl...ication\Token\SamlToken.

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...
141
        $authenticatedToken->setUser($identity);
142
143
        // When dealing with a RA(A), determine for which institution the authenticating user should enter the RA environment
144
        if (!in_array('ROLE_SRAA', $roles)) {
145
            // Start by loading the ra listing for this identity to know what institutions (s)he is RA(A) for.
146
            $institutions = $this->raListingService->searchBy($identity->id, $institution);
147
148
            if ($institutions->getTotalItems() === 1) {
149
                // Change the institution to the first item from the institution listing results
150
                $institution = $institutions->getOnlyElement()->raInstitution;
151
            } elseif ($institutions->getTotalItems() > 1) {
152
                if ($institutions->isListed($identity->institution)) {
153
                    $institution = $identity->institution;
154
                } else {
155
                    // Otherwise pick the first institution in the list and set that for the
156
                    $institution = reset($institutions->getElements())->raInstitution;
0 ignored issues
show
Bug introduced by
$institutions->getElements() cannot be passed to reset() as the parameter $array expects a reference.
Loading history...
157
                }
158
            } else {
159
                throw new AuthenticationException('Unauthorized to authenticate, user is not present in ra listing');
160
            }
161
            $authenticatedToken->changeInstitutionScope($institution, $institutionConfigurationOptions);
162
        }
163
164
        return $authenticatedToken;
165
    }
166
167
    private function getSingleStringValue($attribute, AssertionAdapter $translatedAssertion)
168
    {
169
        $values = $translatedAssertion->getAttributeValue($attribute);
170
171
        if (empty($values)) {
172
            throw new MissingRequiredAttributeException(
173
                sprintf(
174
                    'Missing a required SAML attribute. This application requires the "%s" attribute to function.',
175
                    $attribute
176
                )
177
            );
178
        }
179
180
        // see https://www.pivotaltracker.com/story/show/121296389
181
        if (count($values) > 1) {
182
            $this->logger->warning(sprintf(
183
                'Found "%d" values for attribute "%s", using first value',
184
                count($values),
185
                $attribute
186
            ));
187
        }
188
189
        $value = reset($values);
190
191
        if (!is_string($value)) {
192
            $message = sprintf(
193
                'First value of attribute "%s" must be a string, "%s" given',
194
                $attribute,
195
                is_object($value) ? get_class($value) : gettype($value)
196
            );
197
198
            $this->logger->warning($message);
199
200
            throw new MissingRequiredAttributeException($message);
201
        }
202
203
        return $value;
204
    }
205
206
    public function supports(TokenInterface $token)
207
    {
208
        return $token instanceof SamlToken;
209
    }
210
}
211