From 594bae6127741054fb443252b6380c899cc375d9 Mon Sep 17 00:00:00 2001 From: Jan Schulz-Hofen Date: Tue, 17 Jan 2017 17:00:38 +0100 Subject: [PATCH 6/7] =?UTF-8?q?[oauth]=20Use=20Redmine=E2=80=99s=20permiss?= =?UTF-8?q?ions=20as=20OAuth2=20scopes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User#allowed_to? will deny any actions performed by OAuth2 apps which aren’t authorized for the required scope/permission, regardless of the user’s actual role/membership. --- app/controllers/application_controller.rb | 1 + .../oauth2_applications_controller.rb | 19 +++++++++++++ app/models/user.rb | 26 +++++++++++++---- .../doorkeeper/applications/_form.html.erb | 28 +++++++++++++------ .../doorkeeper/applications/index.html.erb | 2 +- .../doorkeeper/applications/show.html.erb | 2 +- .../doorkeeper/authorizations/new.html.erb | 2 +- config/initializers/doorkeeper.rb | 3 +- config/routes.rb | 4 ++- lib/redmine.rb | 2 +- 10 files changed, 68 insertions(+), 21 deletions(-) create mode 100644 app/controllers/oauth2_applications_controller.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 02f874142..c47592b35 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -127,6 +127,7 @@ class ApplicationController < ActionController::Base # Oauth if access_token.accessible? user = User.active.find_by_id(access_token.resource_owner_id) + user.oauth_scope = access_token.scopes.all.map(&:to_sym) else doorkeeper_render_error end diff --git a/app/controllers/oauth2_applications_controller.rb b/app/controllers/oauth2_applications_controller.rb new file mode 100644 index 000000000..a921886ef --- /dev/null +++ b/app/controllers/oauth2_applications_controller.rb @@ -0,0 +1,19 @@ +class Oauth2ApplicationsController < Doorkeeper::ApplicationsController + + private + + def application_params + params[:doorkeeper_application] ||= {} + params[:doorkeeper_application][:scopes] ||= [] + + scopes = Redmine::AccessControl.public_permissions.map{|p| p.name.to_s} + + if params[:doorkeeper_application][:scopes].is_a?(Array) + scopes |= params[:doorkeeper_application][:scopes] + else + scopes |= params[:doorkeeper_application][:scopes].split(/\s+/) + end + params[:doorkeeper_application][:scopes] = scopes.join(' ') + super + end +end diff --git a/app/models/user.rb b/app/models/user.rb index 8be16b823..b071a1572 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -100,6 +100,7 @@ class User < Principal attr_accessor :password, :password_confirmation, :generate_password attr_accessor :last_before_login_on attr_accessor :remote_ip + attr_writer :oauth_scope LOGIN_LENGTH_LIMIT = 60 MAIL_LENGTH_LIMIT = 60 @@ -704,15 +705,22 @@ class User < Principal def allowed_to?(action, context, options={}, &block) if context && context.is_a?(Project) return false unless context.allows_to?(action) - # Admin users are authorized for anything else - return true if admin? + + # Admin users are authorized for anything or what their oauth scope prescribes + if admin? + if @oauth_scope + return Role.new(permissions: @oauth_scope).allowed_to?(action, @oauth_scope) + else + return true + end + end roles = roles_for_project(context) return false unless roles roles.any? {|role| (context.is_public? || role.member?) && - role.allowed_to?(action) && + role.allowed_to?(action, @oauth_scope) && (block_given? ? yield(role, self) : true) } elsif context && context.is_a?(Array) @@ -725,13 +733,19 @@ class User < Principal elsif context raise ArgumentError.new("#allowed_to? context argument must be a Project, an Array of projects or nil") elsif options[:global] - # Admin users are always authorized - return true if admin? + # Admin users are always authorized, only limited by their oauth scope + if admin? + if @oauth_scope + return Role.new(permissions: @oauth_scope).allowed_to?(action, @oauth_scope) + else + return true + end + end # authorize if user has at least one role that has this permission roles = self.roles.to_a | [builtin_role] roles.any? {|role| - role.allowed_to?(action) && + role.allowed_to?(action, @oauth_scope) && (block_given? ? yield(role, self) : true) } else diff --git a/app/views/doorkeeper/applications/_form.html.erb b/app/views/doorkeeper/applications/_form.html.erb index 8b111b33a..c3398b9cb 100644 --- a/app/views/doorkeeper/applications/_form.html.erb +++ b/app/views/doorkeeper/applications/_form.html.erb @@ -12,14 +12,24 @@ <% end %>

+ -

- <%= f.text_field :scopes, :size => 60, :label => :'activerecord.attributes.doorkeeper/application.scopes' %> - - <%= t('doorkeeper.applications.help.scopes') %> - -

- - - +

<%= l(:'activerecord.attributes.doorkeeper/application.scopes') %>

+
+<% perms_by_module = Redmine::AccessControl.permissions.group_by {|p| p.project_module.to_s} %> +<% perms_by_module.keys.sort.each do |mod| %> +
<%= mod.blank? ? l(:label_project) : l_or_humanize(mod, :prefix => 'project_module_') %> + <% perms_by_module[mod].each do |permission| %> + + <% end %> +
+<% end %> +
<%= check_all_links 'scopes' %> +<%= hidden_field_tag 'doorkeeper_application[scopes][]', '' %>
diff --git a/app/views/doorkeeper/applications/index.html.erb b/app/views/doorkeeper/applications/index.html.erb index e94581017..1232c58d2 100644 --- a/app/views/doorkeeper/applications/index.html.erb +++ b/app/views/doorkeeper/applications/index.html.erb @@ -18,7 +18,7 @@ "> <%= link_to application.name, oauth_application_path(application) %> <%= truncate application.redirect_uri.split.join(', '), length: 50 %> - <%= h application.scopes %> + <%= safe_join application.scopes.map{|scope| h l_or_humanize(scope, prefix: 'permission_')}, ", " %> <%= link_to t('doorkeeper.applications.buttons.edit'), edit_oauth_application_path(application), class: 'icon icon-edit' %> <%= link_to t('doorkeeper.applications.buttons.destroy'), oauth_application_path(application), :data => {:confirm => t('doorkeeper.applications.confirmations.destroy')}, :method => :delete, :class => 'icon icon-del' %> diff --git a/app/views/doorkeeper/applications/show.html.erb b/app/views/doorkeeper/applications/show.html.erb index 5f9bb632a..f7b11f7e3 100644 --- a/app/views/doorkeeper/applications/show.html.erb +++ b/app/views/doorkeeper/applications/show.html.erb @@ -17,7 +17,7 @@

<%= t('.scopes') %>: - <%= h @application.scopes %> + <%= safe_join @application.scopes.map{|scope| h l_or_humanize(scope, prefix: 'permission_')}, ", " %>

diff --git a/app/views/doorkeeper/authorizations/new.html.erb b/app/views/doorkeeper/authorizations/new.html.erb index 44f2be948..38e93a1d1 100644 --- a/app/views/doorkeeper/authorizations/new.html.erb +++ b/app/views/doorkeeper/authorizations/new.html.erb @@ -10,7 +10,7 @@

<%= t('.able_to') %>:

diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 83b2aacc3..b7e408dc3 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -3,7 +3,8 @@ Doorkeeper.configure do reuse_access_token realm Redmine::Info.app_name base_controller 'ApplicationController' - default_scopes :public + default_scopes *Redmine::AccessControl.public_permissions.map(&:name) + optional_scopes *Redmine::AccessControl.permissions.map(&:name) resource_owner_authenticator do if require_login diff --git a/config/routes.rb b/config/routes.rb index 4dc9da569..1d6fc41d0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -19,7 +19,9 @@ Rails.application.routes.draw do - use_doorkeeper + use_doorkeeper do + controllers :applications => 'oauth2_applications' + end root :to => 'welcome#index' root :to => 'welcome#index', :as => 'home' diff --git a/lib/redmine.rb b/lib/redmine.rb index 038c7cb4d..e5a4eb3d4 100644 --- a/lib/redmine.rb +++ b/lib/redmine.rb @@ -267,7 +267,7 @@ Redmine::MenuManager.map :admin_menu do |menu| :html => {:class => 'icon icon-settings'} menu.push :ldap_authentication, {:controller => 'auth_sources', :action => 'index'}, :html => {:class => 'icon icon-server-authentication'} - menu.push :applications, {:controller => 'doorkeeper/applications', :action => 'index'}, + menu.push :applications, {:controller => 'oauth2_applications', :action => 'index'}, :if => Proc.new { Setting.rest_api_enabled? }, :caption => :'doorkeeper.layouts.admin.nav.applications', :html => {:class => 'icon icon-applications'} -- 2.20.1