Refactor code for tokens

Result of multiple mob programming sessions to lay out foundations to a
better SCM/CI integration.

The controllers trigger_controller and services/webhooks_controller are
now merged and a single action is handling the various tokens. The
routes are kept for backward compatibility.

Authorization is done through Pundit, although we couldn't get away from
Package.get_by_project_and_name... it's just doing too much and we
wanted to avoid increasing the scope of the changes even more.

Anything related to extracting tokens out of the various
headers/parameters is now handled by
TriggerControllerService::TokenExtractor. It was previously scattered in
the aforementioned controllers and token models.

Finally, the specs were adapted to follow the changes.

Co-authored-by: Daniel Donisa <daniel.donisa@suse.com>
Co-authored-by: Dany Marcoux <dmarcoux@suse.com>
Co-authored-by: Eduardo Navarro <enavarro@suse.com>
Co-authored-by: Henne Vogelsang <hvogel@opensuse.org>
Co-authored-by: Lukas Krause <lkrause@suse.de>
Co-authored-by: Saray Cabrera Padrón <scabrerapadron@suse.de>
Co-authored-by: Victor Pereira <vpereira@suse.com>

src/api/app/controllers/trigger/errors.rb

@@ -1,26 +1,9 @@
 module Trigger::Errors
   extend ActiveSupport::Concern
 
-  class NoPermissionForInactive < APIError
-    setup 403, 'no permission due to inactive user'
-  end
-
-  class TokenNotFound < APIError
-    setup 404, 'Token not found'
-  end
-
   class InvalidToken < APIError
     setup 'permission_denied',
           403,
-          'No valid token found in the "Authorization" header'
-  end
-
-  class NoPermissionForPackage < APIError
-  end
-
-  class NoRepositoryCouldBeReleased < APIError
-  end
-
-  class NoPermissionForTarget < APIError
+          'No valid token found'
   end
 end

src/api/app/controllers/trigger_controller.rb

@@ -1,106 +1,46 @@
 class TriggerController < ApplicationController
-  validate_action rebuild: { method: :post, response: :status }
-  validate_action release: { method: :post, response: :status }
-  validate_action runservice: { method: :post, response: :status }
+  ALLOWED_GITLAB_EVENTS = ['Push Hook', 'Tag Push Hook', 'Merge Request Hook'].freeze
 
-  before_action :disallow_project_param, only: [:release]
-
-  before_action :extract_auth_from_request, :validate_auth_token, :require_valid_token
-  #
-  # Authentication happens with tokens, so no login is required
-  #
+  # Authentication happens with tokens, so extracting the user is not required
   skip_before_action :extract_user
+  # Authentication happens with tokens, so no login is required
   skip_before_action :require_login
-  skip_before_action :validate_params # new gitlab versions send other data as parameters,
-  # which which we may need to ignore here. Like the project hash.
 
-  # to get access to the method release_package
-  include MaintenanceHelper
+  before_action :validate_gitlab_event
+  before_action :set_token
+  before_action :set_package
 
   include Trigger::Errors
 
-  def rebuild
-    rebuild_trigger = PackageControllerService::RebuildTrigger.new(package: @pkg, project: @prj,
-                                                                   repository: params[:repository],
-                                                                   arch: params[:arch])
-    authorize rebuild_trigger.policy_object, :update?
-    rebuild_trigger.rebuild?
-    render_ok
-  end
-
-  def release
-    raise NoPermissionForPackage.setup('no_permission', 403, "no permission for package #{@pkg} in project #{@pkg.project}") unless policy(@pkg).update?
-
-    manual_release_targets = @pkg.project.release_targets.where(trigger: 'manual')
-    raise NoPermissionForPackage.setup('not_found', 404, "#{@pkg.project} has no release targets that are triggered manually") unless manual_release_targets.any?
-
-    manual_release_targets.each do |release_target|
-      release_package(@pkg,
-                      release_target.target_repository,
-                      @pkg.release_target_name,
-                      { filter_source_repository: release_target.repository,
-                        manual: true,
-                        comment: 'Releasing via trigger event' })
+  def create
+    authorize @token
+    @token.user.run_as do
+      @token.call(params.slice(:repository, :arch).permit!)
+      render_ok
     end
-
-    render_ok
-  end
-
-  def runservice
-    raise NoPermissionForPackage.setup('no_permission', 403, "no permission for package #{@pkg} in project #{@pkg.project}") unless policy(@pkg).update?
-
-    # execute the service in backend
-    pass_to_backend(prepare_path_for_runservice)
-
-    @pkg.sources_changed
   end
 
   private
 
-  def prepare_path_for_runservice
-    path = @pkg.source_path
-    params = { cmd: 'runservice', comment: 'runservice via trigger', user: User.session!.login }
-    URI(path + build_query_from_hash(params, [:cmd, :comment, :user])).to_s
+  # AUTHENTICATION
+  def set_token
+    @token = ::TriggerControllerService::TokenExtractor.new(request).call
+    raise InvalidToken unless @token
   end
 
-  def disallow_project_param
-    render_error(message: 'You specified a project, but the token defines the project/package to release', status: 403, errorcode: 'no_permission') if params[:project].present?
-  end
-
-  def extract_auth_from_request
-    @token_extractor = ::TriggerControllerService::TokenExtractor.new(request).call
-  end
+  def validate_gitlab_event
+    return unless request.env['HTTP_X_GITLAB_EVENT']
 
-  def validate_auth_token
-    raise InvalidToken unless @token_extractor.valid?
+    raise InvalidToken unless request.env['HTTP_X_GITLAB_EVENT'].in?(ALLOWED_GITLAB_EVENTS)
   end
 
-  def require_valid_token
-    @token = @token_extractor.token
-
-    raise TokenNotFound unless @token
-
-    User.session = @token.user
-
-    raise NoPermissionForInactive unless User.session.is_active?
-
-    if @token.package
-      @pkg = @token.package
-      @pkg_name = @pkg.name
-      @prj = @pkg.project
-    else
-      @prj = Project.get_by_name(params[:project])
-      @pkg_name = params[:package] # for multibuild container
-      opts = if @token.instance_of?(Token::Rebuild)
-               { use_source: false,
-                 follow_project_links: true,
-                 follow_multibuild: true }
-             else
-               { use_source: true,
-                 follow_project_links: false,
-                 follow_multibuild: false }
-             end
-      @pkg = Package.get_by_project_and_name(params[:project].to_s, params[:package].to_s, opts)
-    end
+  def set_package
+    # We need to store in memory the package in order to do authorization
+    @token.package_from_association_or_params = @token.package ||
+                                                Package.get_by_project_and_name(params[:project],
+                                                                                params[:package],
+                                                                                @token.package_find_options)
+    # This can happen due to the Package.get_by_project_and_name method
+    raise ActiveRecord::RecordNotFound if @token.package_from_association_or_params.nil?
   end
 end

src/api/app/models/token.rb

@@ -2,9 +2,14 @@ class Token < ApplicationRecord
   belongs_to :user
   belongs_to :package, inverse_of: :tokens
 
+  attr_accessor :package_from_association_or_params
+
   has_secure_token :string
 
   validates :user, presence: true
+  validates :string, uniqueness: { case_sensitive: false }
+
+  include Token::Errors
 
   def token_name
     self.class.token_name
@@ -21,6 +26,14 @@ def self.token_type(action)
       Token::Service
     end
   end
+
+  def call(_options)
+    raise AbstractMethodCalled
+  end
+
+  def package_find_options
+    {}
+  end
 end
 
 # == Schema Information

src/api/app/models/token/errors.rb

@@ -0,0 +1,7 @@
+module Token::Errors
+  extend ActiveSupport::Concern
+
+  class NoReleaseTargetFound < APIError
+    setup 404
+  end
+end

src/api/app/models/token/rebuild.rb

@@ -2,6 +2,17 @@ class Token::Rebuild < Token
   def self.token_name
     'rebuild'
   end
+
+  def call(options)
+    # FIXME: Use the Package#rebuild? instead of calling the Backend directly
+    Backend::Api::Sources::Package.rebuild(package_from_association_or_params.project.name,
+                                           package_from_association_or_params.name,
+                                           options)
+  end
+
+  def package_find_options
+    { use_source: false, follow_project_links: true, follow_multibuild: true }
+  end
 end
 
 # == Schema Information

src/api/app/models/token/release.rb

@@ -1,7 +1,28 @@
 class Token::Release < Token
+  # FIXME: refactor this out of the helper to get access to the method release_package
+  include MaintenanceHelper
+
   def self.token_name
     'release'
   end
+
+  def call(_options)
+    manual_release_targets = package_from_association_or_params.project.release_targets.where(trigger: 'manual')
+    raise NoReleaseTargetFound, "#{package_from_association_or_params.project} has no release targets that are triggered manually" unless manual_release_targets.any?
+
+    manual_release_targets.each do |release_target|
+      release_package(package_from_association_or_params,
+                      release_target.target_repository,
+                      package_from_association_or_params.release_target_name,
+                      { filter_source_repository: release_target.repository,
+                        manual: true,
+                        comment: 'Releasing via trigger event' })
+    end
+  end
+
+  def package_find_options
+    { use_source: true, follow_project_links: false, follow_multibuild: false }
+  end
 end
 
 # == Schema Information

src/api/app/models/token/service.rb

@@ -3,16 +3,14 @@ def self.token_name
     'runservice'
   end
 
-  def valid_signature?(signature, body)
-    return false unless signature
-
-    ActiveSupport::SecurityUtils.secure_compare(signature_of(body), signature)
+  def call(_options)
+    Backend::Api::Sources::Package.trigger_services(package_from_association_or_params.project.name,
+                                                    package_from_association_or_params.name,
+                                                    user.login)
   end
 
-  private
-
-  def signature_of(body)
-    'sha1=' + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new('sha1'), string, body)
+  def package_find_options
+    { use_source: true, follow_project_links: false, follow_multibuild: false }
   end
 end
 

src/api/app/policies/token/rebuild_policy.rb

@@ -0,0 +1,13 @@
+class Token
+  class RebuildPolicy < ApplicationPolicy
+    def initialize(_user, record)
+      super(record.user, record)
+    end
+
+    def create?
+      return false unless record.user.is_active?
+
+      PackagePolicy.new(record.user, record.package_from_association_or_params).update?
+    end
+  end
+end

src/api/app/policies/token/release_policy.rb

@@ -0,0 +1,13 @@
+class Token
+  class ReleasePolicy < ApplicationPolicy
+    def initialize(_user, record)
+      super(record.user, record)
+    end
+
+    def create?
+      return false unless record.user.is_active?
+
+      PackagePolicy.new(record.user, record.package_from_association_or_params).update?
+    end
+  end
+end

src/api/app/policies/token/service_policy.rb

@@ -0,0 +1,13 @@
+class Token
+  class ServicePolicy < ApplicationPolicy
+    def initialize(_user, record)
+      super(record.user, record)
+    end
+
+    def create?
+      return false unless record.user.is_active?
+
+      PackagePolicy.new(record.user, record.package_from_association_or_params).update?
+    end
+  end
+end