From e3aaa56c92f97f8aff3aeb847754794e9eae4706 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 24 Jul 2018 22:30:09 +0800 Subject: [PATCH 1/2] Introduce PolicyCheckable for checking policies --- app/models/deploy_token.rb | 5 +--- app/policies/concerns/policy_checkable.rb | 36 +++++++++++++++++++++++ config/application.rb | 1 + 3 files changed, 38 insertions(+), 4 deletions(-) create mode 100644 app/policies/concerns/policy_checkable.rb diff --git a/app/models/deploy_token.rb b/app/models/deploy_token.rb index 7ab647abe93..51029950dda 100644 --- a/app/models/deploy_token.rb +++ b/app/models/deploy_token.rb @@ -1,6 +1,7 @@ class DeployToken < ActiveRecord::Base include Expirable include TokenAuthenticatable + include PolicyCheckable add_authentication_token_field :token AVAILABLE_SCOPES = %i(read_repository read_registry).freeze @@ -58,10 +59,6 @@ class DeployToken < ActiveRecord::Base write_attribute(:expires_at, value.presence || Forever.date) end - def admin? - false - end - private def ensure_at_least_one_scope diff --git a/app/policies/concerns/policy_checkable.rb b/app/policies/concerns/policy_checkable.rb new file mode 100644 index 00000000000..5aae23c18e9 --- /dev/null +++ b/app/policies/concerns/policy_checkable.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +# Include this module if we want to pass something else than the user to +# check policies. This defines several methods which the policy checker +# would call and check. +module PolicyCheckable + extend ActiveSupport::Concern + + def blocked? + false + end + + def admin? + false + end + + def external? + false + end + + def internal? + false + end + + def access_locked? + false + end + + def required_terms_not_accepted? + false + end + + def can_create_group + false + end +end diff --git a/config/application.rb b/config/application.rb index 0304f466734..80d6ab7e9fa 100644 --- a/config/application.rb +++ b/config/application.rb @@ -43,6 +43,7 @@ module Gitlab #{config.root}/app/models/members #{config.root}/app/models/project_services #{config.root}/app/workers/concerns + #{config.root}/app/policies/concerns #{config.root}/app/services/concerns #{config.root}/app/serializers/concerns #{config.root}/app/finders/concerns From b577faf785a2c04f8376e3dc597dc0d6ec0b753d Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 27 Jul 2018 17:37:19 +0800 Subject: [PATCH 2/2] Rename the module and add a simple test to check if all methods are also presented in the user. --- app/models/deploy_token.rb | 2 +- .../{policy_checkable.rb => policy_actor.rb} | 2 +- spec/policies/concerns/policy_actor_spec.rb | 13 +++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) rename app/policies/concerns/{policy_checkable.rb => policy_actor.rb} (95%) create mode 100644 spec/policies/concerns/policy_actor_spec.rb diff --git a/app/models/deploy_token.rb b/app/models/deploy_token.rb index 51029950dda..fdbe95059e5 100644 --- a/app/models/deploy_token.rb +++ b/app/models/deploy_token.rb @@ -1,7 +1,7 @@ class DeployToken < ActiveRecord::Base include Expirable include TokenAuthenticatable - include PolicyCheckable + include PolicyActor add_authentication_token_field :token AVAILABLE_SCOPES = %i(read_repository read_registry).freeze diff --git a/app/policies/concerns/policy_checkable.rb b/app/policies/concerns/policy_actor.rb similarity index 95% rename from app/policies/concerns/policy_checkable.rb rename to app/policies/concerns/policy_actor.rb index 5aae23c18e9..069d065280e 100644 --- a/app/policies/concerns/policy_checkable.rb +++ b/app/policies/concerns/policy_actor.rb @@ -3,7 +3,7 @@ # Include this module if we want to pass something else than the user to # check policies. This defines several methods which the policy checker # would call and check. -module PolicyCheckable +module PolicyActor extend ActiveSupport::Concern def blocked? diff --git a/spec/policies/concerns/policy_actor_spec.rb b/spec/policies/concerns/policy_actor_spec.rb new file mode 100644 index 00000000000..27db9710a38 --- /dev/null +++ b/spec/policies/concerns/policy_actor_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe PolicyActor do + it 'implements all the methods from user' do + methods = subject.instance_methods + + # User.instance_methods do not return all methods until an instance is + # initialized. So here we just use an instance + expect(build(:user).methods).to include(*methods) + end +end