CommandController::handleAction() - Code Metrics - Inspection of "Use configured institutions for institution lists" - OpenConext/Stepup-Middleware - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — bugfix/institution-listing ( a090c5...2ad763 )

unknown

created 2020-01-21 10:31 UTC

CommandController::handleAction() A

↳ Parent: CommandController

Complexity

Conditions	4
Paths	6

Size

Total Lines

Duplication

Lines	0
Ratio	0 %

Importance

Changes

Metric	Value
dl	0
loc	32
rs	9.408
c	0
b	0
f	0
cc	4
nc	6
nop	3

<?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\StepupMiddleware\ApiBundle\Controller;

use Psr\Log\LoggerInterface;
use Surfnet\Stepup\Identity\Value\IdentityId;
use Surfnet\Stepup\Identity\Value\Institution;
use Surfnet\StepupMiddleware\ApiBundle\Authorization\Service\CommandAuthorizationService;
use Surfnet\StepupMiddleware\ApiBundle\Identity\Service\WhitelistService;
use Surfnet\StepupMiddleware\CommandHandlingBundle\Command\Command;
use Surfnet\StepupMiddleware\CommandHandlingBundle\Command\Metadata;
use Surfnet\StepupMiddleware\CommandHandlingBundle\EventSourcing\MetadataEnricher;
use Surfnet\StepupMiddleware\CommandHandlingBundle\Exception\ForbiddenException;
use Surfnet\StepupMiddleware\CommandHandlingBundle\Identity\Command\CreateIdentityCommand;
use Surfnet\StepupMiddleware\CommandHandlingBundle\Identity\Command\UpdateIdentityCommand;
use Surfnet\StepupMiddleware\CommandHandlingBundle\Pipeline\TransactionAwarePipeline;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
use Symfony\Component\Security\Core\Authorization\AuthorizationChecker;

/**
 * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
 */
class CommandController extends Controller
{
    /**
     * @var WhitelistService
     */
    private $whitelistService;

    /**
     * @var TransactionAwarePipeline
     */
    private $pipeline;

    /**
     * @var MetadataEnricher
     */
    private $metadataEnricher;

    /**
     * @var AuthorizationChecker
     */
    private $authorizationChecker;

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

    /**
     * @var CommandAuthorizationService
     */
    private $commandAuthorizationService;


    public function __construct(
        TransactionAwarePipeline $pipeline,
        WhitelistService $whitelistService,
        MetadataEnricher $enricher,
        AuthorizationChecker $authorizationChecker,
        LoggerInterface $logger,
        CommandAuthorizationService $commandAuthorizationService
    ) {
        $this->pipeline = $pipeline;
        $this->whitelistService = $whitelistService;
        $this->authorizationChecker = $authorizationChecker;
        $this->metadataEnricher = $enricher;
        $this->logger = $logger;
        $this->commandAuthorizationService = $commandAuthorizationService;
    }

    public function handleAction(Command $command, Metadata $metadata, Request $request)
    {
        $this->denyAccessUnlessGranted(['ROLE_RA', 'ROLE_SS']);

        $this->logger->notice(sprintf('Received request to process Command "%s"', $command));

        $this->metadataEnricher->setMetadata($metadata);

        if ($this->authorizationChecker->isGranted('ROLE_MANAGEMENT')) {
            $this->logger->notice('Command sent through Management API, not enforcing Whitelist');
        } else {
            $this->logger->notice('Ensuring that the actor institution is on the whitelist, or the actor is SRAA');

            $this->handleAuthorization($command, $metadata);
        }

        try {
            $command = $this->pipeline->process($command);
        } catch (ForbiddenException $e) {
            throw new AccessDeniedHttpException(
                sprintf('Processing of command "%s" is forbidden for this client', $command),
                $e
            );
        }

        $serverName = $request->server->get('SERVER_NAME') ?: $request->server->get('SERVER_ADDR');
        $response = new JsonResponse(['command' => $command->UUID, 'processed_by' => $serverName]);
interface SomeInterface { }
class SomeClass implements SomeInterface {
    public $a;
}

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

        $this->logger->notice(sprintf('Command "%s" has been successfully processed', $command));

        return $response;
    }

    /**
     * @param Command $command
     * @param Metadata $metadata
     * @return Institution
     */
    private function resolveInstitution(Command $command, Metadata $metadata)
    {
        if ($metadata->actorInstitution) {
            return new Institution($metadata->actorInstitution);
        }

        // the createIdentityCommand is used to create an Identity for a new user,
        // the updateIdentityCommand is used to update name or email of an identity
        // Both are only sent by the SS when the Identity is not logged in yet,
        // thus there is not Metadata::actorInstitution,
        if ($command instanceof CreateIdentityCommand || $command instanceof UpdateIdentityCommand) {
            return new Institution($command->institution);
        }

        // conservative, if we cannot determine an institution, deny processing.
        throw new AccessDeniedHttpException(
            'Cannot reliably determine the institution of the actor, denying processing of command'
        );
    }

    /**
     * @param Command $command
     * @param Metadata $metadata
     */
    private function handleAuthorization(Command $command, Metadata $metadata)
    {
        // Get the actorId and actorInstitution from the metadata
        // Be aware that these values could be null when executing commands where we shouldn't log in for
        // - CreateIdentityCommand
        // - UpdateIdentityCommand
        $actorId = !is_null($metadata->actorId) ? new IdentityId($metadata->actorId) : null;
        $actorInstitution = !is_null($metadata->actorInstitution) ? new Institution($metadata->actorInstitution) : null;

        // The institution of an actor should be whitelisted or the actor should be SRAA
        // Be aware that the actor metadata is not always present, see self::resolveInstitution
        $this->logger->notice('Ensuring that the actor institution is on the whitelist, or the actor is SRAA');
        $institution = $this->resolveInstitution($command, $metadata);
        if(!$this->commandAuthorizationService->isInstitutionWhitelisted($institution, $actorId)){
            throw new AccessDeniedHttpException(sprintf(
                'Institution "%s" is not on the whitelist and actor "%s" is not an SRAA, processing of command denied',
                $institution,
                $metadata->actorId
            ));
        }

        $this->logger->notice('Ensuring that the actor is allowed to execute a command based on the fine grained authorization configuration');

        // Validate that if a command is an SelfServiceExecutable we may execute the command
        // This should be an SRAA or the actor itself
        // Be aware that for the CreateIdentityCommand and UpdateIdentityCommand the actorId is unknown because we aren't logged in yet
        if(!$this->commandAuthorizationService->maySelfserviceCommandBeExecutedOnBehalfOf(
            $command,
            $actorId
        )) {
            throw new AccessDeniedHttpException(sprintf(
                'The actor "%s" is not allowed to act on behalf of identity "%s" processing of SelfService command denied',
                new IdentityId($metadata->actorId),
                $command->getIdentityId()
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.
    }
}
            ));
        }

        // Validate that if a command is an RAExecutable we may execute the command
        // This should be an SRAA or an RAA which is configured to act on behalf of the institution
        if (!$this->commandAuthorizationService->mayRaCommandBeExecutedOnBehalfOf(
            $command,
            $actorId,
            $actorInstitution
        )) {
            throw new AccessDeniedHttpException(sprintf(
                'The actor "%s" is not allowed to act on behalf of institution  "%s" processing of RA command denied',
                new IdentityId($metadata->actorId),
                $institution
            ));
        }
    }
}


Push — bugfix/institution-listing ( a090c5...2ad763 )

CommandController::handleAction() A

Complexity

Size

Duplication

Importance

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\StepupMiddleware\ApiBundle\Controller;
20
21			use Psr\Log\LoggerInterface;
22			use Surfnet\Stepup\Identity\Value\IdentityId;
23			use Surfnet\Stepup\Identity\Value\Institution;
24			use Surfnet\StepupMiddleware\ApiBundle\Authorization\Service\CommandAuthorizationService;
25			use Surfnet\StepupMiddleware\ApiBundle\Identity\Service\WhitelistService;
26			use Surfnet\StepupMiddleware\CommandHandlingBundle\Command\Command;
27			use Surfnet\StepupMiddleware\CommandHandlingBundle\Command\Metadata;
28			use Surfnet\StepupMiddleware\CommandHandlingBundle\EventSourcing\MetadataEnricher;
29			use Surfnet\StepupMiddleware\CommandHandlingBundle\Exception\ForbiddenException;
30			use Surfnet\StepupMiddleware\CommandHandlingBundle\Identity\Command\CreateIdentityCommand;
31			use Surfnet\StepupMiddleware\CommandHandlingBundle\Identity\Command\UpdateIdentityCommand;
32			use Surfnet\StepupMiddleware\CommandHandlingBundle\Pipeline\TransactionAwarePipeline;
33			use Symfony\Bundle\FrameworkBundle\Controller\Controller;
34			use Symfony\Component\HttpFoundation\JsonResponse;
35			use Symfony\Component\HttpFoundation\Request;
36			use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException;
37			use Symfony\Component\Security\Core\Authorization\AuthorizationChecker;
38
39			/**
40			* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
41			*/
42			class CommandController extends Controller
43			{
44			/**
45			* @var WhitelistService
46			*/
47			private $whitelistService;
48
49			/**
50			* @var TransactionAwarePipeline
51			*/
52			private $pipeline;
53
54			/**
55			* @var MetadataEnricher
56			*/
57			private $metadataEnricher;
58
59			/**
60			* @var AuthorizationChecker
61			*/
62			private $authorizationChecker;
63
64			/**
65			* @var LoggerInterface
66			*/
67			private $logger;
68
69			/**
70			* @var CommandAuthorizationService
71			*/
72			private $commandAuthorizationService;
73
74
75			public function __construct(
76			TransactionAwarePipeline $pipeline,
77			WhitelistService $whitelistService,
78			MetadataEnricher $enricher,
79			AuthorizationChecker $authorizationChecker,
80			LoggerInterface $logger,
81			CommandAuthorizationService $commandAuthorizationService
82			) {
83			$this->pipeline = $pipeline;
84			$this->whitelistService = $whitelistService;
85			$this->authorizationChecker = $authorizationChecker;
86			$this->metadataEnricher = $enricher;
87			$this->logger = $logger;
88			$this->commandAuthorizationService = $commandAuthorizationService;
89			}
90
91			public function handleAction(Command $command, Metadata $metadata, Request $request)
92			{
93			$this->denyAccessUnlessGranted(['ROLE_RA', 'ROLE_SS']);
94
95			$this->logger->notice(sprintf('Received request to process Command "%s"', $command));
96
97			$this->metadataEnricher->setMetadata($metadata);
98
99			if ($this->authorizationChecker->isGranted('ROLE_MANAGEMENT')) {
100			$this->logger->notice('Command sent through Management API, not enforcing Whitelist');
101			} else {
102			$this->logger->notice('Ensuring that the actor institution is on the whitelist, or the actor is SRAA');
103
104			$this->handleAuthorization($command, $metadata);
105			}
106
107			try {
108			$command = $this->pipeline->process($command);
109			} catch (ForbiddenException $e) {
110			throw new AccessDeniedHttpException(
111			sprintf('Processing of command "%s" is forbidden for this client', $command),
112			$e
113			);
114			}
115
116			$serverName = $request->server->get('SERVER_NAME') ?: $request->server->get('SERVER_ADDR');
117			$response = new JsonResponse(['command' => $command->UUID, 'processed_by' => $serverName]);
			0 ignored issues – show Bug introduced 2015-12-15 13:40 UTC by Report Bug Copy Issue Report Accessing `UUID` on the interface `Surfnet\StepupMiddleware...gBundle\Command\Command` 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...
118
119			$this->logger->notice(sprintf('Command "%s" has been successfully processed', $command));
120
121			return $response;
122			}
123
124			/**
125			* @param Command $command
126			* @param Metadata $metadata
127			* @return Institution
128			*/
129			private function resolveInstitution(Command $command, Metadata $metadata)
130			{
131			if ($metadata->actorInstitution) {
132			return new Institution($metadata->actorInstitution);
133			}
134
135			// the createIdentityCommand is used to create an Identity for a new user,
136			// the updateIdentityCommand is used to update name or email of an identity
137			// Both are only sent by the SS when the Identity is not logged in yet,
138			// thus there is not Metadata::actorInstitution,
139			if ($command instanceof CreateIdentityCommand \|\| $command instanceof UpdateIdentityCommand) {
140			return new Institution($command->institution);
141			}
142
143			// conservative, if we cannot determine an institution, deny processing.
144			throw new AccessDeniedHttpException(
145			'Cannot reliably determine the institution of the actor, denying processing of command'
146			);
147			}
148
149			/**
150			* @param Command $command
151			* @param Metadata $metadata
152			*/
153			private function handleAuthorization(Command $command, Metadata $metadata)
154			{
155			// Get the actorId and actorInstitution from the metadata
156			// Be aware that these values could be null when executing commands where we shouldn't log in for
157			// - CreateIdentityCommand
158			// - UpdateIdentityCommand
159			$actorId = !is_null($metadata->actorId) ? new IdentityId($metadata->actorId) : null;
160			$actorInstitution = !is_null($metadata->actorInstitution) ? new Institution($metadata->actorInstitution) : null;
161
162			// The institution of an actor should be whitelisted or the actor should be SRAA
163			// Be aware that the actor metadata is not always present, see self::resolveInstitution
164			$this->logger->notice('Ensuring that the actor institution is on the whitelist, or the actor is SRAA');
165			$institution = $this->resolveInstitution($command, $metadata);
166			if(!$this->commandAuthorizationService->isInstitutionWhitelisted($institution, $actorId)){
167			throw new AccessDeniedHttpException(sprintf(
168			'Institution "%s" is not on the whitelist and actor "%s" is not an SRAA, processing of command denied',
169			$institution,
170			$metadata->actorId
171			));
172			}
173
174			$this->logger->notice('Ensuring that the actor is allowed to execute a command based on the fine grained authorization configuration');
175
176			// Validate that if a command is an SelfServiceExecutable we may execute the command
177			// This should be an SRAA or the actor itself
178			// Be aware that for the CreateIdentityCommand and UpdateIdentityCommand the actorId is unknown because we aren't logged in yet
179			if(!$this->commandAuthorizationService->maySelfserviceCommandBeExecutedOnBehalfOf(
180			$command,
181			$actorId
182			)) {
183			throw new AccessDeniedHttpException(sprintf(
184			'The actor "%s" is not allowed to act on behalf of identity "%s" processing of SelfService command denied',
185			new IdentityId($metadata->actorId),
186			$command->getIdentityId()
			0 ignored issues – show Bug introduced 2020-01-21 10:14 UTC by Report Bug Copy Issue Report It seems like you code against a concrete implementation and not the interface `Surfnet\StepupMiddleware...gBundle\Command\Command` as the method `getIdentityId()` does only exist in the following implementations of said interface: `Surfnet\StepupMiddleware...d\CreateIdentityCommand`, `Surfnet\StepupMiddleware...LocalePreferenceCommand`, `Surfnet\StepupMiddleware...veGssfPossessionCommand`, `Surfnet\StepupMiddleware...ePhonePossessionCommand`, `Surfnet\StepupMiddleware...DevicePossessionCommand`, `Surfnet\StepupMiddleware...ubikeyPossessionCommand`, `Surfnet\StepupMiddleware...eOwnSecondFactorCommand`, `Surfnet\StepupMiddleware...d\UpdateIdentityCommand`, `Surfnet\StepupMiddleware...mand\VerifyEmailCommand`. 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...
187			));
188			}
189
190			// Validate that if a command is an RAExecutable we may execute the command
191			// This should be an SRAA or an RAA which is configured to act on behalf of the institution
192			if (!$this->commandAuthorizationService->mayRaCommandBeExecutedOnBehalfOf(
193			$command,
194			$actorId,
195			$actorInstitution
196			)) {
197			throw new AccessDeniedHttpException(sprintf(
198			'The actor "%s" is not allowed to act on behalf of institution "%s" processing of RA command denied',
199			new IdentityId($metadata->actorId),
200			$institution
201			));
202			}
203			}
204			}
205

OpenConext / Stepup-Middleware

Push — bugfix/institution-listing ( a090c5...2ad763 )

CommandController::handleAction() A

Complexity

Size

Duplication

Importance

Available Fixes

Available Fixes

Duplication Side-by-Side

Filter issues like