iainporter / oauth2-provider

Implementation of an oauth2 provider
Apache License 2.0
226 stars 181 forks source link

Issue #5 Authentification Required - should be reopened #10

Closed chartrand22 closed 10 years ago

chartrand22 commented 10 years ago

Issue #5 Authentification Required - should be reopened (or revisited)

The behaviour and response is different in these 2 scenarios :

1) login with INVALID username and INVALID password

2) login with CORRECT username and INVALID password

The fix only works for scenario 1), but why is a HTML response considered RESTful ?

chartrand22 commented 10 years ago

scenario 1) uses the OAuthRestEntryPoint fix and returns a HTTP401 with an HTML response

scenario 2) returns a HTTP 400 with this payload :

{"error":"invalid_grant","error_description":"Bad credentials"}

I would think scenario 2) is a better type of response for REST

chartrand22 commented 10 years ago

Oops, that was fixed by the Nov 2 check-in called "Delegate to Spring for Authentication failures "

chartrand22 commented 10 years ago

Well, now I don't see when and where the "OAuthRestEntryPoint" code gets used anymore. Should that now be removed ?

Also, is there a way to map this type of Oauth error response :

{"error":"invalid_grant","error_description":"Bad credentials"}

to com.porterhead.api.ErrorResponse ? This would keep things consistent in terms of error responses.

iainporter commented 10 years ago

Exception handling needs a bit of work. OAuthRestEntryPoint is invoked when the client credentials are incorrect. I added a json mapping class to handle it more elegantly

I'll catch up with the other exceptions next.

Thanks for the feedback. If you want to submit pull requests then please do.

chartrand22 commented 10 years ago

Are you sure it gets invoked? It doesn't for me but I might be missing some code changes.

iainporter commented 10 years ago

Just mung the Basic Authentication header and put a trace point in com.porterhead.security.OAuthRestEntryPoint

e.g.

curl -v -X POST -H "Content-Type: application/json" \ -H "Authorization: Basic MzUzYjMwMmM0NDU3NGY1NjUwNDU2ODdlNTM0ZTdkNmE6Mjg2OTI0Njk3ZTYxNWE2NzJhNjQ2YTQ5MzU0NTY0NmM=" \ -d '{"username":"foobar@example.com", "password":"password"}' \ 'http://localhost:8080/oauth2-provider/oauth/token'

change the header to this:

MzUzYjMwMmM0NDU3NGY1NjUwNDU2ODdlNTM0ZTdkNmE6Mjg2OTI0Njk3ZTYxNWE2NzJhNjQ2YTQ5MzU0NTY0xxx=

chartrand22 commented 10 years ago

Ok thanks, yes that picks it up. I was trying an incorrect username and/or password.
This can be closed, thanks !