Skip to content

Commit

Permalink
Fix website backends listing being visible for limited admins.
Browse files Browse the repository at this point in the history
The listing of website backends didn't properly account for admin
permissions in some cases. But the admins couldn't edit or display the
individual website backends, so the issue was limited only to the
listing page.

See: 18F/api.data.gov#261

This also adds a "none" scope for mongoid, which a couple of our
policies were already erroneously using (which was resulting in errors
being thrown in certain unexpected admin permission circumstances).
  • Loading branch information
GUI committed May 4, 2016
1 parent ba79855 commit c909dc0
Show file tree
Hide file tree
Showing 8 changed files with 227 additions and 3 deletions.
4 changes: 4 additions & 0 deletions src/api-umbrella/web-app/app/models/api_scope.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,8 @@ def path_prefix_matcher
def display_name
"#{self.name} - #{self.host}#{self.path_prefix}"
end

def root?
(self.path_prefix.blank? || self.path_prefix == "/")
end
end
10 changes: 7 additions & 3 deletions src/api-umbrella/web-app/app/policies/website_backend_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,18 @@ def resolve

query_scopes = []
api_scopes.each do |api_scope|
if(api_scope.path_prefix.blank? || api_scope.path_prefix == "/")
if(api_scope.root?)
query_scopes << {
:frontend_host => api_scope.host,
}
end
end

scope.or(query_scopes)
if(query_scopes.any?)
scope.or(query_scopes)
else
scope.none
end
end
end
end
Expand Down Expand Up @@ -50,7 +54,7 @@ def can?(permission)
api_scopes = user.api_scopes_with_permission(permission)

allowed = api_scopes.any? do |api_scope|
(record.frontend_host == api_scope.host && (api_scope.path_prefix.blank? || api_scope.path_prefix == "/"))
(record.frontend_host == api_scope.host && api_scope.root?)
end
end

Expand Down
72 changes: 72 additions & 0 deletions src/api-umbrella/web-app/config/initializers/mongoid_none_scope.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# Backport "none" scope to Mongoid 3:
# https://gist.github.com/backspace/d77d93c892da8c2577f9
if(Mongoid::VERSION.to_i != 3)
STDERR.puts "WARNING: Mongoid no longer version 3. config/initializers/mongoid_none_scope.rb should no longer be needed in Mongoid 4"
else
# rubocop:disable all
module Mongoid
class Criteria
def none
@none = true and self
end

def empty_and_chainable?
!!@none
end
end

module Contextual
class None
include ::Enumerable

# Previously included Queryable, which has been extracted in v4
attr_reader :collection, :criteria, :klass

def blank?
!exists?
end
alias :empty? :blank?

attr_reader :criteria, :klass

def ==(other)
other.is_a?(None)
end

def each
if block_given?
[].each { |doc| yield(doc) }
self
else
to_enum
end
end

def exists?; false; end

def initialize(criteria)
@criteria, @klass = criteria, criteria.klass
end

def last; nil; end

def length
entries.length
end
alias :size :length
end

private

def create_context
return None.new(self) if empty_and_chainable?
embedded ? Memory.new(self) : Mongo.new(self)
end
end

module Finders
delegate :none, to: :with_default_scope
end
end
# rubocop:enable all
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
require "spec_helper"

describe Api::V1::WebsiteBackendsController do
before(:all) do
@admin = FactoryGirl.create(:admin)
@amazon_root_admin = FactoryGirl.create(:limited_admin, :groups => [FactoryGirl.create(:amazon_admin_group_single_root_scope, :backend_manage_permission)])
@unauthorized_amazon_root_admin = FactoryGirl.create(:limited_admin, :groups => [FactoryGirl.create(:amazon_admin_group_single_root_scope, :backend_publish_permission)])
@amazon_sub_admin = FactoryGirl.create(:limited_admin, :groups => [FactoryGirl.create(:amazon_admin_group_single_sub_scope, :backend_manage_permission)])
@amazon_multi_admin = FactoryGirl.create(:limited_admin, :groups => [FactoryGirl.create(:amazon_admin_group_multi_scope, :backend_manage_permission)])
end

before(:each) do
WebsiteBackend.delete_all

@website_backend = FactoryGirl.create(:website_backend)
@amazon_website_backend = FactoryGirl.create(:amazon_website_backend)
end

describe "GET index" do
it "returns datatables output fields" do
admin_token_auth(@admin)
get :index, :format => "json"

data = MultiJson.load(response.body)
data.keys.sort.should eql([
"data",
"draw",
"recordsFiltered",
"recordsTotal",
])
end

it "paginates results" do
admin_token_auth(@admin)
get :index, :format => "json", :length => "1"

website_backend_count = WebsiteBackend.count
website_backend_count.should be > 1

data = MultiJson.load(response.body)
data["recordsTotal"].should eql(website_backend_count)
data["recordsFiltered"].should eql(website_backend_count)
data["data"].length.should eql(1)
end

describe "admin permissions" do
it "includes all website backends for superuser admins" do
admin_token_auth(@admin)
get :index, :format => "json"

data = MultiJson.load(response.body)
website_backend_ids = data["data"].map { |api| api["id"] }
website_backend_ids.length.should eql(2)
website_backend_ids.should include(@website_backend.id)
website_backend_ids.should include(@amazon_website_backend.id)
end

it "includes website backends the admin has access to" do
admin_token_auth(@amazon_root_admin)
get :index, :format => "json"

data = MultiJson.load(response.body)
website_backend_ids = data["data"].map { |api| api["id"] }
website_backend_ids.should include(@amazon_website_backend.id)
end

it "excludes website backends the admin does not have access to" do
admin_token_auth(@amazon_root_admin)
get :index, :format => "json"

data = MultiJson.load(response.body)
website_backend_ids = data["data"].map { |api| api["id"] }
website_backend_ids.should_not include(@website_backend.id)
end

it "excludes website backends the admin only has partial access to" do
admin_token_auth(@amazon_sub_admin)
get :index, :format => "json"

data = MultiJson.load(response.body)
website_backend_ids = data["data"].map { |api| api["id"] }
website_backend_ids.should_not include(@amazon_website_backend.id)
end

it "excludes all website backends for admins without proper access" do
admin_token_auth(@unauthorized_amazon_root_admin)
get :index, :format => "json"

data = MultiJson.load(response.body)
data["data"].length.should eql(0)
end

it "grants access to website backends with multiple prefixes when the admin has permissions to each prefix via separate scopes and groups" do
admin_token_auth(@amazon_multi_admin)
get :index, :format => "json"

data = MultiJson.load(response.body)
website_backend_ids = data["data"].map { |api| api["id"] }
website_backend_ids.length.should eql(1)
website_backend_ids.should include(@amazon_website_backend.id)
end
end
end
end
17 changes: 17 additions & 0 deletions src/api-umbrella/web-app/spec/factories/admin_groups.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,22 @@
]
end
end

factory :amazon_admin_group_single_root_scope do
api_scopes { [FactoryGirl.create(:amazon_api_scope)] }
end

factory :amazon_admin_group_single_sub_scope do
api_scopes { [FactoryGirl.create(:amazon_books_api_scope)] }
end

factory :amazon_admin_group_multi_scope do
api_scopes do
[
FactoryGirl.create(:amazon_api_scope),
FactoryGirl.create(:amazon_books_api_scope),
]
end
end
end
end
10 changes: 10 additions & 0 deletions src/api-umbrella/web-app/spec/factories/api_scopes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,15 @@
factory :bing_maps_api_scope do
path_prefix "/bing/maps"
end

factory :amazon_api_scope do
host "amazon.com"
path_prefix "/"
end

factory :amazon_books_api_scope do
host "amazon.com"
path_prefix "/books"
end
end
end
12 changes: 12 additions & 0 deletions src/api-umbrella/web-app/spec/factories/website_backends.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
FactoryGirl.define do
factory :website_backend do
frontend_host "localhost"
backend_protocol "http"
server_host "example.com"
server_port 80

factory :amazon_website_backend do
frontend_host "amazon.com"
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

before(:each) do
Api.delete_all
WebsiteBackend.delete_all
ConfigVersion.delete_all
end

Expand Down

0 comments on commit c909dc0

Please sign in to comment.