Skip to content

[SecurityContext] Request listener exception #2753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
idetia opened this issue Nov 30, 2011 · 6 comments
Closed

[SecurityContext] Request listener exception #2753

idetia opened this issue Nov 30, 2011 · 6 comments
Labels

Comments

@idetia
Copy link

idetia commented Nov 30, 2011

When uses the Security Context from a request listener throws an exception: Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException: The security context contains no authentication token

@stof
Copy link
Member

stof commented Nov 30, 2011

Are you running your request listener after the firewall ?

And are you using a catch-all firewall ? If not, you should first check if you have a token before trying to call isGranted

@idetia
Copy link
Author

idetia commented Nov 30, 2011

Ok, I have changed the listener priority to -99 but now the web profiler toolbar isn't show and the same exception is logged: request.CRITICAL: Symfony\Component\Security\Core\Exception\AuthenticationCredentialsNotFoundException: The security context contains no authentication token.

If I call the Security Context out of the request listener (from a Controller for example) there not any issue.

@vicb
Copy link
Contributor

vicb commented Mar 8, 2012

@idetia could you link your listener code and config + security in a gist ?

@stof
Copy link
Member

stof commented Mar 8, 2012

@idetia the issue is that your listener is probably used even for urls for which the firewall is disabled. And for instance, the SE disable the firewall entirely for the url used to load the WDT by ajax (which is a way to ensure that it won't display a login form instead because of a redirection to the login form). Your listener should check that $securityContext->getToken() is not null (it will be set if the firewall was used) before trying to check for permissions.

@vicb
Copy link
Contributor

vicb commented Mar 8, 2012

@idetia I am closing the issue as @stof has just provided the solution. Please post a coment if it does not work.

@vicb vicb closed this as completed Mar 8, 2012
@idetia
Copy link
Author

idetia commented Mar 9, 2012

@vicb, @stof: Yes, that was the problem:

public function onKernelRequest(GetResponseEvent $event)
{
    if (null !== $this->security->getToken() && $this->security->isGranted('IS_AUTHENTICATED_REMEMBERED')) {    
        ...
    }
}

Thanks for the help.

fabpot added a commit that referenced this issue Mar 23, 2012
Commits
-------

5ae76f1 [HttpFoundation] Update documentation.
910b5c7 [HttpFoudation] CS, more tests and some optimization.
b0466e8 [HttpFoundation] Refactored BC Session class methods.
84c2e3c [HttpFoundation] Allow flash messages to have multiple messages per type.

Discussion
----------

[2.1][HttpFoundation] Multiple session flash messages

Bug fix: no
Feature addition: yes
Backwards compatibility break: yes, but this already happened in #2583.  BC `Session` methods remain unbroken.
Symfony2 tests pass: yes
Fixes the following tickets: #1863
References the following tickets: #2714, #2753, #2510, #2543, #2853
Todo: -

This PR alters flash messages so that it is possible to store more than one message per flash type using the `add()` method or by passing an array of messages to `set()`.

__NOTES ABOUT BC__

This PR maintains BC behaviour with the `Session` class in that the old Symfony 2.0 methods will continue to work as before.

---------------------------------------------------------------------------

by drak at 2012-02-13T06:28:33Z

I think this is ready for review @fabpot @lsmith77

---------------------------------------------------------------------------

by lsmith77 at 2012-02-14T19:30:39Z

the FlashBag vs. AutoExpireFlashBag behavior and setup difference should probably also be explained in the upgrading log

---------------------------------------------------------------------------

by drak at 2012-02-15T04:43:14Z

@lsmith77 Those differences are explained already in the changelog

 * Added `FlashBag`. Flashes expire when retrieved by `get()` or `all()`.
   This makes the implementation ESI compatible.
 * Added `AutoExpireFlashBag` (default) to replicate Symfony 2.0.x auto expire behaviour of messages auto expiring
   after one page page load.  Messages must be retrived by `get()` or `all()`.

---------------------------------------------------------------------------

by Crell at 2012-02-19T17:35:34Z

Drak asked me to weigh in here with use cases.  Drupal currently has a similar session-stored-messaging system in place that I'd like to be able to replace with Flash messages.  We frequently have multiple messages within a single request, however, so this change is critical to our being able to do so.

For instance, when saving an article in Drupal there is, by default, a "yay, you saved an article!" type message that gets displayed.  If you also have the site configured to send email when a post is updated, you may see a "email notifications sent" message (depending on your access level).  If you have a Solr server setup for search, and you're in debug mode, there will also be a "record ID X added to Solr, it should update in 2 minutes" message.  And if there's a bug somewhere, you'll also get, as an error message rather than notice message, a "Oops, E_NOTICE on line 54" message.

Form validation is another case.  If you have multiple errors in a single form, we prefer to list all of them.  So if you screw up 4 times on a form, you may get 4 different error messages showing what you screwed up so you can fix it in one go instead of several.

Now sure, one could emulate that by building a multi-message layer on top of single-layer messages, but, really, why?  "One is a special case of many", and there are many many cases where you'll want to post multiple messages.  Like, most of Drupal. :-)

---------------------------------------------------------------------------

by lsmith77 at 2012-03-06T20:55:51Z

@fabpot is there any information you still need before merging this? do you want more discussion in which case you might want to take this to the mailing list ..

---------------------------------------------------------------------------

by drak at 2012-03-08T18:54:13Z

Another plus for this PR is that it requires no extra lines of code in templates etc to display the flashes, see https://github.com/symfony/symfony/pull/3267/files#diff-1

---------------------------------------------------------------------------

by drak at 2012-03-15T06:38:21Z

Rebased against current `master`, should be mergeable again..

---------------------------------------------------------------------------

by evillemez at 2012-03-17T03:08:41Z

+1 to this, I have an extended version of HttpFoundation just for this... would love to get rid of it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants