Completed
Pull Request — master (#145)
by Tobias
16:26 queued 06:19
created

Configuration::validateAuthenticationType()   A

Complexity

Conditions 2
Paths 2

Size

Total Lines 18
Code Lines 12

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 5
CRAP Score 2

Importance

Changes 0
Metric Value
dl 0
loc 18
ccs 5
cts 5
cp 1
rs 9.4285
c 0
b 0
f 0
cc 2
eloc 12
nc 2
nop 3
crap 2
1
<?php
2
3
namespace Http\HttplugBundle\DependencyInjection;
4
5
use Symfony\Component\Config\Definition\Builder\ArrayNodeDefinition;
6
use Symfony\Component\Config\Definition\Builder\NodeDefinition;
7
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
8
use Symfony\Component\Config\Definition\ConfigurationInterface;
9
use Symfony\Component\Config\Definition\Exception\InvalidConfigurationException;
10
use Symfony\Component\Config\Definition\Exception\InvalidTypeException;
11
12
/**
13
 * This class contains the configuration information for the bundle.
14
 *
15
 * This information is solely responsible for how the different configuration
16
 * sections are normalized, and merged.
17
 *
18
 * @author David Buchmann <[email protected]>
19
 * @author Tobias Nyholm <[email protected]>
20
 */
21
class Configuration implements ConfigurationInterface
22
{
23
    /**
24
     * Whether to use the debug mode.
25
     *
26
     * @see https://github.com/doctrine/DoctrineBundle/blob/v1.5.2/DependencyInjection/Configuration.php#L31-L41
27
     *
28
     * @var bool
29
     */
30
    private $debug;
31
32
    /**
33
     * @param bool $debug
34 16
     */
35
    public function __construct($debug)
36 16
    {
37 16
        $this->debug = (bool) $debug;
38
    }
39
40
    /**
41
     * {@inheritdoc}
42 16
     */
43
    public function getConfigTreeBuilder()
44 16
    {
45 16
        $treeBuilder = new TreeBuilder();
46
        $rootNode = $treeBuilder->root('httplug');
47 16
48 16
        $this->configureClients($rootNode);
49
        $this->configureSharedPlugins($rootNode);
50
51 16
        $rootNode
1 ignored issue
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 fixXmlConfig() 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...
52
            ->validate()
53 13
                ->ifTrue(function ($v) {
54 13
                    return !empty($v['classes']['client'])
55 10
                        || !empty($v['classes']['message_factory'])
56 13
                        || !empty($v['classes']['uri_factory'])
57 16
                        || !empty($v['classes']['stream_factory']);
58
                })
59 3
                ->then(function ($v) {
60 3
                    foreach ($v['classes'] as $key => $class) {
61 1
                        if (null !== $class && !class_exists($class)) {
62 1
                            throw new InvalidConfigurationException(sprintf(
63 1
                                'Class %s specified for httplug.classes.%s does not exist.',
64
                                $class,
65 1
                                $key
66
                            ));
67 2
                        }
68
                    }
69 2
70 16
                    return $v;
71 16
                })
72 16
            ->end()
73
            ->beforeNormalization()
74 16
                ->ifTrue(function ($v) {
75 16
                    return is_array($v) && array_key_exists('toolbar', $v) && is_array($v['toolbar']);
76
                })
77 4
                ->then(function ($v) {
78 1
                    if (array_key_exists('profiling', $v)) {
79
                        throw new InvalidConfigurationException('Can\'t configure both "toolbar" and "profiling" section. The "toolbar" config is deprecated as of version 1.3.0, please only use "profiling".');
80
                    }
81 3
82
                    @trigger_error('"httplug.toolbar" config is deprecated since version 1.3 and will be removed in 2.0. Use "httplug.profiling" instead.', E_USER_DEPRECATED);
0 ignored issues
show
Security Best Practice introduced by
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.

If you suppress an error, we recommend checking for the error condition explicitly:

// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
Loading history...
83 3
84 1
                    if (array_key_exists('enabled', $v['toolbar']) && 'auto' === $v['toolbar']['enabled']) {
85 1
                        @trigger_error('"auto" value in "httplug.toolbar" config is deprecated since version 1.3 and will be removed in 2.0. Use a boolean value instead.', E_USER_DEPRECATED);
1 ignored issue
show
Security Best Practice introduced by
It seems like you do not handle an error condition here. This can introduce security issues, and is generally not recommended.

If you suppress an error, we recommend checking for the error condition explicitly:

// For example instead of
@mkdir($dir);

// Better use
if (@mkdir($dir) === false) {
    throw new \RuntimeException('The directory '.$dir.' could not be created.');
}
Loading history...
86 1
                        $v['toolbar']['enabled'] = $this->debug;
87
                    }
88 3
89
                    $v['profiling'] = $v['toolbar'];
90 3
91
                    unset($v['toolbar']);
92 3
93 16
                    return $v;
94 16
                })
95 16
            ->end()
96 16
            ->fixXmlConfig('client')
97 16
            ->children()
98 16
                ->arrayNode('main_alias')
99 16
                    ->addDefaultsIfNotSet()
100 16
                    ->info('Configure which service the main alias point to.')
101 16
                    ->children()
102 16
                        ->scalarNode('client')->defaultValue('httplug.client.default')->end()
103 16
                        ->scalarNode('message_factory')->defaultValue('httplug.message_factory.default')->end()
104 16
                        ->scalarNode('uri_factory')->defaultValue('httplug.uri_factory.default')->end()
105 16
                        ->scalarNode('stream_factory')->defaultValue('httplug.stream_factory.default')->end()
106 16
                    ->end()
107 16
                ->end()
108 16
                ->arrayNode('classes')
109 16
                    ->addDefaultsIfNotSet()
110 16
                    ->info('Overwrite a service class instead of using the discovery mechanism.')
111 16
                    ->children()
112 16
                        ->scalarNode('client')->defaultNull()->end()
113 16
                        ->scalarNode('message_factory')->defaultNull()->end()
114 16
                        ->scalarNode('uri_factory')->defaultNull()->end()
115 16
                        ->scalarNode('stream_factory')->defaultNull()->end()
116 16
                    ->end()
117 16
                ->end()
118 16
                ->arrayNode('profiling')
119 16
                    ->addDefaultsIfNotSet()
120 16
                    ->treatFalseLike(['enabled' => false])
121 16
                    ->treatTrueLike(['enabled' => true])
122 16
                    ->treatNullLike(['enabled' => $this->debug])
123 16
                    ->info('Extend the debug profiler with information about requests.')
124 16
                    ->children()
125 16
                        ->booleanNode('enabled')
126 16
                            ->info('Turn the toolbar on or off. Defaults to kernel debug mode.')
127 16
                            ->defaultValue($this->debug)
128 16
                        ->end()
129 16
                        ->scalarNode('formatter')->defaultNull()->end()
130 16
                        ->integerNode('captured_body_length')
131 16
                            ->defaultValue(0)
132 16
                            ->info('Limit long HTTP message bodies to x characters. If set to 0 we do not read the message body. Only available with the default formatter (FullHttpMessageFormatter).')
133 16
                        ->end()
134 16
                    ->end()
135 16
                ->end()
136 16
                ->arrayNode('discovery')
137 16
                    ->addDefaultsIfNotSet()
138 16
                    ->info('Control what clients should be found by the discovery.')
139 16
                    ->children()
140 16
                        ->scalarNode('client')
141 16
                            ->defaultValue('auto')
142 16
                            ->info('Set to "auto" to see auto discovered client in the web profiler. If provided a service id for a client then this client will be found by auto discovery.')
143 16
                        ->end()
144 16
                        ->scalarNode('async_client')
145 16
                            ->defaultNull()
146 16
                            ->info('Set to "auto" to see auto discovered client in the web profiler. If provided a service id for a client then this client will be found by auto discovery.')
147 16
                        ->end()
148 16
                    ->end()
149 16
                ->end()
150
            ->end();
151 16
152
        return $treeBuilder;
153
    }
154 16
155
    private function configureClients(ArrayNodeDefinition $root)
156 16
    {
157 16
        $root->children()
1 ignored issue
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 fixXmlConfig() 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...
158 16
            ->arrayNode('clients')
159 16
                ->useAttributeAsKey('name')
160 16
                ->prototype('array')
161 16
                ->fixXmlConfig('plugin')
162
                ->validate()
163
                    ->ifTrue(function ($config) {
164 7
                        // Make sure we only allow one of these to be true
165 16
                        return (bool) $config['flexible_client'] + (bool) $config['http_methods_client'] + (bool) $config['batch_client'] >= 2;
166 16
                    })
167 16
                    ->thenInvalid('A http client can\'t be decorated with several of FlexibleHttpClient, HttpMethodsClient and BatchClient. Only one of the following options can be true. ("flexible_client", "http_methods_client", "batch_client")')
168 16
                ->end()
169 16
                ->children()
170 16
                    ->scalarNode('factory')
171 16
                        ->isRequired()
172 16
                        ->cannotBeEmpty()
173 16
                        ->info('The service id of a factory to use when creating the adapter.')
174 16
                    ->end()
175 16
                    ->booleanNode('flexible_client')
176 16
                        ->defaultFalse()
177 16
                        ->info('Set to true to get the client wrapped in a FlexibleHttpClient which emulates async or sync behavior.')
178 16
                    ->end()
179 16
                    ->booleanNode('http_methods_client')
180 16
                        ->defaultFalse()
181 16
                        ->info('Set to true to get the client wrapped in a HttpMethodsClient which emulates provides functions for HTTP verbs.')
182 16
                    ->end()
183 16
                    ->booleanNode('batch_client')
184 16
                        ->defaultFalse()
185 16
                        ->info('Set to true to get the client wrapped in a BatchClient which allows you to send multiple request at the same time.')
186 16
                    ->end()
187 16
                    ->variableNode('config')->defaultValue([])->end()
188 16
                    ->append($this->createClientPluginNode())
189 16
                ->end()
190 16
            ->end()
191 16
        ->end();
192
    }
193
194
    /**
195
     * @param ArrayNodeDefinition $root
196 16
     */
197
    private function configureSharedPlugins(ArrayNodeDefinition $root)
198
    {
199 16
        $pluginsNode = $root
200 16
            ->children()
201 16
                ->arrayNode('plugins')
202 16
                ->info('Global plugin configuration. Plugins need to be explicitly added to clients.')
203
                ->addDefaultsIfNotSet()
204 16
            // don't call end to get the plugins node
205 16
        ;
206 16
        $this->addSharedPluginNodes($pluginsNode);
207
    }
208
209
    /**
210
     * Createplugins node of a client.
211
     *
212
     * @return ArrayNodeDefinition The plugin node
213 16
     */
214
    private function createClientPluginNode()
215 16
    {
216 16
        $builder = new TreeBuilder();
217
        $node = $builder->root('plugins');
218
219
        /** @var ArrayNodeDefinition $pluginList */
220 16
        $pluginList = $node
221 16
            ->info('A list of plugin service ids and client specific plugin definitions. The order is important.')
222 16
            ->prototype('array')
223
        ;
224
        $pluginList
225 16
            // support having just a service id in the list
226
            ->beforeNormalization()
227 8
                ->always(function ($plugin) {
228
                    if (is_string($plugin)) {
229
                        return [
230 7
                            'reference' => [
231 7
                                'enabled' => true,
232 7
                                'id' => $plugin,
233 7
                            ],
234
                        ];
235
                    }
236 5
237 16
                    return $plugin;
238 16
                })
239
            ->end()
240 16
241
            ->validate()
242 7
                ->always(function ($plugins) {
243 7
                    foreach ($plugins as $name => $definition) {
244 7
                        if ('authentication' === $name) {
245 7
                            if (!count($definition)) {
246 7
                                unset($plugins['authentication']);
247 7
                            }
248 7
                        } elseif (!$definition['enabled']) {
249 7
                            unset($plugins[$name]);
250 7
                        }
251
                    }
252 7
253 16
                    return $plugins;
254 16
                })
255
            ->end()
256 16
        ;
257
        $this->addSharedPluginNodes($pluginList, true);
258
259 16
        $pluginList
260 16
            ->children()
261 16
                ->arrayNode('reference')
262 16
                    ->canBeEnabled()
263 16
                    ->info('Reference to a plugin service')
264 16
                    ->children()
265 16
                        ->scalarNode('id')
266 16
                            ->info('Service id of a plugin')
267 16
                            ->isRequired()
268 16
                            ->cannotBeEmpty()
269 16
                        ->end()
270 16
                    ->end()
271 16
                ->end()
272 16
                ->arrayNode('add_host')
273 16
                    ->canBeEnabled()
274 16
                    ->addDefaultsIfNotSet()
275 16
                    ->info('Set scheme, host and port in the request URI.')
276 16
                    ->children()
277 16
                        ->scalarNode('host')
278 16
                            ->info('Host name including protocol and optionally the port number, e.g. https://api.local:8000')
279 16
                            ->isRequired()
280 16
                            ->cannotBeEmpty()
281 16
                        ->end()
282 16
                        ->scalarNode('replace')
283 16
                            ->info('Whether to replace the host if request already specifies one')
284 16
                            ->defaultValue(false)
285 16
                        ->end()
286 16
                    ->end()
287 16
                ->end()
288 16
                ->arrayNode('header_append')
289 16
                    ->canBeEnabled()
290 16
                    ->info('Append headers to the request. If the header already exists the value will be appended to the current value.')
291 16
                    ->fixXmlConfig('header')
292 16
                    ->children()
293 16
                        ->arrayNode('headers')
294 16
                            ->info('Keys are the header names, values the header values')
295 16
                            ->normalizeKeys(false)
296 16
                            ->useAttributeAsKey('name')
297 16
                            ->prototype('scalar')->end()
298 16
                        ->end()
299 16
                    ->end()
300 16
                ->end()
301 16
                ->arrayNode('header_defaults')
302 16
                    ->canBeEnabled()
303 16
                    ->info('Set header to default value if it does not exist.')
304 16
                    ->fixXmlConfig('header')
305 16
                    ->children()
306 16
                        ->arrayNode('headers')
307 16
                            ->info('Keys are the header names, values the header values')
308 16
                            ->normalizeKeys(false)
309 16
                            ->useAttributeAsKey('name')
310 16
                            ->prototype('scalar')->end()
311 16
                        ->end()
312 16
                    ->end()
313 16
                ->end()
314 16
                ->arrayNode('header_set')
315 16
                    ->canBeEnabled()
316 16
                    ->info('Set headers to requests. If the header does not exist it wil be set, if the header already exists it will be replaced.')
317 16
                    ->fixXmlConfig('header')
318 16
                    ->children()
319 16
                        ->arrayNode('headers')
320 16
                            ->info('Keys are the header names, values the header values')
321 16
                            ->normalizeKeys(false)
322 16
                            ->useAttributeAsKey('name')
323 16
                            ->prototype('scalar')->end()
324 16
                        ->end()
325 16
                    ->end()
326 16
                ->end()
327 16
                ->arrayNode('header_remove')
328 16
                    ->canBeEnabled()
329 16
                    ->info('Remove headers from requests.')
330 16
                    ->fixXmlConfig('header')
331 16
                    ->children()
332 16
                        ->arrayNode('headers')
333 16
                            ->info('List of header names to remove')
334 16
                            ->prototype('scalar')->end()
335 16
                        ->end()
336 16
                    ->end()
337 16
                ->end()
338 16
            ->end()
339
        ->end();
340 16
341
        return $node;
342
    }
343
344
    /**
345
     * Add the definitions for shared plugin configurations.
346
     *
347
     * @param ArrayNodeDefinition $pluginNode The node to add to.
348
     * @param bool                $disableAll Some shared plugins are enabled by default. On the client, all are disabled by default.
349 16
     */
350
    private function addSharedPluginNodes(ArrayNodeDefinition $pluginNode, $disableAll = false)
351 16
    {
352
        $children = $pluginNode->children();
353 16
354
        $children->append($this->createAuthenticationPluginNode());
355 16
356 16
        $children->arrayNode('cache')
357 16
            ->canBeEnabled()
358 16
            ->addDefaultsIfNotSet()
359 16
                ->children()
360 16
                    ->scalarNode('cache_pool')
361 16
                        ->info('This must be a service id to a service implementing Psr\Cache\CacheItemPoolInterface')
362 16
                        ->isRequired()
363 16
                        ->cannotBeEmpty()
364 16
                    ->end()
365 16
                    ->scalarNode('stream_factory')
366 16
                        ->info('This must be a service id to a service implementing Http\Message\StreamFactory')
367 16
                        ->defaultValue('httplug.stream_factory')
368 16
                        ->cannotBeEmpty()
369 16
                    ->end()
370 16
                    ->arrayNode('config')
371 16
                        ->addDefaultsIfNotSet()
372 16
                        ->children()
373 16
                            ->scalarNode('default_ttl')->defaultValue(0)->end()
374 16
                            ->scalarNode('respect_cache_headers')->defaultNull()->end()
375 16
                            ->variableNode('respect_response_cache_directives')
376 16
                                ->validate()
377 16
                                ->always(function ($v) {
378
                                    if (is_null($v) || is_array($v)) {
379
                                        return $v;
380 16
                                    }
381 16
                                    throw new InvalidTypeException();
382 16
                                })
383 16
                                ->end()
384 16
                            ->defaultNull()
385 16
                            ->end()
386 16
                        ->end()
387 16
                    ->end()
388 16
                ->end()
389 16
            ->end();
390
        // End cache plugin
391
392 16
        $children->arrayNode('cookie')
393 16
            ->canBeEnabled()
394 16
                ->children()
395 16
                    ->scalarNode('cookie_jar')
396 16
                        ->info('This must be a service id to a service implementing Http\Message\CookieJar')
397 16
                        ->isRequired()
398 16
                        ->cannotBeEmpty()
399
                    ->end()
400
                ->end()
401 16
            ->end();
402 16
        // End cookie plugin
403 16
404 16
        $decoder = $children->arrayNode('decoder');
405 16
        $disableAll ? $decoder->canBeEnabled() : $decoder->canBeDisabled();
406 16
        $decoder->addDefaultsIfNotSet()
407 16
            ->children()
408 16
                ->scalarNode('use_content_encoding')->defaultTrue()->end()
409 16
            ->end()
410 16
        ->end();
411
        // End decoder plugin
412
413 16
        $children->arrayNode('history')
414 16
            ->canBeEnabled()
415 16
                ->children()
416 16
                    ->scalarNode('journal')
417 16
                        ->info('This must be a service id to a service implementing Http\Client\Plugin\Journal')
418 16
                        ->isRequired()
419 16
                        ->cannotBeEmpty()
420 16
                    ->end()
421 16
                ->end()
422 16
            ->end();
423 16
        // End history plugin
424 16
425 16
        $logger = $children->arrayNode('logger');
426 16
        $disableAll ? $logger->canBeEnabled() : $logger->canBeDisabled();
427 16
        $logger->addDefaultsIfNotSet()
428
            ->children()
429
                ->scalarNode('logger')
430 16
                    ->info('This must be a service id to a service implementing Psr\Log\LoggerInterface')
431 16
                    ->defaultValue('logger')
432 16
                    ->cannotBeEmpty()
433 16
                ->end()
434 16
                ->scalarNode('formatter')
435 16
                    ->info('This must be a service id to a service implementing Http\Message\Formatter')
436 16
                    ->defaultNull()
437 16
                ->end()
438
            ->end()
439
        ->end();
440 16
        // End logger plugin
441 16
442 16
        $redirect = $children->arrayNode('redirect');
443 16
        $disableAll ? $redirect->canBeEnabled() : $redirect->canBeDisabled();
444 16
        $redirect->addDefaultsIfNotSet()
445 16
            ->children()
446 16
                ->scalarNode('preserve_header')->defaultTrue()->end()
447
                ->scalarNode('use_default_for_multiple')->defaultTrue()->end()
448
            ->end()
449 16
        ->end();
450 16
        // End redirect plugin
451 16
452 16
        $retry = $children->arrayNode('retry');
453 16
        $disableAll ? $retry->canBeEnabled() : $retry->canBeDisabled();
454 16
        $retry->addDefaultsIfNotSet()
455 16
            ->children()
456 16
                ->scalarNode('retry')->defaultValue(1)->end() // TODO: should be called retries for consistency with the class
457 16
            ->end()
458 16
        ->end();
459 16
        // End retry plugin
460
461 16
        $stopwatch = $children->arrayNode('stopwatch');
462
        $disableAll ? $stopwatch->canBeEnabled() : $stopwatch->canBeDisabled();
463
        $stopwatch->addDefaultsIfNotSet()
464
            ->children()
465
                ->scalarNode('stopwatch')
466
                    ->info('This must be a service id to a service extending Symfony\Component\Stopwatch\Stopwatch')
467
                    ->defaultValue('debug.stopwatch')
468 16
                    ->cannotBeEmpty()
469
                ->end()
470 16
            ->end()
471 16
        ->end();
472
        // End stopwatch plugin
473 16
    }
474 16
475 16
    /**
476 16
     * Create configuration for authentication plugin.
477 16
     *
478 5
     * @return NodeDefinition Definition for the authentication node in the plugins list.
479 5
     */
480 4
    private function createAuthenticationPluginNode()
481 4
    {
482 2
        $builder = new TreeBuilder();
483 1
        $node = $builder->root('authentication');
484 1
        $node
1 ignored issue
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 children() 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...
485 2
            ->useAttributeAsKey('name')
486 2
            ->prototype('array')
487 1
                ->validate()
488 1
                    ->always()
489 1
                    ->then(function ($config) {
490 1
                        switch ($config['type']) {
491 4
                            case 'basic':
492
                                $this->validateAuthenticationType(['username', 'password'], $config, 'basic');
493 4
                                break;
494 16
                            case 'bearer':
495 16
                                $this->validateAuthenticationType(['token'], $config, 'bearer');
496 16
                                break;
497 16
                            case 'service':
498 16
                                $this->validateAuthenticationType(['service'], $config, 'service');
499 16
                                break;
500 16
                            case 'wsse':
501 16
                                $this->validateAuthenticationType(['username', 'password'], $config, 'wsse');
502 16
                                break;
503 16
                        }
504 16
505 16
                        return $config;
506 16
                    })
507 16
                ->end()
508 16
                ->children()
509
                    ->enumNode('type')
510 16
                        ->values(['basic', 'bearer', 'wsse', 'service'])
511
                        ->isRequired()
512
                        ->cannotBeEmpty()
513
                    ->end()
514
                    ->scalarNode('username')->end()
515
                    ->scalarNode('password')->end()
516
                    ->scalarNode('token')->end()
517
                    ->scalarNode('service')->end()
518
                    ->end()
519
                ->end()
520
            ->end(); // End authentication plugin
521
522 5
        return $node;
523
    }
524 5
525 5
    /**
526 5
     * Validate that the configuration fragment has the specified keys and none other.
527 5
     *
528
     * @param array  $expected Fields that must exist
529 5
     * @param array  $actual   Actual configuration hashmap
530 4
     * @param string $authName Name of authentication method for error messages
531
     *
532
     * @throws InvalidConfigurationException If $actual does not have exactly the keys specified in $expected (plus 'type')
533 1
     */
534 1
    private function validateAuthenticationType(array $expected, array $actual, $authName)
535 1
    {
536 1
        unset($actual['type']);
537 1
        $actual = array_keys($actual);
538 1
        sort($actual);
539
        sort($expected);
540
541
        if ($expected === $actual) {
542
            return;
543
        }
544
545
        throw new InvalidConfigurationException(sprintf(
546
            'Authentication "%s" requires %s but got %s',
547
            $authName,
548
            implode(', ', $expected),
549
            implode(', ', $actual)
550
        ));
551
    }
552
}
553