Allow controller level authentication#175
Conversation
mcelicalderon
force-pushed
the
allow-controller-level-authentication
branch
from
a69a763 to
941cf68
Compare
mcelicalderon
force-pushed
the
allow-controller-level-authentication
branch
from
e4f7e6a to
82155a8
Compare
| def self.mount_resource(mapping_name) | ||
| @mounted_resources << mapping_name | ||
| def self.mount_resource(model) | ||
| @mounted_resources << model |
There was a problem hiding this comment.
Should we check the resource is not already there before adding?
There was a problem hiding this comment.
we do on the resource_loader
There was a problem hiding this comment.
I think it would less error prone if the check is closer to the assignation, like with add_mapping bellow where the key is looked among the devise mapping before adding
There was a problem hiding this comment.
With add_mapping it is important to prevent adding a new Devise mapping as it has mnay implications, with this one, no real problem if we ever mount the same model more than once. I think it would just add unnecessary overhead to check the list each time we mount a resource. This array is only used to later check the model is or not already on the list.
mcelicalderon
force-pushed
the
allow-controller-level-authentication
branch
from
148e000 to
82155a8
Compare
| end | ||
|
|
||
| def resource_class(resource = nil) | ||
| return resource if resource.respond_to?(:find_by) |
There was a problem hiding this comment.
Can we add a comment here indicating this is meant to assert an AR or MongoId instance?
| def self.mount_resource(mapping_name) | ||
| @mounted_resources << mapping_name | ||
| def self.mount_resource(model) | ||
| @mounted_resources << model |
There was a problem hiding this comment.
I think it would less error prone if the check is closer to the assignation, like with add_mapping bellow where the key is looked among the devise mapping before adding
| end | ||
|
|
||
| def resource_class(resource = nil) | ||
| # Return the resource class instead of looking for a Devise mapping if resource is already a resource class |
There was a problem hiding this comment.
I think this comment should mention find_by is meant to use duck typing to indentify AR and Mongo ID models.
|
|
||
| def resource_class(resource = nil) | ||
| # Return the resource class instead of looking for a Devise mapping if resource is already a resource class | ||
| return resource if resource.respond_to?(:find_by) |
There was a problem hiding this comment.
In which cases will this not be an AR or Mongo Id model?
mcelicalderon
force-pushed
the
allow-controller-level-authentication
branch
from
5c935cd to
2155f5d
Compare
2 participants
Right now authentication happens during GQL query execution and there's no good way to authenticate at the controller level first.
This change will allow controllers so authenticate a resource before reaching the GQL schema. This could be useful in scenarios like the one discussed in #151
This way you will be able to send
current_resourceto the GQL context and I think we will probably deprecate not sending that one when you expect the resource to be authenticated.