Remove the 'ResponseFactoryInterface $responseFactory' parameter from the 'Authentication' constructor

[klsoft-web]

Proposed new feature or change

The 'ResponseFactoryInterface $responseFactory' parameter is proposed for removal from the 'Authentication' constructor in order to simplify the 'Authentication' configuration, while retaining the same logic.

Current:

final class Authentication implements MiddlewareInterface
{
    // ...
    public function __construct(
        private AuthenticatorInterface|AuthenticationMethodInterface $authenticationMethod,
        ResponseFactoryInterface $responseFactory,
        ?RequestHandlerInterface $authenticationFailureHandler = null,
    ) {
        $this->failureHandler = $authenticationFailureHandler ?? new AuthenticationFailureHandler(
            $responseFactory,
        );
    }

    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
    {
        // ...
        if ($identity === null && !$this->isOptional($request)) {
            $response = $this->failureHandler->handle($request);
            // ...
        }
        // ...
    }
    // ...
}

Proposed:

final class Authentication implements MiddlewareInterface
{
    // ...
    public function __construct(
        private AuthenticatorInterface|AuthenticationMethodInterface $authenticationMethod,
        private RequestHandlerInterface $authenticationFailureHandler
    ) {}

    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
    { 
        // ...
        if ($identity === null && !$this->isOptional($request)) {
            $response = $this->authenticationFailureHandler->handle($request);
            // ...
        }
        // ...
    }
    // ...
}

//config/di-web.php
use App\Web\Auth\AuthenticationFailureHandler;
use Psr\Http\Server\RequestHandlerInterface;

return [
    RequestHandlerInterface::class => AuthenticationFailureHandler::class
];

[vjik]

I like this idea, but it's better configure middleware:

//config/di-web.php
return [
    Authentication::class => [
        '__construct()' => [
            'authenticationFailureHandler' => Reference::to(AuthenticationFailureHandler::class),
        ],
    ],
];

⚠️ Don't implement it before start version 4.0 development.

[klsoft-web]

I like this idea, but it's better configure middleware:

//config/di-web.php
return [
Authentication::class => [
'__construct()' => [
'authenticationFailureHandler' => Reference::to(AuthenticationFailureHandler::class),
],
],
];

⚠️ Don't implement it before start version 4.0 development.

I don't agree that it's better, since the 'ResponseFactoryInterface $responseFactory' parameter is still required to configure authentication via a closure:

//config/di-web.php

use Psr\Http\Message\ResponseFactoryInterface;
use Yiisoft\Access\AccessCheckerInterface;
use Yiisoft\Auth\AuthenticationMethodInterface;
use Yiisoft\Auth\AuthenticatorInterface;
use Yiisoft\Auth\Middleware\Authentication;
use Yiisoft\User\Method\WebAuth;


return [
    // ...
    AuthenticatorInterface::class => WebAuth::class,
    Authentication::class => static function (ContainerInterface $container) {
        return new Authentication(
            authenticationMethod: $container->get(AuthenticatorInterface::class),
            responseFactory: $container->get(ResponseFactoryInterface::class),
            authenticationFailureHandler: $container->get(MyAuthenticationFailureHandler::class)
        );
    },
];

and this part of the code looks redundant:

final class Authentication implements MiddlewareInterface
{
   // ...
    public function __construct(
        private AuthenticatorInterface|AuthenticationMethodInterface $authenticationMethod,
        ResponseFactoryInterface $responseFactory,
        ?RequestHandlerInterface $authenticationFailureHandler = null,
    ) {
        $this->failureHandler = $authenticationFailureHandler ?? new AuthenticationFailureHandler(
            $responseFactory,
        );
    }
}

[vjik]

I don't agree that it's better, since the 'ResponseFactoryInterface $responseFactory' parameter is still required to configure authentication via a closure:

I agree with you about removing $responseFactory parameter, but suggest another configuration.

[klsoft-web]

Thank you for your suggestion!
Did you add the issue to the 4.0 milestone if you agree with removing it?

[vjik]

Let's wait for the other maintainers' opinions.

[klsoft-web]

Let me explain the main purpose of this issue.
In a real-world application, the 'redirect_uri' parameter is must-have for redirecting to the login page.

In the current version of Yiisoft\Auth\Middleware\Authentication, you are forced to write this configuration:

//config/web/di/application.php
return [
    Authentication::class => [
        '__construct()' => [
            'authenticationFailureHandler' => Reference::to(AuthenticationFailureWithRedirectToLoginUrl::class),
        ],
    ],
];

After the proposed solution:

//yiisoft/auth Middleware\Authentication.php
final class Authentication implements MiddlewareInterface
{
    // ...
    public function __construct(
        private AuthenticatorInterface|AuthenticationMethodInterface $authenticationMethod,
        private RequestHandlerInterface $authenticationFailureHandler
    ) {}

    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
    { 
        // ...
        if ($identity === null && !$this->isOptional($request)) {
            $response = $this->authenticationFailureHandler->handle($request);
            // ...
        }
        // ...
    }
    // ...
}

//yiisoft/user, yiisoft/yii-auth-session AuthenticationFailureWithRedirectToLoginUrl.php
use App\Data\Auth\AuthRepositoryInterface;
use Psr\Http\Message\ResponseFactoryInterface;
use Psr\Http\Message\ResponseInterface;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Server\RequestHandlerInterface;
use Yiisoft\Http\Status;
use Yiisoft\Router\UrlGeneratorInterface;

final readonly class AuthenticationFailureWithRedirectToLoginUrl implements RequestHandlerInterface
{
    private string $loginRouteName = 'login';
     
    public function __construct(
        private AuthRepositoryInterface  $authRepository,
        private ResponseFactoryInterface $responseFactory,
        private UrlGeneratorInterface    $urlGenerator)
    {
    }

    public function handle(ServerRequestInterface $request): ResponseInterface
    {
        $queryParameters = ['redirect_uri' => $request->getUri()->__toString()];

        return $this->responseFactory
            ->createResponse(Status::FOUND)
            ->withHeader('Location', $this->urlGenerator->generate($this->loginRouteName, [], $queryParameters));
    }
    
    public function withLoginRouteName(string $name): self
    {
        $new = clone $this;
        $new->loginRouteName = $name;
        return $new;
    }
}

//yiisoft/user, yiisoft/yii-auth-session config/di-web.php
use Yiisoft\Auth\AuthenticatorInterface;
use Psr\Http\Server\RequestHandlerInterface;

return [
    // ...
    AuthenticatorInterface::class => WebAuth::class,
    RequestHandlerInterface::class => AuthenticationFailureWithRedirectToLoginUrl::class
];

You will get a real-world application by default, with no need to write a single line of code!

[samdark]

Sounds good to me 👍

[klsoft-web]

Sounds good to me 👍

Glad to hear it!
Did you add the issue to the 4.0 or 3.4 milestone?

I'll also remind you that, despite the fact that the issue breaks backwards compatibility, but only if you use the $responseFactory parameter when configuring Yiisoft\Auth\Middleware\Authentication.

//config/web/di/application.php
use Psr\Http\Message\ResponseFactoryInterface;
use Yiisoft\Access\AccessCheckerInterface;
use Yiisoft\Auth\AuthenticationMethodInterface;
use Yiisoft\Auth\AuthenticatorInterface;
use Yiisoft\Auth\Middleware\Authentication;
use Yiisoft\User\Method\WebAuth;


return [
    // ...
    AuthenticatorInterface::class => WebAuth::class,
    Authentication::class => static function (ContainerInterface $container) {
        return new Authentication(
            authenticationMethod: $container->get(AuthenticatorInterface::class),
            responseFactory: $container->get(ResponseFactoryInterface::class),
            authenticationFailureHandler: $container->get(MyAuthenticationFailureHandler::class)
        );
    },
];

and, you can still resolve the issue of breaking backwards compatibility:

//config/web/di/application.php
return [
    Authentication::class => [
        '__construct()' => [
            'authenticationFailureHandler' => Reference::to(MyAuthenticationFailureHandler::class),
        ],
    ],
];

However, if you use the standard configuration:

//config/web/di/application.php
use Yiisoft\Auth\AuthenticationMethodInterface;
use Yiisoft\User\Method\WebAuth;

return [
    // ...
    AuthenticationMethodInterface::class => WebAuth::class,
];

or like this:

//config/web/di/application.php
return [
    Authentication::class => [
        '__construct()' => [
            'authenticationFailureHandler' => Reference::to(MyAuthenticationFailureHandler::class),
        ],
    ],
];

You get the same result.

So while the backwards compatibility issue affects a small number of users and you are still resolving it, you will get an additional feature that allows you to configure the RequestHandlerInterface.

[samdark]

I've actually rechecked, and there is a problem that shaped the current state. We have two valid use cases. One is for the web, and another is for APIs. And for APIs, redirect should not happen. It should be a 401 response instead. So the default should stay as is, 401.

[samdark]

As for version, since that's a backwards compatibility break, I would go with 4.0.

An alternative is to release 3.4 but with the parameter deprecated and nullable.

[klsoft-web]

I've actually rechecked, and there is a problem that shaped the current state. We have two valid use cases. One is for the web, and another is for APIs. And for APIs, redirect should not happen. It should be a 401 response instead. So the default should stay as is, 401.

Of course, it will stay as it is. Sorry, I made a typo in the namespace of the AuthenticationFailureHandler in the issue description. The correct code is as follows:

//config/di-web.php
use Yiisoft\Auth\Handler\AuthenticationFailureHandler;
use Psr\Http\Server\RequestHandlerInterface;

return [
    RequestHandlerInterface::class => AuthenticationFailureHandler::class
];

[samdark]

RequestHandlerInterface::class => AuthenticationFailureHandler::class

Wait, but RequestHandlerInterface is a generic interface, not specific for auth.

[klsoft-web]

Wait, but RequestHandlerInterface is a generic interface, not specific for auth.

I agree it's a reasonable concern.
What about changing the type of the authenticationFailureHandler parameter in the Yiisoft\Auth\Middleware\Authentication class to AuthenticationFailureHandlerInterface?

//Handler\AuthenticationFailureHandlerInterface.php
interface AuthenticationFailureHandlerInterface extends RequestHandlerInterface {}


//Handler\AuthenticationFailureHandler.php
final class AuthenticationFailureHandler implements AuthenticationFailureHandlerInterface
{
    // ...
}

//Middleware\Authentication.php
final class Authentication implements MiddlewareInterface
{
    // ...
    public function __construct(
        private AuthenticatorInterface|AuthenticationMethodInterface $authenticationMethod,
        private AuthenticationFailureHandlerInterface $authenticationFailureHandler
    ) {}

    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
    { 
        // ...
        if ($identity === null && !$this->isOptional($request)) {
            $response = $this->authenticationFailureHandler->handle($request);
            // ...
        }
        // ...
    }
    // ...
}

//config/di-web.php
use Yiisoft\Auth\Handler\AuthenticationFailureHandler;
use Yiisoft\Auth\Handler\AuthenticationFailureHandlerInterface;

return [
    AuthenticationFailureHandlerInterface::class => AuthenticationFailureHandler::class
];