Completed
Pull Request — master (#145)
by Rob
06:37
created

Configuration::createAuthenticationPluginNode()   B

Complexity

Conditions 5
Paths 1

Size

Total Lines 44
Code Lines 38

Duplication

Lines 0
Ratio 0 %

Code Coverage

Tests 39
CRAP Score 5

Importance

Changes 0
Metric Value
dl 0
loc 44
ccs 39
cts 39
cp 1
rs 8.439
c 0
b 0
f 0
cc 5
eloc 38
nc 1
nop 0
crap 5
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
     */
35 16
    public function __construct($debug)
36
    {
37 16
        $this->debug = (bool) $debug;
38 16
    }
39
40
    /**
41
     * {@inheritdoc}
42
     */
43 16
    public function getConfigTreeBuilder()
44
    {
45 16
        $treeBuilder = new TreeBuilder();
46 16
        $rootNode = $treeBuilder->root('httplug');
47
48 16
        $this->configureClients($rootNode);
49 16
        $this->configureSharedPlugins($rootNode);
50
51
        $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 16
            ->validate()
53
                ->ifTrue(function ($v) {
54 13
                    return !empty($v['classes']['client'])
55 13
                        || !empty($v['classes']['message_factory'])
56 10
                        || !empty($v['classes']['uri_factory'])
57 13
                        || !empty($v['classes']['stream_factory']);
58 16
                })
59
                ->then(function ($v) {
60 3
                    foreach ($v['classes'] as $key => $class) {
61 3
                        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 1
                                $class,
65
                                $key
66 1
                            ));
67
                        }
68 2
                    }
69
70 2
                    return $v;
71 16
                })
72 16
            ->end()
73 16
            ->beforeNormalization()
74
                ->ifTrue(function ($v) {
75 16
                    return is_array($v) && array_key_exists('toolbar', $v) && is_array($v['toolbar']);
76 16
                })
77
                ->then(function ($v) {
78 4
                    if (array_key_exists('profiling', $v)) {
79 1
                        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
82 3
                    @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
84 3
                    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 1
                    }
88
89 3
                    $v['profiling'] = $v['toolbar'];
90
91 3
                    unset($v['toolbar']);
92
93 3
                    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 16
            ->end();
151
152 16
        return $treeBuilder;
153
    }
154
155 16
    private function configureClients(ArrayNodeDefinition $root)
156
    {
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 16
                ->validate()
163
                    ->ifTrue(function ($config) {
164
                        // Make sure we only allow one of these to be true
165 7
                        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 16
    }
193
194
    /**
195
     * @param ArrayNodeDefinition $root
196
     */
197 16
    private function configureSharedPlugins(ArrayNodeDefinition $root)
198
    {
199
        $pluginsNode = $root
200 16
            ->children()
201 16
                ->arrayNode('plugins')
202 16
                ->info('Global plugin configuration. Plugins need to be explicitly added to clients.')
203 16
                ->addDefaultsIfNotSet()
204
            // don't call end to get the plugins node
205 16
        ;
206 16
        $this->addSharedPluginNodes($pluginsNode);
207 16
    }
208
209
    /**
210
     * Createplugins node of a client.
211
     *
212
     * @return ArrayNodeDefinition The plugin node
213
     */
214 16
    private function createClientPluginNode()
215
    {
216 16
        $builder = new TreeBuilder();
217 16
        $node = $builder->root('plugins');
218
219
        /** @var ArrayNodeDefinition $pluginList */
220
        $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 16
        ;
224
        $pluginList
225
            // support having just a service id in the list
226 16
            ->beforeNormalization()
227
                ->always(function ($plugin) {
228 8
                    if (is_string($plugin)) {
229
                        return [
230
                            'reference' => [
231 7
                                'enabled' => true,
232 7
                                'id' => $plugin,
233 7
                            ],
234 7
                        ];
235
                    }
236
237 5
                    return $plugin;
238 16
                })
239 16
            ->end()
240
241 16
            ->validate()
242
                ->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 7
                    }
252
253 7
                    return $plugins;
254 16
                })
255 16
            ->end()
256
        ;
257 16
        $this->addSharedPluginNodes($pluginList, true);
258
259
        $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 16
        ->end();
340
341 16
        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
     */
350 16
    private function addSharedPluginNodes(ArrayNodeDefinition $pluginNode, $disableAll = false)
351
    {
352 16
        $children = $pluginNode->children();
353
354 16
        $children->append($this->createAuthenticationPluginNode());
355
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
                                ->always(function ($v) {
378 1
                                    if (is_null($v) || is_array($v)) {
379 1
                                        return $v;
380
                                    }
381
                                    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 16
                    ->end()
400 16
                ->end()
401 16
            ->end();
402
        // End cookie plugin
403
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
        // End history plugin
424
425 16
        $logger = $children->arrayNode('logger');
426 16
        $disableAll ? $logger->canBeEnabled() : $logger->canBeDisabled();
427 16
        $logger->addDefaultsIfNotSet()
428 16
            ->children()
429 16
                ->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 16
            ->end()
439 16
        ->end();
440
        // End logger plugin
441
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 16
                ->scalarNode('use_default_for_multiple')->defaultTrue()->end()
448 16
            ->end()
449 16
        ->end();
450
        // End redirect plugin
451
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
        // End retry plugin
460
461 16
        $stopwatch = $children->arrayNode('stopwatch');
462 16
        $disableAll ? $stopwatch->canBeEnabled() : $stopwatch->canBeDisabled();
463 16
        $stopwatch->addDefaultsIfNotSet()
464 16
            ->children()
465 16
                ->scalarNode('stopwatch')
466 16
                    ->info('This must be a service id to a service extending Symfony\Component\Stopwatch\Stopwatch')
467 16
                    ->defaultValue('debug.stopwatch')
468 16
                    ->cannotBeEmpty()
469 16
                ->end()
470 16
            ->end()
471 16
        ->end();
472
        // End stopwatch plugin
473 16
    }
474
475
    /**
476
     * Create configuration for authentication plugin.
477
     *
478
     * @return NodeDefinition Definition for the authentication node in the plugins list.
479
     */
480 16
    private function createAuthenticationPluginNode()
481
    {
482 16
        $builder = new TreeBuilder();
483 16
        $node = $builder->root('authentication');
484
        $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 16
            ->useAttributeAsKey('name')
486 16
            ->prototype('array')
487 16
                ->validate()
488 16
                    ->always()
489 16
                    ->then(function ($config) {
490 5
                        switch ($config['type']) {
491 5
                            case 'basic':
492 4
                                $this->validateAuthenticationType(['username', 'password'], $config, 'basic');
493 4
                                break;
494 2
                            case 'bearer':
495 1
                                $this->validateAuthenticationType(['token'], $config, 'bearer');
496 1
                                break;
497 2
                            case 'service':
498 2
                                $this->validateAuthenticationType(['service'], $config, 'service');
499 1
                                break;
500 1
                            case 'wsse':
501 1
                                $this->validateAuthenticationType(['username', 'password'], $config, 'wsse');
502 1
                                break;
503 4
                        }
504
505 4
                        return $config;
506 16
                    })
507 16
                ->end()
508 16
                ->children()
509 16
                    ->enumNode('type')
510 16
                        ->values(['basic', 'bearer', 'wsse', 'service'])
511 16
                        ->isRequired()
512 16
                        ->cannotBeEmpty()
513 16
                    ->end()
514 16
                    ->scalarNode('username')->end()
515 16
                    ->scalarNode('password')->end()
516 16
                    ->scalarNode('token')->end()
517 16
                    ->scalarNode('service')->end()
518 16
                    ->end()
519 16
                ->end()
520 16
            ->end(); // End authentication plugin
521
522 16
        return $node;
523
    }
524
525
    /**
526
     * Validate that the configuration fragment has the specified keys and none other.
527
     *
528
     * @param array  $expected Fields that must exist
529
     * @param array  $actual   Actual configuration hashmap
530
     * @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
     */
534 5
    private function validateAuthenticationType(array $expected, array $actual, $authName)
535
    {
536 5
        unset($actual['type']);
537 5
        $actual = array_keys($actual);
538 5
        sort($actual);
539 5
        sort($expected);
540
541 5
        if ($expected === $actual) {
542 4
            return;
543
        }
544
545 1
        throw new InvalidConfigurationException(sprintf(
546 1
            'Authentication "%s" requires %s but got %s',
547 1
            $authName,
548 1
            implode(', ', $expected),
549 1
            implode(', ', $actual)
550 1
        ));
551
    }
552
}
553