Merge branch 'security-if-51113-hash_tokens-11-3' into 'security-11-3'

[11.3] Persist only SHA digest of PersonalAccessToken#token

See merge request gitlab/gitlabhq!2552

6 files changed, 178 insertions, 46 deletions

diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb
index 522b65e4205..66db4bd92de 100644
--- a/app/models/concerns/token_authenticatable.rb
+++ b/app/models/concerns/token_authenticatable.rb
@@ -5,57 +5,50 @@ module TokenAuthenticatable
 
   private
 
-  def write_new_token(token_field)
-    new_token = generate_available_token(token_field)
-    write_attribute(token_field, new_token)
-  end
-
-  def generate_available_token(token_field)
-    loop do
-      token = generate_token(token_field)
-      break token unless self.class.unscoped.find_by(token_field => token)
-    end
-  end
-
-  def generate_token(token_field)
-    Devise.friendly_token
-  end
-
   class_methods do
-    def authentication_token_fields
-      @token_fields || []
-    end
-
     private # rubocop:disable Lint/UselessAccessModifier
 
-    def add_authentication_token_field(token_field)
+    def add_authentication_token_field(token_field, options = {})
       @token_fields = [] unless @token_fields
+
+      if @token_fields.include?(token_field)
+        raise ArgumentError.new("#{token_field} already configured via add_authentication_token_field")
+      end
+
       @token_fields << token_field
 
+      attr_accessor :cleartext_tokens
+
+      strategy = if options[:digest]
+                   TokenAuthenticatableStrategies::Digest.new(self, token_field, options)
+                 else
+                   TokenAuthenticatableStrategies::Insecure.new(self, token_field, options)
+                 end
+
       define_singleton_method("find_by_#{token_field}") do |token|
-        find_by(token_field => token) if token
+        strategy.find_token_authenticatable(token)
       end
 
-      define_method("ensure_#{token_field}") do
-        current_token = read_attribute(token_field)
-        current_token.blank? ? write_new_token(token_field) : current_token
+      define_method(token_field) do
+        strategy.get_token(self)
       end
 
       define_method("set_#{token_field}") do |token|
-        write_attribute(token_field, token) if token
+        strategy.set_token(self, token)
+      end
+
+      define_method("ensure_#{token_field}") do
+        strategy.ensure_token(self)
       end
 
       # Returns a token, but only saves when the database is in read & write mode
       define_method("ensure_#{token_field}!") do
-        send("reset_#{token_field}!") if read_attribute(token_field).blank? # rubocop:disable GitlabSecurity/PublicSend
-
-        read_attribute(token_field)
+        strategy.ensure_token!(self)
       end
 
       # Resets the token, but only saves when the database is in read & write mode
       define_method("reset_#{token_field}!") do
-        write_new_token(token_field)
-        save! if Gitlab::Database.read_write?
+        strategy.reset_token!(self)
       end
     end
   end
diff --git a/app/models/concerns/token_authenticatable_strategies/base.rb b/app/models/concerns/token_authenticatable_strategies/base.rb
new file mode 100644
index 00000000000..f0f7107d627
--- /dev/null
+++ b/app/models/concerns/token_authenticatable_strategies/base.rb
@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+module TokenAuthenticatableStrategies
+  class Base
+    def initialize(klass, token_field, options)
+      @klass = klass
+      @token_field = token_field
+      @options = options
+    end
+
+    def find_token_authenticatable(instance, unscoped = false)
+      raise NotImplementedError
+    end
+
+    def get_token(instance)
+      raise NotImplementedError
+    end
+
+    def set_token(instance)
+      raise NotImplementedError
+    end
+
+    def ensure_token(instance)
+      write_new_token(instance) unless token_set?(instance)
+    end
+
+    # Returns a token, but only saves when the database is in read & write mode
+    def ensure_token!(instance)
+      reset_token!(instance) unless token_set?(instance)
+      get_token(instance)
+    end
+
+    # Resets the token, but only saves when the database is in read & write mode
+    def reset_token!(instance)
+      write_new_token(instance)
+      instance.save! if Gitlab::Database.read_write?
+    end
+
+    protected
+
+    def write_new_token(instance)
+      new_token = generate_available_token
+      set_token(instance, new_token)
+    end
+
+    def generate_available_token
+      loop do
+        token = generate_token
+        break token unless find_token_authenticatable(token, true)
+      end
+    end
+
+    def generate_token
+      @options[:token_generator] ? @options[:token_generator].call : Devise.friendly_token
+    end
+
+    def relation(unscoped)
+      unscoped ? @klass.unscoped : @klass
+    end
+
+    def token_set?(instance)
+      raise NotImplementedError
+    end
+
+    def token_field_name
+      @token_field
+    end
+  end
+end
diff --git a/app/models/concerns/token_authenticatable_strategies/digest.rb b/app/models/concerns/token_authenticatable_strategies/digest.rb
new file mode 100644
index 00000000000..9926662ed66
--- /dev/null
+++ b/app/models/concerns/token_authenticatable_strategies/digest.rb
@@ -0,0 +1,50 @@
+# frozen_string_literal: true
+
+module TokenAuthenticatableStrategies
+  class Digest < Base
+    def find_token_authenticatable(token, unscoped = false)
+      return unless token
+
+      token_authenticatable = relation(unscoped).find_by(token_field_name => Gitlab::CryptoHelper.sha256(token))
+
+      if @options[:fallback]
+        token_authenticatable ||= fallback_strategy.find_token_authenticatable(token)
+      end
+
+      token_authenticatable
+    end
+
+    def get_token(instance)
+      token = instance.cleartext_tokens&.[](@token_field)
+      token ||= fallback_strategy.get_token(instance) if @options[:fallback]
+
+      token
+    end
+
+    def set_token(instance, token)
+      return unless token
+
+      instance.cleartext_tokens ||= {}
+      instance.cleartext_tokens[@token_field] = token
+      instance[token_field_name] = Gitlab::CryptoHelper.sha256(token)
+      instance[@token_field] = nil if @options[:fallback]
+    end
+
+    protected
+
+    def fallback_strategy
+      @fallback_strategy ||= TokenAuthenticatableStrategies::Insecure.new(@klass, @token_field, @options)
+    end
+
+    def token_set?(instance)
+      token_digest = instance.read_attribute(token_field_name)
+      token_digest ||= instance.read_attribute(@token_field) if @options[:fallback]
+
+      token_digest.present?
+    end
+
+    def token_field_name
+      "#{@token_field}_digest"
+    end
+  end
+end
diff --git a/app/models/concerns/token_authenticatable_strategies/insecure.rb b/app/models/concerns/token_authenticatable_strategies/insecure.rb
new file mode 100644
index 00000000000..5f915259521
--- /dev/null
+++ b/app/models/concerns/token_authenticatable_strategies/insecure.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+module TokenAuthenticatableStrategies
+  class Insecure < Base
+    def find_token_authenticatable(token, unscoped = false)
+      relation(unscoped).find_by(@token_field => token) if token
+    end
+
+    def get_token(instance)
+      instance.read_attribute(@token_field)
+    end
+
+    def set_token(instance, token)
+      instance[@token_field] = token if token
+    end
+
+    protected
+
+    def token_set?(instance)
+      instance.read_attribute(@token_field).present?
+    end
+  end
+end
diff --git a/app/models/personal_access_token.rb b/app/models/personal_access_token.rb
index 207146479c0..73a58f2420e 100644
--- a/app/models/personal_access_token.rb
+++ b/app/models/personal_access_token.rb
@@ -3,7 +3,7 @@
 class PersonalAccessToken < ActiveRecord::Base
   include Expirable
   include TokenAuthenticatable
-  add_authentication_token_field :token
+  add_authentication_token_field :token, digest: true, fallback: true
 
   REDIS_EXPIRY_TIME = 3.minutes
 
@@ -33,16 +33,22 @@ class PersonalAccessToken < ActiveRecord::Base
 
   def self.redis_getdel(user_id)
     Gitlab::Redis::SharedState.with do |redis|
-      token = redis.get(redis_shared_state_key(user_id))
+      encrypted_token = redis.get(redis_shared_state_key(user_id))
       redis.del(redis_shared_state_key(user_id))
-      token
+      begin
+        Gitlab::CryptoHelper.aes256_gcm_decrypt(encrypted_token)
+      rescue => ex
+        logger.warn "Failed to decrypt PersonalAccessToken value stored in Redis for User ##{user_id}: #{ex.class}"
+        encrypted_token
+      end
     end
   end
 
   def self.redis_store!(user_id, token)
+    encrypted_token = Gitlab::CryptoHelper.aes256_gcm_encrypt(token)
+
     Gitlab::Redis::SharedState.with do |redis|
-      redis.set(redis_shared_state_key(user_id), token, ex: REDIS_EXPIRY_TIME)
-      token
+      redis.set(redis_shared_state_key(user_id), encrypted_token, ex: REDIS_EXPIRY_TIME)
     end
   end
 
diff --git a/app/models/user.rb b/app/models/user.rb
index 671657388fb..7c8c994352e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -27,7 +27,7 @@ class User < ActiveRecord::Base
   ignore_column :email_provider
   ignore_column :authentication_token
 
-  add_authentication_token_field :incoming_email_token
+  add_authentication_token_field :incoming_email_token, token_generator: -> { SecureRandom.hex.to_i(16).to_s(36) }
   add_authentication_token_field :feed_token
 
   default_value_for :admin, false
@@ -1397,15 +1397,6 @@ class User < ActiveRecord::Base
     end
   end
 
-  def generate_token(token_field)
-    if token_field == :incoming_email_token
-      # Needs to be all lowercase and alphanumeric because it's gonna be used in an email address.
-      SecureRandom.hex.to_i(16).to_s(36)
-    else
-      super
-    end
-  end
-
   def self.unique_internal(scope, username, email_pattern, &block)
     scope.first || create_unique_internal(scope, username, email_pattern, &block)
   end