From b3586a2512d549b092840f802280bf108f1e99a4 Mon Sep 17 00:00:00 2001 From: ArayB Date: Fri, 4 Mar 2022 08:37:11 +0000 Subject: [PATCH 1/5] Create draft PR for #25 From d0f7a04472d307dfe6a16229f3d9aa368d3e3b12 Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Fri, 4 Mar 2022 13:50:48 +0000 Subject: [PATCH 2/5] Add projects controller --- app/controllers/api/projects_controller.rb | 32 ++++++++++++++++++++++ config/routes.rb | 2 +- spec/request/projects/show_spec.rb | 30 ++++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 app/controllers/api/projects_controller.rb create mode 100644 spec/request/projects/show_spec.rb diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb new file mode 100644 index 000000000..129290aec --- /dev/null +++ b/app/controllers/api/projects_controller.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Api + class ProjectsController < ApiController + require 'phrase_identifier' + + def show + @project = Project.find_by!(identifier: params[:id]) + render :show, formats: [:json] + end + + def update + components = project_params[:components] + + components.each do |comp_params| + component = Component.find(comp_params[:id]) + component.update(comp_params) + end + + head :ok + end + + private + + def project_params + params.require(:project) + .permit(:identifier, + :type, + components: %i[id name extension content]) + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 3903d8011..5f9a666f9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -12,7 +12,7 @@ get '/python', to: 'default_projects#python' end - namespace :projects do + resources :projects, only: %i[show update] do resources :phrases, only: %i[show update] do resource :remix, only: %i[create] end diff --git a/spec/request/projects/show_spec.rb b/spec/request/projects/show_spec.rb new file mode 100644 index 000000000..4df8f8b3e --- /dev/null +++ b/spec/request/projects/show_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Project show requests', type: :request do + let!(:project) { create(:project) } + + it 'returns success response' do + get "/api/projects/#{project.identifier}" + + expect(response.status).to eq(200) + end + + it 'returns json' do + get "/api/projects/#{project.identifier}" + expect(response.content_type).to eq('application/json; charset=utf-8') + end + + it 'returns the project json' do + expected = { identifier: project.identifier, project_type: 'python', name: project.name, components: [] }.to_json + get "/api/projects/#{project.identifier}" + expect(response.body).to eq(expected) + end + + it 'returns 404 response if invalid project' do + get '/api/projects/no-such-project' + + expect(response.status).to eq(404) + end +end From 8589561f8ed8b44ffebbf6cf09703f85c15275be Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Fri, 4 Mar 2022 15:30:12 +0000 Subject: [PATCH 3/5] Remove "phrases" from routing --- .../project/operation/create_remix.rb | 4 +- .../api/projects/phrases_controller.rb | 36 ------------------ .../api/projects/remixes_controller.rb | 2 +- app/controllers/api/projects_controller.rb | 19 +++++++--- config/routes.rb | 4 +- docker-compose.yml | 4 +- .../project/operation/create_remix_spec.rb | 4 +- spec/request/projects/remix_spec.rb | 6 +-- spec/request/projects/show_spec.rb | 12 +++++- spec/request/projects/update_spec.rb | 38 +++++++++++++++++++ 10 files changed, 73 insertions(+), 56 deletions(-) delete mode 100644 app/controllers/api/projects/phrases_controller.rb create mode 100644 spec/request/projects/update_spec.rb diff --git a/app/concepts/project/operation/create_remix.rb b/app/concepts/project/operation/create_remix.rb index cb617cec5..22a7c3818 100644 --- a/app/concepts/project/operation/create_remix.rb +++ b/app/concepts/project/operation/create_remix.rb @@ -19,12 +19,12 @@ class << self private def validate_params(response, params, user_id) - valid = params[:phrase_id].present? && user_id.present? + valid = params[:project_id].present? && user_id.present? response[:error] = 'Invalid parameters' unless valid end def remix_project(response, params, user_id) - original_project = Project.find_by!(identifier: params[:phrase_id]) + original_project = Project.find_by!(identifier: params[:project_id]) response[:project] = create_remix(original_project, user_id) diff --git a/app/controllers/api/projects/phrases_controller.rb b/app/controllers/api/projects/phrases_controller.rb deleted file mode 100644 index d92373ad8..000000000 --- a/app/controllers/api/projects/phrases_controller.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -module Api - module Projects - class PhrasesController < ApiController - require 'phrase_identifier' - - def show - @project = Project.find_by!(identifier: params[:id]) - render '/api/projects/show', formats: [:json] - end - - def update - @project = Project.find_by!(identifier: params[:id]) - - if oauth_user_id && oauth_user_id == @project.user_id - components = project_params[:components] - - components.each do |comp_params| - component = Component.find(comp_params[:id]) - component.update(comp_params) - end - head :ok - else - head :unauthorized - end - end - - private - - def project_params - params.require(:project).permit(:identifier, :type, components: %i[id name extension content]) - end - end - end -end diff --git a/app/controllers/api/projects/remixes_controller.rb b/app/controllers/api/projects/remixes_controller.rb index eb25f0a46..f24a87963 100644 --- a/app/controllers/api/projects/remixes_controller.rb +++ b/app/controllers/api/projects/remixes_controller.rb @@ -19,7 +19,7 @@ def create private def remix_params - params.permit(:phrase_id) + params.permit(:project_id) end end end diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb index 129290aec..f57f0d0a8 100644 --- a/app/controllers/api/projects_controller.rb +++ b/app/controllers/api/projects_controller.rb @@ -4,20 +4,27 @@ module Api class ProjectsController < ApiController require 'phrase_identifier' + before_action :require_oauth_user, only: %i[update] + def show @project = Project.find_by!(identifier: params[:id]) render :show, formats: [:json] end def update - components = project_params[:components] + @project = Project.find_by!(identifier: params[:id]) - components.each do |comp_params| - component = Component.find(comp_params[:id]) - component.update(comp_params) - end + if oauth_user_id && oauth_user_id == @project.user_id + components = project_params[:components] - head :ok + components.each do |comp_params| + component = Component.find(comp_params[:id]) + component.update(comp_params) + end + head :ok + else + head :unauthorized + end end private diff --git a/config/routes.rb b/config/routes.rb index 5f9a666f9..0b07b8978 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -13,9 +13,7 @@ end resources :projects, only: %i[show update] do - resources :phrases, only: %i[show update] do - resource :remix, only: %i[create] - end + resource :remix, only: %i[create], controller: 'projects/remixes' end end end diff --git a/docker-compose.yml b/docker-compose.yml index 6591b4a01..662fc10ce 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -14,6 +14,7 @@ services: command: bash -c "rm -f tmp/pids/server.pid && bundle exec rails s -p 3009 -b '0.0.0.0'" volumes: - .:/app + - bundle:/usr/local/bundle ports: - "3009:3009" depends_on: @@ -22,4 +23,5 @@ services: tty: true volumes: - pg-data: + pg-data: null + bundle: null diff --git a/spec/concepts/project/operation/create_remix_spec.rb b/spec/concepts/project/operation/create_remix_spec.rb index 9cc6a6c69..c2c58e221 100644 --- a/spec/concepts/project/operation/create_remix_spec.rb +++ b/spec/concepts/project/operation/create_remix_spec.rb @@ -14,7 +14,7 @@ describe '.call' do context 'when all params valid' do - let(:params) { { phrase_id: original_project.identifier } } + let(:params) { { project_id: original_project.identifier } } it 'returns success' do result = create_remix @@ -54,7 +54,7 @@ context 'when user_id is not present' do let(:user_id) { nil } - let(:params) { { phrase_id: original_project.identifier } } + let(:params) { { project_id: original_project.identifier } } it 'returns failure' do result = create_remix diff --git a/spec/request/projects/remix_spec.rb b/spec/request/projects/remix_spec.rb index 1f1a62c46..bb8a28fc7 100644 --- a/spec/request/projects/remix_spec.rb +++ b/spec/request/projects/remix_spec.rb @@ -17,13 +17,13 @@ end it 'returns success response' do - post "/api/projects/phrases/#{original_project.identifier}/remix" + post "/api/projects/#{original_project.identifier}/remix" expect(response.status).to eq(200) end it 'returns 404 response if invalid project' do - post '/api/projects/phrases/no-such-project/remix' + post '/api/projects/no-such-project/remix' expect(response.status).to eq(404) end @@ -31,7 +31,7 @@ context 'when auth is invalid' do it 'returns unauthorized' do - post "/api/projects/phrases/#{original_project.identifier}/remix" + post "/api/projects/#{original_project.identifier}/remix" expect(response.status).to eq(401) end diff --git a/spec/request/projects/show_spec.rb b/spec/request/projects/show_spec.rb index 4df8f8b3e..02ac6aba9 100644 --- a/spec/request/projects/show_spec.rb +++ b/spec/request/projects/show_spec.rb @@ -4,6 +4,15 @@ RSpec.describe 'Project show requests', type: :request do let!(:project) { create(:project) } + let(:expected_json) do + { + identifier: project.identifier, + project_type: 'python', + name: project.name, + user_id: project.user_id, + components: [] + }.to_json + end it 'returns success response' do get "/api/projects/#{project.identifier}" @@ -17,9 +26,8 @@ end it 'returns the project json' do - expected = { identifier: project.identifier, project_type: 'python', name: project.name, components: [] }.to_json get "/api/projects/#{project.identifier}" - expect(response.body).to eq(expected) + expect(response.body).to eq(expected_json) end it 'returns 404 response if invalid project' do diff --git a/spec/request/projects/update_spec.rb b/spec/request/projects/update_spec.rb new file mode 100644 index 000000000..5d171f5e9 --- /dev/null +++ b/spec/request/projects/update_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Project update requests', type: :request do + let(:user_id) { 'e0675b6c-dc48-4cd6-8c04-0f7ac05af51a' } + let(:project) { create(:project, user_id: user_id) } + + context 'when authed user is project creator' do + let(:project) { create(:project, user_id: user_id) } + let!(:component) { create(:component, project: project) } + let(:changes) { { name: 'updated', extension: 'py', content: 'updated content' } } + let(:params) { { project: { components: [{ id: component.id, **changes }] } } } + + before do + mock_oauth_user(user_id) + end + + it 'returns success response' do + put "/api/projects/#{project.identifier}", params: params + expect(response.status).to eq(200) + end + + it 'updates component' do + put "/api/projects/#{project.identifier}", params: params + updated = component.reload.attributes.symbolize_keys.slice(:name, :content, :extension) + expect(updated).to eq(changes) + end + end + + context 'when auth is invalid' do + it 'returns unauthorized' do + put "/api/projects/#{project.identifier}" + + expect(response.status).to eq(401) + end + end +end From 70893431a31fc2f40b6d68897081a7f963091309 Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Fri, 4 Mar 2022 15:59:27 +0000 Subject: [PATCH 4/5] Add CanCanCan and use it to authorize updating projects --- Gemfile | 1 + Gemfile.lock | 2 ++ app/controllers/api/projects_controller.rb | 16 ++++++---------- app/controllers/api_controller.rb | 13 ++++++++++++- app/models/ability.rb | 11 +++++++++++ spec/request/projects/update_spec.rb | 15 +++++++++++++++ 6 files changed, 47 insertions(+), 11 deletions(-) create mode 100644 app/models/ability.rb diff --git a/Gemfile b/Gemfile index cb2a7d483..69896c150 100644 --- a/Gemfile +++ b/Gemfile @@ -6,6 +6,7 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" } ruby '3.0.3' gem 'bootsnap', require: false +gem 'cancancan', '~> 3.3' gem 'faraday' gem 'importmap-rails' gem 'jbuilder' diff --git a/Gemfile.lock b/Gemfile.lock index 2ac248da0..affd4caa7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -67,6 +67,7 @@ GEM msgpack (~> 1.0) builder (3.2.4) byebug (11.1.3) + cancancan (3.3.0) coderay (1.1.3) concurrent-ruby (1.1.9) crack (0.4.5) @@ -250,6 +251,7 @@ PLATFORMS DEPENDENCIES bootsnap + cancancan (~> 3.3) dotenv-rails factory_bot_rails faker diff --git a/app/controllers/api/projects_controller.rb b/app/controllers/api/projects_controller.rb index f57f0d0a8..f773d8b66 100644 --- a/app/controllers/api/projects_controller.rb +++ b/app/controllers/api/projects_controller.rb @@ -13,18 +13,14 @@ def show def update @project = Project.find_by!(identifier: params[:id]) + authorize! :update, @project - if oauth_user_id && oauth_user_id == @project.user_id - components = project_params[:components] - - components.each do |comp_params| - component = Component.find(comp_params[:id]) - component.update(comp_params) - end - head :ok - else - head :unauthorized + components = project_params[:components] + components.each do |comp_params| + component = Component.find(comp_params[:id]) + component.update(comp_params) end + head :ok end private diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index 1a43feefd..ed2e93956 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -5,15 +5,26 @@ class ApiController < ActionController::API unless Rails.application.config.consider_all_requests_local rescue_from ActiveRecord::RecordNotFound, with: -> { return404 } + rescue_from CanCan::AccessDenied, with: -> { return401 } end + private def require_oauth_user head :unauthorized unless oauth_user_id end + def current_user + # current_user is required by CanCanCan + oauth_user_id + end + def return404 - render json: { error: '404 Not found' }, status: :not_found + head :not_found + end + + def return401 + head :unauthorized end end diff --git a/app/models/ability.rb b/app/models/ability.rb new file mode 100644 index 000000000..b5e6044dd --- /dev/null +++ b/app/models/ability.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class Ability + include CanCan::Ability + + def initialize(user) + return unless user.present? + + can :update, Project, user_id: user + end +end diff --git a/spec/request/projects/update_spec.rb b/spec/request/projects/update_spec.rb index 5d171f5e9..5fd984713 100644 --- a/spec/request/projects/update_spec.rb +++ b/spec/request/projects/update_spec.rb @@ -28,6 +28,21 @@ end end + context 'when authed user is not creator' do + let(:project) { create(:project) } + let!(:component) { create(:component, project: project) } + let(:params) { { project: { components: [] } } } + + before do + mock_oauth_user(user_id) + end + + it 'returns unauthorized response' do + put "/api/projects/#{project.identifier}", params: params + expect(response.status).to eq(401) + end + end + context 'when auth is invalid' do it 'returns unauthorized' do put "/api/projects/#{project.identifier}" From eaf50038edd8e4262242014ec0703f1c26353c19 Mon Sep 17 00:00:00 2001 From: Ant Barnes Date: Fri, 4 Mar 2022 16:01:45 +0000 Subject: [PATCH 5/5] Tidy up --- app/controllers/api_controller.rb | 1 - app/models/ability.rb | 2 +- spec/request/projects/update_spec.rb | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/app/controllers/api_controller.rb b/app/controllers/api_controller.rb index ed2e93956..fecf2da8c 100644 --- a/app/controllers/api_controller.rb +++ b/app/controllers/api_controller.rb @@ -8,7 +8,6 @@ class ApiController < ActionController::API rescue_from CanCan::AccessDenied, with: -> { return401 } end - private def require_oauth_user diff --git a/app/models/ability.rb b/app/models/ability.rb index b5e6044dd..438d12901 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -4,7 +4,7 @@ class Ability include CanCan::Ability def initialize(user) - return unless user.present? + return if user.blank? can :update, Project, user_id: user end diff --git a/spec/request/projects/update_spec.rb b/spec/request/projects/update_spec.rb index 5fd984713..0c97d046e 100644 --- a/spec/request/projects/update_spec.rb +++ b/spec/request/projects/update_spec.rb @@ -30,7 +30,6 @@ context 'when authed user is not creator' do let(:project) { create(:project) } - let!(:component) { create(:component, project: project) } let(:params) { { project: { components: [] } } } before do