FIX ci code review

app/controllers/ci/base_controller.rb

@@ -7,4 +7,40 @@ class Ci::BaseController < ApplicationController
 
     @user, @repo = Ci::Repo.find_with_namespace(namespace, id)
   end
+
+  private
+    def authorize_access_project!
+      unless @project.manager?(current_user)
+        return render_forbidden
+      end
+    end
+
+    def authorize_manage_builds!
+      unless @project.owner?(current_user)
+        return render_forbidden
+      end
+    end
+
+    def authenticate_admin!
+      return render_forbidden unless current_user.admin?
+    end
+
+    def authorize_owner_project!
+      unless @project.owner?(current_user)
+        return render_forbidden
+      end
+    end
+
+    def find_cloud_account
+      @cloud_account ||= current_user.ci_cloud_account
+    end
+
+    def load_ci_user
+      begin
+        @ci_user = Ci::User.find_by(user_login: params[:owner])
+      rescue
+        render_not_found
+      end
+    end
+
 end

app/controllers/ci/builds_controller.rb

@@ -1,9 +1,8 @@
 class Ci::BuildsController < Ci::BaseController
   include RepositoriesHelper
-  include Devopsable
 
   before_action :load_project
-  before_action :ci_authorize!
+  before_action :authorize_owner_project!
   before_action :load_repo
   before_action :find_cloud_account, except: [:index, :show]
 
@@ -36,17 +35,4 @@ class Ci::BuildsController < Ci::BaseController
 
     render json: result
   end
-
-  private
-    def find_cloud_account
-      @cloud_account = current_user.ci_cloud_account
-    end
-
-    def load_ci_user
-      begin
-        @ci_user = Ci::User.find_by(user_login: params[:owner])
-      rescue
-        render_not_found
-      end
-    end
 end

app/controllers/ci/cloud_accounts_controller.rb

@@ -1,16 +1,15 @@
 class Ci::CloudAccountsController < Ci::BaseController
-  include Devopsable
+  include Ci::CloudAccountManageable
 
   before_action :load_project, only: %i[create activate]
-  before_action :ci_authorize!, only: %i[create activate]
-  before_action :find_cloud_account, only: %i[activate]
+  before_action :authorize_owner_project!, only: %i[create activate]
   before_action :load_repo, only: %i[activate]
+  before_action :find_cloud_account, only: %i[show]
+  before_action :validate_params!, only: %i[create bind]
 
   def create
     ActiveRecord::Base.transaction do
-      Ci::CreateCloudAccountForm.new(devops_params).validate!
-
-      @cloud_account = bind_account(current_user)
+      @cloud_account = bind_account!
       if @cloud_account.blank?
         render_error('激活失败, 请检查你的云服务器信息是否正确.')
         raise ActiveRecord::Rollback
@@ -27,6 +26,8 @@ class Ci::CloudAccountsController < Ci::BaseController
     return render_error('请先在指定地址做用户认证') unless @user.ci_certification?
 
     return render_error('该项目已经激活') if @repo && @repo.repo_active?
+
+    @cloud_account = Ci::CloudAccount.find params[:id]
     begin
       ActiveRecord::Base.transaction do
         repo.activate!(@user.user_id)
@@ -41,14 +42,11 @@ class Ci::CloudAccountsController < Ci::BaseController
   end
 
   def show
-    @cloud_account = current_user.ci_cloud_account
   end
 
   def bind
-    Ci::CreateCloudAccountForm.new(devops_params).validate!
-
     ActiveRecord::Base.transaction do
-      @cloud_account = bind_account(current_user)
+      @cloud_account = bind_account!
       if @cloud_account.blank?
         render_error('激活失败, 请检查你的云服务器信息是否正确.')
         raise ActiveRecord::Rollback
@@ -62,7 +60,7 @@ class Ci::CloudAccountsController < Ci::BaseController
 
   def unbind
     ActiveRecord::Base.transaction do
-      unbind_account!(current_user)
+      unbind_account!
       render_ok
     end
   rescue Exception => ex
@@ -71,92 +69,7 @@ class Ci::CloudAccountsController < Ci::BaseController
 
 
   private
-    def devops_params
-      params.permit(:account, :secret, :ip_num)
-    end
-
-    def find_cloud_account
-      @cloud_account = Ci::CloudAccount.find params[:id]
-    end
-
-    def bind_account(user)
-      # 1. 保存华为云服务器帐号
-      create_params = devops_params.merge(ip_num: IPAddr.new(devops_params[:ip_num]).to_i, secret: Ci::CloudAccount.encrypted_secret(devops_params[:secret]))
-
-      return render_error('你已绑定了云帐号.') unless user.ci_cloud_account.blank?
-
-      cloud_account = Ci::CloudAccount.new(create_params)
-      cloud_account.user = user
-      cloud_account.save!
-
-      # 2. 生成oauth2应用程序的client_id和client_secrete
-      gitea_oauth = Gitea::Oauth2::CreateService.call(user.gitea_token, {name: "pipeline", redirect_uris: ["#{cloud_account.drone_url}/login"]})
-      logger.info "######### gitea_oauth: #{gitea_oauth}"
-      oauth = Oauth.new(client_id: gitea_oauth['client_id'],
-        client_secret: gitea_oauth['client_secret'],
-        redirect_uri: gitea_oauth['redirect_uris'],
-        gitea_oauth_id: gitea_oauth['id'],
-        user_id: current_user.id)
-      oauth.save
-
-      rpc_secret = SecureRandom.hex 16
-      logger.info "######### rpc_secret: #{rpc_secret}"
-
-      # 3. 创建drone server
-      drone_server_cmd = Ci::Drone::Server.new(oauth.client_id, oauth.client_secret, cloud_account.drone_host, rpc_secret).generate_cmd
-      logger.info "######### drone_server_cmd: #{drone_server_cmd}"
-
-      # 4. 创建drone client
-      drone_client_cmd = Ci::Drone::Client.new(oauth.client_id, cloud_account.drone_ip, rpc_secret).generate_cmd
-      logger.info "######### drone_client_cmd: #{drone_client_cmd}"
-
-      # 5. 登录远程服务器，启动drone服务
-      result = Ci::Drone::Start.new(cloud_account.account, cloud_account.visible_secret, cloud_account.drone_ip, drone_server_cmd, drone_client_cmd).run
-      logger.info "######### result: #{result}"
-
-
-      redirect_url = "#{cloud_account.drone_url}/login"
-      logger.info "######### redirect_url: #{redirect_url}"
-
-      result && !result.blank? ? cloud_account : nil
-    end
-
-    def unbind_account!(user)
-      cloud_account = user.ci_cloud_account
-      ci_user = cloud_account.ci_user || Ci::User.find_by(user_login: user.login)
-
-      if user.devops_step == User::DEVOPS_UNINIT || cloud_account.blank?
-        return render_error('你未绑定CI服务器')
-      elsif user.devops_step == User::DEVOPS_UNVERIFIED || user.ci_certification?
-        ci_user.destroy!
-        Ci::Repo.where(repo_namespace: user.login).delete_all
-        cloud_account.destroy!
-      end
-
-      user.projects.update_all(open_devops: false)
-      user.set_drone_step!(User::DEVOPS_UNINIT)
-
-      # TODO
-      # 删除用户项目下的与ci相关的所有webhook
-      user.projects.select(:id, :identifier, :gitea_webhook_id).each do |project|
-        if project.gitea_webhook_id
-          result = Gitea::Hooks::DestroyService.call(user.gitea_token, user.login, project.identifier, project.gitea_webhook_id)
-          project.update_column(:gitea_webhook_id, nil) if result.status == 204
-        end
-      end
-    end
-
-    def bind_hook!(user, cloud_account, repo)
-      hook_params = {
-        active: true,
-        config: {
-          content_type: "json",
-          url: cloud_account.drone_url + "/hook?secret=#{repo.repo_signer}"
-        },
-        type: "gitea"
-      }
-      result = Gitea::Hooks::CreateService.call(user.gitea_token, user.login, repo.repo_name, hook_params)
-
-      result[:status].present? ? nil : result
+    def validate_params!
+      Ci::CreateCloudAccountForm.new(devops_params).validate!
     end
 end

app/controllers/ci/projects_controller.rb

@@ -1,15 +1,13 @@
 class Ci::ProjectsController < Ci::BaseController
-  include Devopsable
   include RepositoriesHelper
 
   before_action :load_project
   before_action :load_repo, only: [:update_trustie_pipeline]
+  before_action :authorize_owner_project!, only: [:authorize]
+  before_action :find_cloud_account, only: [:authorize]
 
   def authorize
     @user = current_user
-    limit_project_owner_can_devops!(@user, @project)
-
-    @cloud_account = @user.ci_cloud_account
   end
 
   # get .trustie-pipeline.yml file

app/controllers/concerns/ci/cloud_account_manageable.rb

@@ -0,0 +1,93 @@
+module Ci::CloudAccountManageable
+  extend ActiveSupport::Concern
+
+  included do
+  end
+
+  def bind_account!
+    # 1. 保存华为云服务器帐号
+    create_params = devops_params.merge(ip_num: IPAddr.new(devops_params[:ip_num]).to_i, secret: Ci::CloudAccount.encrypted_secret(devops_params[:secret]))
+
+    return render_error('你已绑定了云帐号.') unless current_user.ci_cloud_account.blank?
+
+    cloud_account = Ci::CloudAccount.new(create_params)
+    cloud_account.user = current_user
+    cloud_account.save!
+
+    # 2. 生成oauth2应用程序的client_id和client_secrete
+    gitea_oauth = Gitea::Oauth2::CreateService.call(current_user.gitea_token, {name: "pipeline", redirect_uris: ["#{cloud_account.drone_url}/login"]})
+    logger.info "######### gitea_oauth: #{gitea_oauth}"
+    oauth = Oauth.new(client_id: gitea_oauth['client_id'],
+      client_secret: gitea_oauth['client_secret'],
+      redirect_uri: gitea_oauth['redirect_uris'],
+      gitea_oauth_id: gitea_oauth['id'],
+      user_id: current_user.id)
+    oauth.save
+
+    rpc_secret = SecureRandom.hex 16
+    logger.info "######### rpc_secret: #{rpc_secret}"
+
+    # 3. 创建drone server
+    drone_server_cmd = Ci::Drone::Server.new(oauth.client_id, oauth.client_secret, cloud_account.drone_host, rpc_secret).generate_cmd
+    logger.info "######### drone_server_cmd: #{drone_server_cmd}"
+
+    # 4. 创建drone client
+    drone_client_cmd = Ci::Drone::Client.new(oauth.client_id, cloud_account.drone_ip, rpc_secret).generate_cmd
+    logger.info "######### drone_client_cmd: #{drone_client_cmd}"
+
+    # 5. 登录远程服务器，启动drone服务
+    result = Ci::Drone::Start.new(cloud_account.account, cloud_account.visible_secret, cloud_account.drone_ip, drone_server_cmd, drone_client_cmd).run
+    logger.info "######### result: #{result}"
+
+
+    redirect_url = "#{cloud_account.drone_url}/login"
+    logger.info "######### redirect_url: #{redirect_url}"
+
+    result && !result.blank? ? cloud_account : nil
+  end
+
+  def unbind_account!
+    cloud_account = current_user.ci_cloud_account
+    ci_user = cloud_account.ci_user || Ci::User.find_by(user_login: current_user.login)
+
+    if current_user.devops_step == User::DEVOPS_UNINIT || cloud_account.blank?
+      return render_error('你未绑定CI服务器')
+    elsif current_user.devops_step == User::DEVOPS_UNVERIFIED || current_user.ci_certification?
+      ci_user.destroy!
+      Ci::Repo.where(repo_namespace: current_user.login).delete_all
+      cloud_account.destroy!
+    end
+
+    current_user.projects.update_all(open_devops: false)
+    current_user.set_drone_step!(User::DEVOPS_UNINIT)
+
+    # TODO
+    # 删除用户项目下的与ci相关的所有webhook
+    current_user.projects.select(:id, :identifier, :gitea_webhook_id).each do |project|
+      if project.gitea_webhook_id
+        result = Gitea::Hooks::DestroyService.call(current_user.gitea_token, current_user.login, project.identifier, project.gitea_webhook_id)
+        project.update_column(:gitea_webhook_id, nil) if result.status == 204
+      end
+    end
+  end
+
+  def bind_hook!(user, cloud_account, repo)
+    hook_params = {
+      active: true,
+      config: {
+        content_type: "json",
+        url: cloud_account.drone_url + "/hook?secret=#{repo.repo_signer}"
+      },
+      type: "gitea"
+    }
+    result = Gitea::Hooks::CreateService.call(user.gitea_token, user.login, repo.repo_name, hook_params)
+
+    result[:status].present? ? nil : result
+  end
+
+  private
+    def devops_params
+      params.permit(:account, :secret, :ip_num)
+    end
+
+end

app/controllers/concerns/devopsable.rb

@@ -1,32 +0,0 @@
-module Devopsable
-  extend ActiveSupport::Concern
-
-  included do
-  end
-
-  # ci 权限验证
-  def ci_authorize!
-		render_forbidden unless @project.owner?(current_user)
-	end
-
-	def auto_load_project
-		@project = Project.find_by(id: params[:project_id]) || Project.find_by(identifier: params[:project_id])
-		render_not_found('未找到相关的项目') if @project.blank?
-	end
-
-  # TODO 暂时限制项目拥有者才有权限操作
-  def limit_project_owner_can_devops!(user, project)
-    return if project.owner? user
-    render_forbidden
-  end
-
-  def find_cloud_account
-    @cloud_account = Ci::CloudAccount.find params[:id]
-  end
-
-  def set_drone_token!(user, cloud_account, drone_token)
-    return if user.devops_has_token?
-    cloud_account.update_column(:drone_token, drone_token)
-    user.set_drone_step!(User::DEVOPS_HAS_TOKEN)
-  end
-end

app/controllers/users_controller.rb

@@ -1,10 +1,7 @@
 class UsersController < ApplicationController
-  include Devopsable
-
   before_action :load_user, only: [:show, :homepage_info, :sync_token, :sync_gitea_pwd, :projects, :watch_users, :fan_users]
   before_action :check_user_exist, only: [:show, :homepage_info,:projects, :watch_users, :fan_users]
-  before_action :require_login, only: %i[me list devops_authenticate devops]
-  before_action :auto_load_project, only: %i[devops devops_authenticate]
+  before_action :require_login, only: %i[me list]
   skip_before_action :check_sign, only: [:attachment_show]
 
   def list