diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d8e789703..f9c7c2f57b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Add complete plan flag to V2 API [#3595](https://github.com/DMPRoadmap/roadmap/pull/3595) - chore(deps): `bundle update && yarn upgrade` and bump `node-version` in workflows [#3601](https://github.com/DMPRoadmap/roadmap/pull/3601) - Update `database.yml.sample`: add separate test db [#3521](https://github.com/DMPRoadmap/roadmap/pull/3521) +- Patch inactive user issue in V2 API [#3610](https://github.com/DMPRoadmap/roadmap/pull/3610) ## v5.0.2 - Bump Ruby to v3.1.4 and use `.ruby-version` in CI diff --git a/app/controllers/api/v2/base_api_controller.rb b/app/controllers/api/v2/base_api_controller.rb index fa4aad2b79..a0c5cefc54 100644 --- a/app/controllers/api/v2/base_api_controller.rb +++ b/app/controllers/api/v2/base_api_controller.rb @@ -8,6 +8,8 @@ class BaseApiController < ApplicationController # rubocop:todo Style/Documentati # call doorkeeper to authorize the request before_action :doorkeeper_authorize!, except: %i[heartbeat] + # Authorize resource owner, check if the user account associated with the token is active + before_action :authorize_resource_owner, except: %i[heartbeat] # get details of server (e.g. DMPonline) and client app before_action :base_response_content @@ -35,18 +37,32 @@ def me ) end + protected + + def render_error(errors:, status:, details: nil) + @payload = { errors: errors, details: details } + render '/api/v2/error', status: status + end + private # define instance variable json and associated getter and setter methods attr_accessor :json + def authorize_resource_owner + return unless doorkeeper_token&.resource_owner_id.present? + + @resource_owner = User.find_by(id: doorkeeper_token.resource_owner_id) + + return if @resource_owner.present? && @resource_owner.active? + + render_error(errors: _('User account has been deactivated.'), status: :unauthorized) + end + def base_response_content @application = ApplicationService.application_name @client = doorkeeper_token&.application @caller = @client&.name || request.remote_ip - return unless doorkeeper_token&.resource_owner_id - - @resource_owner = User.find(doorkeeper_token.resource_owner_id) end def log_access @@ -68,17 +84,14 @@ def handle_exception(exception) def handle_internal_server_error(exception) # log server errors Rails.logger.error "Exception message: #{exception.message}" + Rails.logger.error exception.backtrace.join("\n") if exception.backtrace.present? # inform client of server error - message = _('There was a problem in the server.') - @payload = { message: [message] } - render '/api/v2/error', status: :internal_server_error + render_error(errors: _('There was a problem in the server.'), status: :internal_server_error) end def handle_client_not_authorized - message = _('The client is not authorized to perform this action.') - @payload = { message: [message] } - render '/api/v2/error', status: :forbidden + render_error(errors: _('The client is not authorized to perform this action.'), status: :forbidden) end # retrieve the requested pagination params or use defaults diff --git a/app/views/api/v2/error.json.jbuilder b/app/views/api/v2/error.json.jbuilder index ac08f26d9f..2576ad3e9c 100644 --- a/app/views/api/v2/error.json.jbuilder +++ b/app/views/api/v2/error.json.jbuilder @@ -3,5 +3,5 @@ json.partial! 'api/v2/standard_response' # json.items [] -json.message @payload[:message] +json.errors @payload[:errors] json.details @payload[:details] diff --git a/spec/requests/api/v2/base_controller_spec.rb b/spec/requests/api/v2/base_controller_spec.rb new file mode 100644 index 0000000000..831aa85b2f --- /dev/null +++ b/spec/requests/api/v2/base_controller_spec.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Api::V2::BaseApiController do + include ApiHelper + + describe 'GET /api/v2/me' do + context 'OAuth (authorization_code grant type) — on behalf of a user' do + before do + @user = create(:user) + @client = create(:oauth_application) + token = mock_authorization_code_token(oauth_application: @client, user: @user).plaintext_token + + @headers = { + Accept: 'application/json', + 'Content-Type': 'application/json', + Authorization: "Bearer #{token}" + } + end + + it 'returns 200 OK and user details when user is active' do + get(api_v2_me_path, headers: @headers) + + expect(response).to have_http_status(:ok) + + json = JSON.parse(response.body) + expect(json['email']).to eq(@user.email) + expect(json['firstname']).to eq(@user.firstname) + expect(json['surname']).to eq(@user.surname) + expect(json['organisation']).to eq(@user.org.name) + end + + it 'returns 401 Unauthorized when user account is deactivated' do + @user.update(active: false) + + get(api_v2_me_path, headers: @headers) + + expect(response).to have_http_status(:unauthorized) + + json = JSON.parse(response.body) + expect(json['errors']).to include('User account has been deactivated.') + end + end + + context 'when no authorization token is provided' do + it 'returns 401 Unauthorized' do + get(api_v2_me_path, headers: { Accept: 'application/json' }) + + expect(response).to have_http_status(:unauthorized) + end + end + end +end diff --git a/spec/views/api/v2/error.json.jbuilder_spec.rb b/spec/views/api/v2/error.json.jbuilder_spec.rb index ca2337ff6d..9732637ab7 100644 --- a/spec/views/api/v2/error.json.jbuilder_spec.rb +++ b/spec/views/api/v2/error.json.jbuilder_spec.rb @@ -8,7 +8,7 @@ @code = [200, 400, 404, 500].sample @errors = [Faker::Lorem.sentence, Faker::Lorem.sentence] - assign :payload, { message: @errors } + assign :payload, { errors: @errors } @resp = OpenStruct.new(status: @code) @req = Net::HTTPGenericRequest.new('GET', nil, nil, @url) @@ -23,7 +23,7 @@ end it ':errors contains an array of error messages' do - expect(@json[:message]).to eql(@errors) + expect(@json[:errors]).to eql(@errors) end end end