Authentication with Panoptes Roles 💯 (#100)

* Add Pundit, remove dupes

* Add Pundit

* Roles are pulled out of a JSONB and keys must be strings

* Add ApplicationPolicy and spec

* Add UserRoles concern

* * Call super instead of repeating the gem
* Rescue from Pundit::Unauthorized
* Don't call verify_authorized until this is done

* Update project roles

* ProjectPolicy, spec, and AppPolicy updates

* ProjectController updates

* Workflow policy & controller updates

* Authorize singletons on #show routes

* Return a 403 on failed auth. Give each policy its own scope.

* TranscriptionPolicy & spec, only allow approvers to approve

* Add auth verifier after_actions

* Don't join a table if you've already got a foreign key

Co-Authored-By: Campbell Allen <[email protected]>

* Inline projects method, don't needlessly compact

* Move PanoptesApi out of app/lib and into app/services

* Move UserRole logic out of concern and into RolesChecker service

* Skip auth on root status route

* Move permissions questions to policy, use new policy action for approval check

* Remove comment

* Properly memoize project ids. Move empty? check to role checker.

* Reduce number of db calls to get transcription project policy

* Rename some methods+classes, privatize and un-spec others

Co-authored-by: Campbell Allen <[email protected]>

Gemfile

@@ -6,6 +6,7 @@ gem 'pg', '>= 0.18', '< 2.0'
 gem 'puma', '~> 4.3'
 
 gem 'panoptes-client'
+gem 'pundit'
 
 # jsonapi.rb is a bundle that incorporates fast_jsonapi (serialization),
 # ransack (filtration), and some RSpec matchers along with some
@@ -26,8 +27,6 @@ group :development, :test do
   gem 'rspec-rails'
   gem 'factory_bot_rails'
   gem 'jsonapi-rspec', require: false
-  gem 'pry-byebug'
-  gem 'rspec-rails'
 end
 
 group :development do
@@ -38,6 +37,7 @@ end
 
 group :test do
   gem 'simplecov'
+  gem 'pundit-matchers'
 end
 
 # Windows does not include zoneinfo files, so bundle the tzinfo-data gem

Gemfile.lock

@@ -147,6 +147,10 @@ GEM
       pry (>= 0.10.4)
     puma (4.3.1)
       nio4r (~> 2.0)
+    pundit (2.1.0)
+      activesupport (>= 3.0.0)
+    pundit-matchers (1.6.0)
+      rspec-rails (>= 3.0.0)
     rack (2.0.8)
     rack-cors (1.1.0)
       rack (>= 2.0.0)
@@ -230,7 +234,7 @@ GEM
       sprockets (>= 3.0.0)
     term-ansicolor (1.7.1)
       tins (~> 1.0)
-    thor (0.20.3)
+    thor (1.0.1)
     thread_safe (0.3.6)
     tins (1.22.2)
     tzinfo (1.2.5)
@@ -258,6 +262,8 @@ DEPENDENCIES
   pry-byebug
   pry-rails
   puma (~> 4.3)
+  pundit
+  pundit-matchers
   rack-cors
   rails (~> 6.0.1)
   rspec-rails

app/controllers/application_controller.rb

@@ -1,8 +1,12 @@
 class ApplicationController < ActionController::Base
+  include Pundit
+
   protect_from_forgery unless: -> { request.format.json? }
 
   attr_reader :current_user, :auth_token
   before_action :set_user
+  after_action :verify_authorized, except: :index
+  after_action :verify_policy_scoped, only: :index
 
   include ErrorExtender
   include JSONAPI::Pagination

app/controllers/concerns/error_extender.rb

@@ -6,6 +6,7 @@ module ErrorExtender
     rescue_from ActionController::BadRequest, with: :render_jsonapi_bad_request
     rescue_from ActiveModel::UnknownAttributeError, with: :render_jsonapi_unknown_attribute
     rescue_from Panoptes::Client::AuthenticationExpired, with: :render_jsonapi_token_expired
+    rescue_from Pundit::NotAuthorizedError, with: :render_jsonapi_not_authorized
   end
 
   def report_to_sentry(exception)
@@ -22,8 +23,7 @@ def report_to_sentry(exception)
   # Overriding this JSONAPI::Errors method to add Sentry reporting
   def render_jsonapi_internal_server_error(exception)
     report_to_sentry(exception)
-    error = { status: '500', title: Rack::Utils::HTTP_STATUS_CODES[500] }
-    render jsonapi_errors: [error], status: :internal_server_error
+    super(exception)
   end
 
   def render_jsonapi_bad_request(exception)
@@ -45,4 +45,9 @@ def render_jsonapi_unknown_attribute(exception)
 
     render jsonapi_errors: [error], status: :unprocessable_entity
   end
+
+  def render_jsonapi_not_authorized
+    error = { status: '403', title: Rack::Utils::HTTP_STATUS_CODES[403] }
+    render jsonapi_errors: [error], status: :forbidden
+  end
 end

app/controllers/projects_controller.rb

@@ -1,11 +1,12 @@
 class ProjectsController < ApplicationController
   def index
-    @projects = Project.all
+    @projects = policy_scope(Project)
     jsonapi_render(@projects, allowed_filters)
   end
 
   def show
     @project = Project.find(params[:id])
+    authorize @project
     render jsonapi: @project
   end
 

app/controllers/status_controller.rb

@@ -1,5 +1,6 @@
 class StatusController < ApplicationController
   def show
+    skip_authorization
     @status = ApplicationStatus.new
     render json: @status
   end

app/controllers/transcriptions_controller.rb

@@ -4,19 +4,27 @@ class TranscriptionsController < ApplicationController
   before_action :status_filter_to_int, only: :index
 
   def index
-    @transcriptions = Transcription.all
+    @transcriptions = policy_scope(Transcription)
     jsonapi_render(@transcriptions, allowed_filters)
   end
 
-  def update
+  def show
     @transcription = Transcription.find(params[:id])
-    raise ActionController::BadRequest if type_invalid?
-    @transcription.update!(update_attrs)
+    authorize @transcription
     render jsonapi: @transcription
   end
 
-  def show
+  def update
     @transcription = Transcription.find(params[:id])
+    raise ActionController::BadRequest if type_invalid?
+
+    if approve?
+      authorize @transcription, :approve?
+    else
+      authorize @transcription
+    end
+
+    @transcription.update!(update_attrs)
     render jsonapi: @transcription
   end
 
@@ -26,25 +34,25 @@ def update_attrs
     jsonapi_deserialize(params)
   end
 
-  # jsonapi.rb filtering doesn't handle filtering by enum term (e.g. 'ready'),
+  # Ransack filtering doesn't handle filtering by enum term (e.g. 'ready'),
   # so we must translate status terms to their integer value if they're present
   def status_filter_to_int
     if params[:filter]
       params[:filter].each do |key, value|
-        # filter key is comprised of <filterterm>_<relationship> 
+        # filter key is comprised of <filterterm>_<relationship>
         # e.g. id_eq, status_in, etc - check if filter term is status
         if key.split('_').first == 'status'
           # split status terms in case there is a list of them
           status_terms = value.split(',')
           status_enum_values = []
-          
+
           # for each status term, try to convert to enum value,
           # and add to list of converted enum values
           status_terms.each do |term|
             enum_value = Transcription.statuses[term]
             status_enum_values.append(enum_value.to_s) if enum_value
           end
-          
+
           # if list of converted enum values is not empty,
           # update params to reflect converted values
           unless status_enum_values.empty?
@@ -59,6 +67,10 @@ def type_invalid?
     params[:data][:type] != "transcriptions"
   end
 
+  def approve?
+    update_attrs["status"] == "approved"
+  end
+
   def allowed_filters
     [:id, :workflow_id, :group_id, :flagged, :status]
   end

app/controllers/workflows_controller.rb

@@ -1,11 +1,12 @@
 class WorkflowsController < ApplicationController
   def index
-    @workflows = Workflow.all
+    @workflows = policy_scope(Workflow)
     jsonapi_render(@workflows, allowed_filters)
   end
 
   def show
     @workflow = Workflow.find(params[:id])
+    authorize @workflow
     render jsonapi: @workflow
   end
 

app/policies/application_policy.rb

@@ -0,0 +1,37 @@
+class ApplicationPolicy
+  attr_reader :user, :records, :role_checker
+
+  def initialize(user, records)
+    @user = user
+    @records = Array.wrap(records)
+    raise Pundit::NotAuthorizedError, "must be logged in to Panoptes" unless logged_in?
+    @role_checker = ProjectRoleChecker.new(user, @records)
+  end
+
+  def index?
+    admin? || (logged_in? && viewer?)
+  end
+
+  def show?
+    admin? || (logged_in? && viewer?)
+  end
+
+  def admin?
+    logged_in? && user.admin
+  end
+
+  def logged_in?
+    !!user
+  end
+
+  class Scope
+    attr_reader :user, :scope, :role_checker
+
+    def initialize(user, scope)
+      raise Pundit::NotAuthorizedError, "must be logged in to Panoptes" unless user
+      @user = user
+      @scope = scope
+      @role_checker = ProjectRoleChecker.new(user, scope)
+    end
+  end
+end

app/policies/project_policy.rb

@@ -0,0 +1,27 @@
+class ProjectPolicy < ApplicationPolicy
+  def editor?
+    role_checker.can_edit?
+  end
+
+  def approver?
+    role_checker.can_approve?
+  end
+
+  def viewer?
+    role_checker.can_view?
+  end
+
+  class Scope < Scope
+    def resolve
+      if user.admin?
+        scope.all
+      else
+        viewer_policy_scope
+      end
+    end
+
+    def viewer_policy_scope
+      scope.where id: role_checker.viewer_project_ids
+    end
+  end
+end

app/policies/transcription_policy.rb

@@ -0,0 +1,38 @@
+class TranscriptionPolicy < ApplicationPolicy
+  delegate :editor?, :approver?, :viewer?, to: :project_policy
+
+  def update?
+    has_update_rights?
+  end
+
+  def approve?
+    if has_update_rights?
+      approver? || admin?
+    else
+      false
+    end
+  end
+
+  def has_update_rights?
+    admin? || (logged_in? && editor?)
+  end
+
+  def project_policy
+    workflow_ids = records.map(&:workflow_id).uniq
+    ProjectPolicy.new(user, Project.joins(:workflows).where(workflows: { id: workflow_ids }).distinct)
+  end
+
+  class Scope < Scope
+    def resolve
+      viewer_policy_scope
+    end
+
+    def viewer_policy_scope
+      if user.admin?
+        scope.all
+      elsif user
+        scope.joins(:workflow).where(workflows: { project_id: role_checker.viewer_project_ids } )
+      end
+    end
+  end
+end

app/policies/workflow_policy.rb

@@ -0,0 +1,21 @@
+class WorkflowPolicy < ApplicationPolicy
+  delegate :editor?, :approver?, :viewer?, to: :project_policy
+
+  def project_policy
+    ProjectPolicy.new(user, Project.where(id: records.pluck(:project_id).uniq))
+  end
+
+  class Scope < Scope
+    def resolve
+      viewer_policy_scope
+    end
+
+    def viewer_policy_scope
+      if user.admin?
+        scope.all
+      elsif user
+        scope.joins(:project).where project_id: role_checker.viewer_project_ids
+      end
+    end
+  end
+end

app/lib/panoptes_api.rb → app/services/panoptes_api.rb

@@ -18,7 +18,7 @@ def roles(user_id)
     { }.tap do |roles|
       response = get_roles(user_id)
       response['project_roles'].map do |role|
-        project_id = role['links']['project'].to_i
+        project_id = role['links']['project']
         roles[project_id] ||= []
         roles[project_id] |= role['roles']
       end

app/services/project_role_checker.rb

@@ -0,0 +1,51 @@
+class ProjectRoleChecker
+  attr_reader :user, :records, :viewer_project_ids
+
+  EDITOR_ROLES = %w(owner collaborator expert scientist moderator)
+  APPROVER_ROLES = %w(owner collaborator)
+  VIEWER_ROLES = %w(owner collaborator expert scientist tester)
+
+  def initialize(user, records)
+    @user = user
+    @records = records
+    @viewer_project_ids = get_viewer_project_ids
+  end
+
+  def can_edit?
+    ids = user_project_ids(user.roles, EDITOR_ROLES)
+    check_roles(ids, records)
+  end
+
+  def can_approve?
+    ids = user_project_ids(user.roles, APPROVER_ROLES)
+    check_roles(ids, records)
+  end
+
+  def can_view?
+    ids = user_project_ids(user.roles, VIEWER_ROLES)
+    check_roles(ids, records)
+  end
+
+  def get_viewer_project_ids
+    user_project_ids(user.roles, VIEWER_ROLES)
+  end
+
+  private
+
+  def user_project_ids(user_roles, allowed_roles)
+    allowed_role_ids = []
+    user_roles.each do |id, roles|
+      if (roles & allowed_roles).any?
+        allowed_role_ids << id
+      end
+    end
+    allowed_role_ids
+  end
+
+  def check_roles(project_ids, records)
+    return false if records.empty?
+    records.all? do |record|
+      project_ids.include? record.id.to_s
+    end
+  end
+end