Skip to content
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

[Security] [Firewall] Configuring multiple HTTP authentication methods #10035

Closed
SimonSimCity opened this issue Jan 16, 2014 · 6 comments
Closed

Comments

@SimonSimCity
Copy link
Contributor

Hi,

I would like to discuss the possibility of how to improve the outcome of a failing authentication process.

I saw, that the two http-authentication methods for basic and digest authentication have their own AuthenticationEntryPoint where each of them creates a response, sets the corresponding response-header and returns a 401 status-code.

All this is correct. But if you define both, the basic and the digest authentication, just one of their AuthenticationEntryPoint classes will be called on a failing login and the client does not get informed about it.

In the first answer of http://stackoverflow.com/questions/3576197/http-authentication-www-authenticate-header-multiple-realms I read, that it's possible to return a set of possible authentication methods to be used, and the clients responsibility now is to take the most secure one it supports.

Now I tried to activate both, http_basic and http_digest authentication methods, and it just failed ... here's the error-message:

ContextErrorException: Catchable Fatal Error: Argument 4 passed to Symfony\Component\Security\Http\Firewall\DigestAuthenticationListener::__construct() must be an instance of Symfony\Component\Security\Http\EntryPoint\DigestAuthenticationEntryPoint, instance of Symfony\Component\Security\Http\EntryPoint\BasicAuthenticationEntryPoint given, called in /var/www/sf2.local/app/cache/dev/appDevDebugProjectContainer.php on line 2912 and defined in /var/www/sf2.local/vendor/symfony/symfony/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php line 40

    in /var/www/sf2.local/vendor/symfony/symfony/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php line 40
    at ErrorHandler->handle('4096', 'Argument 4 passed to Symfony\Component\Security\Http\Firewall\DigestAuthenticationListener::__construct() must be an instance of Symfony\Component\Security\Http\EntryPoint\DigestAuthenticationEntryPoint, instance of Symfony\Component\Security\Http\EntryPoint\BasicAuthenticationEntryPoint given, called in /var/www/sf2.local/app/cache/dev/appDevDebugProjectContainer.php on line 2912 and defined', '/var/www/sf2.local/vendor/symfony/symfony/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php', '40', array('securityContext' => object(SecurityContext), 'provider' => object(ChainUserProvider), 'providerKey' => 'secured_area')) in /var/www/sf2.local/vendor/symfony/symfony/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php line 40
    at DigestAuthenticationListener->__construct(object(SecurityContext), object(ChainUserProvider), 'secured_area', object(BasicAuthenticationEntryPoint), object(Logger)) in /var/www/sf2.local/app/cache/dev/appDevDebugProjectContainer.php line 2912
    at appDevDebugProjectContainer->getSecurity_Firewall_Map_Context_SecuredAreaService() in /var/www/sf2.local/app/bootstrap.php.cache line 2033
    at Container->get('security.firewall.map.context.secured_area') in /var/www/sf2.local/app/cache/dev/classes.php line 2758
    at FirewallMap->getListeners(object(Request)) in /var/www/sf2.local/app/cache/dev/classes.php line 2418
    at Firewall->onKernelRequest(object(GetResponseEvent), 'kernel.request', object(TraceableEventDispatcher))
    at call_user_func(array(object(Firewall), 'onKernelRequest'), object(GetResponseEvent), 'kernel.request', object(TraceableEventDispatcher)) in /var/www/sf2.local/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/Debug/TraceableEventDispatcher.php line 392
    at TraceableEventDispatcher->Symfony\Component\HttpKernel\Debug\{closure}(object(GetResponseEvent), 'kernel.request', object(ContainerAwareEventDispatcher))
    at call_user_func(object(Closure), object(GetResponseEvent), 'kernel.request', object(ContainerAwareEventDispatcher)) in /var/www/sf2.local/app/cache/dev/classes.php line 1747
    at EventDispatcher->doDispatch(array(object(Closure), object(Closure), object(Closure), object(Closure), object(Closure), object(Closure), object(Closure), object(Closure), object(Closure), object(Closure), object(Closure), object(Closure), object(Closure), object(Closure), object(Closure)), 'kernel.request', object(GetResponseEvent)) in /var/www/sf2.local/app/cache/dev/classes.php line 1680
    at EventDispatcher->dispatch('kernel.request', object(GetResponseEvent)) in /var/www/sf2.local/app/cache/dev/classes.php line 1844
    at ContainerAwareEventDispatcher->dispatch('kernel.request', object(GetResponseEvent)) in /var/www/sf2.local/vendor/symfony/symfony/src/Symfony/Component/HttpKernel/Debug/TraceableEventDispatcher.php line 139
    at TraceableEventDispatcher->dispatch('kernel.request', object(GetResponseEvent)) in /var/www/sf2.local/app/bootstrap.php.cache line 2900
    at HttpKernel->handleRaw(object(Request), '1') in /var/www/sf2.local/app/bootstrap.php.cache line 2883
    at HttpKernel->handle(object(Request), '1', true) in /var/www/sf2.local/app/bootstrap.php.cache line 3022
    at ContainerAwareHttpKernel->handle(object(Request), '1', true) in /var/www/sf2.local/app/bootstrap.php.cache line 2303
    at Kernel->handle(object(Request)) in /var/www/sf2.local/web/app_dev.php line 28

Any comments on that? I, personally, think this should be fixed and the code should be rethought to come up with a way, where I can activate both authentication-methods and I get both as possible methods to authenticate on a 401-response.

@SimonSimCity
Copy link
Contributor Author

Just as a hint if you need it: It is kind of possible, yea ...

If you have the full project secured by multiple challenges, you can create your own listener, that listens on KernelEvents::EXCEPTION and catches all exceptions that are an instance of Symfony\Component\Security\Core\Exception\AuthenticationException. Just keep the priority of your listener above 0.

Here's an example:

    public function onKernelException(GetResponseForExceptionEvent $event)
    {
        static $handling;

        if (true === $handling) {
            return false;
        }

        $request = $event->getRequest();

        $handling = true;

        $exception = $event->getException();

        $yourChallenge = 'Basic realm="basic", Newauth realm="newauth"';
        if ($exception instanceof AuthenticationException) {
            $exception = new UnauthorizedHttpException($yourChallenge, 'You are not authenticated', $exception);
            $event->setException($exception);
            parent::onKernelException($event);
        }

        $handling = false;
    }

Please also take a look at this website, where you can see a list of supported challenge-combinations: http://greenbytes.de/tech/tc/httpauth/ None of the browser really keeps the spec here ;)

EDIT:
Please keep in mind, that this sample implementation is for your whole API! Not just the ones you put under your authentication. If you just want to have this triggered if the service does not provide it's own AuthenticationFailureHandler, keep the priority under 0.

@weaverryan
Copy link
Member

If you have 2 auth methods and you want to control the "entry point" (i.e. the response sent back to the user to say "hey, you need to authenticate"), you can do this by setting your own "entry point" via the entry_point configuration under a firewall: http://symfony.com/doc/current/reference/configuration/security.html. Just create a class that implements AuthenticationEntryPointInterface, register it as a service, and put the id there:

security:
    firewalls:
        some_firewall_name:
            # ...
            entry_point: service_id_to_entry_point

I think this is the correct solution - there's no way we could automatically create 1 entry point Response for multiple authentication providers).

I hope this helps someone :).

fabpot added a commit that referenced this issue Sep 24, 2015
…back into security) (weaverryan)

This PR was merged into the 2.8 branch.

Discussion
----------

New Guard Authentication System (e.g. putting the joy back into security)

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | at least partially: #14300, #11158, #11451, #10035, #10463, #8606, probably more
| License       | MIT
| Doc PR        | symfony/symfony-docs#5265

Hi guys!

Though it got much easier in 2.4 with `pre_auth`, authentication is a pain in Symfony. This introduces a new authentication provider called guard, with one goal in mind: put everything you need for *any* authentication system into one spot.

### How it works

With guard, you can perform custom authentication just by implementing the [GuardAuthenticatorInterface](https://github.com/weaverryan/symfony/blob/guard/src/Symfony/Component/Security/Guard/GuardAuthenticatorInterface.php) and registering it as a service. It has methods for every part of a custom authentication flow I can think of.

For a working example, see https://github.com/weaverryan/symfony-demo/tree/guard-auth. This uses 2 authenticators simultaneously, creating a system that handles [form login](https://github.com/weaverryan/symfony-demo/blob/guard-auth/src/AppBundle/Security/FormLoginAuthenticator.php) and [api token auth](https://github.com/weaverryan/symfony-demo/blob/guard-auth/src/AppBundle/Security/TokenAuthenticator.php) with a respectable amount of code. The [security.yml](https://github.com/weaverryan/symfony-demo/blob/guard-auth/app/config/security.yml) is also quite simple.

This also supports "manual login" without jumping through hoops: https://github.com/weaverryan/symfony-demo/blob/guard-auth/src/AppBundle/Controller/SecurityController.php#L45

I've also tested with "remember me" and "switch user" - no problems with either.

I hope you like it :).

### What's Needed

1) **Other Use-Cases?**: Please think about the code and try it. What use-cases are we *not* covering? I want Guard to be simple, but cover the 99.9% use-cases.

2) **Remember me** functionality cannot be triggered via manual login. That's true now, and it's not fixed, and it's tricky.

### Deprecations?

This is a new feature, so no deprecations. But, creating a login form with a guard authenticator is a whole heck of a lot easier to understand than `form_login` or even `simple_form`. In a perfect world, we'd either deprecate those or make them use "guard" internally so that we have just **one** way of performing authentication.

Thanks!

Commits
-------

a01ed35 Adding the necessary files so that Guard can be its own installable component
d763134 Removing unnecessary override
e353833 fabbot
dd485f4 Adding a new exception and throwing it when the User changes
302235e Fixing a bug where having an authentication failure would log you out.
396a162 Tweaks thanks to Wouter
c9d9430 Adding logging  on this step and switching the order - not for any huge reason
31f9cae Adding a base class to assist with form login authentication
0501761 Allowing for other authenticators to be checked
293c8a1 meaningless author and license changes
81432f9 Adding missing factory registration
7a94994 Thanks again fabbot!
7de05be A few more changes thanks to @iltar
ffdbc66 Splitting the getting of the user and checking credentials into two steps
6edb9e1 Tweaking docblock on interface thanks to @iltar
d693721 Adding periods at the end of exceptions, and changing one class name to LogicException thanks to @iltar
eb158cb Updating interface method per suggestion - makes sense to me, Request is redundant
c73c32e Thanks fabbot!
6c180c7 Adding an edge case - this should not happen anyways
180e2c7 Properly handles "post auth" tokens that have become not authenticated
873ed28 Renaming the tokens to be clear they are "post" and "pre" auth - also adding an interface
a0bceb4 adding Guard tests
05af97c Initial commit (but after some polished work) of the new Guard authentication system
330aa7f Improving phpdoc on AuthenticationEntryPointInterface so people that implement this understand it
@kgilden
Copy link
Contributor

kgilden commented Aug 12, 2016

I would like to work on resolving this. Yep, @weaverryan's suggestion works. However, wouldn't it be nice to simply configure both HTTP Basic and HTTP Digest and have them just work? Possible use cases seem few and far in between except for feature completeness. Another one would be to provide a graceful way to migrate from one method to the other (i.e. for REST API-s).

security:
  firewalls:
    main:
      http_basic: ~
      http_digest:
        secret: '%secret%'
        realm: my-realm

These authentication methods can in theory coexist. So I was thinking of writing some glue code to basically use a facade for the two entry point classes along the following lines:

class HttpAuthenticationEntryPoint implements AuthenticationEntryPointInterface

    // [...]

    /**
     * {@inheritdoc}
     */
    public function start(Request $request, AuthenticationException $authException = null)
    {
        $responseForDigest = $this->digest->start($request, $authException);
        $responseForBasic = $this->basic->start($request, $authException);

        $responseForDigest->headers->set('WWW-Authenticate', [
            $responseForDigest->headers->get('WWW-Authenticate'),
            $responseForBasic->headers->get('WWW-Authenticate')
        ]);

        return $responseForDigest;
    }

    // [...]
}

I have a working proof-of-concept so it seems doable. Would this be considered a valid fix? Also, could this by any chance be merged with 2.8?

@kgilden
Copy link
Contributor

kgilden commented Nov 18, 2016

ping @stof

@chalasr
Copy link
Member

chalasr commented Sep 8, 2017

This is a bug that should be fixed.
For instance, configuring guard along with http_digest breaks, because the default entry point for the firewall is automatically the guard authenticator, which is not a DigestAuthenticationEntryPoint.
We do create the entry point if there is no default entry point configured, so we should keep creating it if the default one is not an instance of DigestAuthenticationEntryPoint.
Related to lexik/LexikJWTAuthenticationBundle#384.

@chalasr
Copy link
Member

chalasr commented Sep 14, 2017

In fact, the entry point needs to be configured at the firewall level, there is no kind of per-listener entry point.

The current behavior is: if no entry_point configured at the firewall root level, then the first listener which registers an entry point is the one defining the firewall entry point.

So, Ryan's answer is the correct one. If you are using multiple listeners and at least one of them define an entry point but another (or your firewall) needs a specific entry point, you need to register the entry point as a service and make it the firewall entry point explicitly.

Here is how I solved it for this specific case (digest requiring specific entry point):

# security.yaml
firewalls:
    main:
        pattern:   ^/
        entry_point: app.digest_entry_point
        http_basic: ~
        http_digest:
            realm:    "%app.digest_realm%"
            secret:   "%kernel.secret%"
services.yaml
    app.digest_entry_point:
        class: Symfony\Component\Security\Http\EntryPoint\DigestAuthenticationEntryPoint
        arguments:
            - '%app.digest_realm%'
            - '%kernel.secret%'

Hope it helps, there is nothing we can do here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants