Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 22 additions & 9 deletions app/controllers/api/v2/base_api_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/views/api/v2/error.json.jbuilder
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
json.partial! 'api/v2/standard_response'
Comment thread
momo3404 marked this conversation as resolved.

# json.items []
json.message @payload[:message]
json.errors @payload[:errors]
json.details @payload[:details]
54 changes: 54 additions & 0 deletions spec/requests/api/v2/base_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions spec/views/api/v2/error.json.jbuilder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Loading