diff options
author | Timothy Andrew <mail@timothyandrew.net> | 2017-06-23 14:18:44 +0300 |
---|---|---|
committer | Timothy Andrew <mail@timothyandrew.net> | 2017-06-28 10:17:13 +0300 |
commit | 4dbfa14e160e0d9bca11941adcf04b3d272aa1a2 (patch) | |
tree | 9affe145ebecd6e57ced495fa18d29d2a406d37a | |
parent | 1b8223dd51345f6075172a92dab610f9dee89d84 (diff) |
Implement review comments from @dbalexandre for !12300.
-rw-r--r-- | lib/api/api_guard.rb | 12 | ||||
-rw-r--r-- | lib/api/helpers.rb | 4 | ||||
-rw-r--r-- | spec/services/access_token_validation_service_spec.rb | 24 |
3 files changed, 18 insertions, 22 deletions
diff --git a/lib/api/api_guard.rb b/lib/api/api_guard.rb index d4599aaeed0..8411ad8ec34 100644 --- a/lib/api/api_guard.rb +++ b/lib/api/api_guard.rb @@ -30,15 +30,13 @@ module API # endpoint class. If this method is called multiple times on the same class, # the scopes are all aggregated. def allow_access_with_scope(scopes, options = {}) - @scopes ||= [] - - params = Array.wrap(scopes).map { |scope| { name: scope, if: options[:if] } } - - @scopes.concat(params) + Array(scopes).each do |scope| + allowed_scopes << { name: scope, if: options[:if] } + end end - def scopes - @scopes + def allowed_scopes + @scopes ||= [] end end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 5c0b82587ab..a2a661b205c 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -416,8 +416,8 @@ module API begin endpoint_classes = [options[:for].presence, ::API::API].compact endpoint_classes.reduce([]) do |memo, endpoint| - if endpoint.respond_to?(:scopes) - memo.concat(endpoint.scopes) + if endpoint.respond_to?(:allowed_scopes) + memo.concat(endpoint.allowed_scopes) else memo end diff --git a/spec/services/access_token_validation_service_spec.rb b/spec/services/access_token_validation_service_spec.rb index eff4269a4d5..c8189aa14d8 100644 --- a/spec/services/access_token_validation_service_spec.rb +++ b/spec/services/access_token_validation_service_spec.rb @@ -41,24 +41,22 @@ describe AccessTokenValidationService, services: true do end context "conditions" do - context "if" do - it "ignores any scopes whose `if` condition returns false" do - token = double("token", scopes: [:api, :read_user]) + it "ignores any scopes whose `if` condition returns false" do + token = double("token", scopes: [:api, :read_user]) - expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { false } }])).to be(false) - end + expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { false } }])).to be(false) + end - it "does not ignore scopes whose `if` condition is not set" do - token = double("token", scopes: [:api, :read_user]) + it "does not ignore scopes whose `if` condition is not set" do + token = double("token", scopes: [:api, :read_user]) - expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { false } }, { name: :read_user }])).to be(true) - end + expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { false } }, { name: :read_user }])).to be(true) + end - it "does not ignore scopes whose `if` condition returns true" do - token = double("token", scopes: [:api, :read_user]) + it "does not ignore scopes whose `if` condition returns true" do + token = double("token", scopes: [:api, :read_user]) - expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { true } }, { name: :read_user, if: ->(_) { false } }])).to be(true) - end + expect(described_class.new(token, request: request).include_any_scope?([{ name: :api, if: ->(_) { true } }, { name: :read_user, if: ->(_) { false } }])).to be(true) end end end |