symfony / symfony

The Symfony PHP framework
https://symfony.com
MIT License
29.81k stars 9.47k forks source link

Simpler testing of security protected resources #26839

Closed javiereguiluz closed 4 years ago

javiereguiluz commented 6 years ago
Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? yes
Symfony version 4.2

The official Symfony solution to log in a user in the app before testing security-protected resources looks like this:

public function testSomething()
{
    $client = static::createClient();

    // --- log in the user ----------------------------------------------------
    $username = 'admin';
    $session = $client->getContainer()->get('session');

    // the firewall context defaults to the firewall name
    $firewallContext = 'secured_area';

    $token = new UsernamePasswordToken($username, null, $firewallContext, array('ROLE_ADMIN'));
    $session->set('_security_'.$firewallContext, serialize($token));
    $session->save();

    $cookie = new Cookie($session->getName(), $session->getId());
    $client->getCookieJar()->set($cookie);
    // --- end of user log in -------------------------------------------------

    // ... test something ...
}

I think we can do better. I'd like to have a different way to test security-protected resources. A solution that is simple to use and easy to understand. Thanks!

sstok commented 6 years ago

For Park-Manager I created a special Guard Authenticator that allows to provide the username and password (plain) as server variables:

```php namespace ParkManager\Bundle\UserBundle\Security; use ParkManager\Component\User\Security\SecurityUser; // UserInterface implementation use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Encoder\UserPasswordEncoderInterface; use Symfony\Component\Security\Core\Exception\AuthenticationException; use Symfony\Component\Security\Core\Exception\BadCredentialsException; use Symfony\Component\Security\Core\User\UserInterface; use Symfony\Component\Security\Core\User\UserProviderInterface; use Symfony\Component\Security\Guard\AbstractGuardAuthenticator; final class BrowserKitAuthenticator extends AbstractGuardAuthenticator { private $passwordEncoder; public function __construct(UserPasswordEncoderInterface $passwordEncoder) { $this->passwordEncoder = $passwordEncoder; } public function getCredentials(Request $request): array { return [ 'username' => $request->server->get('TEST_AUTH_USERNAME'), 'password' => $request->server->get('TEST_AUTH_PASSWORD'), 'password_new' => $request->server->get('TEST_AUTH_PASSWORD_NEW'), // This is only used when the password changes (properly need to fix this in a better way :-) ) ]; } public function getUser($credentials, UserProviderInterface $userProvider): ?SecurityUser { $email = $credentials['username']; if (null === $email) { return null; } return $userProvider->loadUserByUsername($email); } public function checkCredentials($credentials, UserInterface $user): bool { if (!$user->isEnabled()) { throw new AuthenticationException(); } if (!$this->passwordEncoder->isPasswordValid($user, $credentials['password']) && (null !== $credentials['password_new'] && !$this->passwordEncoder->isPasswordValid($user, $credentials['password_new'])) ) { throw new BadCredentialsException(); } return true; } public function onAuthenticationSuccess(Request $request, TokenInterface $token, $providerKey) { return null; } public function start(Request $request, AuthenticationException $authException = null): Response { return new Response('Auth header required', 401); } public function onAuthenticationFailure(Request $request, AuthenticationException $exception): Response { $data = [ 'message' => strtr($exception->getMessageKey(), $exception->getMessageData()), ]; return new JsonResponse($data, Response::HTTP_FORBIDDEN); } public function supportsRememberMe() { return false; } public function supports(Request $request) { return $request->server->has('TEST_AUTH_USERNAME'); } } ```

This needs to be registered after the firewalls are registered (in config/packages/test/security.yaml):

firewalls:
    main:
        guard:
            entry_point: 'ParkManager\Bundle\UserBundle\Security\BrowserKitAuthenticator'
            authenticators:
                - 'ParkManager\Bundle\UserBundle\Security\BrowserKitAuthenticator'

Used as:

$client = self::createClient([], [
    'TEST_AUTH_USERNAME' => 'rick',
    'TEST_AUTH_PASSWORD' => 'never-gonna-give-you-up',
]);

$crawler = $client->request('GET', '/profile');

Basically this works similar to HTTP Basic, but keeps the token after the current request (like a log-in form), so you only need it in createClient :+1: as a pre-condition.

I am not saying this is perfect but maybe this can be used as a step-up for something better (test security firewall listener or something?)

I believe the simplest solution is to set the username and password with the create client (as default) but allow them to change per request (for example a password reset/change operation), you can provide the new password to overwrite the client default (this covers the 90% use-case while still allowing the 10% exceptional use-case).

javiereguiluz commented 6 years ago

@sstok thanks for your proposal! However, I was thinking about something easier to learn and use. For example: login() and logout() methods:

// login when creating the client...
$client = static::createClient();
$client->login('the_username', 'the_password');

// ...or when making a request...
$crawler = $client->login('the_username', 'the_password')->request('GET', '/profile');

// logout for some special request...
$crawler = $client->logout()->request('GET', '/contact');

// ... or logout for all the next requests
$client->logout()

Other frameworks like Laravel also lets you pass an object representing the user:

$client->login(UserInterface $theUserObject)->...
linaori commented 6 years ago

I think the $client->login solution looks the cleanest tbh. Just have to figure how to define the firewall and token.

normisg commented 6 years ago

I just created a class in tests directory and put symfony suggested code in it

namespace App\Tests;

use Symfony\Bundle\FrameworkBundle\Test\WebTestCase;
use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken;
use Symfony\Component\BrowserKit\Cookie;
use Symfony\Component\Security\Core\User\UserInterface;

class ControllerTestCase extends WebTestCase
{

    /**
     * @param UserInterface $user
     * @return \Symfony\Bundle\FrameworkBundle\Client
     */
    protected static function createAuthClient(UserInterface $user)
    {
        $options = [];
        $server = [];
        $client = parent::createClient($options, $server);
        $session = $client->getContainer()->get('session');

        // the firewall context defaults to the firewall name
        $firewallContext = 'main';

        $token = new UsernamePasswordToken($user, null, $firewallContext, $user->getRoles());
        $session->set('_security_'.$firewallContext, serialize($token));
        $session->save();

        $cookie = new Cookie($session->getName(), $session->getId());
        $client->getCookieJar()->set($cookie);

        return $client;
    }
.
.
.

Than just used it when I need.

namespace App\Tests\Controller;

use App\Controller\SecurityController;

class SecurityControllerTest extends ControllerTestCase
{

/**
     * {@inheritdoc}
     */
    public function setUp()
    {
        $this->user = new User();
        ...
    }

    public function testUserCanNotOpenAdmin()
    {
        $this->user->setRoles(['ROLE_USER']);
        $client = self::createAuthClient($this->user);
        $client->request('GET', '/admin');
        $this->assertTrue($client->getResponse()->isForbidden());
    }

.
.
.
Pierstoval commented 5 years ago

I also implemented something similar here: https://gist.github.com/Pierstoval/db09556cff141aada5b608a26b23f260 , there's one for native WebTestCase and another one for Panther.

Most implementations tend to use the same kind of system.

To me, a clean solution would be to implement a mix of all existing solutions, and provide an additionnal argument for the firewall name. We could even make the firewall name totally optional and default to "the first firewall with active security" ( = not the "dev" one).

Also, important thing here, I'm not sure this logic should be put in the Client class, but rather in the WebTestCase itself, with a trait maybe, to make it customizable by the end user (which wouldn't be possible if put in the Client class)