Use actor when we don't know if it's a Key or User

* Use gl_id when we don't know if it's a key-X or user-X
* Use Actor.new_from(gl_id) which will figure out if it's a Key or User
* Use key_str when we're referring to key-X as key_id is confusing

8 files changed, 75 insertions, 68 deletions

diff --git a/lib/action/api_2fa_recovery.rb b/lib/action/api_2fa_recovery.rb
index 827f8aa..5597ff0 100644
--- a/lib/action/api_2fa_recovery.rb
+++ b/lib/action/api_2fa_recovery.rb
@@ -3,8 +3,8 @@ require_relative '../gitlab_logger'
 
 module Action
   class API2FARecovery < Base
-    def initialize(key_id)
-      @key_id = key_id
+    def initialize(key)
+      @key = key
     end
 
     def execute(_, _)
@@ -13,7 +13,7 @@ module Action
 
     private
 
-    attr_reader :key_id
+    attr_reader :key
 
     def continue?(question)
       puts "#{question} (yes/no)"
@@ -34,10 +34,10 @@ module Action
         return
       end
 
-      resp = api.two_factor_recovery_codes(key_id)
+      resp = api.two_factor_recovery_codes(key.key_id)
       if resp['success']
         codes = resp['recovery_codes'].join("\n")
-        $logger.info('API 2FA recovery success', user: user.log_username)
+        $logger.info('API 2FA recovery success', user: key.log_username)
         puts "Your two-factor authentication recovery codes are:\n\n" \
             "#{codes}\n\n" \
             "During sign in, use one of the codes above when prompted for\n" \
@@ -45,7 +45,7 @@ module Action
             "a new device so you do not lose access to your account again."
         true
       else
-        $logger.info('API 2FA recovery error', user: user.log_username)
+        $logger.info('API 2FA recovery error', user: key.log_username)
         puts "An error occurred while trying to generate new recovery codes.\n" \
             "#{resp['message']}"
       end
diff --git a/lib/action/base.rb b/lib/action/base.rb
index 1f24c8c..fe8c836 100644
--- a/lib/action/base.rb
+++ b/lib/action/base.rb
@@ -3,18 +3,19 @@ require 'json'
 require_relative '../gitlab_config'
 require_relative '../gitlab_net'
 require_relative '../gitlab_metrics'
-require_relative '../user'
 
 module Action
   class Base
+    def initialize
+      raise NotImplementedError
+    end
+
     def self.create_from_json(_)
       raise NotImplementedError
     end
 
     private
 
-    attr_reader :key_id
-
     def config
       @config ||= GitlabConfig.new
     end
@@ -22,9 +23,5 @@ module Action
     def api
       @api ||= GitlabNet.new
     end
-
-    def user
-      @user ||= User.new(key_id, audit_usernames: config.audit_usernames)
-    end
   end
 end
diff --git a/lib/action/git_lfs_authenticate.rb b/lib/action/git_lfs_authenticate.rb
index d38d845..218c71e 100644
--- a/lib/action/git_lfs_authenticate.rb
+++ b/lib/action/git_lfs_authenticate.rb
@@ -3,15 +3,15 @@ require_relative '../gitlab_logger'
 
 module Action
   class GitLFSAuthenticate < Base
-    def initialize(key_id, repo_name)
-      @key_id = key_id
+    def initialize(key, repo_name)
+      @key = key
       @repo_name = repo_name
     end
 
     def execute(_, _)
       GitlabMetrics.measure('lfs-authenticate') do
-        $logger.info('Processing LFS authentication', user: user.log_username)
-        lfs_access = api.lfs_authenticate(key_id, repo_name)
+        $logger.info('Processing LFS authentication', user: key.log_username)
+        lfs_access = api.lfs_authenticate(key.key_id, repo_name)
         return unless lfs_access
 
         puts lfs_access.authentication_payload
@@ -21,6 +21,6 @@ module Action
 
     private
 
-    attr_reader :key_id, :repo_name
+    attr_reader :key, :repo_name
   end
 end
diff --git a/lib/action/gitaly.rb b/lib/action/gitaly.rb
index 65397e6..569a1b7 100644
--- a/lib/action/gitaly.rb
+++ b/lib/action/gitaly.rb
@@ -11,16 +11,16 @@ module Action
       'git-receive-pack' => File.join(ROOT_PATH, 'bin', 'gitaly-receive-pack')
     }.freeze
 
-    def initialize(key_id, gl_repository, gl_username, repository_path, gitaly)
-      @key_id = key_id
+    def initialize(actor, gl_repository, gl_username, repository_path, gitaly)
+      @actor = actor
       @gl_repository = gl_repository
       @gl_username = gl_username
       @repository_path = repository_path
       @gitaly = gitaly
     end
 
-    def self.create_from_json(key_id, json)
-      new(key_id,
+    def self.create_from_json(actor, json)
+      new(actor,
           json['gl_repository'],
           json['gl_username'],
           json['repository_path'],
@@ -31,13 +31,13 @@ module Action
       raise ArgumentError, REPOSITORY_PATH_NOT_PROVIDED unless repository_path
       raise InvalidRepositoryPathError unless valid_repository?
 
-      $logger.info('Performing Gitaly command', user: user.log_username)
+      $logger.info('Performing Gitaly command', user: actor.log_username)
       process(command, args)
     end
 
     private
 
-    attr_reader :gl_repository, :gl_username, :repository_path, :gitaly
+    attr_reader :actor, :gl_repository, :gl_username, :repository_path, :gitaly
 
     def process(command, args)
       executable = command
@@ -50,7 +50,7 @@ module Action
       end
 
       args_string = [File.basename(executable), *args].join(' ')
-      $logger.info('executing git command', command: args_string, user: user.log_username)
+      $logger.info('executing git command', command: args_string, user: actor.log_username)
 
       exec_cmd(executable, *args)
     end
@@ -77,7 +77,7 @@ module Action
         'PATH' => ENV['PATH'],
         'LD_LIBRARY_PATH' => ENV['LD_LIBRARY_PATH'],
         'LANG' => ENV['LANG'],
-        'GL_ID' => key_id,
+        'GL_ID' => actor.identifier,
         'GL_PROTOCOL' => GitlabNet::GL_PROTOCOL,
         'GL_REPOSITORY' => gl_repository,
         'GL_USERNAME' => gl_username
@@ -90,7 +90,7 @@ module Action
       {
         'repository' => gitaly['repository'],
         'gl_repository' => gl_repository,
-        'gl_id' => key_id,
+        'gl_id' => actor.identifier,
         'gl_username' => gl_username
       }
     end
diff --git a/lib/gitlab_access.rb b/lib/gitlab_access.rb
index a1a8b93..6683ee7 100644
--- a/lib/gitlab_access.rb
+++ b/lib/gitlab_access.rb
@@ -1,6 +1,7 @@
 require 'json'
 
 require_relative 'errors'
+require_relative 'actor'
 require_relative 'gitlab_init'
 require_relative 'gitlab_net'
 require_relative 'names_helper'
@@ -10,17 +11,17 @@ require_relative 'object_dirs_helper'
 class GitlabAccess
   include NamesHelper
 
-  def initialize(gl_repository, repo_path, key_id, changes, protocol)
+  def initialize(gl_repository, repo_path, gl_id, changes, protocol)
     @gl_repository = gl_repository
     @repo_path = repo_path.strip
-    @key_id = key_id
+    @gl_id = gl_id
     @changes = changes.lines
     @protocol = protocol
   end
 
   def exec
     GitlabMetrics.measure('check-access:git-receive-pack') do
-      api.check_access('git-receive-pack', gl_repository, repo_path, key_id, changes, protocol, env: ObjectDirsHelper.all_attributes.to_json)
+      api.check_access('git-receive-pack', gl_repository, repo_path, actor, changes, protocol, env: ObjectDirsHelper.all_attributes.to_json)
     end
     true
   rescue GitlabNet::ApiUnreachableError
@@ -33,9 +34,17 @@ class GitlabAccess
 
   private
 
-  attr_reader :gl_repository, :repo_path, :key_id, :changes, :protocol
+  attr_reader :gl_repository, :repo_path, :gl_id, :changes, :protocol
 
   def api
-    GitlabNet.new
+    @api ||= GitlabNet.new
+  end
+
+  def config
+    @config ||= GitlabConfig.new
+  end
+
+  def actor
+    @actor ||= Actor.new_from(gl_id, audit_usernames: config.audit_usernames)
   end
 end
diff --git a/lib/gitlab_net.rb b/lib/gitlab_net.rb
index 29a91f4..34e2ff6 100644
--- a/lib/gitlab_net.rb
+++ b/lib/gitlab_net.rb
@@ -14,34 +14,34 @@ class GitlabNet
   GL_PROTOCOL = 'ssh'.freeze
   API_INACCESSIBLE_ERROR = 'API is not accessible'.freeze
 
-  def check_access(cmd, gl_repository, repo, key_id, changes, protocol = GL_PROTOCOL, env: {})
+  def check_access(cmd, gl_repository, repo, actor, changes, protocol = GL_PROTOCOL, env: {})
     changes = changes.join("\n") unless changes.is_a?(String)
 
     params = {
       action: cmd,
       changes: changes,
       gl_repository: gl_repository,
-      key_id: key_id.gsub('key-', ''),
       project: sanitize_path(repo),
       protocol: protocol,
       env: env
     }
 
+    params[actor.class.identifier_key.to_sym] = actor.id
+
     resp = post("#{internal_api_endpoint}/allowed", params)
 
-    determine_action(key_id, resp)
+    determine_action(actor, resp)
   end
 
-  def discover(key)
-    key_id = key.gsub("key-", "")
+  def discover(key_id)
     resp = get("#{internal_api_endpoint}/discover?key_id=#{key_id}")
     JSON.parse(resp.body)
   rescue JSON::ParserError, ApiUnreachableError
     nil
   end
 
-  def lfs_authenticate(key, repo)
-    params = { project: sanitize_path(repo), key_id: key.gsub('key-', '') }
+  def lfs_authenticate(key_id, repo)
+    params = { project: sanitize_path(repo), key_id: key_id }
     resp = post("#{internal_api_endpoint}/lfs_authenticate", params)
 
     GitlabLfsAuthentication.build_from_json(resp.body) if resp.code == HTTP_SUCCESS
@@ -68,15 +68,14 @@ class GitlabNet
     get("#{internal_api_endpoint}/check", options: { read_timeout: CHECK_TIMEOUT })
   end
 
-  def authorized_key(key)
-    resp = get("#{internal_api_endpoint}/authorized_keys?key=#{URI.escape(key, '+/=')}")
+  def authorized_key(full_key)
+    resp = get("#{internal_api_endpoint}/authorized_keys?key=#{URI.escape(full_key, '+/=')}")
     JSON.parse(resp.body) if resp.code == HTTP_SUCCESS
   rescue
     nil
   end
 
-  def two_factor_recovery_codes(key)
-    key_id = key.gsub('key-', '')
+  def two_factor_recovery_codes(key_id)
     resp = post("#{internal_api_endpoint}/two_factor_recovery_codes", key_id: key_id)
     JSON.parse(resp.body) if resp.code == HTTP_SUCCESS
   rescue
@@ -92,8 +91,8 @@ class GitlabNet
     false
   end
 
-  def post_receive(gl_repository, identifier, changes)
-    params = { gl_repository: gl_repository, identifier: identifier, changes: changes }
+  def post_receive(gl_repository, actor, changes)
+    params = { gl_repository: gl_repository, identifier: actor.identifier, changes: changes }
     resp = post("#{internal_api_endpoint}/post_receive", params)
     raise NotFoundError if resp.code == HTTP_NOT_FOUND
 
@@ -113,7 +112,7 @@ class GitlabNet
     repo.delete("'")
   end
 
-  def determine_action(key_id, resp)
+  def determine_action(actor, resp)
     json = JSON.parse(resp.body)
     message = json['message']
 
@@ -124,7 +123,7 @@ class GitlabNet
       #       accessing the 'status' key.
       raise AccessDeniedError, message unless json['status']
 
-      Action::Gitaly.create_from_json(key_id, json)
+      Action::Gitaly.create_from_json(actor, json)
     when HTTP_UNAUTHORIZED, HTTP_NOT_FOUND
       raise AccessDeniedError, message
     else
diff --git a/lib/gitlab_post_receive.rb b/lib/gitlab_post_receive.rb
index cb9931d..9248582 100644
--- a/lib/gitlab_post_receive.rb
+++ b/lib/gitlab_post_receive.rb
@@ -8,20 +8,18 @@ require 'securerandom'
 class GitlabPostReceive
   include NamesHelper
 
-  attr_reader :config, :gl_repository, :repo_path, :changes, :jid
-
-  def initialize(gl_repository, repo_path, actor, changes)
+  def initialize(gl_repository, repo_path, gl_id, changes)
     @config = GitlabConfig.new
     @gl_repository = gl_repository
     @repo_path = repo_path.strip
-    @actor = actor
+    @gl_id = gl_id
     @changes = changes
     @jid = SecureRandom.hex(12)
   end
 
   def exec
     response = GitlabMetrics.measure("post-receive") do
-      api.post_receive(gl_repository, @actor, changes)
+      api.post_receive(gl_repository, actor, changes)
     end
 
     return false unless response
@@ -35,12 +33,18 @@ class GitlabPostReceive
     false
   end
 
-  protected
+  private
+
+  attr_reader :config, :gl_repository, :repo_path, :gl_id, :changes, :jid
 
   def api
     @api ||= GitlabNet.new
   end
 
+  def actor
+    @actor ||= Actor.new_from(gl_id, audit_usernames: config.audit_usernames)
+  end
+
   def print_merge_request_links(merge_request_urls)
     return if merge_request_urls.empty?
     puts
@@ -100,8 +104,6 @@ class GitlabPostReceive
     puts "=" * total_width
   end
 
-  private
-
   def parse_broadcast_msg(msg, text_length)
     msg ||= ""
     # just return msg if shorter than or equal to text length
diff --git a/lib/gitlab_shell.rb b/lib/gitlab_shell.rb
index 5c51bc1..6fde88a 100644
--- a/lib/gitlab_shell.rb
+++ b/lib/gitlab_shell.rb
@@ -3,7 +3,7 @@ require 'pathname'
 
 require_relative 'gitlab_net'
 require_relative 'gitlab_metrics'
-require_relative 'user'
+require_relative 'actor/key'
 
 class GitlabShell
   API_2FA_RECOVERY_CODES_COMMAND = '2fa_recovery_codes'.freeze
@@ -16,8 +16,8 @@ class GitlabShell
   GIT_COMMANDS = [GIT_UPLOAD_PACK_COMMAND, GIT_RECEIVE_PACK_COMMAND,
                   GIT_UPLOAD_ARCHIVE_COMMAND, GIT_LFS_AUTHENTICATE_COMMAND].freeze
 
-  def initialize(key_id)
-    @key_id = key_id
+  def initialize(key_str)
+    @key_str = key_str
     @config = GitlabConfig.new
   end
 
@@ -26,7 +26,7 @@ class GitlabShell
   # 'evil command'.
   def exec(origin_cmd)
     if !origin_cmd || origin_cmd.empty?
-      puts "Welcome to GitLab, #{user.username}!"
+      puts "Welcome to GitLab, #{key.username}!"
       return true
     end
 
@@ -38,11 +38,11 @@ class GitlabShell
     $stderr.puts "GitLab: Failed to authorize your Git request: internal API unreachable"
     false
   rescue AccessDeniedError, UnknownError => ex
-    $logger.warn('Access denied', command: origin_cmd, user: user.log_username)
+    $logger.warn('Access denied', command: origin_cmd, user: key.log_username)
     $stderr.puts "GitLab: #{ex.message}"
     false
   rescue DisallowedCommandError
-    $logger.warn('Denied disallowed command', command: origin_cmd, user: user.log_username)
+    $logger.warn('Denied disallowed command', command: origin_cmd, user: key.log_username)
     $stderr.puts 'GitLab: Disallowed command'
     false
   rescue InvalidRepositoryPathError
@@ -52,10 +52,10 @@ class GitlabShell
 
   private
 
-  attr_reader :config, :key_id
+  attr_reader :config, :key_str
 
-  def user
-    @user ||= User.new(key_id, audit_usernames: config.audit_usernames)
+  def key
+    @key ||= Actor::Key.from(key_str, audit_usernames: config.audit_usernames)
   end
 
   def parse_cmd(cmd)
@@ -96,16 +96,16 @@ class GitlabShell
   end
 
   def determine_action(command, git_access_command, repo_name)
-    return Action::API2FARecovery.new(key_id) if command == API_2FA_RECOVERY_CODES_COMMAND
+    return Action::API2FARecovery.new(key) if command == API_2FA_RECOVERY_CODES_COMMAND
 
     GitlabMetrics.measure('verify-access') do
       # GitlatNet#check_access will raise exception in the event of a problem
       initial_action = api.check_access(git_access_command, nil,
-                                        repo_name, key_id, '_any')
+                                        repo_name, key, '_any')
 
       case command
       when GIT_LFS_AUTHENTICATE_COMMAND
-        Action::GitLFSAuthenticate.new(key_id, repo_name)
+        Action::GitLFSAuthenticate.new(key, repo_name)
       else
         initial_action
       end
@@ -113,6 +113,6 @@ class GitlabShell
   end
 
   def api
-    GitlabNet.new
+    @api ||= GitlabNet.new
   end
 end