Inconsistency between HttpBasicAuthenticator and TokenAuthenticator for Unauthorized Response

[viraj-khatavkar]

@markstory I found inconsistency for unauthorized response between HttpBasicAuthenticator and TokenAuthenticator when the user is trying to access with the wrong credentials. I am not sure if this is intended or a bug. Let me explain the issue and a possible solution:

Here's how the unauthorizedChallenge is implemented for both the authenticators in this plugin:

### HttpBasicAuthenticator

public function unauthorizedChallenge(ServerRequestInterface $request): void
{
    if ($this->getConfig('skipChallenge')) {
        return;
    }

    throw new AuthenticationRequiredException($this->loginHeaders($request), '');
}

### TokenAuthenticator

public function unauthorizedChallenge(ServerRequestInterface $request): void
{
}

HttpBasicAuthenticaor responds with a 401 Unauthorized if identity is not found. While TokenAuthenticator responds with a 500 Error which isn't the right API response for the end users consuming the API. If we could modify the TokenAuthenticator as follows, it will resolve the issue:

public function unauthorizedChallenge(ServerRequestInterface $request): void
{
    throw new AuthenticationRequiredException([], '');
}

Let me know if this works, I can create a PR. I am also open to any other solution that might resolve this inconsistency and make it appropriate for the users who are consuming the API. As JwtAuthenticator extends TokenAuthenticator this should resolve the issue for that as well

[ADmad]

While TokenAuthenticator responds with a 500 Error

How did you end up with a 500 error? Authentication service does not throw any exception in case of authentication failure except for HTTP Basic/Digest Auth. It returns a result with failure status. A 401 Unauthorized with challenge headers is thrown only for HTTP Auth as it's specs dictates so.

[ADmad]

If you are using the ~~AuthComponent~~AuthenticationComponent then it would throw a UnauthenticatedException if it doesn't find the identity in the request.

[viraj-khatavkar]

@ADmad I am not using the AuthComponent - here's an example response with the TokenAuthenticator when a wrong token is passed

[ADmad]

Sorry I meant the AuthenticationComponent. How/where are you loading the component?

[viraj-khatavkar]

@ADmad I am doing it like this:

### Application.php::getAuthenticationService

$service->loadIdentifier('Authentication.Token', [
    'tokenField' => 'api_key'
]);

$service->loadAuthenticator('Authentication.Token', [
    'queryParam' => 'token',
    'header' => 'Authorization',
    'tokenPrefix' => 'Bearer'
]);

[viraj-khatavkar]

@ADmad I think I found the issue: even if the Result::FAILURE_IDENTITY_NOT_FOUND is returned from TokenAuthenticator the process goes on to the app's XyzController and it was failing there at $this->Authentication->getIdentityData(...)

But I am not sure why it goes to the controller even if the response is Result::FAILURE_IDENTITY_NOT_FOUND - it seems that in AuthenticationMiddleware::process this result isn't handled

[viraj-khatavkar]

Due to which the AuthenticationMiddleware continues it's call to the startupProcess of controller

[ADmad]

You haven't answered my question how/where are you loading the AuthenticationComponent?

[viraj-khatavkar]

@ADmad sorry, I am loading it in the AppController::initialize method:

$this->loadComponent('Authentication.Authentication', [
    'logoutRedirect' => '/'  // Default is false
]);

[ADmad]

Are you making the $this->Authentication->getIdentityData(...) call also in a controller's initialize() method?

[viraj-khatavkar]

@ADmad nope, it's in a normal get method

[ADmad]

That doesn't make sense. Before the controller's action is called the Controller.startup event is triggered. AuthenticationComponent::startup() would be run for that event which calls doIdentityCheck() method which throws the UnauthenticatedException. So the execution should never reach your action method.

[viraj-khatavkar]

let me check that flow once

[markstory]

Having a full stacktrace from where that 500 is coming from would help. You can see the source in that error message. AuthenticationComponent line 256. Adding stackTrace() at that line should output a trace to the response.

[viraj-khatavkar]

@markstory after following the stackTrace() it seems that it is failing at Authorization, but it shouldn't actually receive Authroization and fail at Authention itself - so still trying to figure it out!

Full Disclosure: We have a ControllerHookPolicy and ControllerResolver for supporting legacy isAuthorizaed methods

[othercorey]

@markstory after following the stackTrace() it seems that it is failing at Authorization, but it shouldn't actually receive Authroization and fail at Authention itself - so still trying to figure it out!

I don't understand. Are you saying authentication fails and then authorization fails also? Or are you saying authentication passes and authorization fails?

[github-actions[bot]]

This issue is stale because it has been open for 120 days with no activity. Remove the stale label or comment or this will be closed in 15 days