Fix handle_auth_errors :raise NotImplementedError

[camero2734]

Summary

Fixes #1679, and follow-up to #1676

ErrorResponse requires a subclass to override its exception_class method, otherwise it throws an error when raise_exception! is called. However, with the recent change in #1676, we call ErrorResponse#raise_exception! quite often, which causes NotImplementedErrors to be raised when handle_auth_errors is set to :raise

This PR fixes this by creating a new Error type for each standard OAuth error that Doorkeeper handles, so that you can rescue specific errors when in :raise mode.

Instead of validate taking a symbol, it now expects a class that extends BaseResponseError:

# Before
validate :something,  error: :some_symbol

# After
validate :something,  error: Errors::Something

This accounts for the majority of the changes in the PR. This is the error that will be thrown in :raise mode.

Possible Improvements

I just recovered the symbol name (which is used for the error query param according to the spec) via Class.to_s.demodulize.underscore, but perhaps we would like to explicitly provide this when constructing the classes.

Alternatives

We could create a XXXErrorResponse for every OAuth error, such as InvalidClientErrorResponse and InvalidGrantErrorResponse

We could just remove the NotImplementedError from ErrorResponse#exception_class and replace it with a new, single error type like Errors::GenericOAuth or something

[nbulaj]

Looks dangerous 😟 need some proper review

[camero2734]

@nbulaj Hmmm, if you'd prefer we could just do the last alternative, which is to just remove the NotImplementedError and just create a single Errors::GenericOAuth that's raised. Not too big of an issue since you can find out the type of error via error.response.name I believe. Probably only a few files changed in that route as compared to 20+ 😅

ETA: Simpler one is here just in case main...camero2734:doorkeeper:fix-not-implemented-error-raise-generic

[nbulaj]

Yeah actually looks good, nice work 💪 Have to just check openid-connect gem - maybe it uses some custom errors and we can break something for it.

[camero2734]

@nbulaj After the last commit I pushed, I got all ✅ in openid-connect gem tests locally, just had to change these tests:

--- a/spec/controllers/doorkeeper/authorizations_controller_spec.rb
+++ b/spec/controllers/doorkeeper/authorizations_controller_spec.rb
@@ -82,14 +82,14 @@ describe Doorkeeper::AuthorizationsController, type: :controller do
         it 'render error when client_id is missing' do
           authorize!(client_id: nil)

-          expect(response).to be_successful
+          expect(response).to have_http_status(:bad_request)
           expect(response).to render_template('doorkeeper/authorizations/error')
         end

         it 'render error when response_type is missing' do
           authorize!(response_type: nil)

-          expect(response).to be_successful
+          expect(response).to have_http_status(:bad_request)
           expect(response).to render_template('doorkeeper/authorizations/error')
         end
       end

which is just a result of the last PR's status code changes

[nbulaj]

LGTM 👍