Commit be71e069 authored by Thong Kuah's avatar Thong Kuah

TokenAuthenticatableStrategies should register fields

This prevents knowledge of which fields are configured leaking into
SensitiveSerializableHash.
parent 48d4c8e8
...@@ -26,16 +26,6 @@ module SensitiveSerializableHash ...@@ -26,16 +26,6 @@ module SensitiveSerializableHash
options[:except].concat self.class.attributes_exempt_from_serializable_hash options[:except].concat self.class.attributes_exempt_from_serializable_hash
if self.class.respond_to?(:token_authenticatable_fields)
options[:except].concat self.class.token_authenticatable_fields
# See https://gitlab.com/gitlab-org/security/gitlab/-/tree/master/app/models/concerns/token_authenticatable_strategies
# TODO expose this fields from the TokenAuthenticatable module instead
options[:except].concat self.class.token_authenticatable_fields.map { |token_field| "#{token_field}_expires_at" }
options[:except].concat self.class.token_authenticatable_fields.map { |token_field| "#{token_field}_digest" }
options[:except].concat self.class.token_authenticatable_fields.map { |token_field| "#{token_field}_encrypted" }
end
if self.class.respond_to?(:encrypted_attributes) if self.class.respond_to?(:encrypted_attributes)
options[:except].concat self.class.encrypted_attributes.keys options[:except].concat self.class.encrypted_attributes.keys
......
...@@ -27,6 +27,8 @@ module TokenAuthenticatable ...@@ -27,6 +27,8 @@ module TokenAuthenticatable
strategy = TokenAuthenticatableStrategies::Base strategy = TokenAuthenticatableStrategies::Base
.fabricate(self, token_field, options) .fabricate(self, token_field, options)
prevent_from_serialization(*strategy.token_fields) if respond_to?(:prevent_from_serialization)
if options.fetch(:unique, true) if options.fetch(:unique, true)
define_singleton_method("find_by_#{token_field}") do |token| define_singleton_method("find_by_#{token_field}") do |token|
strategy.find_token_authenticatable(token) strategy.find_token_authenticatable(token)
......
...@@ -23,6 +23,14 @@ module TokenAuthenticatableStrategies ...@@ -23,6 +23,14 @@ module TokenAuthenticatableStrategies
raise NotImplementedError raise NotImplementedError
end end
def token_fields
result = [token_field]
result << @expires_at_field if expirable?
result
end
# Default implementation returns the token as-is # Default implementation returns the token as-is
def format_token(instance, token) def format_token(instance, token)
instance.send("format_#{@token_field}", token) # rubocop:disable GitlabSecurity/PublicSend instance.send("format_#{@token_field}", token) # rubocop:disable GitlabSecurity/PublicSend
......
...@@ -2,6 +2,10 @@ ...@@ -2,6 +2,10 @@
module TokenAuthenticatableStrategies module TokenAuthenticatableStrategies
class Digest < Base class Digest < Base
def token_fields
super + [token_field_name]
end
def find_token_authenticatable(token, unscoped = false) def find_token_authenticatable(token, unscoped = false)
return unless token return unless token
......
...@@ -2,6 +2,10 @@ ...@@ -2,6 +2,10 @@
module TokenAuthenticatableStrategies module TokenAuthenticatableStrategies
class Encrypted < Base class Encrypted < Base
def token_fields
super + [encrypted_field]
end
def find_token_authenticatable(token, unscoped = false) def find_token_authenticatable(token, unscoped = false)
return if token.blank? return if token.blank?
......
...@@ -9,6 +9,12 @@ RSpec.shared_examples 'TokenAuthenticatable' do ...@@ -9,6 +9,12 @@ RSpec.shared_examples 'TokenAuthenticatable' do
it { is_expected.to respond_to("set_#{token_field}") } it { is_expected.to respond_to("set_#{token_field}") }
it { is_expected.to respond_to("reset_#{token_field}!") } it { is_expected.to respond_to("reset_#{token_field}!") }
end end
describe 'SensitiveSerializableHash' do
it 'includes the token field in list of sensitive attributes prevented from serialization' do
expect(described_class.attributes_exempt_from_serializable_hash).to include(token_field)
end
end
end end
RSpec.describe User, 'TokenAuthenticatable' do RSpec.describe User, 'TokenAuthenticatable' do
......
...@@ -6,6 +6,24 @@ RSpec.describe TokenAuthenticatableStrategies::Base do ...@@ -6,6 +6,24 @@ RSpec.describe TokenAuthenticatableStrategies::Base do
let(:instance) { double(:instance) } let(:instance) { double(:instance) }
let(:field) { double(:field) } let(:field) { double(:field) }
describe '#token_fields' do
let(:strategy) { described_class.new(instance, field, options) }
let(:field) { 'some_token' }
let(:options) { {} }
it 'includes the token field' do
expect(strategy.token_fields).to contain_exactly(field)
end
context 'with expires_at option' do
let(:options) { { expires_at: true } }
it 'includes the token_expires_at field' do
expect(strategy.token_fields).to contain_exactly(field, 'some_token_expires_at')
end
end
end
describe '.fabricate' do describe '.fabricate' do
context 'when digest stragegy is specified' do context 'when digest stragegy is specified' do
it 'fabricates digest strategy object' do it 'fabricates digest strategy object' do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe TokenAuthenticatableStrategies::Digest do
let(:model) { class_double('Project') }
let(:options) { { digest: true } }
subject(:strategy) do
described_class.new(model, 'some_field', options)
end
describe '#token_fields' do
it 'includes the digest field' do
expect(strategy.token_fields).to contain_exactly('some_field', 'some_field_digest')
end
end
end
...@@ -14,10 +14,18 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do ...@@ -14,10 +14,18 @@ RSpec.describe TokenAuthenticatableStrategies::Encrypted do
Gitlab::CryptoHelper.aes256_gcm_encrypt('my-value') Gitlab::CryptoHelper.aes256_gcm_encrypt('my-value')
end end
subject do subject(:strategy) do
described_class.new(model, 'some_field', options) described_class.new(model, 'some_field', options)
end end
describe '#token_fields' do
let(:options) { { encrypted: :required } }
it 'includes the encrypted field' do
expect(strategy.token_fields).to contain_exactly('some_field', 'some_field_encrypted')
end
end
describe '#find_token_authenticatable' do describe '#find_token_authenticatable' do
context 'when encryption is required' do context 'when encryption is required' do
let(:options) { { encrypted: :required } } let(:options) { { encrypted: :required } }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment