Completed
Push — master ( 00f21d...76f138 )
by Boy
19:52 queued 16:05
created

Configuration   A

Complexity

Total Complexity 6

Size/Duplication

Total Lines 148
Duplicated Lines 0 %

Coupling/Cohesion

Components 0
Dependencies 3

Importance

Changes 0
Metric Value
wmc 6
c 0
b 0
f 0
lcom 0
cbo 3
dl 0
loc 148
rs 10

3 Methods

Rating   Name   Duplication   Size   Complexity  
A getConfigTreeBuilder() 0 19 1
B addRoutesSection() 0 36 4
B addProvidersSection() 0 82 1
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\StepupGateway\SamlStepupProviderBundle\DependencyInjection;
20
21
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
22
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
23
use Symfony\Component\Config\Definition\ConfigurationInterface;
24
25
class Configuration implements ConfigurationInterface
26
{
27
    public function getConfigTreeBuilder()
28
    {
29
        $treeBuilder = new TreeBuilder();
30
        $rootNode = $treeBuilder->root('surfnet_stepup_gateway_saml_stepup_provider');
31
32
        $rootNode
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Symfony\Component\Config...\Builder\NodeDefinition as the method requiresAtLeastOneElement() does only exist in the following sub-classes of Symfony\Component\Config...\Builder\NodeDefinition: Symfony\Component\Config...der\ArrayNodeDefinition. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

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

class MyUser extends 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 sub-classes 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 parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
33
            ->children()
34
            ->arrayNode('allowed_sps')
35
                ->isRequired()
36
                ->requiresAtLeastOneElement()
37
                ->prototype('scalar')
38
                ->end()
39
            ->end();
40
41
        $this->addRoutesSection($rootNode);
42
        $this->addProvidersSection($rootNode);
43
44
        return $treeBuilder;
45
    }
46
47
    /**
48
     * @param ArrayNodeDefinition $rootNode
49
     */
50
    private function addRoutesSection(ArrayNodeDefinition $rootNode)
51
    {
52
        $rootNode
53
            ->children()
54
            ->arrayNode('routes')
55
                ->children()
56
                    ->scalarNode('sso')
57
                        ->isRequired()
58
                        ->validate()
59
                            ->ifTrue(function ($v) {
60
                                return !is_string($v) || strlen($v) === 0;
61
                            })
62
                            ->thenInvalid('SSO route must be a non-empty string')
63
                        ->end()
64
                    ->end()
65
                    ->scalarNode('consume_assertion')
66
                        ->isRequired()
67
                        ->validate()
68
                            ->ifTrue(function ($v) {
69
                                return !is_string($v) || strlen($v) === 0;
70
                            })
71
                            ->thenInvalid('Consume assertion route must be a non-empty string')
72
                        ->end()
73
                    ->end()
74
                    ->scalarNode('metadata')
75
                        ->isRequired()
76
                        ->validate()
77
                            ->ifTrue(function ($v) {
78
                                return !is_string($v) || strlen($v) === 0;
79
                            })
80
                            ->thenInvalid('Metadata route must be a non-empty string')
81
                        ->end()
82
                    ->end()
83
                ->end()
84
            ->end();
85
    }
86
87
    /**
88
     * @param ArrayNodeDefinition $rootNode
89
     */
90
    private function addProvidersSection(ArrayNodeDefinition $rootNode)
91
    {
92
        /** @var ArrayNodeDefinition $protoType */
93
        $protoType = $rootNode
0 ignored issues
show
Bug introduced by
It seems like you code against a specific sub-type and not the parent class Symfony\Component\Config...\Builder\NodeDefinition as the method requiresAtLeastOneElement() does only exist in the following sub-classes of Symfony\Component\Config...\Builder\NodeDefinition: Symfony\Component\Config...der\ArrayNodeDefinition. Maybe you want to instanceof check for one of these explicitly?

Let’s take a look at an example:

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

class MyUser extends 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 sub-classes 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 parent class:

    abstract class User
    {
        /** @return string */
        abstract public function getPassword();
    
        /** @return string */
        abstract public function getDisplayName();
    }
    
Loading history...
94
            ->children()
95
            ->arrayNode('providers')
96
                ->isRequired()
97
                ->requiresAtLeastOneElement()
98
                ->useAttributeAsKey('type')
99
                ->prototype('array');
100
101
        $protoType
102
            ->canBeDisabled()
103
            ->children()
104
                ->arrayNode('hosted')
105
                    ->children()
106
                        ->arrayNode('service_provider')
107
                            ->children()
108
                                ->scalarNode('public_key')
109
                                    ->isRequired()
110
                                    ->info('The absolute path to the public key used to sign AuthnRequests')
111
                                ->end()
112
                                ->scalarNode('private_key')
113
                                    ->isRequired()
114
                                    ->info('The absolute path to the private key used to sign AuthnRequests')
115
                                ->end()
116
                            ->end()
117
                        ->end()
118
                        ->arrayNode('identity_provider')
119
                            ->children()
120
                                ->scalarNode('service_provider_repository')
121
                                    ->isRequired()
122
                                    ->info(
123
                                        'Name of the service that is the Entity Repository. Must implement the '
124
                                        . ' Surfnet\SamlBundle\Entity\ServiceProviderRepository interface.'
125
                                    )
126
                                ->end()
127
                                ->scalarNode('public_key')
128
                                    ->isRequired()
129
                                    ->info('The absolute path to the public key used to sign Responses to AuthRequests with')
0 ignored issues
show
Coding Style introduced by
This line exceeds maximum limit of 120 characters; contains 125 characters

Overly long lines are hard to read on any screen. Most code styles therefor impose a maximum limit on the number of characters in a line.

Loading history...
130
                                ->end()
131
                                ->scalarNode('private_key')
132
                                    ->isRequired()
133
                                    ->info('The absolute path to the private key used to sign Responses to AuthRequests with')
0 ignored issues
show
Coding Style introduced by
This line exceeds maximum limit of 120 characters; contains 126 characters

Overly long lines are hard to read on any screen. Most code styles therefor impose a maximum limit on the number of characters in a line.

Loading history...
134
                                ->end()
135
                            ->end()
136
                        ->end()
137
                        ->arrayNode('metadata')
138
                            ->children()
139
                                ->scalarNode('public_key')
140
                                    ->isRequired()
141
                                    ->info('The absolute path to the public key used to sign the metadata')
142
                                ->end()
143
                                ->scalarNode('private_key')
144
                                    ->isRequired()
145
                                    ->info('The absolute path to the private key used to sign the metadata')
146
                                ->end()
147
                            ->end()
148
                        ->end()
149
                    ->end()
150
                ->end()
151
                ->arrayNode('remote')
152
                    ->children()
153
                        ->scalarNode('entity_id')
154
                            ->isRequired()
155
                            ->info('The EntityID of the remote identity provider')
156
                        ->end()
157
                        ->scalarNode('sso_url')
158
                            ->isRequired()
159
                            ->info('The name of the route to generate the SSO URL')
160
                        ->end()
161
                        ->scalarNode('certificate')
162
                            ->isRequired()
163
                            ->info(
164
                                'The contents of the certificate used to sign the AuthnResponse with, if different from'
165
                                . ' the public key configured below'
166
                            )
167
                        ->end()
168
                    ->end()
169
                ->end()
170
            ->end();
171
    }
172
}
173