8000 Json authentication listener tries to authenticate on all routes · Issue #21948 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Json authentication listener tries to authenticate on all routes #21948

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.

Sign up for GitHub

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

Jump to bottom
Closed
tplantta opened this issue Mar 9, 2017 · 5 comments
Closed

Json authentication listener tries to authenticate on all routes #21948

tplantta opened this issue Mar 9, 2017 · 5 comments
Labels
Bug Security Status: Needs Review

Comments

@tplantta
Copy link
tplantta commented Mar 9, 2017
Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version master

Json login listener tries to authenticate on all routes on the firewall it is registered on, not just the configured check_path. It even blocks access to routes that are configured to allow anonymous access.

With this configuration

security:
    firewalls:
        customer:
            anonymous: ~
            stateless: true
            pattern: ^/customerapi
            provider: my_db_customers
            guard:
                authenticators:
                    - app.token_authenticator
            json_login:
                success_handler: app.customer_login_success_handler
                check_path: customer_login 
    access_control:
        - { path: ^/customerapi/login, roles: IS_AUTHENTICATED_ANONYMOUSLY }
        - { path: ^/customerapi/account/create, roles: IS_AUTHENTICATED_ANONYMOUSLY }

no route starting with ^/customerapi can be accessed without the json format credentials for a valid user. Error messages in dev.log clearly show it is json authentication listener denying access.
For example GET request to some route requiring authentication, with proper access token and empty HTTP body results in:

[2017-03-09 13:51:33] security.INFO: Guard authentication successful! {"token":"[object] (Symfony\\Component\\Security\\Guard\\Token\\PostAuthenticationGuardToken: PostAuthenticationGuardToken(user=\"*******@*****\", authenticated=true, roles=\"ROLE_CUSTOMER\"))","authenticator":"AppBundle\\Security\\CustomerTokenAuthenticator"} [] [2017-03-09 13:51:33] security.DEBUG: Guard authenticator set no success response: request continues. {"authenticator":"AppBundle\\Security\\CustomerTokenAuthenticator"} [] [2017-03-09 13:51:33] security.DEBUG: Remember me skipped: it is not configured for the firewall. {"authenticator":"AppBundle\\Security\\CustomerTokenAuthenticator"} [] [2017-03-09 13:51:33] security.INFO: An AuthenticationException was thrown; redirecting to authentication entry point. {"exception":"[object] (Symfony\\Component\\Security\\Core\\Exception\\BadCredentialsException(code: 0): Invalid JSON. at /home/lanttto/NetBeansProjects/AAh/vendor/symfony/symfony/src/Symfony/Component/Security/Http/Firewall/UsernamePasswordJsonAuthenticationListener.php:74)"}

As far as I undestand from the documentation this is not intended behavior. Seems to lack a check against request path to only act when check_path is requested.
@fabpot
Copy link
Member
fabpot commented Mar 9, 2017

/cc @dunglas

@dmaicher
Copy link
Contributor

Following the PR that provided the feature (#18952) it seems the authentication is supposed to be stateless. So it does not store the token in the session and you need to authenticate every single request you perform.

Is that correct @dunglas ? Just wondering for what exactly do we need the check_path then?

@chalasr chalasr mentioned this issue Apr 13, 2017
Closed
@lsmith77
Copy link
Contributor

if indeed its supposed to be stateless then yeah the check_path isn't needed and the docs should clarify this aspect so that one knows it makes no sense to combine it with form_login for example.

@lsmith77
Copy link
Contributor

hmm thinking about it .. how would I then use this on a GET request if its intended for stateless only?

in case there is doubt on if it makes sense to send a request body with a GET, see:
https://groups.yahoo.com/neo/groups/rest-discuss/conversations/messages/9962

Yes. In other words, any HTTP request message is allowed to contain
a message body, and thus must parse messages with that in mind.
Server semantics for GET, however, are restricted such that a body,
if any, has no semantic meaning to the request. The requirements
on parsing are separate from the requirements on method semantics.

So, yes, you can send a body with GET, and no, it is never useful
to do so.

This is part of the layered design of HTTP/1.1 that will become
clear again once the spec is partitioned (work in progress).

....Roy```

@lsmith77
Copy link
Contributor

anyway .. at this point I think I will just combine the form_login with the JsonDecoder in FOSRestBundle to get a stateful login via JSON.

@chalasr chalasr mentioned this issue Apr 13, 2017
Closed
fabpot added a commit that referenced this issue Apr 18, 2017
@fabpot
bug #22425 [Security] Allow to set a check_path on json_login listene…
6c7bced 
…r (chalasr)

This PR was squashed before being merged into the 3.3-dev branch (closes #22425).

Discussion
----------

[Security] Allow to set a check_path on json_login listener

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no, master only
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #21948, ~~#22423~~
| License       | MIT
| Doc PR        | n/a

The listener should allow to restrict authentication to a given check_path, as stated in the docs http://symfony.com/doc/master/security/json_login_setup.html

Commits
-------

9f7eb61 [Security] Allow to set a check_path on json_login listener
@fabpot fabpot closed this as completed Apr 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Security Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @lsmith77 @dmaicher @xabbuh @carsonbot @tplantta
0