Refactor ActiveRecord::TokenFor to not rely on relation delegation

Ref: https://github.com/rails/rails/pull/50396
Ref: https://github.com/rails/rails/pull/51776

`ActiveRecord::Relation` automatically delegates missing methods
to the model class wrapped in a `scoping { }` block.

This is to support scoping in user defined class methods. The problem
however is that it's very error prone for the framework, because we
can mistakenly call model methods from inside `Relation` and not
realized we're applying a global scope.

In the best case scenario it's just a waste of performance, but
it can also lead to bugs like https://github.com/rails/rails/issues/51775

I'm planning to restrict this automatic delegation to methods defined
in childs of `ActiveRecord::Base` only: https://github.com/rails/rails/pull/50396
but for this to work we must first refactor any Rails code that rely on it.
This commit is contained in:
Jean Boussier 2024-05-13 10:53:18 +09:00
parent 41888b1c79
commit c08c2efda2
2 changed files with 23 additions and 13 deletions

@ -66,7 +66,7 @@ def exec_explain(&block)
include Enumerable
include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches, Explain, Delegation
include SignedId::RelationMethods
include SignedId::RelationMethods, TokenFor::RelationMethods
attr_reader :table, :klass, :loaded, :predicate_builder
attr_accessor :skip_preloading_value

@ -35,6 +35,24 @@ def resolve_token(token)
end
end
module RelationMethods
# Finds a record using a given +token+ for a predefined +purpose+. Returns
# +nil+ if the token is invalid or the record was not found.
def find_by_token_for(purpose, token)
raise UnknownPrimaryKey.new(self) unless model.primary_key
model.token_definitions.fetch(purpose).resolve_token(token) { |id| find_by(model.primary_key => id) }
end
# Finds a record using a given +token+ for a predefined +purpose+. Raises
# ActiveSupport::MessageVerifier::InvalidSignature if the token is invalid
# (e.g. expired, bad format, etc). Raises ActiveRecord::RecordNotFound if
# the token is valid but the record was not found.
def find_by_token_for!(purpose, token)
model.token_definitions.fetch(purpose).resolve_token(token) { |id| find(id) } ||
(raise ActiveSupport::MessageVerifier::InvalidSignature)
end
end
module ClassMethods
# Defines the behavior of tokens generated for a specific +purpose+.
# A token can be generated by calling TokenFor#generate_token_for on a
@ -85,20 +103,12 @@ def generates_token_for(purpose, expires_in: nil, &block)
self.token_definitions = token_definitions.merge(purpose => TokenDefinition.new(self, purpose, expires_in, block))
end
# Finds a record using a given +token+ for a predefined +purpose+. Returns
# +nil+ if the token is invalid or the record was not found.
def find_by_token_for(purpose, token)
raise UnknownPrimaryKey.new(self) unless primary_key
token_definitions.fetch(purpose).resolve_token(token) { |id| find_by(primary_key => id) }
def find_by_token_for(purpose, token) # :nodoc:
all.find_by_token_for(purpose, token)
end
# Finds a record using a given +token+ for a predefined +purpose+. Raises
# ActiveSupport::MessageVerifier::InvalidSignature if the token is invalid
# (e.g. expired, bad format, etc). Raises ActiveRecord::RecordNotFound if
# the token is valid but the record was not found.
def find_by_token_for!(purpose, token)
token_definitions.fetch(purpose).resolve_token(token) { |id| find(id) } ||
(raise ActiveSupport::MessageVerifier::InvalidSignature)
def find_by_token_for!(purpose, token) # :nodoc:
all.find_by_token_for!(purpose, token)
end
end