diff --git a/Gemfile b/Gemfile index 02cc8702..08cdd7d8 100644 --- a/Gemfile +++ b/Gemfile @@ -29,6 +29,7 @@ gem 'gemini_upload-rails', gemini_gem_spec # for admins to upload images gem 'graphql' # A lovely API gem 'graphiql-rails' # A lovely interface to the API +gem 'graphql-guard' # Authorization helpers for graphQL watt_gem_spec = { git: 'https://github.com/artsy/watt.git', branch: 'master' } gem 'watt', watt_gem_spec # artsy bootstrap diff --git a/Gemfile.lock b/Gemfile.lock index 6888388b..898f7482 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -124,6 +124,8 @@ GEM graphiql-rails (1.4.7) rails graphql (1.6.7) + graphql-guard (1.0.0) + graphql (>= 1.6.0, < 2) haml (5.0.1) temple (>= 0.8.0) tilt @@ -356,6 +358,7 @@ DEPENDENCIES gemini_upload-rails! graphiql-rails graphql + graphql-guard haml-rails hyperclient jquery-rails diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index 9bf4c15d..48fab6ed 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -33,6 +33,10 @@ def require_authorized_submission raise ApplicationController::NotAuthorized unless current_user && current_user == @submission.user_id end + def require_app + raise ApplicationController::NotAuthorized unless current_app + end + private # For now, require that signature is valid by verifying payload w/ secret. diff --git a/app/controllers/api/graphql_controller.rb b/app/controllers/api/graphql_controller.rb index 2557df6e..7508557e 100644 --- a/app/controllers/api/graphql_controller.rb +++ b/app/controllers/api/graphql_controller.rb @@ -1,7 +1,5 @@ module Api class GraphqlController < BaseController - before_action :require_authentication - def execute result = RootSchema.execute( params[:query], @@ -9,7 +7,8 @@ def execute context: { current_application: current_app, current_user: current_user - } + }, + except: PermissionBlacklist ) render json: result, status: 200 end diff --git a/app/graph/permission_blacklist.rb b/app/graph/permission_blacklist.rb new file mode 100644 index 00000000..ba28758e --- /dev/null +++ b/app/graph/permission_blacklist.rb @@ -0,0 +1,7 @@ +class PermissionBlacklist + def self.call(schema_member, context) + if schema_member.name == 'user_id' + return context[:current_user].blank? + end + end +end diff --git a/spec/requests/api/graphql/query_spec.rb b/spec/requests/api/graphql/query_spec.rb index 94ea55a2..4ea24be8 100644 --- a/spec/requests/api/graphql/query_spec.rb +++ b/spec/requests/api/graphql/query_spec.rb @@ -20,11 +20,40 @@ end describe 'POST /graphql' do - it 'rejects unauthorized requests' do + it 'does not return the user_id if there is no user' do + introspection_query = <<-graphql + query { + __type(name: "Submission") { + name + fields { + name + } + } + } + graphql post '/api/graphql', params: { - query: query_submissions - }, headers: { 'Authorization' => 'Bearer foo.bar.baz' } - expect(response.status).to eq 401 + query: introspection_query + } + expect(JSON.parse(response.body)['data']['__type']['fields'].map{|f| f['name']}).to_not include('user_id') + expect(response.status).to eq 200 + end + + it 'includes the user_id param if there is a user present' do + introspection_query = <<-graphql + query { + __type(name: "Submission") { + name + fields { + name + } + } + } + graphql + post '/api/graphql', params: { + query: introspection_query + }, headers: headers + expect(JSON.parse(response.body)['data']['__type']['fields'].map{|f| f['name']}).to include('user_id') + expect(response.status).to eq 200 end it 'finds two existing submissions' do