From 4dbfa14e160e0d9bca11941adcf04b3d272aa1a2 Mon Sep 17 00:00:00 2001
From: Timothy Andrew <mail@timothyandrew.net>
Date: Fri, 23 Jun 2017 11:18:44 +0000
Subject: [PATCH] Implement review comments from @dbalexandre for !12300.

---
 lib/api/api_guard.rb                          | 12 ++++------
 lib/api/helpers.rb                            |  4 ++--
 .../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