[ELY-2548] BasicAuthenticationMechanism should return FORBIDDEN inste… #1899
Conversation
There was a problem hiding this comment.
Hi @keshav-725 , looks good, can you please add a simple test for it in this PR? I know you have tests with your HttpClient implementation but it would be good to have a test related to this change in this PR as well.
The test you add here can have a hard coded value in the Authorization header and you can update the callbackhandler in AbstractBaseHttpTest class to handle an unauthorized user. Thanks!
| @@ -170,7 +171,7 @@ public void evaluateRequest(final HttpServerRequest request) throws HttpAuthenti | |||
| httpBasic.debugf("User %s authorization failed.", username); | |||
| fail(); | |||
|
|
|||
| request.authenticationFailed(httpBasic.authorizationFailed(username), response -> prepareResponse(request, displayRealmName, response)); | |||
| request.authenticationFailed(httpBasic.authorizationFailed(username), httpResponse -> httpResponse.setStatusCode(HttpConstants.FORBIDDEN)); | |||
There was a problem hiding this comment.
hi @keshav-725 , just a nitpick, but just to be consistent with the lines below (line 182) and above in this class I would rename to response :
response -> response.setStatusCode(HttpConstants.FORBIDDEN));
|
@keshav-725 Please squash the commits into 1, otherwise this LGTM, thanks! |
…ad of UNAUTHORIZED
Hi @Skyllarr ,squashed the commit.Thanks! |
|
@keshav-725 Thanks! |
|
Superseded by #2108 |
https://issues.redhat.com/browse/ELY-2548