{"action":"created","issue":{"url":"https://api.github.com/repos/symfony/symfony/issues/13433","labels_url":"https://api.github.com/repos/symfony/symfony/issues/13433/labels{/name}","comments_url":"https://api.github.com/repos/symfony/symfony/issues/13433/comments","events_url":"https://api.github.com/repos/symfony/symfony/issues/13433/events","html_url":"https://github.com/symfony/symfony/issues/13433","id":54591144,"number":13433,"title":"[Security] Setting DefaultAuthenticationFailureHandler options more than once doesn't work","user":{"login":"petert82","id":3646420,"avatar_url":"https://avatars.githubusercontent.com/u/3646420?v=3","gravatar_id":"","url":"https://api.github.com/users/petert82","html_url":"https://github.com/petert82","followers_url":"https://api.github.com/users/petert82/followers","following_url":"https://api.github.com/users/petert82/following{/other_user}","gists_url":"https://api.github.com/users/petert82/gists{/gist_id}","starred_url":"https://api.github.com/users/petert82/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/petert82/subscriptions","organizations_url":"https://api.github.com/users/petert82/orgs","repos_url":"https://api.github.com/users/petert82/repos","events_url":"https://api.github.com/users/petert82/events{/privacy}","received_events_url":"https://api.github.com/users/petert82/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":4,"created_at":"2015-01-16T15:59:28Z","updated_at":"2015-01-21T14:58:10Z","closed_at":null,"body":"I *think* there is bug in the DefaultAuthenticationFailureHandler's `setOptions` method which means that if it is called more than once, only the options from the *last* call will be applied.\r\n\r\nFor example, given this:\r\n\r\n```php\r\n// Assume $handler is an instance of DefaultAuthenticationFailureHandler\r\n$handler->setOptions(array('foo' => 'bar'));\r\n$handler->setOptions(array('baz' => 'qux'));\r\n```\r\n\r\nI would expect this when I call `getOptions`:\r\n\r\n```php\r\narray(\r\n    'failure_path' => null,\r\n    'failure_forward' => false,\r\n    'login_path' => '/login',\r\n    'failure_path_parameter' => '_failure_path',\r\n    'foo' => 'bar',\r\n    'baz' => 'qux',\r\n)\r\n```\r\n\r\nBut what I would actually get is (note the missing `foo` key):\r\n\r\n```php\r\narray(\r\n    'failure_path' => null,\r\n    'failure_forward' => false,\r\n    'login_path' => '/login',\r\n    'failure_path_parameter' => '_failure_path',\r\n    'baz' => 'qux',\r\n)\r\n```\r\n\r\nSince `setOptions` is called by the DefaultAuthenticationFailureHandler constructor, this means that when overriding the `security.authentication.failure_handler` service, any custom options provided as service arguments are lost when `setOptions` is called later.\r\n\r\nFrom a quick glance at the code DefaultAuthenticationSuccessHandler would exhibit the same behaviour.\r\n\r\nIt seems to me that the `setOptions` method in both of these classes should be doing something more like this, unless the current behaviour is intentional?\r\n\r\n```php\r\n$this->options = array_merge($this->defaultOptions, $this->options, $options);\r\n```\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/symfony/symfony/issues/comments/70851386","html_url":"https://github.com/symfony/symfony/issues/13433#issuecomment-70851386","issue_url":"https://api.github.com/repos/symfony/symfony/issues/13433","id":70851386,"user":{"login":"petert82","id":3646420,"avatar_url":"https://avatars.githubusercontent.com/u/3646420?v=3","gravatar_id":"","url":"https://api.github.com/users/petert82","html_url":"https://github.com/petert82","followers_url":"https://api.github.com/users/petert82/followers","following_url":"https://api.github.com/users/petert82/following{/other_user}","gists_url":"https://api.github.com/users/petert82/gists{/gist_id}","starred_url":"https://api.github.com/users/petert82/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/petert82/subscriptions","organizations_url":"https://api.github.com/users/petert82/orgs","repos_url":"https://api.github.com/users/petert82/repos","events_url":"https://api.github.com/users/petert82/events{/privacy}","received_events_url":"https://api.github.com/users/petert82/received_events","type":"User","site_admin":false},"created_at":"2015-01-21T14:58:10Z","updated_at":"2015-01-21T14:58:10Z","body":"But, even ignoring the issue of setting extra options, with the behaviour as it stands, the options provided to the constructor interact quite strangely with options provided via `setOptions`.\r\n\r\nFor example, say I construct a `DefaultAuthenticationFailureHandler` with this as its `$options`:\r\n\r\n```php\r\narray(\r\n    'login_path' => '/my-login-path',\r\n)\r\n```\r\n\r\nAnd then call `setOptions` with these options:\r\n\r\n```php\r\narray(\r\n    'failure_forward' => true,\r\n)\r\n```\r\n\r\nI end up with an options array that looks like this:\r\n\r\n```php\r\narray(\r\n    'failure_path' => null,\r\n    'failure_forward' => true,\r\n    'login_path' => '/login',\r\n    'failure_path_parameter' => '_failure_path',\r\n)\r\n```\r\n\r\nMy `login_path` has been replaced with the default value. This seems quite counter-intuitive, to say the least - essentially, as is, I have to get the current options, merge my changes into them and then apply the result of that via `setOptions` if I don't want to accidentally lose a previously set option."}}