-
Notifications
You must be signed in to change notification settings - Fork 641
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use cookies only when available #684
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems like a reasonable change, but without it the tests still pass, so is it really necessary? |
||
after_save :update_session | ||
|
||
after_destroy :destroy_cookie | ||
after_destroy :destroy_cookie, if: :cookie_enabled? | ||
after_destroy :update_session | ||
|
||
# `validate` callbacks, in deliberate order. For example, | ||
|
@@ -1611,12 +1611,18 @@ def cookie_key | |
# @api private | ||
# @return ::Authlogic::CookieCredentials or if no cookie is found, nil | ||
def cookie_credentials | ||
return unless cookie_enabled? | ||
|
||
cookie_value = cookie_jar[cookie_key] | ||
unless cookie_value.nil? | ||
::Authlogic::CookieCredentials.parse(cookie_value) | ||
end | ||
end | ||
|
||
def cookie_enabled? | ||
!controller.cookies.nil? | ||
end | ||
|
||
def cookie_jar | ||
if self.class.sign_cookie | ||
controller.cookies.signed | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
# frozen_string_literal: true | ||
|
||
module Authlogic | ||
module TestCase | ||
# Basically acts like an API controller but doesn't do anything. | ||
# Authlogic can interact with this, do it's thing and then you can look at | ||
# the controller object to see if anything changed. | ||
class MockAPIController < ControllerAdapters::AbstractAdapter | ||
attr_writer :request_content_type | ||
|
||
def initialize | ||
end | ||
|
||
# Expected API controller has no cookies method. | ||
undef :cookies | ||
|
||
def cookie_domain | ||
nil | ||
end | ||
|
||
def logger | ||
@logger ||= MockLogger.new | ||
end | ||
|
||
def params | ||
@params ||= {} | ||
end | ||
|
||
def request | ||
@request ||= MockRequest.new(self) | ||
end | ||
|
||
def request_content_type | ||
@request_content_type ||= "text/html" | ||
end | ||
|
||
def session | ||
@session ||= {} | ||
end | ||
|
||
# If method is defined, it causes below behavior... | ||
# controller = Authlogic::ControllerAdapters::RailsAdapter.new( | ||
# Authlogic::TestCase::MockAPIController.new | ||
# ) | ||
# controller.responds_to_single_access_allowed? #=> true | ||
# controller.single_access_allowed? | ||
# #=> NoMethodError: undefined method `single_access_allowed?' for nil:NilClass | ||
# | ||
undef :single_access_allowed? | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,7 +39,7 @@ def params | |
end | ||
|
||
def request | ||
@request ||= MockRequest.new(controller) | ||
@request ||= MockRequest.new(self) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand this change. The tests pass without it. Can you please explain? |
||
end | ||
|
||
def request_content_type | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The conditional seems unnecessary. I can remove the |
||
end | ||
|
||
def ip | ||
controller&.respond_to?(:env) && | ||
controller.env.is_a?(Hash) && | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. I spent some time playing around with
ActionController::API
and I was able to reproduce the need for this.