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

[RFC] Make easier configuration of basic auth #2

Open
Nek- opened this issue Oct 15, 2018 · 2 comments
Open

[RFC] Make easier configuration of basic auth #2

Nek- opened this issue Oct 15, 2018 · 2 comments

Comments

@Nek-
Copy link
Contributor

Nek- commented Oct 15, 2018

This component is supposed to help you bootstrap a project with basic auth. But it only provides a guard that supports basic authentication.

We need to add this controller and its route:

public function __construct(JwtGeneratorInterface $generateJwtTokenForAuthentication, AuthenticationSuccessHandlerInterface $authenticationSuccessHandler)
{
    $this->generateJwtTokenForAuthentication = $generateJwtTokenForAuthentication;
    $this->authenticationSuccessHandler      = $authenticationSuccessHandler;
}

public function __invoke()
{
    $user = $this->getUser();

    $user->setLoginAt(new \DateTimeImmutable());
    $user->setToken($this->generateJwtTokenForAuthentication->create($user));

    $response        = $this->authenticationSuccessHandler->handleAuthenticationSuccess($user, $user->getToken());
    $decodedResponse = json_decode($response->getContent(), true);
    $user->setRefreshToken($decodedResponse['refresh_token']);

    return $user;
}

This controller (and some other classes of this package) needs a JwtGenerator that the user must define. No doc about (#1) and weird errors. Here is an example of implementation.

class StandardJwtGenerator implements JwtGeneratorInterface
{
    /**
     * @var JWTEncoderInterface
     */
    private $encoder;

    /**
     * @var string|\DateTime
     */
    private $expiration;

    /**
     * GenerateJwtTokenForAuthentication constructor.
     *
     * @param JWTEncoderInterface $encoder
     */
    public function __construct(JWTEncoderInterface $encoder, $expiration = '+1 day')
    {
        $this->encoder    = $encoder;
        $this->expiration = $expiration;
    }

    /**
     * @param UserInterface $user
     *
     * @return string
     */
    public function create(UserInterface $user)
    {
        $expiration = $this->expiration;
        if (!$expiration instanceof \DateTimeInterface) {
            $expiration = new \DateTime($this->expiration);
        }

        $payload = [
            'id'    => $user->getId(),
            'email' => $user->getEmail(),
            'exp'   => $expiration->getTimestamp(),
        ];

        return $this->encoder->encode($payload);
    }
}

Aliasing interfaces is also required:

services:
    Biig\Component\User\Jwt\JwtGeneratorInterface: '@App\Domain\User\Security\StandardJwtGenerator'
    Biig\Component\User\Persistence\RepositoryInterface: '@App\Infrastructure\ORM\User\Repository\UserRepository'

This is a problem because it removes the possibility to have many services within the interface. It should be done via configuration and automatic creation of services.

This is absolutely not documented. And it could be added automatically by the bundle with a little line of configuration... That modifies the behavior of the guard which is a little bit too much hardcoded. See here:

return 1 === \preg_match('/login$/', $request->getPathInfo()) && $this->isAllowedRequest($request);

So, to summarize here is what to do:

  1. Add the given controller
  2. Add a way of configuration to enable service and controller (basic auth to start)
  3. Document this
@Awkan
Copy link
Contributor

Awkan commented Oct 15, 2018

I agree with this 👍

  • Component should provide a User model (eg. User model which doesn't have setLoginAt method)
  • Indeed guard is too much hardcoded which leads to some problems (eg. force user to finish it login url with /login). This should be passed at least as configuration params

@Nek-
Copy link
Contributor Author

Nek- commented Oct 15, 2018

@Awkan disagree with the user model. This is far too much specific. (btw it's one of the biggest problems of FOSUserBundle)

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

No branches or pull requests

2 participants