Currently oauth2 errors end up in a plain text message being displayed on a blank page. It could be better if errors are handled more gracefully.

CommentFileSizeAuthor
#4 oauth2_server-2302391-garceful_error_handeling-4.patch700 bytesdashohoxha
#1 oauth2_server-2302391-garceful_error_handeling-1.patch1.02 KBdashohoxha

Comments

dashohoxha’s picture

dashohoxha commented
Status: Active » Needs review
StatusFileSize
new oauth2_server-2302391-garceful_error_handeling-1.patch1.02 KB

This patch tries to improve error handeling.

bojanz’s picture

bojanz commented
Status: Needs review » Closed (won't fix)

These errors are thrown only from the oauth2 endpoints, which are primarily meant to be consumed by code.
By making a Drupal page render around the error, you've essentially made it unparsable, breaking the tests and making our behavior completely different from the library one.

Hence, "won't fix".

An end-user would see an error directly only in cases of misconfiguration, which is rare enough to be irrelevant.

dashohoxha’s picture

dashohoxha commented

I agree with your argument, and it could break some existing code.
But at least you could give a chance to the application to do something different in case of certain errors:

function oauth2_server_send_response(OAuth2\Response $response) {
  if ($response->getStatusCode() != 200) {
    module_invoke_all('oauth2_server_error', $response->getStatusCode(), $response->getParameters());
  }
  . . . . .
}

This does not break the tests or the existing code, but still gives a chance to applications to display more informative messages in certain cases (by implementing hook_oauth2_server_error).

One case like this is when redirect_uri is wrong (for example client has moved to another location). Since the server cannot redirect, the error message will never reach the client application.

dashohoxha’s picture

dashohoxha commented
Status: Closed (won't fix) » Needs review
StatusFileSize
new oauth2_server-2302391-garceful_error_handeling-4.patch700 bytes

Here is an updated patch.
However this feature is not so important, so you can close it again with "won't fix".

dashohoxha’s picture

dashohoxha commented
Status: Needs review » Closed (won't fix)