Ensure the all_public_actions method works for Grape APIs #23
Conversation
| @@ -155,8 +155,14 @@ def this_is_an_abstract_controller_so_it_needs_no_access_tests | |||
| alias :access_tests_not_required :this_is_an_abstract_controller_so_it_needs_no_access_tests | |||
|
|
|||
| def all_public_actions | |||
| actions = controller_class.public_instance_methods(false) | |||
| actions += controller_class.superclass.public_instance_methods(false) | |||
| actions = [] | |||
There was a problem hiding this comment.
Could you update the README as well?
Currently it says:
Support for Rails 4 and 5
Instead, I think it should follow the Appraisals:
https://github.com/appfolio/ae_declarative_authorization/blob/master/Appraisals#L1-L2
There was a problem hiding this comment.
I agree it should be updated, but that seems unrelated to this change.
| actions += controller_class.superclass.public_instance_methods(false) | ||
| actions = [] | ||
| if defined?(Grape) && [Grape::API, Grape::API::Instance].any? { |base| controller_class < base } | ||
| actions += controller_class.routes.map { |api| "#{api.request_method} #{api.origin}" } |
There was a problem hiding this comment.
I don't really understand how this works but I guess it adds a string that is somehow parsed somewhere else?
There was a problem hiding this comment.
In rails controllers, actions are uniquely identified by their method name (eg index, show, etc). In Grape APIs the endpoints are not generated in the same way, so we instead need to use a combination of the HTTP Verb and Path.
This means authorization rules for APIs look like this:
role :tenant do
allowed to: ['GET /occupancies/:id'], when: [:read, :write_without_delete, :write]
endThe string GET /occupancies/:id is what is being checked here.
There was a problem hiding this comment.
Oh right, that string. Yeah makes sense.
|
Is there a way to add tests for this additional behavior? |
The
DeclarativeAuthorization::Test::Helpersmodule includes a method calledall_public_actionsthat is responsible for determining the set of publically available methods on a controller. This is used to verify that every public method has associated access tests.This method assumes that the controller is a Rails controller, so does not work correctly with Grape APIs. This means that it is possible for APIs to be missing authorization rules.
This PR fixes the issue by adjusting the
all_public_actionsmethod to behave differently when run against Grape APIs.