diff --git a/app/controllers/api/v1/base_controller.rb b/app/controllers/api/v1/base_controller.rb
index 9a6cc1d0..7b92a287 100644
--- a/app/controllers/api/v1/base_controller.rb
+++ b/app/controllers/api/v1/base_controller.rb
@@ -1,4 +1,6 @@
class Api::V1::BaseController < ApplicationController
+ include Concerns::AuthApi
+
protect_from_forgery with: :null_session
before_action :skip_session
@@ -11,16 +13,6 @@ class Api::V1::BaseController < ApplicationController
private
- def authenticate
- doorkeeper_authorize!
- super if current_user
- end
-
- # @return [User] Current user, or +nil+ if no valid token.
- def current_user
- @current_user ||= User.undeleted.find(doorkeeper_token.resource_owner_id) if doorkeeper_token
- end
-
# @return [Ordergroup] Current user's ordergroup, or +nil+ if no valid token or user has no ordergroup.
def current_ordergroup
current_user.try(:ordergroup)
@@ -28,7 +20,9 @@ class Api::V1::BaseController < ApplicationController
def require_ordergroup
authenticate
- raise Api::Errors::PermissionRequired unless current_user.ordergroup.present?
+ unless current_ordergroup.present?
+ raise Api::Errors::PermissionRequired.new('Forbidden, must be in an ordergroup')
+ end
end
def skip_session
@@ -42,13 +36,18 @@ class Api::V1::BaseController < ApplicationController
end
def not_acceptable_handler(e)
- render status: 422, json: {error: 'not_acceptable', error_description: e.message || 'Data not acceptable' }
+ msg = e.message || 'Data not acceptable'
+ render status: 422, json: {error: 'not_acceptable', error_description: msg}
end
def doorkeeper_unauthorized_render_options(error:)
{json: {error: error.name, error_description: error.description}}
end
+ def doorkeeper_forbidden_render_options(error:)
+ {json: {error: error.name, error_description: error.description}}
+ end
+
def permission_required_handler(e)
msg = e.message || 'Forbidden, user has no access'
render status: 403, json: {error: 'forbidden', error_description: msg}
@@ -58,5 +57,4 @@ class Api::V1::BaseController < ApplicationController
def show_user(user = current_user, **options)
user.display
end
-
end
diff --git a/app/controllers/api/v1/configs_controller.rb b/app/controllers/api/v1/configs_controller.rb
index 3ba0ed4b..8edb73c4 100644
--- a/app/controllers/api/v1/configs_controller.rb
+++ b/app/controllers/api/v1/configs_controller.rb
@@ -1,5 +1,7 @@
class Api::V1::ConfigsController < Api::V1::BaseController
+ before_action ->{ doorkeeper_authorize! 'config:user', 'config:read', 'config:write' }
+
def show
render json: FoodsoftConfig, serializer: ConfigSerializer, root: 'config'
end
diff --git a/app/controllers/api/v1/user/users_controller.rb b/app/controllers/api/v1/user/users_controller.rb
new file mode 100644
index 00000000..e95e4790
--- /dev/null
+++ b/app/controllers/api/v1/user/users_controller.rb
@@ -0,0 +1,9 @@
+class Api::V1::User::UsersController < Api::V1::BaseController
+
+ before_action ->{ doorkeeper_authorize! 'user:read', 'user:write' }
+
+ def show
+ render json: current_user
+ end
+
+end
diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb
deleted file mode 100644
index f260c0c9..00000000
--- a/app/controllers/api/v1/users_controller.rb
+++ /dev/null
@@ -1,7 +0,0 @@
-class Api::V1::UsersController < Api::V1::BaseController
-
- def show
- render json: current_user
- end
-
-end
diff --git a/app/controllers/concerns/auth_api.rb b/app/controllers/concerns/auth_api.rb
new file mode 100644
index 00000000..be3389ec
--- /dev/null
+++ b/app/controllers/concerns/auth_api.rb
@@ -0,0 +1,66 @@
+# Controller concern for authentication methods
+#
+# Split off from main +ApplicationController+ to allow e.g.
+# Doorkeeper to use it too.
+module Concerns::AuthApi
+ extend ActiveSupport::Concern
+
+ protected
+
+ def authenticate
+ # authenticate does not look at scopes, we just want to know if there's a valid token
+ # @see https://github.com/doorkeeper-gem/doorkeeper/blob/v5.0.1/lib/doorkeeper/models/access_token_mixin.rb#L218
+ doorkeeper_render_error unless doorkeeper_token && doorkeeper_token.accessible?
+ super if current_user
+ end
+
+ # @return [User] Current user, or +nil+ if no valid token.
+ def current_user
+ @current_user ||= User.undeleted.find(doorkeeper_token.resource_owner_id) if doorkeeper_token
+ end
+
+ def doorkeeper_authorize!(*scopes)
+ super(*scopes)
+ # In addition to Doorkeeper's authorization and scope check, we also verify
+ # that the user has permissions for the scope (through its workgroups).
+ # Unless no scopes were supplied, which means we only want to make sure there
+ # is a valid user.
+ #
+ # If Doorkeeper's +handle_auth_errors+ is set to +:raise+, we don't get here,
+ # but otherwise we need to check whether +super+ rendered an error.
+ doorkeeper_authorize_roles!(*scopes) if scopes.present? && !performed?
+ end
+
+ private
+
+ # Make sure that at least one the given OAuth scopes is valid for the current user's permissions.
+ # @raise Api::Errors::PermissionsRequired
+ def doorkeeper_authorize_roles!(*scopes)
+ unless scopes.any? {|scope| doorkeeper_scope_permitted?(scope) }
+ raise Api::Errors::PermissionRequired.new('Forbidden, no permission')
+ end
+ end
+
+ # Check whether a given OAuth scope is permitted for the current user.
+ # @note This does not validate whether the given scope is a valid scope.
+ # @return [Boolean] Whether the given scope is valid for the current user.
+ # @see https://github.com/foodcoops/foodsoft/issues/582#issuecomment-442513237
+ def doorkeeper_scope_permitted?(scope)
+ scope_parts = scope.split(':')
+ # user sub-scopes like +config:user+ are always permitted
+ if scope_parts.last == 'user'
+ return true
+ end
+
+ case scope_parts.first
+ when 'user' then true # access to the current user's own profile
+ when 'config' then current_user.role_admin?
+ when 'users' then current_user.role_admin?
+ when 'workgroups' then current_user.role_admin?
+ when 'suppliers' then current_user.role_suppliers?
+ when 'group_orders' then current_user.role_orders?
+ when 'finance' then current_user.role_finance?
+ # please note that offline_access does not belong here, since it is not used for permission checking
+ end
+ end
+end
diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb
index ad5be092..253e7b42 100644
--- a/config/initializers/doorkeeper.rb
+++ b/config/initializers/doorkeeper.rb
@@ -47,8 +47,13 @@ Doorkeeper.configure do
# Define access token scopes for your provider
# For more information go to
# https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes
- # default_scopes :public
- # optional_scopes :write, :update
+
+ # default is a collection of read-only scopes
+ default_scopes 'config:user', 'user:read'
+
+ optional_scopes 'config:read', 'config:write',
+ 'user:write',
+ 'offline_access'
# Change the way client credentials are retrieved from the request object.
# By default it retrieves first from the `HTTP_AUTHORIZATION` header, then
diff --git a/config/routes.rb b/config/routes.rb
index 94037c63..9e3cad02 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -245,9 +245,12 @@ Foodsoft::Application.routes.draw do
namespace :api do
namespace :v1 do
- resource :user, only: [:show]
resource :config, only: [:show]
resource :navigation, only: [:show]
+
+ namespace :user do
+ root to: 'users#show'
+ end
end
end
diff --git a/doc/API.md b/doc/API.md
index 5c544149..2e09cfa4 100644
--- a/doc/API.md
+++ b/doc/API.md
@@ -9,6 +9,8 @@ The API is documented using [Open API 2.0](https://github.com/OAI/OpenAPI-Specif
/ [Swagger](https://swagger.io/) in [swagger.v1.yml](swagger.v1.yml).
This provides a machine-readable reference that is used to provide documentation.
+**Note:** the current OAuth scopes may be subject to change, until the next release of Foodsoft.
+
## API endpoint documentation
>> [View API documentation](http://petstore.swagger.io/?url=https%3A%2F%2Fraw.githubusercontent.com%2Ffoodcoops%2Ffoodsoft%2Fmaster%2Fdoc%2Fswagger.v1.yml) <<
diff --git a/doc/swagger.v1.yml b/doc/swagger.v1.yml
index eeee5f35..62321496 100644
--- a/doc/swagger.v1.yml
+++ b/doc/swagger.v1.yml
@@ -46,8 +46,12 @@ paths:
description: not logged-in
schema:
$ref: '#/definitions/Error401'
+ 403:
+ description: missing scope
+ schema:
+ $ref: '#/definitions/Error403'
security:
- - foodsoft_auth: ['all']
+ - foodsoft_auth: ['user:read', 'user:write']
/config:
get:
summary: configuration variables
@@ -62,8 +66,12 @@ paths:
description: not logged-in
schema:
$ref: '#/definitions/Error401'
+ 403:
+ description: missing scope or no permission
+ schema:
+ $ref: '#/definitions/Error403'
security:
- - foodsoft_auth: ['all']
+ - foodsoft_auth: ['config:user', 'config:read', 'config:write']
/navigation:
get:
summary: navigation
@@ -82,7 +90,7 @@ paths:
schema:
$ref: '#/definitions/Error401'
security:
- - foodsoft_auth: ['all']
+ - foodsoft_auth: []
definitions:
# models
@@ -142,6 +150,14 @@ definitions:
description: 'unauthorized'
error_description:
$ref: '#/definitions/Error/properties/error_description'
+ Error403:
+ type: object
+ properties:
+ error:
+ type: string
+ description: 'forbidden or invalid_scope'
+ error_description:
+ $ref: '#/definitions/Error/properties/error_description'
securityDefinitions:
foodsoft_auth:
@@ -149,5 +165,9 @@ securityDefinitions:
flow: implicit
authorizationUrl: http://localhost:3000/f/oauth/authorize
scopes:
- all: full access to user functions
+ config:user: reading Foodsoft configuration for regular users
+ config:read: reading Foodsoft configuration values
+ config:write: reading and updating Foodsoft configuration values
+ user:read: reading your own user profile
+ user:write: reading and updating your own user profile
offline_access: retain access after user has logged out
diff --git a/spec/api/v1/swagger_spec.rb b/spec/api/v1/swagger_spec.rb
index ab0197aa..33bf4851 100644
--- a/spec/api/v1/swagger_spec.rb
+++ b/spec/api/v1/swagger_spec.rb
@@ -15,6 +15,7 @@ describe 'API v1', type: :apivore, order: :defined do
context 'has valid paths' do
context 'user' do
+ let(:api_scopes) { ['user:read'] }
# create multiple users to make sure we're getting the authenticated user, not just any
let!(:other_user_1) { create :user }
let!(:user) { create :user }
@@ -23,20 +24,23 @@ describe 'API v1', type: :apivore, order: :defined do
it { is_expected.to validate(:get, '/user', 200, api_auth) }
it { is_expected.to validate(:get, '/user', 401) }
- context 'with invalid access token' do
- let(:api_access_token) { 'abc' }
- it { is_expected.to validate(:get, '/user', 401, api_auth) }
- end
+ it_handles_invalid_token_and_scope(:get, '/user')
end
context 'config' do
+ let(:api_scopes) { ['config:user'] }
+
it { is_expected.to validate(:get, '/config', 200, api_auth) }
it { is_expected.to validate(:get, '/config', 401) }
+
+ it_handles_invalid_token_and_scope(:get, '/config')
end
context 'navigation' do
it { is_expected.to validate(:get, '/navigation', 200, api_auth) }
it { is_expected.to validate(:get, '/navigation', 401) }
+
+ it_handles_invalid_token(:get, '/navigation')
end
end
diff --git a/spec/factories/user.rb b/spec/factories/user.rb
index 51b35809..c4bcb6a2 100644
--- a/spec/factories/user.rb
+++ b/spec/factories/user.rb
@@ -21,6 +21,14 @@ FactoryBot.define do
create :ordergroup, user_ids: [user.id]
end
end
+
+ [:ordergroup, :finance, :invoices, :article_meta, :suppliers, :pickups, :orders].each do |role|
+ trait "role_#{role}".to_sym do
+ after :create do |user, evaluator|
+ create :workgroup, "role_#{role}" => true, user_ids: [user.id]
+ end
+ end
+ end
end
factory :group do
diff --git a/spec/support/api_helper.rb b/spec/support/api_helper.rb
index 4ab6a55f..66f9bd95 100644
--- a/spec/support/api_helper.rb
+++ b/spec/support/api_helper.rb
@@ -3,8 +3,28 @@ module ApiHelper
included do
let(:user) { create(:user) }
- let(:api_access_token) { create(:oauth2_access_token, resource_owner_id: user.id).token }
+ let(:api_scopes) { [] } # empty scopes for stricter testing (in reality this would be default_scopes)
+ let(:api_access_token) { create(:oauth2_access_token, resource_owner_id: user.id, scopes: api_scopes&.join(' ')).token }
let(:api_authorization) { "Bearer #{api_access_token}" }
+
+ def self.it_handles_invalid_token(method, path, params_block = ->{ api_auth })
+ context 'with invalid access token' do
+ let(:api_access_token) { 'abc' }
+ it { is_expected.to validate(method, path, 401, instance_exec(¶ms_block)) }
+ end
+ end
+
+ def self.it_handles_invalid_scope(method, path, params_block = ->{ api_auth })
+ context 'with invalid scope' do
+ let(:api_scopes) { ['none'] }
+ it { is_expected.to validate(method, path, 403, instance_exec(¶ms_block)) }
+ end
+ end
+
+ def self.it_handles_invalid_token_and_scope(*args)
+ it_handles_invalid_token(*args)
+ it_handles_invalid_scope(*args)
+ end
end
# Add authentication to parameters for {Swagger::RspecHelpers#validate}