Closed viraj-khatavkar closed 1 year ago
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.
If you are using the AuthComponentAuthenticationComponent
then it would throw a UnauthenticatedException
if it doesn't find the identity in the request.
@ADmad I am not using the AuthComponent
- here's an example response with the TokenAuthenticator
when a wrong token is passed
Sorry I meant the AuthenticationComponent
. How/where are you loading the component?
@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'
]);
@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
Due to which the AuthenticationMiddleware
continues it's call to the startupProcess
of controller
You haven't answered my question how/where are you loading the AuthenticationComponent?
@ADmad sorry, I am loading it in the AppController::initialize
method:
$this->loadComponent('Authentication.Authentication', [
'logoutRedirect' => '/' // Default is false
]);
Are you making the $this->Authentication->getIdentityData(...)
call also in a controller's initialize()
method?
@ADmad nope, it's in a normal get method
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.
let me check that flow once
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.
@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
@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?
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
@markstory I found inconsistency for unauthorized response between
HttpBasicAuthenticator
andTokenAuthenticator
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:HttpBasicAuthenticaor
responds with a401 Unauthorized
if identity is not found. WhileTokenAuthenticator
responds with a500 Error
which isn't the right API response for the end users consuming the API. If we could modify theTokenAuthenticator
as follows, it will resolve the issue: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
extendsTokenAuthenticator
this should resolve the issue for that as well