[Delivers #102256768] client slug authz

.env.example

@@ -8,6 +8,7 @@ MYUSA_SECRET: consumer_secret_key
 # optional
 #
 # API_ENABLED=true
+# ASSETS_DEBUG=true
 # BULLET_ENABLED=true
 # BUDGET_REPORT_RECIPIENT
 # DATABASE_URL

app/controllers/application_controller.rb

@@ -1,5 +1,5 @@
 class ApplicationController < ActionController::Base
-  include Pundit    # For authorization checks
+  include Pundit
   include ReturnToHelper
   include MarkdownHelper
 
@@ -11,11 +11,10 @@ class ApplicationController < ActionController::Base
 
   before_action :disable_peek_by_default
 
-
   protected
 
   # We are overriding this method to account for ExceptionPolicies
-  def authorize(record, query=nil, user=nil)
+  def authorize(record, query = nil, user = nil)
     user ||= @current_user
     policy = ::PolicyFinder.policy_for(user, record)
 
@@ -25,9 +24,9 @@ def authorize(record, query=nil, user=nil)
       # the method might raise its own exception, or it might return a
       # boolean. Both systems are accommodated
       # will need to replace this when a new version of pundit arrives
-      ex = NotAuthorizedError.new("not allowed to #{q} this #{record}")
-      ex.query, ex.record, ex.policy = q, record, pol
-      raise ex
+      msg = "not allowed to #{query} this #{record}"
+      ex = NotAuthorizedError.new(query: query, record: record, policy: policy, message: msg)
+      fail ex
     end
   end
 
@@ -63,7 +62,7 @@ def sign_out
   end
 
   def signed_in?
-    !!current_user
+    current_user ? true : false
   end
 
   def authenticate_user!

app/controllers/use_case_controller.rb

@@ -83,7 +83,8 @@ def auth_errors(exception)
     path = polymorphic_path(self.model_class, action: :new)
     # prevent redirect loop
     if path == request.path
-      redirect_to proposals_path, alert: exception.message
+      flash[:notice] = exception.message
+      render 'communicarts/authorization_error', status: 403
     else
       redirect_to path, alert: exception.message
     end

app/policies/proposal_policy.rb

@@ -20,8 +20,7 @@ def can_show!
   end
 
   def can_create!
-    # TODO restrict by client_slug
-    true
+    slug_matches? || @user.admin?
   end
   alias_method :can_new!, :can_create!
 
@@ -32,6 +31,15 @@ def can_cancel!
 
   protected
 
+  def use_case_namespace
+    cls = self.class.to_s
+    cls.gsub("::#{cls.demodulize}", '')
+  end
+
+  def slug_matches?
+    use_case_namespace.downcase == @user.client_slug
+  end
+
   def restricted?
     ENV['RESTRICT_ACCESS'] == 'true'
   end

app/views/communicarts/authentication_error.html.haml

@@ -1,8 +1,8 @@
-%h1
-  Authentication Error
-%h4
-  - if flash[:notice]
-    = flash[:notice]
-  - else
-    We were unable to authorize your request. Please check with the administrator.
-  =# TODO: Update with support email
+%div.inset
+  %h1
+    Authentication Error
+  %h4
+    - if flash[:notice]
+      = flash[:notice]
+    - else
+      We were unable to authorize your request. Please check with the administrator.

app/views/communicarts/authorization_error.html.haml

@@ -0,0 +1,7 @@
+%div.inset
+  %h1
+    Authorization error
+  %h4
+    You do not have access to this page
+    - if flash[:notice]
+      = flash[:notice]

config/environments/development.rb

@@ -25,7 +25,7 @@
   # Debug mode disables concatenation and preprocessing of assets.
   # This option may cause significant delays in view rendering with a large
   # number of complex assets.
-  config.assets.debug = true
+  config.assets.debug = ENV['ASSETS_DEBUG'] ? true : false
 
   # Use letter opener to avoid sending real emails. The "web" version makes
   # the emails visible at /letter_opener

doc/setup.md

@@ -37,7 +37,7 @@ Per [the Twelve-Factor guidelines](http://12factor.net/config), all necessary co
 ## Starting the application
 
 ```bash
-PORT=3000 ./script/start
+PORT=3000 ASSETS_DEBUG=true ./script/start
 open http://localhost:3000
 ```
 

lib/tasks/check.rake

@@ -0,0 +1,9 @@
+namespace :check do
+  desc "Report all non-admin Users with a null client_slug"
+  task client_slug: :environment do
+    User.where(client_slug: nil).each do |user|
+      next if user.admin?
+      puts "missing: #{user.id} #{user.email_address}"
+    end
+  end
+end

spec/controllers/ncr/work_orders_controller_spec.rb

@@ -1,7 +1,7 @@
 describe Ncr::WorkOrdersController do
   describe 'creating' do
     before do
-      login_as(FactoryGirl.create(:user))
+      login_as(FactoryGirl.create(:user, client_slug: 'ncr'))
     end
     let (:params) {{
       ncr_work_order: {

spec/features/18f_procurement_spec.rb

@@ -11,7 +11,7 @@
 
   context "when signed in" do
 
-    let(:requester) { FactoryGirl.create(:user) }
+    let(:requester) { FactoryGirl.create(:user, client_slug: 'gsa18f') }
     let(:procurement) {
       pr = FactoryGirl.create(:gsa18f_procurement, requester: requester)
       pr.add_approvals

spec/features/client_slug_authz_spec.rb

@@ -0,0 +1,8 @@
+describe "client_slug confers authz rules" do
+  it "rejects requests for user with no client_slug" do
+    user = FactoryGirl.create(:user)
+    login_as(user)
+    visit '/ncr/work_orders/new'
+    expect(page.status_code).to eq(403)
+  end
+end

spec/features/ncr_work_orders_spec.rb

@@ -16,18 +16,18 @@
 
     with_feature 'RESTRICT_ACCESS' do
       it "requires a GSA email address" do
-        user = FactoryGirl.create(:user, email_address: 'intruder@some.com')
+        user = FactoryGirl.create(:user, email_address: 'intruder@some.com', client_slug: 'ncr')
         login_as(user)
 
         visit '/ncr/work_orders/new'
 
-        expect(current_path).to eq('/proposals')
+        expect(page.status_code).to eq(403)
         expect(page).to have_content("You must be logged in with a GSA email address")
       end
     end
 
     context "when signed in as the requester" do
-      let(:requester) { FactoryGirl.create(:user) }
+      let(:requester) { FactoryGirl.create(:user, client_slug: 'ncr') }
       let(:ncr_helper_class) { Class.new { extend Ncr::WorkOrdersHelper } }
 
       before do
@@ -375,7 +375,7 @@
     end
 
     it "does not show a edit link for non requester" do
-      ncr_proposal.set_requester(FactoryGirl.create(:user))
+      ncr_proposal.set_requester(FactoryGirl.create(:user, client_slug: 'ncr'))
       visit "/proposals/#{ncr_proposal.id}"
       expect(page).not_to have_content('Modify Request')
     end
@@ -636,7 +636,7 @@
     end
 
     it "cannot be edited by someone other than the requester" do
-      stranger = FactoryGirl.create(:user)
+      stranger = FactoryGirl.create(:user, client_slug: 'ncr')
       login_as(stranger)
 
       visit "/ncr/work_orders/#{work_order.id}/edit"
@@ -648,7 +648,7 @@
   describe "delegate on a work order" do
     let(:work_order) { FactoryGirl.create(:ncr_work_order, description: 'test') }
     let(:proposal) { work_order.proposal }
-    let(:delegate) { FactoryGirl.create(:user) }
+    let(:delegate) { FactoryGirl.create(:user, client_slug: 'ncr') }
 
     before do
       work_order.setup_approvals_and_observers('approver@example.com')

spec/policies/gsa18f/procurement_policy_spec.rb

@@ -3,20 +3,20 @@
 
   permissions :can_create? do
     it "allows a user with an arbitrary email to create" do
-      user = User.new(email_address: 'user@some.com')
+      user = User.new(email_address: 'user@some.com', client_slug: 'gsa18f')
       procurement = Gsa18f::Procurement.new
       expect(subject).to permit(user, procurement)
     end
 
     with_feature 'RESTRICT_ACCESS' do
       it "allows someone with a GSA email to create" do
-        user = User.new(email_address: 'user@gsa.gov')
+        user = User.new(email_address: 'user@gsa.gov', client_slug: 'gsa18f')
         procurement = Gsa18f::Procurement.new
         expect(subject).to permit(user, procurement)
       end
 
       it "doesn't allow someone with a non-GSA email to create" do
-        user = User.new(email_address: 'intruder@some.com')
+        user = User.new(email_address: 'intruder@some.com', client_slug: 'gsa18f')
         procurement = Gsa18f::Procurement.new
         expect(subject).not_to permit(user, procurement)
       end

spec/policies/ncr/work_order_policy_spec.rb

@@ -15,7 +15,7 @@
     end
 
     it "allows an observer to edit it" do
-      observer = FactoryGirl.create(:user)
+      observer = FactoryGirl.create(:user, client_slug: 'ncr')
       proposal.add_observer(observer)
       expect(subject).to permit(observer, work_order)
     end
@@ -32,20 +32,20 @@
 
   permissions :can_create? do
     it "allows a user with an arbitrary email to create" do
-      user = User.new(email_address: 'user@some.com')
+      user = User.new(email_address: 'user@some.com', client_slug: 'ncr')
       work_order = Ncr::WorkOrder.new
       expect(subject).to permit(user, work_order)
     end
 
     with_feature 'RESTRICT_ACCESS' do
       it "allows someone with a GSA email to create" do
-        user = User.new(email_address: 'user@gsa.gov')
+        user = User.new(email_address: 'user@gsa.gov', client_slug: 'ncr')
         work_order = Ncr::WorkOrder.new
         expect(subject).to permit(user, work_order)
       end
 
       it "doesn't allow someone with a non-GSA email to create" do
-        user = User.new(email_address: 'intruder@some.com')
+        user = User.new(email_address: 'intruder@some.com', client_slug: 'ncr')
         work_order = Ncr::WorkOrder.new
         expect(subject).not_to permit(user, work_order)
       end