From 6e5459eb77742cf5214d268f21d1df2f8b9b6252 Mon Sep 17 00:00:00 2001
From: Nick Muerdter <nick.muerdter@nrel.gov>
Date: Tue, 25 Nov 2014 17:16:19 -0700
Subject: [PATCH] Fix two issues with api scopes and admin permissions to api
 backends:

1. If an api backend had multiple url prefixes, a limited admin account
   couldn't access the backend even if the admin was granted access to
   all the individual prefixes involved. admin account belonged to
   multiple. This is now fixed so that as long as your admin account has
   access to *all* the url prefixes involved in a backend, then you can
   access the backend.
2. Any admin could access an API Backend that had no URL prefixes
   defined on it. An API Backend without prefixes isn't actually valid
   or functional, so not a huge deal, but it led to some confusing
   results, since in some cases, that backend would not be visible, and
   then other times it would show up.
---
 app/policies/api_policy.rb                    | 10 ++--
 .../api/v1/apis_controller_spec.rb            | 59 ++++++++++++++++++-
 spec/factories/admin_groups.rb                | 18 ++++++
 spec/factories/api_scopes.rb                  | 16 +++++
 spec/factories/apis.rb                        | 23 ++++++++
 5 files changed, 118 insertions(+), 8 deletions(-)

diff --git a/app/policies/api_policy.rb b/app/policies/api_policy.rb
index 745adeb2..c29edd8d 100644
--- a/app/policies/api_policy.rb
+++ b/app/policies/api_policy.rb
@@ -69,14 +69,12 @@ def can?(permission)
         api_scopes = user.api_scopes_with_permission(permission)
       end
 
-      api_scopes.each do |api_scope|
-        if(record.frontend_host == api_scope.host)
-          allowed = record.url_matches.all? do |url_match|
-            api_scope.path_prefix_matcher.match(url_match.frontend_prefix)
+      if(record.url_matches.present?)
+        allowed = record.url_matches.all? do |url_match|
+          api_scopes.any? do |api_scope|
+            (record.frontend_host == api_scope.host && api_scope.path_prefix_matcher.match(url_match.frontend_prefix))
           end
         end
-
-        break if(allowed)
       end
     end
 
diff --git a/spec/controllers/api/v1/apis_controller_spec.rb b/spec/controllers/api/v1/apis_controller_spec.rb
index 44824641..c2fec2a0 100644
--- a/spec/controllers/api/v1/apis_controller_spec.rb
+++ b/spec/controllers/api/v1/apis_controller_spec.rb
@@ -5,6 +5,9 @@
     @admin = FactoryGirl.create(:admin)
     @google_admin = FactoryGirl.create(:limited_admin, :groups => [FactoryGirl.create(:google_admin_group, :backend_manage_permission)])
     @unauthorized_google_admin = FactoryGirl.create(:limited_admin, :groups => [FactoryGirl.create(:google_admin_group, :backend_publish_permission)])
+    @bing_single_all_scope_admin = FactoryGirl.create(:limited_admin, :groups => [FactoryGirl.create(:bing_admin_group_single_all_scope, :backend_manage_permission)])
+    @bing_single_restricted_scope_admin = FactoryGirl.create(:limited_admin, :groups => [FactoryGirl.create(:bing_admin_group_single_restricted_scope, :backend_manage_permission)])
+    @bing_multi_scope_admin = FactoryGirl.create(:limited_admin, :groups => [FactoryGirl.create(:bing_admin_group_multi_scope, :backend_manage_permission)])
   end
 
   before(:each) do
@@ -27,6 +30,11 @@
     })
     @google_extra_url_match_api = FactoryGirl.create(:google_extra_url_match_api)
     @yahoo_api = FactoryGirl.create(:yahoo_api)
+    @bing_api = FactoryGirl.create(:bing_api)
+    @bing_search_api = FactoryGirl.create(:bing_search_api)
+    @empty_url_prefixes_api = FactoryGirl.create(:google_api, {
+      :url_matches => [],
+    })
   end
 
   describe "GET index" do
@@ -50,10 +58,15 @@
 
         data = MultiJson.load(response.body)
         api_ids = data["data"].map { |api| api["id"] }
+        api_ids.length.should eql(8)
         api_ids.should include(@api.id)
         api_ids.should include(@google_api.id)
+        api_ids.should include(@google2_api.id)
         api_ids.should include(@google_extra_url_match_api.id)
         api_ids.should include(@yahoo_api.id)
+        api_ids.should include(@bing_api.id)
+        api_ids.should include(@bing_search_api.id)
+        api_ids.should include(@empty_url_prefixes_api.id)
       end
 
       it "includes apis the admin has access to" do
@@ -90,6 +103,39 @@
         data = MultiJson.load(response.body)
         data["data"].length.should eql(0)
       end
+
+      it "grants access to apis for any apis falling under the prefix of the scope" do
+        admin_token_auth(@bing_single_all_scope_admin)
+        get :index, :format => "json"
+
+        data = MultiJson.load(response.body)
+        api_ids = data["data"].map { |api| api["id"] }
+        api_ids.length.should eql(2)
+        api_ids.should include(@bing_api.id)
+        api_ids.should include(@bing_search_api.id)
+      end
+
+      it "grants access to apis with multiple prefixes when the admin has permissions to each prefix via separate scopes and groups" do
+        admin_token_auth(@bing_multi_scope_admin)
+        get :index, :format => "json"
+
+        data = MultiJson.load(response.body)
+        api_ids = data["data"].map { |api| api["id"] }
+        api_ids.length.should eql(2)
+        api_ids.should include(@bing_api.id)
+        api_ids.should include(@bing_search_api.id)
+      end
+
+      it "only grants access to apis when the admin has permission to all of the url prefixes" do
+        admin_token_auth(@bing_single_restricted_scope_admin)
+        get :index, :format => "json"
+
+        data = MultiJson.load(response.body)
+        api_ids = data["data"].map { |api| api["id"] }
+        api_ids.should_not include(@bing_api.id)
+        api_ids.length.should eql(1)
+        api_ids.should include(@bing_search_api.id)
+      end
     end
   end
 
@@ -104,7 +150,7 @@
         data["api"]["id"].should eql(@api.id)
       end
 
-      it "allows admins to create apis within the scope it has access to" do
+      it "allows admins to view apis within the scope it has access to" do
         admin_token_auth(@google_admin)
         get :show, :format => "json", :id => @google_api.id
 
@@ -113,7 +159,7 @@
         data["api"]["id"].should eql(@google_api.id)
       end
 
-      it "prevents admins from creating apis outside the scope it has access to" do
+      it "prevents admins from viewing apis outside the scope it has access to" do
         admin_token_auth(@google_admin)
         get :show, :format => "json", :id => @google_extra_url_match_api.id
 
@@ -130,6 +176,15 @@
         data = MultiJson.load(response.body)
         data.keys.should eql(["errors"])
       end
+
+      it "prevents limited admins from viewing incomplete apis without url prefixes" do
+        admin_token_auth(@google_admin)
+        get :show, :format => "json", :id => @empty_url_prefixes_api.id
+
+        response.status.should eql(403)
+        data = MultiJson.load(response.body)
+        data.keys.should eql(["errors"])
+      end
     end
   end
 
diff --git a/spec/factories/admin_groups.rb b/spec/factories/admin_groups.rb
index 89fb97ee..f093b566 100644
--- a/spec/factories/admin_groups.rb
+++ b/spec/factories/admin_groups.rb
@@ -43,5 +43,23 @@
     factory :yahoo_admin_group do
       api_scopes { [FactoryGirl.create(:yahoo_api_scope)] }
     end
+
+    factory :bing_admin_group_single_all_scope do
+      api_scopes { [FactoryGirl.create(:bing_all_api_scope)] }
+    end
+
+    factory :bing_admin_group_single_restricted_scope do
+      api_scopes { [FactoryGirl.create(:bing_search_api_scope)] }
+    end
+
+    factory :bing_admin_group_multi_scope do
+      api_scopes do
+        [
+          FactoryGirl.create(:bing_search_api_scope),
+          FactoryGirl.create(:bing_images_api_scope),
+          FactoryGirl.create(:bing_maps_api_scope),
+        ]
+      end
+    end
   end
 end
diff --git a/spec/factories/api_scopes.rb b/spec/factories/api_scopes.rb
index a3353ec4..339ba189 100644
--- a/spec/factories/api_scopes.rb
+++ b/spec/factories/api_scopes.rb
@@ -13,5 +13,21 @@
     factory :yahoo_api_scope do
       path_prefix "/yahoo"
     end
+
+    factory :bing_all_api_scope do
+      path_prefix "/bing"
+    end
+
+    factory :bing_search_api_scope do
+      path_prefix "/bing/search"
+    end
+
+    factory :bing_images_api_scope do
+      path_prefix "/bing/images"
+    end
+
+    factory :bing_maps_api_scope do
+      path_prefix "/bing/maps"
+    end
   end
 end
diff --git a/spec/factories/apis.rb b/spec/factories/apis.rb
index 21715cdd..4e7670a5 100644
--- a/spec/factories/apis.rb
+++ b/spec/factories/apis.rb
@@ -88,5 +88,28 @@
         ]
       end
     end
+
+    factory :bing_api do
+      sequence(:name) { |n| "Bing #{n}" }
+      backend_host "bing.com"
+
+      servers do
+        [FactoryGirl.attributes_for(:api_server, :host => "bing.com", :port => 80)]
+      end
+
+      url_matches do
+        [
+          FactoryGirl.attributes_for(:api_url_match, :frontend_prefix => "/bing/search", :backend_prefix => "/"),
+          FactoryGirl.attributes_for(:api_url_match, :frontend_prefix => "/bing/images", :backend_prefix => "/"),
+          FactoryGirl.attributes_for(:api_url_match, :frontend_prefix => "/bing/maps", :backend_prefix => "/"),
+        ]
+      end
+
+      factory :bing_search_api do
+        url_matches do
+          [FactoryGirl.attributes_for(:api_url_match, :frontend_prefix => "/bing/search", :backend_prefix => "/")]
+        end
+      end
+    end
   end
 end