Skip to content

Commit

Permalink
Pundit authorizations for users (#18)
Browse files Browse the repository at this point in the history
* add panoptes_client and require login when querying for /user/<id>

* update faraday to get panoptes-client.rb to work. remove typo on require_login on app controller

* add policy to check if current user(i.e. querying user) can view stats of the queried user

* Update application_controller.rb

* Update user_classification_count_controller.rb

* fix user_classification_count specs to take care of authorizatons

* add spec for unauthorized user

* remove unneeded scopes in app policies

* add specs for policies

* update missing token specs

* adding specs for missing headers/token authenticaton

* update typo

* remove parens from queried_user policy and remove logged_in? method on application_policy

* rename specs for logged in user

* Update README.md to link to repo wiki

* typo on application_controller user client had hard coded env, removing wrapper object on pundit policy in favor of  headless policy, sending param to current_user,

* remove error raising in application policy for cases when we expect user to be nil

* remove spec that checks if error raised if user nil on application policy to allow more open scope
  • Loading branch information
yuenmichelle1 authored Jul 25, 2023
1 parent 7a5a261 commit dc50012
Show file tree
Hide file tree
Showing 13 changed files with 284 additions and 32 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" }
# Reduces boot times through caching; required in config/boot.rb
gem 'bootsnap', require: false
gem 'composite_primary_keys'
gem 'panoptes-client'
# Use postgresql as the database for Active Record
gem 'pg', '~> 1.1'
# Use the Puma web server [https://github.com/puma/puma]
Expand Down
38 changes: 38 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ GEM
debug (1.8.0)
irb (>= 1.5.0)
reline (>= 0.3.1)
deprecate (0.0.0)
diff-lcs (1.5.0)
docile (1.4.0)
erubi (1.12.0)
Expand All @@ -94,6 +95,34 @@ GEM
factory_bot_rails (6.2.0)
factory_bot (~> 6.2.0)
railties (>= 5.0.0)
faraday (1.10.3)
faraday-em_http (~> 1.0)
faraday-em_synchrony (~> 1.0)
faraday-excon (~> 1.1)
faraday-httpclient (~> 1.0)
faraday-multipart (~> 1.0)
faraday-net_http (~> 1.0)
faraday-net_http_persistent (~> 1.0)
faraday-patron (~> 1.0)
faraday-rack (~> 1.0)
faraday-retry (~> 1.0)
ruby2_keywords (>= 0.0.4)
faraday-em_http (1.0.0)
faraday-em_synchrony (1.0.0)
faraday-excon (1.1.0)
faraday-httpclient (1.0.1)
faraday-multipart (1.0.4)
multipart-post (~> 2)
faraday-net_http (1.0.1)
faraday-net_http_persistent (1.2.0)
faraday-panoptes (0.4.0)
faraday (~> 1.0)
faraday_middleware (~> 1.0)
faraday-patron (1.0.0)
faraday-rack (1.0.0)
faraday-retry (1.0.3)
faraday_middleware (1.2.0)
faraday (~> 1.0)
globalid (1.1.0)
activesupport (>= 5.0)
i18n (1.14.1)
Expand All @@ -102,6 +131,7 @@ GEM
irb (1.6.4)
reline (>= 0.3.0)
json (2.6.3)
jwt (1.5.6)
loofah (2.21.3)
crass (~> 1.0.2)
nokogiri (>= 1.12.0)
Expand All @@ -115,6 +145,7 @@ GEM
mini_mime (1.1.2)
minitest (5.18.1)
msgpack (1.7.0)
multipart-post (2.3.0)
net-imap (0.3.6)
date
net-protocol
Expand All @@ -129,6 +160,11 @@ GEM
racc (~> 1.4)
nokogiri (1.15.3-x86_64-linux)
racc (~> 1.4)
panoptes-client (1.2.0)
deprecate
faraday
faraday-panoptes (~> 0.3)
jwt (~> 1.5.0)
parallel (1.23.0)
parser (3.2.2.1)
ast (~> 2.4.1)
Expand Down Expand Up @@ -213,6 +249,7 @@ GEM
rubocop-ast (1.28.1)
parser (>= 3.2.1.0)
ruby-progressbar (1.13.0)
ruby2_keywords (0.0.5)
simplecov (0.22.0)
docile (~> 1.1)
simplecov-html (~> 0.11)
Expand Down Expand Up @@ -242,6 +279,7 @@ DEPENDENCIES
database_cleaner
debug
factory_bot_rails
panoptes-client
pg (~> 1.1)
pry-byebug
puma (~> 5.0)
Expand Down
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ Enhanced Running Average Stats Service

Zooniverse's Stats Service Measuring Volunteer Effort and Contribution.

## Full Documentation

See the [Wiki](https://github.com/zooniverse/eras/wiki) for full documentation, examples, operational details and other information

## Running App Locally With Docker
```
docker-compose build
Expand Down
45 changes: 45 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,56 @@
class ApplicationController < ActionController::API
include Pundit::Authorization
class ValidationError < StandardError; end
class Unauthorized < StandardError; end

attr_reader :current_user

after_action :verify_authorized, except: [:index]
after_action :verify_policy_scoped, only: [:index]

rescue_from ValidationError, with: :render_bad_request
rescue_from Unauthorized, with: :not_authorized
rescue_from Pundit::NotAuthorizedError, with: :not_authorized
rescue_from Panoptes::Client::ResourceNotFound, with: :not_found

private

def require_login
@current_user = client.me
rescue Panoptes::Client::ServerError
raise Unauthorized, 'Could not check authentication with Panoptes'
end

def client
return @client if @client

authorization_header = request.headers['Authorization']
raise Unauthorized, 'Missing Authorization header' unless authorization_header

authorization_token = authorization_header.match(/\ABearer (.*)\Z/).try { |match| match[1] }
raise Unauthorized, 'Missing Bearer token' unless authorization_token

@client = Panoptes::Client.new \
env: Rails.env.to_sym,
auth: { token: authorization_token }
end

def panoptes_application_client
@panoptes_application_client ||= Panoptes::Client.new \
env: Rails.env.to_sym,
auth: { client_id: Rails.application.credentials.panoptes_client_id,
client_secret: Rails.application.credentials.panoptes_client_secret },
params: { admin: true }
end

def not_authorized(exception)
render_exception :forbidden, exception
end

def not_found(exception)
render_exception :not_found, exception
end

def render_bad_request(exception)
render_exception(400, exception)
end
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/kinesis_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

class KinesisController < ApplicationController
def create
# TODO: Authorizations on Kinesis
skip_authorization
kinesis_stream.create_events(params['payload'])
head :no_content
end
Expand Down
4 changes: 3 additions & 1 deletion app/controllers/user_classification_count_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
class UserClassificationCountController < ApplicationController
before_action :validate_params
before_action :sanitize_params
before_action :require_login

def query
# TODO: policies and scopes should be added
current_user['queried_user_id'] = params[:id]
authorize :queried_user_context, :show?
user_classification_counts = CountUserClassifications.new(user_classification_count_params).call(user_classification_count_params)
render json: UserClassificationCountsSerializer.new(user_classification_counts), serializer_options: serializer_opts_from_params
end
Expand Down
14 changes: 14 additions & 0 deletions app/policies/application_policy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# frozen_string_literal: true

class ApplicationPolicy
attr_reader :user, :record

def initialize(user, record)
@user = user
@record = record
end

def panoptes_admin?
user['admin'] == true
end
end
18 changes: 18 additions & 0 deletions app/policies/queried_user_context_policy.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# frozen_string_literal: true

class QueriedUserContextPolicy < ApplicationPolicy
attr_reader :user

def initialize(user, _record)
super
@user = user
end

def show?
current_user_is_queried_user? || panoptes_admin?
end

def current_user_is_queried_user?
user['id']&.to_i == user['queried_user_id']&.to_i
end
end
112 changes: 81 additions & 31 deletions spec/controllers/user_classification_count_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,45 +3,95 @@
require 'rails_helper'

RSpec.describe UserClassificationCountController do
include AuthenticationHelpers

describe 'GET query' do
let!(:classification_event) { create(:classification_event) }

it 'returns total count of user classification events' do
get :query, params: { id: classification_event.user_id }
expected_response = { total_count: 1 }
expect(response.status).to eq(200)
expect(response.body).to eq(expected_response.to_json)
context 'user querying their own stats' do
before(:each) { authenticate!(classification_event.user_id) }

it 'returns total count of user classification events' do
get :query, params: { id: classification_event.user_id.to_s }
expected_response = { total_count: 1 }
expect(response.status).to eq(200)
expect(response.body).to eq(expected_response.to_json)
end

it 'returns total_count and breakdown of user classifications when period given' do
get :query, params: { id: classification_event.user_id, period: 'day' }
expect(response.status).to eq(200)
response_body = JSON.parse(response.body)
expect(response_body['total_count']).to eq(1)
expect(response_body['data'].length).to eq(1)
expect(response_body['data'][0]['period']).to eq("#{Date.today}T00:00:00.000Z")
expect(response_body['data'][0]['count']).to eq(1)
end

it 'returns total_count and time_spent and breakdown of user classifications wher period is given' do
get :query, params: { id: classification_event.user_id, period: 'day', time_spent: true }
expect(response.status).to eq(200)
response_body = JSON.parse(response.body)
expect(response_body['total_count']).to eq(1)
expect(response_body['time_spent']).to eq(classification_event.session_time)
expect(response_body['data'].length).to eq(1)
expect(response_body['data'][0]['period']).to eq("#{Date.today}T00:00:00.000Z")
expect(response_body['data'][0]['count']).to eq(1)
expect(response_body['data'][0]['session_time']).to eq(classification_event.session_time)
end

it 'returns top contributions and unique project contributions if querying for top_project_contributions' do
get :query, params: { id: classification_event.user_id, top_project_contributions: 10 }
expect(response.status).to eq(200)
response_body = JSON.parse(response.body)
expect(response_body['unique_project_contributions']).to eq(1)
expect(response_body['top_project_contributions'].length).to eq(1)
expect(response_body['top_project_contributions'][0]['project_id']).to eq(classification_event.project_id)
end
end

it 'returns total_count and breakdown of user classifications when period given' do
get :query, params: { id: classification_event.user_id, period: 'day' }
expect(response.status).to eq(200)
response_body = JSON.parse(response.body)
expect(response_body['total_count']).to eq(1)
expect(response_body['data'].length).to eq(1)
expect(response_body['data'][0]['period']).to eq("#{Date.today}T00:00:00.000Z")
expect(response_body['data'][0]['count']).to eq(1)
context 'zooniverse_admin' do
before(:each) { authenticate!(is_panoptes_admin: true) }
it 'returns successful response' do
get :query, params: { id: classification_event.user_id.to_s }
expected_response = { total_count: 1 }
expect(response.status).to eq(200)
expect(response.body).to eq(expected_response.to_json)
end
end

it 'returns total_count and time_spent and breakdown of user classifications wher period is given' do
get :query, params: { id: classification_event.user_id, period: 'day', time_spent: true }
expect(response.status).to eq(200)
response_body = JSON.parse(response.body)
expect(response_body['total_count']).to eq(1)
expect(response_body['time_spent']).to eq(classification_event.session_time)
expect(response_body['data'].length).to eq(1)
expect(response_body['data'][0]['period']).to eq("#{Date.today}T00:00:00.000Z")
expect(response_body['data'][0]['count']).to eq(1)
expect(response_body['data'][0]['session_time']).to eq(classification_event.session_time)
context 'not authorized user' do
before(:each) { authenticate! }

it 'returns a 403 not authorized response' do
get :query, params: { id: classification_event.user_id.to_s }
expect(response.status).to eq(403)
end
end

it 'returns top contributions and unique project contributions if querying for top_project_contributions' do
get :query, params: { id: classification_event.user_id, top_project_contributions: 10 }
expect(response.status).to eq(200)
response_body = JSON.parse(response.body)
expect(response_body['unique_project_contributions']).to eq(1)
expect(response_body['top_project_contributions'].length).to eq(1)
expect(response_body['top_project_contributions'][0]['project_id']).to eq(classification_event.project_id)
context 'missing token' do
it 'returns a 403 missing authorization header' do
get :query, params: { id: classification_event.user_id.to_s }
expected_response = { error: 'Missing Authorization header' }
expect(response.status).to eq(403)
expect(response.body).to eq(expected_response.to_json)
end

it 'returns a 403 missing when missing bearer token' do
request.headers['Authorization'] = 'asjdhaskdhsa'
get :query, params: { id: classification_event.user_id.to_s }
expected_response = { error: 'Missing Bearer token' }
expect(response.status).to eq(403)
expect(response.body).to eq(expected_response.to_json)
end
end

it 'returns forbidden if panoptes fails to find user' do
allow(controller).to receive(:client).and_raise(Panoptes::Client::ServerError, 'an error')
get :query, params: { id: classification_event.user_id.to_s }
expected_response = { error: 'Could not check authentication with Panoptes' }
expect(response.status).to eq(403)
expect(response.body).to eq(expected_response.to_json)
end

context 'param validations' do
Expand All @@ -59,7 +109,7 @@
expect(response.body).to include('Cannot query top projects and query by project/workflow')
end

it 'ensures top_project_cotributions is an integer' do
it 'ensures top_project_contributions is an integer' do
get :query, params: { id: 1, top_project_contributions: '20' }
expect(controller.params[:top_project_contributions]).to eq(20)
end
Expand Down
25 changes: 25 additions & 0 deletions spec/policies/application_policy_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# frozen_string_literal: true

RSpec.describe ApplicationPolicy, type: :policy do
let(:records) { [] }
let(:user) {
{
'id' => '1234',
'login' => 'login',
'display_name' => 'display_name'
}
}
let(:policy) { ApplicationPolicy.new user, records }

context 'with a user' do
it 'sets panoptes_admin? to be false if user is not a panoptes admin' do
expect(policy.panoptes_admin?).to be false
end

it 'sets panoptes_admin? to true if user is panoptes admin' do
user['admin'] = true
ApplicationPolicy.new user, records
expect(policy.panoptes_admin?).to be true
end
end
end
Loading

0 comments on commit dc50012

Please sign in to comment.