Ap 602 refactor roles (#46)

* Adding role-based auth and refactor require_framework_admin

* Tweak stackpass auth

* Tweak proxy borrower; remove redundant authorize call from S.P.

* Remove hardcoded admin functionality

* Rolling back rubocop disables

* Use authenticate_with_role in ref card; update specs

Author: steve-sullivan

app/controllers/concerns/auth_support.rb

@@ -12,6 +12,15 @@ def authenticate!
     raise Error::UnauthorizedError, "Endpoint #{controller_name}/#{action_name} requires authentication" unless authenticated?
   end
 
+  # Require current user be authenticated and have a role
+  def authenticate_with_role!(*roles)
+    authenticate!
+    return true if current_user.any_role?(*roles)
+
+    raise Error::ForbiddenError,
+          "Endpoint #{controller_name}/#{action_name} requires one of: #{roles.join(', ')}"
+  end
+
   # Return whether the current user is authenticated
   #
   # @return [Boolean]
@@ -46,11 +55,7 @@ def sign_out
   end
 
   def require_framework_admin!
-    authenticate!
-    return true if framework_admin?
-
-    raise Error::ForbiddenError,
-          "Endpoint #{controller_name}/#{action_name} requires framework admin CalGroup"
+    authenticate_with_role!(:framework_admin)
   end
 
   def framework_admin?

app/controllers/proxy_borrower_admin_controller.rb

@@ -109,7 +109,7 @@ def init_form!; end
 
   # You shall not pass....unless you're an admin
   def require_admin!
-    @user_is_admin = current_user.role?(Role.proxyborrow_admin)
+    @user_is_admin = current_user.any_role?(Role.proxyborrow_admin, :framework_admin)
     redirect_to proxy_borrower_forms_path unless @user_is_admin
   end
 

app/controllers/proxy_borrower_forms_controller.rb

@@ -9,7 +9,7 @@ def index
     # I think I want to get the users role now... if they're in the DB
     # then I'll want to pass that info so they have the
     # admin link...otherwise, NO admin link!
-    @user_is_admin = current_user.role?(Role.proxyborrow_admin)
+    @user_is_admin = current_user.any_role?(Role.proxyborrow_admin, :framework_admin)
   end
 
   def dsp_form
@@ -25,9 +25,6 @@ def faculty_form
                                       department: @current_user.department_number)
   end
 
-  # TODO: do we still need this?
-  def forbidden; end
-
   # Processes a request from DSP form: (eventually dry this up)
   def process_dsp_request
     @form = ProxyBorrowerRequests.new form_params(:student_name, :dsp_rep)

app/controllers/reference_card_forms_controller.rb

@@ -84,8 +84,6 @@ def validate_recaptcha!
   end
 
   def require_admin!
-    authenticate!
-    @user_is_admin = current_user.role?(Role.stackpass_admin)
-    raise Error::ForbiddenError unless @user_is_admin
+    @user_is_admin = authenticate_with_role!(Role.stackpass_admin, :framework_admin)
   end
 end

app/controllers/stack_pass_admin_controller.rb

@@ -51,7 +51,7 @@ def init_form!; end
 
   # You shall not pass....unless you're an admin
   def require_admin!
-    @user_is_admin = current_user.role?(Role.stackpass_admin)
+    @user_is_admin = current_user.any_role?(Role.stackpass_admin, :framework_admin)
     redirect_to stack_pass_forms_path unless @user_is_admin
   end
 

app/controllers/stack_pass_forms_controller.rb

@@ -86,8 +86,6 @@ def validate_recaptcha!
   end
 
   def require_admin!
-    authenticate!
-    @user_is_admin = current_user.role?(Role.stackpass_admin)
-    raise Error::ForbiddenError unless @user_is_admin
+    @user_is_admin = authenticate_with_role!(Role.stackpass_admin, :framework_admin)
   end
 end

app/controllers/stack_requests_controller.rb

@@ -8,7 +8,7 @@ class StackRequestsController < ApplicationController
   def forbidden; end
 
   def index
-    @user_is_admin = current_user.role?(Role.stackpass_admin)
+    @user_is_admin = current_user.any_role?(Role.stackpass_admin, :framework_admin)
   end
 
 end

app/models/framework_users.rb

@@ -21,18 +21,4 @@ class FrameworkUsers < ActiveRecord::Base
   validates :role,
             presence: true
 
-  class << self
-    # Hardcoded admins - so if for some reason all of the
-    # admins in the DB are deleted, we still have a way of
-    # getting in and managing things!
-    HARDCODED_ADMIN_UIDS = [
-      '7165',    # Lisa Weber
-      '1707532'  # Steve Sullivan
-    ].freeze
-
-    def hardcoded_admin?(uid)
-      HARDCODED_ADMIN_UIDS.include?(uid.to_s)
-    end
-  end
-
 end

app/models/user.rb

@@ -72,19 +72,26 @@ def primary_patron_record
     @primary_patron_record ||= find_primary_record
   end
 
-  # TODO: Unify this, faculty/staff checks, framework/alma admin checks
-  #       (and improve the design)
   def role?(role)
-    # First check if user is a hardcoded admin
-    return true if FrameworkUsers.hardcoded_admin?(uid)
+    role_name = role_name_for(role)
+
+    case role_name
+    when :framework_admin
+      return true if framework_admin?
+    when :alma_admin
+      return true if alma_admin?
+    end
 
-    # If user is not, then check if the user was added to the DB as an admin:
     user = FrameworkUsers.find_by(lcasid: uid)
     return false unless user
 
     user.assignments.exists?(role:)
   end
 
+  def any_role?(*roles)
+    roles.flatten.any? { |role| role?(role) }
+  end
+
   def ucb_faculty?
     affiliations&.include?('EMPLOYEE-TYPE-ACADEMIC')
   end
@@ -103,4 +110,17 @@ def uid_patron_record
   def find_primary_record
     uid_patron_record
   end
+
+  def role_name_for(role)
+    role_value =
+      if role.respond_to?(:role)
+        role.role
+      elsif role.respond_to?(:name)
+        role.name
+      else
+        role
+      end
+
+    role_value.to_sym
+  end
 end

spec/calnet_helper.rb

@@ -2,8 +2,8 @@
 require 'alma_helper'
 
 module CalnetHelper
-  # Lisa Weber's UID, hard-coded in FrameworkUsers
-  STACK_REQUEST_ADMIN_UID = '7165'.freeze
+  # UID used for test authentication
+  TEST_UID = '7165'.freeze
 
   # Mocks a calnet login as the specified patron, and stubs the corresponding
   # Millennium patron dump file. Suitable for calling from a before() block.