SamlProvider::authenticate() - Code Metrics - Inspection of "Allow all RA roles to change the institution scope" - OpenConext/Stepup-RA - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — feature/fine-grained-authoriza... (#182)

by Michiel

created 2018-11-01 15:21 UTC

SamlProvider::authenticate() C

↳ Parent: SamlProvider

Complexity

Conditions	10
Paths	26

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	77
rs	6.6351
c	0
b	0
f	0
cc	10
nc	26
nop	1

How to fix Long Method Complexity

<?php

/**
 * Copyright 2014 SURFnet bv
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

namespace Surfnet\StepupRa\RaBundle\Security\Authentication\Provider;

use Psr\Log\LoggerInterface;
use Surfnet\SamlBundle\SAML2\Attribute\AttributeDictionary;
use Surfnet\SamlBundle\SAML2\Response\AssertionAdapter;
use Surfnet\StepupRa\RaBundle\Exception\InconsistentStateException;
use Surfnet\StepupRa\RaBundle\Exception\MissingRequiredAttributeException;
use Surfnet\StepupRa\RaBundle\Exception\UserNotRaException;
use Surfnet\StepupRa\RaBundle\Security\Authentication\Token\SamlToken;
use Surfnet\StepupRa\RaBundle\Service\IdentityService;
use Surfnet\StepupRa\RaBundle\Service\InstitutionConfigurationOptionsService;
use Surfnet\StepupRa\RaBundle\Service\RaListingService;
use Symfony\Component\Security\Core\Authentication\Provider\AuthenticationProviderInterface;
use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
use Symfony\Component\Security\Core\Exception\AuthenticationException;
use Symfony\Component\Security\Core\Exception\BadCredentialsException;

/**
 * @SuppressWarnings(PHPMD.CouplingBetweenObjects) - The SamlProvider needs to test several authorizations in order
 *  to determine the user may, or may not log in. This causes the coupling.
 */
class SamlProvider implements AuthenticationProviderInterface
{
    /**
     * @var \Surfnet\StepupRa\RaBundle\Service\IdentityService
     */
    private $identityService;

    /**
     * @var \Surfnet\SamlBundle\SAML2\Attribute\AttributeDictionary
     */
    private $attributeDictionary;

    /**
     * @var InstitutionConfigurationOptionsService
     */
    private $institutionConfigurationOptionsService;

    /**
     * @var RaListingService
     */
    private $raListingService;

    /**
     * @var LoggerInterface
     */
    private $logger;

    public function __construct(
        IdentityService $identityService,
        AttributeDictionary $attributeDictionary,
        InstitutionConfigurationOptionsService $institutionConfigurationOptionsService,
        RaListingService $raListingService,
        LoggerInterface $logger
    ) {
        $this->identityService                        = $identityService;
        $this->attributeDictionary                    = $attributeDictionary;
        $this->institutionConfigurationOptionsService = $institutionConfigurationOptionsService;
        $this->raListingService                       = $raListingService;
        $this->logger                                 = $logger;
    }

    /**
     * @SuppressWarnings(PHPMD.CyclomaticComplexity) - The authorization tests cause the complexity to raise, could and
     *  might be changed by introducing additional utility classes. Consider rebuilding this in the future.
     *
     * @param SamlToken|TokenInterface $token
     * @return TokenInterface|void
     */
    public function authenticate(TokenInterface $token)
    {
        $translatedAssertion = $this->attributeDictionary->translate($token->assertion);
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

function someFunction(SomeInterface $object) {
    if ($object instanceof SomeClass) {
        $a = $object->a;
    }
}

        $nameId      = $translatedAssertion->getNameID();
        $institution = $this->getSingleStringValue('schacHomeOrganization', $translatedAssertion);

        $identity = $this->identityService->findByNameIdAndInstitution($nameId, $institution);

        // if no identity can be found, we're done.
        if ($identity === null) {
            throw new BadCredentialsException(
                'Unable to find Identity matching the criteria. Has the identity been registered before?'
            );
        }

        $raCredentials = $this->identityService->getRaCredentials($identity);

        // if no credentials can be found, we're done.
        if (!$raCredentials) {
            throw new UserNotRaException(
                'The Identity is not registered as (S)RA(A) and therefor does not have access to this application'
            );
        }

        // determine the role based on the credentials given
        $roles = [];
        if ($raCredentials->isSraa) {
            $roles[] = 'ROLE_SRAA';
        }

        if ($raCredentials->isRaa) {
            $roles[] = 'ROLE_RAA';
        } else {
            $roles[] = 'ROLE_RA';
        }

        $institutionConfigurationOptions = $this->institutionConfigurationOptionsService
            ->getInstitutionConfigurationOptionsFor($identity->institution);

        if ($institutionConfigurationOptions === null) {
            throw new InconsistentStateException(
                sprintf(
                    'InstitutionConfigurationOptions for institution "%s" '
                    . 'must exist but cannot be found after authenticating Identity "%s"',
                    $identity->institution,
                    $identity->id
                )
            );
        }

        // set the token
        $authenticatedToken = new SamlToken($token->getLoa(), $roles, $institutionConfigurationOptions);
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.
    }
}
        $authenticatedToken->setUser($identity);

        // When dealing with a RA(A), determine for which institution the authenticating user should enter the RA environment
        if (!in_array('ROLE_SRAA', $roles)) {
            // Start by loading the ra listing for this identity to know what institutions (s)he is RA(A) for.
            $institutions = $this->raListingService->searchBy($identity->id, $institution);
            if ($institutions->getTotalItems() === 1) {
                // Change the institution to the first item from the institution listing results
                $institution = $institutions->getOnlyElement()->institution;
            } elseif ($institutions->getTotalItems() > 1) {
                if ($institutions->isListed($identity->institution)) {
                    $institution = $identity->institution;
                } else {
                    // Otherwise pick the first institution in the list and set that for the
                    $institution = reset($institutions->getElements())->institution;

                }
            } else {
                throw new AuthenticationException('Unauthorized to authenticate, user is not present in ra listing');
            }
            $authenticatedToken->changeInstitutionScope($institution, $institutionConfigurationOptions);
        }

        return $authenticatedToken;
    }

    private function getSingleStringValue($attribute, AssertionAdapter $translatedAssertion)
    {
        $values = $translatedAssertion->getAttributeValue($attribute);

        if (empty($values)) {
            throw new MissingRequiredAttributeException(
                sprintf(
                    'Missing a required SAML attribute. This application requires the "%s" attribute to function.',
                    $attribute
                )
            );
        }

        // see https://www.pivotaltracker.com/story/show/121296389
        if (count($values) > 1) {
            $this->logger->warning(sprintf(
                'Found "%d" values for attribute "%s", using first value',
                count($values),
                $attribute
            ));
        }

        $value = reset($values);

        if (!is_string($value)) {
            $message = sprintf(
                'First value of attribute "%s" must be a string, "%s" given',
                $attribute,
                is_object($value) ? get_class($value) : gettype($value)
            );

            $this->logger->warning($message);

            throw new MissingRequiredAttributeException($message);
        }

        return $value;
    }

    public function supports(TokenInterface $token)
    {
        return $token instanceof SamlToken;
    }
}


Pull Request — feature/fine-grained-authoriza... (#182)

SamlProvider::authenticate() C

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Available Fixes

Available Fixes

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 2015-12-15 13:46 UTC by Report Bug Copy Issue Report 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 Adding an additional type check: interface SomeInterface { } class SomeClass implements SomeInterface { public $a; } function someFunction(SomeInterface $object) { if ($object instanceof SomeClass) { $a = $object->a; } } 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 2016-08-02 13:40 UTC by Report Bug Copy Issue Report 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 Change the type-hint for the parameter: class AuthSystem { public function authenticate(MyUser $user) { / ... / } } 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. 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			if ($institutions->getTotalItems() === 1) {
148			// Change the institution to the first item from the institution listing results
149			$institution = $institutions->getOnlyElement()->institution;
150			} elseif ($institutions->getTotalItems() > 1) {
151			if ($institutions->isListed($identity->institution)) {
152			$institution = $identity->institution;
153			} else {
154			// Otherwise pick the first institution in the list and set that for the
155			$institution = reset($institutions->getElements())->institution;
			0 ignored issues – show Bug introduced 2018-11-01 15:23 UTC by Report Bug Copy Issue Report `$institutions->getElements()` cannot be passed to `reset()` as the parameter `$array` expects a reference. Loading history...
156			}
157			} else {
158			throw new AuthenticationException('Unauthorized to authenticate, user is not present in ra listing');
159			}
160			$authenticatedToken->changeInstitutionScope($institution, $institutionConfigurationOptions);
161			}
162
163			return $authenticatedToken;
164			}
165
166			private function getSingleStringValue($attribute, AssertionAdapter $translatedAssertion)
167			{
168			$values = $translatedAssertion->getAttributeValue($attribute);
169
170			if (empty($values)) {
171			throw new MissingRequiredAttributeException(
172			sprintf(
173			'Missing a required SAML attribute. This application requires the "%s" attribute to function.',
174			$attribute
175			)
176			);
177			}
178
179			// see https://www.pivotaltracker.com/story/show/121296389
180			if (count($values) > 1) {
181			$this->logger->warning(sprintf(
182			'Found "%d" values for attribute "%s", using first value',
183			count($values),
184			$attribute
185			));
186			}
187
188			$value = reset($values);
189
190			if (!is_string($value)) {
191			$message = sprintf(
192			'First value of attribute "%s" must be a string, "%s" given',
193			$attribute,
194			is_object($value) ? get_class($value) : gettype($value)
195			);
196
197			$this->logger->warning($message);
198
199			throw new MissingRequiredAttributeException($message);
200			}
201
202			return $value;
203			}
204
205			public function supports(TokenInterface $token)
206			{
207			return $token instanceof SamlToken;
208			}
209			}
210

OpenConext / Stepup-RA

Pull Request — feature/fine-grained-authoriza... (#182)

SamlProvider::authenticate() C

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like