search

zfcampus / zf-oauth2

OAuth2 server module for ZF2
BSD 3-Clause "New" or "Revised" License
104 stars 92 forks source link

Exception thrown by bshaffer/oauth2-server-php is not caught #79

Closed metanav closed 8 years ago

metanav commented 9 years ago

If any exception is thrown by bshaffer/oauth2-server-php methods, it is not get captured and only we see an exception was thrown in the php error log. We should capture all exceptions thrown by bshaffer/oauth2-server-php method calls and convert them into ApiProblemResponse.

alfaproject commented 8 years ago

I have forked tag 1.2.1 (because we are still using PHP 5.4 for a few more weeks at least), to be able to accomplish this on login.

Here is the commit: https://github.com/cherrytech/zf-oauth2/commit/bafff4dffee73e08ef02262c24f5a2b3aa9682f3

I was wondering if I should trap other exception points, and make unit tests for this, or should I give up? Currently this is being used on production on one of our products and works wonders. (:

The use case is to be able to show different error messages to our customers like: 1) you are banned 2) your account is locked temporarily 3) your account is not activated 4) etc

We have a custom OAuth adapter, so throwing an exception and catching it to throw an ApiProblem was the best solution I could find.

@weierophinney what do you think about this? Is it something you guys thought of?

metanav commented 8 years ago

@alfaproject Does it catch all kind of exceptions thrown by bshaffer/oauth2-server-php?

alfaproject commented 8 years ago

It should, yes.

jguittard commented 8 years ago

@alfaproject that's an interesting idea IMO. Would you consider adapting your fork to 1.3.2 and submit a PR? (btw, unit tests are of course expected ;)) @ezimuel could you give your advice on this one ? I'd go for it.