Use cookies only when available #684
Conversation
|
Hello Maeda-san, thanks for the contribution! I haven't personally used Just curious, how do you authenticate API requests, with Regarding your implementation, it all makes sense at first glance. What suggestions to you have for how we can test these changes? We have |
|
Hello, Jared. Thank you for your response.
Yes, I plan to get single_access_token from params (or HTTP Authorization header, with monkey patching) for authentication.
I tried to add |
There was a problem hiding this comment.
I think this is close! I like how you've written an "adapter test" and a "persistence test".
- I've commented on a few lines that I don't understand. Please explain.
- Please add an entry to CHANGELOG.md under "Unreleased -> Added -> Support for ActionController::API"
- Please add a section
2.b.2. ActionController::APIto the readme. Please include a small, complete example of an API controller, and a link to the "single access token" documentation inbase.rb.
| @@ -14,7 +14,7 @@ def authenticate_with_http_basic(&block) | |||
| # Returns a `ActionDispatch::Cookies::CookieJar`. See the AC guide | |||
| # http://guides.rubyonrails.org/action_controller_overview.html#cookies | |||
| def cookies | |||
| controller.send(:cookies) | |||
| controller.respond_to?(:cookies, true) ? controller.send(:cookies) : nil | |||
There was a problem hiding this comment.
This looks good. I spent some time playing around with ActionController::API and I was able to reproduce the need for this.
| @@ -415,10 +415,10 @@ def self.#{method}(*filter_list, &block) | |||
| before_save :set_last_request_at | |||
|
|
|||
| after_save :reset_perishable_token! | |||
| after_save :save_cookie | |||
| after_save :save_cookie, if: :cookie_enabled? | |||
There was a problem hiding this comment.
This seems like a reasonable change, but without it the tests still pass, so is it really necessary?
| @@ -39,7 +39,7 @@ def params | |||
| end | |||
|
|
|||
| def request | |||
| @request ||= MockRequest.new(controller) | |||
| @request ||= MockRequest.new(self) | |||
There was a problem hiding this comment.
I don't understand this change. The tests pass without it. Can you please explain?
| @@ -9,6 +9,10 @@ def initialize(controller) | |||
| self.controller = controller | |||
| end | |||
|
|
|||
| def format | |||
| controller.request_content_type if controller.respond_to? :request_content_type | |||
There was a problem hiding this comment.
The conditional seems unnecessary. I can remove the if controller.respond_to? and the tests still pass. I guess because both of our "test controllers" respond to request_content_type?
|
Merged, thanks!
I've left a documentation place-holder at Let me know how it goes with |
|
@jaredbeck @epaew Do you mind to share example for It's still missing from the documentation. |
Because
ActionController::APIdoes not includeActionController::Cookiesmetal andActionDispatch::Cookiesrack module,Therefore, our controller can not use the
cookiesmethod.In this patch,
Authlogic::Session::Basecallscontroller#cookiesonly when available.