From b3237082da411728032fac8b145eb141ea4c9ccc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marius=20B=C4=82LTEANU?= Date: Sun, 25 May 2025 01:30:53 +0300 Subject: [PATCH 1/3] Oauth provider --- Gemfile | 7 + app/assets/images/icons.svg | 11 ++ app/assets/stylesheets/application.css | 3 + app/controllers/application_controller.rb | 8 ++ .../oauth2_applications_controller.rb | 38 ++++++ app/models/role.rb | 26 ++-- app/models/user.rb | 19 ++- .../doorkeeper/applications/_form.html.erb | 39 ++++++ .../doorkeeper/applications/edit.html.erb | 6 + .../doorkeeper/applications/index.html.erb | 33 +++++ .../doorkeeper/applications/new.html.erb | 6 + .../doorkeeper/applications/show.html.erb | 54 ++++++++ .../doorkeeper/authorizations/error.html.erb | 6 + .../doorkeeper/authorizations/new.html.erb | 48 +++++++ .../doorkeeper/authorizations/show.html.erb | 8 ++ .../authorized_applications/index.html.erb | 31 +++++ app/views/my/account.html.erb | 1 + app/views/users/show.api.rsb | 2 +- config/icon_source.yml | 4 + config/initializers/30-redmine.rb | 74 ++++++++++ config/initializers/doorkeeper.rb | 9 ++ config/locales/de.yml | 12 ++ config/locales/en.yml | 16 +++ config/routes.rb | 5 + ...20170107092155_create_doorkeeper_tables.rb | 68 ++++++++++ db/migrate/20200812065227_enable_pkce.rb | 8 ++ lib/redmine/preparation.rb | 5 + test/system/oauth_provider_test.rb | 128 ++++++++++++++++++ test/unit/role_test.rb | 26 ++++ test/unit/user_test.rb | 61 +++++++++ 30 files changed, 751 insertions(+), 11 deletions(-) create mode 100644 app/controllers/oauth2_applications_controller.rb create mode 100644 app/views/doorkeeper/applications/_form.html.erb create mode 100644 app/views/doorkeeper/applications/edit.html.erb create mode 100644 app/views/doorkeeper/applications/index.html.erb create mode 100644 app/views/doorkeeper/applications/new.html.erb create mode 100644 app/views/doorkeeper/applications/show.html.erb create mode 100644 app/views/doorkeeper/authorizations/error.html.erb create mode 100644 app/views/doorkeeper/authorizations/new.html.erb create mode 100644 app/views/doorkeeper/authorizations/show.html.erb create mode 100644 app/views/doorkeeper/authorized_applications/index.html.erb create mode 100644 config/initializers/doorkeeper.rb create mode 100644 db/migrate/20170107092155_create_doorkeeper_tables.rb create mode 100644 db/migrate/20200812065227_enable_pkce.rb create mode 100644 test/system/oauth_provider_test.rb diff --git a/Gemfile b/Gemfile index 2b40e9e5e..2323556a1 100644 --- a/Gemfile +++ b/Gemfile @@ -19,6 +19,9 @@ gem 'rack', '>= 3.1.3' gem "stimulus-rails", "~> 1.3" gem "importmap-rails", "~> 2.0" gem 'commonmarker', '~> 2.3.0' +gem "doorkeeper", "~> 5.8.2" +gem "bcrypt", require: false +gem "doorkeeper-i18n", "~> 5.2" # Ruby Standard Gems gem 'csv', '~> 3.3.2' @@ -115,6 +118,10 @@ group :test do gem 'rubocop-performance', '~> 1.25.0', require: false gem 'rubocop-rails', '~> 2.31.0', require: false gem 'bundle-audit', require: false + # for testing oauth provider capabilities + gem 'oauth2' + gem 'rest-client' + gem 'webrick' end local_gemfile = File.join(File.dirname(__FILE__), "Gemfile.local") diff --git a/app/assets/images/icons.svg b/app/assets/images/icons.svg index df635523a..ae709d1a5 100644 --- a/app/assets/images/icons.svg +++ b/app/assets/images/icons.svg @@ -59,6 +59,13 @@ + + + + + + + @@ -394,6 +401,10 @@ + + + + diff --git a/app/assets/stylesheets/application.css b/app/assets/stylesheets/application.css index 6571c05d1..7e257b57c 100644 --- a/app/assets/stylesheets/application.css +++ b/app/assets/stylesheets/application.css @@ -1285,6 +1285,9 @@ div.flash.warning svg.icon-svg, .conflict svg.icon-svg { color: #A6750C; } +.warning .oauth-permissions { display:inline-block;text-align:left; } +.warning .oauth-permissions p { margin-top:0;-webkit-margin-before:0;} + #errorExplanation ul { font-size: 0.9em;} #errorExplanation h2, #errorExplanation p { display: none; } diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 074392709..a01d5c75f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -131,6 +131,14 @@ class ApplicationController < ActionController::Base if (key = api_key_from_request) # Use API key user = User.find_by_api_key(key) + elsif access_token = Doorkeeper.authenticate(request) + # 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 elsif /\ABasic /i.match?(request.authorization.to_s) # HTTP Basic, either username/password or API key/random authenticate_with_http_basic do |username, password| diff --git a/app/controllers/oauth2_applications_controller.rb b/app/controllers/oauth2_applications_controller.rb new file mode 100644 index 000000000..107af2ec0 --- /dev/null +++ b/app/controllers/oauth2_applications_controller.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +# +# Redmine - project management software +# Copyright (C) 2006- Jean-Philippe Lang +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +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/role.rb b/app/models/role.rb index 3ca4f92a1..870bbe945 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -198,11 +198,14 @@ class Role < ApplicationRecord # action can be: # * a parameter-like Hash (eg. :controller => 'projects', :action => 'edit') # * a permission Symbol (eg. :edit_project) - def allowed_to?(action) + # scope can be: + # * an array of permissions which will be used as filter (logical AND) + + def allowed_to?(action, scope=nil) if action.is_a? Hash - allowed_actions.include? "#{action[:controller]}/#{action[:action]}" + allowed_actions(scope).include? "#{action[:controller]}/#{action[:action]}" else - allowed_permissions.include? action + allowed_permissions(scope).include? action end end @@ -298,13 +301,20 @@ class Role < ApplicationRecord private - def allowed_permissions - @allowed_permissions ||= permissions + Redmine::AccessControl.public_permissions.collect {|p| p.name} + def allowed_permissions(scope = nil) + scope = scope.sort if scope.present? # to maintain stable cache keys + @allowed_permissions ||= {} + @allowed_permissions[scope] ||= begin + unscoped = permissions + Redmine::AccessControl.public_permissions.collect {|p| p.name} + scope.present? ? unscoped & scope : unscoped + end end - def allowed_actions - @actions_allowed ||= - allowed_permissions.inject([]) do |actions, permission| + def allowed_actions(scope = nil) + scope = scope.sort if scope.present? # to maintain stable cache keys + @actions_allowed ||= {} + @actions_allowed[scope] ||= + allowed_permissions(scope).inject([]) do |actions, permission| actions += Redmine::AccessControl.allowed_actions(permission) end.flatten end diff --git a/app/models/user.rb b/app/models/user.rb index c1a860f5a..496084ceb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -112,6 +112,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 = 254 @@ -732,6 +733,20 @@ class User < Principal end end + def admin? + if authorized_by_oauth? + # when signed in via oauth, the user only acts as admin when the admin scope is set + super and @oauth_scope.include?(:admin) + else + super + end + end + + # true if the user has signed in via oauth + def authorized_by_oauth? + !@oauth_scope.nil? + end + # Return true if the user is allowed to do the specified action on a specific context # Action can be: # * a parameter-like Hash (eg. :controller => 'projects', :action => 'edit') @@ -752,7 +767,7 @@ class User < Principal roles.any? do |role| (context.is_public? || role.member?) && - role.allowed_to?(action) && + role.allowed_to?(action, @oauth_scope) && (block ? yield(role, self) : true) end elsif context && context.is_a?(Array) @@ -771,7 +786,7 @@ class User < Principal # authorize if user has at least one role that has this permission roles = self.roles.to_a | [builtin_role] roles.any? do |role| - role.allowed_to?(action) && + role.allowed_to?(action, @oauth_scope) && (block ? yield(role, self) : true) end else diff --git a/app/views/doorkeeper/applications/_form.html.erb b/app/views/doorkeeper/applications/_form.html.erb new file mode 100644 index 000000000..e4f778f63 --- /dev/null +++ b/app/views/doorkeeper/applications/_form.html.erb @@ -0,0 +1,39 @@ +<%= error_messages_for 'application' %> +
+

<%= f.text_field :name, :required => true %>

+ +

+ <%= f.text_area :redirect_uri, :required => true, :size => 60, :label => :'activerecord.attributes.doorkeeper/application.redirect_uri' %> + + <%= t('doorkeeper.applications.help.redirect_uri') %> + +

+
+ +

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

+

<%= l :text_oauth_info_scopes %>

+
+
<%= l :label_oauth_admin_access %> + +
+<% 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/edit.html.erb b/app/views/doorkeeper/applications/edit.html.erb new file mode 100644 index 000000000..aebc1a841 --- /dev/null +++ b/app/views/doorkeeper/applications/edit.html.erb @@ -0,0 +1,6 @@ +<%= title [l('label_oauth_application_plural'), oauth_applications_path], @application.name %> + +<%= labelled_form_for @application, url: doorkeeper_submit_path(@application) do |f| %> + <%= render :partial => 'form', :locals => {:f => f} %> + <%= submit_tag l(:button_save) %> +<% end %> diff --git a/app/views/doorkeeper/applications/index.html.erb b/app/views/doorkeeper/applications/index.html.erb new file mode 100644 index 000000000..0ba31c0e8 --- /dev/null +++ b/app/views/doorkeeper/applications/index.html.erb @@ -0,0 +1,33 @@ +
+<%= link_to sprite_icon('add', t('.new')), new_oauth_application_path, :class => 'icon icon-add' %> +
+ +<%= title l 'label_oauth_application_plural' %> + +<% if @applications.any? %> +
+ + + + + + + + + <% @applications.each do |application| %> + "> + + + + + + <% end %> + +
<%= t('.name') %><%= t('.callback_url') %><%= t('.scopes') %>
<%= link_to application.name, oauth_application_path(application) %><%= truncate application.redirect_uri.split.join(', '), length: 50 %><%= safe_join application.scopes.map{|scope| h l_or_humanize(scope, prefix: 'permission_')}, ", " %> + <%= link_to sprite_icon('edit', t('doorkeeper.applications.buttons.edit')), edit_oauth_application_path(application), class: 'icon icon-edit' %> + <%= link_to sprite_icon('del', t('doorkeeper.applications.buttons.destroy')), oauth_application_path(application), :data => {:confirm => t('doorkeeper.applications.confirmations.destroy')}, :method => :delete, :class => 'icon icon-del' %> +
+
+<% else %> +

<%= l(:label_no_data) %>

+<% end %> diff --git a/app/views/doorkeeper/applications/new.html.erb b/app/views/doorkeeper/applications/new.html.erb new file mode 100644 index 000000000..e2a39ac93 --- /dev/null +++ b/app/views/doorkeeper/applications/new.html.erb @@ -0,0 +1,6 @@ +<%= title [l('label_oauth_application_plural'), oauth_applications_path], t('.title') %> + +<%= labelled_form_for @application, url: doorkeeper_submit_path(@application) do |f| %> +<%= render :partial => 'form', :locals => { :f => f } %> +<%= submit_tag l(:button_create) %> +<% end %> diff --git a/app/views/doorkeeper/applications/show.html.erb b/app/views/doorkeeper/applications/show.html.erb new file mode 100644 index 000000000..c98e7d29c --- /dev/null +++ b/app/views/doorkeeper/applications/show.html.erb @@ -0,0 +1,54 @@ +
+<%= link_to sprite_icon('edit', t('doorkeeper.applications.buttons.edit')), edit_oauth_application_path(@application), :accesskey => accesskey(:edit), class: 'icon icon-edit' %> +<%= link_to sprite_icon('del', t('doorkeeper.applications.buttons.destroy')), oauth_application_path(@application), :data => {:confirm => t('doorkeeper.applications.confirmations.destroy')}, :method => :delete, :class => 'icon icon-del' %> +
+ +<%= title [l('label_oauth_application_plural'), oauth_applications_path], @application.name %> + +
+

<%= sprite_icon('key', l(:label_information_plural)) %>

+

+ <%= t('.application_id') %>: + <%= h @application.uid %> +

+

+ <%= t('.secret') %>: + + <% secret = flash[:application_secret].presence || @application.plaintext_secret %> + <% flash.delete :application_secret %> + <% if secret.blank? && Doorkeeper.config.application_secret_hashed? %> + <%= t('.secret_hashed') %> + <% else %> + <%= secret %> + <% end %> + + <% if secret.present? && Doorkeeper.config.application_secret_hashed? %> + <%= t "text_oauth_copy_secret_now" %> + <% end %> +

+

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

+
+ +

<%= t('.callback_urls') %>

+ +
+ + + + + + + <% @application.redirect_uri.split.each do |uri| %> + "> + + + + <% end %> + +
<%= t('.callback_url') %>
<%= uri %> + <%= link_to sprite_icon('shield-check', t('doorkeeper.applications.buttons.authorize')), oauth_authorization_path(client_id: @application.uid, redirect_uri: uri, response_type: 'code', scope: @application.scopes), class: 'icon icon-authorize', target: '_blank' %> +
+
diff --git a/app/views/doorkeeper/authorizations/error.html.erb b/app/views/doorkeeper/authorizations/error.html.erb new file mode 100644 index 000000000..59cedf8f3 --- /dev/null +++ b/app/views/doorkeeper/authorizations/error.html.erb @@ -0,0 +1,6 @@ +

<%= t('doorkeeper.authorizations.error.title') %>

+ +

<%= @pre_auth.error_response.body[:error_description] %>

+

<%= l(:button_back) %>

+ +<% html_title t('doorkeeper.authorizations.error.title') %> diff --git a/app/views/doorkeeper/authorizations/new.html.erb b/app/views/doorkeeper/authorizations/new.html.erb new file mode 100644 index 000000000..898f2e645 --- /dev/null +++ b/app/views/doorkeeper/authorizations/new.html.erb @@ -0,0 +1,48 @@ +<%= title t('.title') %> + +
+

<%=h @pre_auth.client.name %>

+ +

<%= raw t('.prompt', client_name: content_tag(:strong, class: "text-info") { @pre_auth.client.name }) %>

+ +
+

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

+
    +
  • <%= l :text_oauth_implicit_permissions %>
  • + <% @pre_auth.scopes.each do |scope| %> + <% if scope == 'admin' %> +
  • <%= l :label_oauth_permission_admin %>
  • + <% else %> +
  • <%= l_or_humanize(scope, prefix: 'permission_') %>
  • + <% end %> + <% end %> +
+
+ +<% if @pre_auth.scopes.include?('admin') %> +

<%= l :text_oauth_admin_permission_info %>

+<% end %> +
+ +

+ <%= form_tag oauth_authorization_path, method: :post do %> + <%= hidden_field_tag :client_id, @pre_auth.client.uid %> + <%= hidden_field_tag :redirect_uri, @pre_auth.redirect_uri %> + <%= hidden_field_tag :state, @pre_auth.state %> + <%= hidden_field_tag :response_type, @pre_auth.response_type %> + <%= hidden_field_tag :scope, @pre_auth.scope %> + <%= hidden_field_tag :code_challenge, @pre_auth.code_challenge %> + <%= hidden_field_tag :code_challenge_method, @pre_auth.code_challenge_method %> + <%= submit_tag t('doorkeeper.authorizations.buttons.authorize') %> + <% end %> + <%= form_tag oauth_authorization_path, method: :delete do %> + <%= hidden_field_tag :client_id, @pre_auth.client.uid %> + <%= hidden_field_tag :redirect_uri, @pre_auth.redirect_uri %> + <%= hidden_field_tag :state, @pre_auth.state %> + <%= hidden_field_tag :response_type, @pre_auth.response_type %> + <%= hidden_field_tag :scope, @pre_auth.scope %> + <%= hidden_field_tag :code_challenge, @pre_auth.code_challenge %> + <%= hidden_field_tag :code_challenge_method, @pre_auth.code_challenge_method %> + <%= submit_tag t('doorkeeper.authorizations.buttons.deny') %> + <% end %> +

diff --git a/app/views/doorkeeper/authorizations/show.html.erb b/app/views/doorkeeper/authorizations/show.html.erb new file mode 100644 index 000000000..25ee88a87 --- /dev/null +++ b/app/views/doorkeeper/authorizations/show.html.erb @@ -0,0 +1,8 @@ +<%= title [l('label_oauth_authorized_application_plural'), oauth_authorized_applications_path] %> + +
<%= l(:label_information_plural) %> +

+ + <%= params[:code] %> +

+
diff --git a/app/views/doorkeeper/authorized_applications/index.html.erb b/app/views/doorkeeper/authorized_applications/index.html.erb new file mode 100644 index 000000000..0a1fc8a00 --- /dev/null +++ b/app/views/doorkeeper/authorized_applications/index.html.erb @@ -0,0 +1,31 @@ +<%= title [t(:label_my_account), my_account_path], l('label_oauth_authorized_application_plural') %> + +<% if @applications.any? %> +
+ + + + + + + + <% @applications.each do |application| %> + "> + + + + + <% end %> + +
<%= t('doorkeeper.authorized_applications.index.application') %><%= t('doorkeeper.authorized_applications.index.created_at') %>
<%= application.name %><%= format_date application.created_at %> + <%= link_to sprite_icon('del', t('doorkeeper.authorized_applications.buttons.revoke')), oauth_authorized_application_path(application), :data => {:confirm => t('doorkeeper.authorized_applications.confirmations.revoke')}, :method => :delete, :class => 'icon icon-del' %> +
+
+<% else %> +

<%= l(:label_no_data) %>

+<% end %> + +<% content_for :sidebar do %> +<% @user = User.current %> +<%= render :partial => 'my/sidebar' %> +<% end %> diff --git a/app/views/my/account.html.erb b/app/views/my/account.html.erb index c8706a5f5..95afbabac 100644 --- a/app/views/my/account.html.erb +++ b/app/views/my/account.html.erb @@ -1,6 +1,7 @@
<%= additional_emails_link(@user) %> <%= link_to(sprite_icon('key', l(:button_change_password)), { :action => 'password'}, :class => 'icon icon-passwd') if @user.change_password_allowed? %> +<%= link_to(sprite_icon('apps', l('label_oauth_authorized_application_plural')), oauth_authorized_applications_path, :class => 'icon icon-applications') if Setting.rest_api_enabled? %> <%= call_hook(:view_my_account_contextual, :user => @user)%>
diff --git a/app/views/users/show.api.rsb b/app/views/users/show.api.rsb index bf415795d..0681903b8 100644 --- a/app/views/users/show.api.rsb +++ b/app/views/users/show.api.rsb @@ -11,7 +11,7 @@ api.user do api.passwd_changed_on @user.passwd_changed_on api.avatar_url gravatar_url(@user.mail, {rating: nil, size: nil, default: Setting.gravatar_default}) if @user.mail && Setting.gravatar_enabled? api.twofa_scheme @user.twofa_scheme if User.current.admin? || (User.current == @user) - api.api_key @user.api_key if User.current.admin? || (User.current == @user) + api.api_key @user.api_key if (User.current.admin? || (User.current == @user && !User.current.authorized_by_oauth?)) api.status @user.status if User.current.admin? render_api_custom_values @user.visible_custom_field_values, api diff --git a/config/icon_source.yml b/config/icon_source.yml index 38ad2fbf4..cd7686dd1 100644 --- a/config/icon_source.yml +++ b/config/icon_source.yml @@ -233,3 +233,7 @@ svg: bulb - name: message-report svg: message-report +- name: apps + svg: apps +- name: shield-check + svg: shield-check \ No newline at end of file diff --git a/config/initializers/30-redmine.rb b/config/initializers/30-redmine.rb index cf13cab20..47f86da0b 100644 --- a/config/initializers/30-redmine.rb +++ b/config/initializers/30-redmine.rb @@ -12,6 +12,72 @@ Rails.application.config.to_prepare do ActiveSupport::XmlMini.backend = 'Nokogiri' Redmine::Preparation.prepare + + Doorkeeper.configure do + orm :active_record + + # Issue access tokens with refresh token + use_refresh_token + + # Authorization Code expiration time (default: 10 minutes). + # + # authorization_code_expires_in 10.minutes + + # Access token expiration time (default: 2 hours). + # If you want to disable expiration, set this to `nil`. + # + # access_token_expires_in 2.hours + + # Hash access and refresh tokens before persisting them. + # https://doorkeeper.gitbook.io/guides/security/token-and-application-secrets + hash_token_secrets + + # Hash application secrets before persisting them. + hash_application_secrets using: '::Doorkeeper::SecretStoring::BCrypt' + + # limit supported flows to Auth code + grant_flows ['authorization_code'] + + realm Redmine::Info.app_name + base_controller 'ApplicationController' + default_scopes(*Redmine::AccessControl.public_permissions.map(&:name)) + optional_scopes(*(Redmine::AccessControl.permissions.map(&:name) << :admin)) + + # Forbids creating/updating applications with arbitrary scopes that are + # not in configuration, i.e. +default_scopes+ or +optional_scopes+. + enforce_configured_scopes + + allow_token_introspection false + + # allow http loopback redirect URIs but require https for all others + force_ssl_in_redirect_uri { |uri| !%w[localhost 127.0.0.1 web localohst:8080].include?(uri.host) } + + # Specify what redirect URI's you want to block during Application creation. + forbid_redirect_uri { |uri| %w[data vbscript javascript].include?(uri.scheme.to_s.downcase) } + + resource_owner_authenticator do + if require_login + if Setting.rest_api_enabled? + User.current + else + deny_access + end + end + end + + admin_authenticator do |_routes| + if !Setting.rest_api_enabled? || !User.current.admin? + deny_access + end + end + end + + # Use Redmine standard layouts and helpers for Doorkeeper OAuth2 screens + Doorkeeper::ApplicationsController.layout "admin" + Doorkeeper::ApplicationsController.main_menu = false + Doorkeeper::AuthorizationsController.layout "base" + Doorkeeper::AuthorizedApplicationsController.layout "base" + Doorkeeper::AuthorizedApplicationsController.main_menu = false end # Load the secret token from the Redmine configuration file @@ -42,6 +108,14 @@ Rails.application.config.to_prepare do paths = theme.asset_paths Rails.application.config.assets.redmine_extension_paths << paths if paths.present? end + + Doorkeeper::ApplicationsController.class_eval do + require_sudo_mode :create, :show, :update, :destroy + end + + Doorkeeper::AuthorizationsController.class_eval do + require_sudo_mode :create, :destroy + end end Rails.application.deprecators[:redmine] = ActiveSupport::Deprecation.new('7.0', 'Redmine') diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb new file mode 100644 index 000000000..40888ad8b --- /dev/null +++ b/config/initializers/doorkeeper.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +# rubocop:disable Lint/EmptyBlock +Doorkeeper.configure do +end + +Rails.application.config.to_prepare do +end +# rubocop:enable Lint/EmptyBlock diff --git a/config/locales/de.yml b/config/locales/de.yml index 5b0f9bd35..36e52fb3a 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -971,6 +971,9 @@ de: permission_view_time_entries: Gebuchte Aufwände ansehen permission_view_wiki_edits: Wiki-Versionsgeschichte ansehen permission_view_wiki_pages: Wiki ansehen + permission_view_project: Projekte ansehen + permission_search_project: Projekte suchen + permission_view_members: Projektmitglieder anzeigen project_module_boards: Foren project_module_calendar: Kalender @@ -1477,3 +1480,12 @@ de: other: "%{count} others" text_setting_gravatar_default_initials_html: Users' initials are sent to https://www.gravatar.com to generate their avatars. + label_oauth_permission_admin: Administrator-Zugriff + label_oauth_admin_access: Administration + label_oauth_application_plural: Applikationen + label_oauth_authorized_application_plural: Autorisierte Applikationen + text_oauth_admin_permission: Voller Admin-Zugriff. Wenn diese Applikation durch einen Administrator autorisiert wird, kann sie alle Daten lesen und schreiben, auch im Namen anderer Benutzer. + text_oauth_admin_permission_info: Diese Applikation verlangt vollen Administrator-Zugriff. Wenn Sie ein Administrator sind (oder in Zukunft Administrator werden), wird sie in der Lage sein, alle Daten zu lesen und zu schreiben, auch im Namen anderer Benutzer. Dies kann vermieden werden, indem die Applikation mit einem anderen Benutzerkonto ohne Administrator-Privileg autorisiert wird. + text_oauth_copy_secret_now: Das Geheimnis bitte jetzt an einen sicheren Ort kopieren, es kann nicht erneut angezeigt werden. + text_oauth_implicit_permissions: Zugriff auf Benutzername, Login sowie auf die primäre Email-Adresse + text_oauth_info_scopes: Scopes für die Applikation auswählen. Die Applikation wird niemals mehr Rechte haben als hier ausgewählt. Sie wird außerdem auf die Rollen und Projektmitgliedschaften des Benutzers, der sie autorisiert hat, beschränkt sein. \ No newline at end of file diff --git a/config/locales/en.yml b/config/locales/en.yml index dca278e23..947a8642f 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -139,6 +139,9 @@ en: must_contain_special_chars: "must contain special characters (!, $, %, ...)" domain_not_allowed: "contains a domain not allowed (%{domain})" too_simple: "is too simple" + attributes: + doorkeeper/application: + scopes: Scopes actionview_instancetag_blank_option: Please select @@ -605,6 +608,10 @@ en: permission_manage_related_issues: Manage related issues permission_import_issues: Import issues permission_log_time_for_other_users: Log spent time for other users + permission_view_project: View projects + permission_search_project: Search projects + permission_view_members: View project members + project_module_issue_tracking: Issue tracking project_module_time_tracking: Time tracking @@ -1158,6 +1165,10 @@ en: label_time_by_author: "%{time} by %{author}" label_involved_principals: Author / Previous assignee label_progressbar: Progress bar + label_oauth_permission_admin: Administrate this Redmine + label_oauth_admin_access: Administration + label_oauth_application_plural: Applications + label_oauth_authorized_application_plural: Authorized applications button_login: Login button_submit: Submit @@ -1343,6 +1354,11 @@ en: text_allowed_queries_to_select: Public (to any users) queries only selectable text_setting_config_change: You can configure the behaviour in config/configuration.yml. Please restart the application after editing it. text_setting_gravatar_default_initials_html: Users' initials are sent to https://www.gravatar.com to generate their avatars. + text_oauth_admin_permission: Full administrative access. When authorized by an Administrator, this application will be able to read and write all data and impersonate other users. + text_oauth_admin_permission_info: This application requests full administrative access. If you are an Administrator (or become one in the future), it will be able to read and write all data and impersonate other users on your behalf. If you want to avoid this, authorize it as a user without Administrator privileges instead. + text_oauth_copy_secret_now: Copy the secret to a safe place now, it will not be shown again. + text_oauth_implicit_permissions: View your name, login and primary email address + text_oauth_info_scopes: Select the scopes this application may request. The application will not be allowed to do more than what is selected here. It will also always be limited by the roles and project memberships of the user who authorized it. default_role_manager: Manager default_role_developer: Developer diff --git a/config/routes.rb b/config/routes.rb index 20a7e826a..52c95c6a4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -18,6 +18,11 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. Rails.application.routes.draw do + use_doorkeeper do + controllers :applications => 'oauth2_applications' + end + + root :to => 'welcome#index' root :to => 'welcome#index', :as => 'home' match 'login', :to => 'account#login', :as => 'signin', :via => [:get, :post] diff --git a/db/migrate/20170107092155_create_doorkeeper_tables.rb b/db/migrate/20170107092155_create_doorkeeper_tables.rb new file mode 100644 index 000000000..b5585c105 --- /dev/null +++ b/db/migrate/20170107092155_create_doorkeeper_tables.rb @@ -0,0 +1,68 @@ +class CreateDoorkeeperTables < ActiveRecord::Migration[4.2] + def change + create_table :oauth_applications do |t| + t.string :name, null: false + t.string :uid, null: false + t.string :secret, null: false + t.text :redirect_uri, null: false + t.text :scopes, null: false + t.boolean :confidential, null: false, default: true + t.timestamps null: false + end + + add_index :oauth_applications, :uid, unique: true + + create_table :oauth_access_grants do |t| + t.integer :resource_owner_id, null: false + t.references :application, null: false + t.string :token, null: false + t.integer :expires_in, null: false + t.text :redirect_uri, null: false + t.datetime :created_at, null: false + t.datetime :revoked_at + t.text :scopes + end + + add_index :oauth_access_grants, :token, unique: true + add_foreign_key( + :oauth_access_grants, + :oauth_applications, + column: :application_id + ) + add_foreign_key( + :oauth_access_grants, + :users, + column: :resource_owner_id + ) + + create_table :oauth_access_tokens do |t| + t.integer :resource_owner_id + t.references :application + + t.string :token, null: false + + t.string :refresh_token + t.integer :expires_in + t.datetime :revoked_at + t.datetime :created_at, null: false + t.text :scopes + + t.string :previous_refresh_token, null: false, default: "" + end + + add_index :oauth_access_tokens, :token, unique: true + add_index :oauth_access_tokens, :resource_owner_id + add_index :oauth_access_tokens, :refresh_token, unique: true + + add_foreign_key( + :oauth_access_tokens, + :oauth_applications, + column: :application_id + ) + add_foreign_key( + :oauth_access_tokens, + :users, + column: :resource_owner_id + ) + end +end diff --git a/db/migrate/20200812065227_enable_pkce.rb b/db/migrate/20200812065227_enable_pkce.rb new file mode 100644 index 000000000..7b1e4582e --- /dev/null +++ b/db/migrate/20200812065227_enable_pkce.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +class EnablePkce < ActiveRecord::Migration[5.2] + def change + add_column :oauth_access_grants, :code_challenge, :string, null: true + add_column :oauth_access_grants, :code_challenge_method, :string, null: true + end +end diff --git a/lib/redmine/preparation.rb b/lib/redmine/preparation.rb index a31204904..a7387f5dc 100644 --- a/lib/redmine/preparation.rb +++ b/lib/redmine/preparation.rb @@ -280,6 +280,11 @@ module Redmine {:controller => 'auth_sources', :action => 'index'}, :icon => 'server-authentication', :html => {:class => 'icon icon-server-authentication'} + menu.push :applications, {:controller => 'oauth2_applications', :action => 'index'}, + :if => Proc.new { Setting.rest_api_enabled? }, + :caption => :'doorkeeper.layouts.admin.nav.applications', + :icon => 'apps', + :html => {:class => 'icon icon-applications'} menu.push :plugins, {:controller => 'admin', :action => 'plugins'}, :last => true, :icon => 'plugins', diff --git a/test/system/oauth_provider_test.rb b/test/system/oauth_provider_test.rb new file mode 100644 index 000000000..f9224f382 --- /dev/null +++ b/test/system/oauth_provider_test.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require File.expand_path('../application_system_test_case', __dir__) +require 'oauth2' +require 'webrick' + +class OauthProviderSystemTest < ApplicationSystemTestCase + fixtures :projects, :users, :email_addresses, :roles, :members, :member_roles, + :trackers, :projects_trackers, :enabled_modules, :issue_statuses, :issues, + :enumerations, :custom_fields, :custom_values, :custom_fields_trackers, + :watchers, :journals, :journal_details, :versions, + :workflows + test 'application creation and authorization' do + # + # admin creates the application, granting permissions and generating a uuid + # and secret. + # + log_user 'admin', 'admin' + with_settings rest_api_enabled: 1 do + visit '/admin' + within 'div#admin-menu ul' do + click_link 'Applications' + end + click_link 'New Application' + fill_in 'Name', with: 'Oauth Test' + + # as per https://tools.ietf.org/html/rfc8252#section-7.3, the port can be + # anything when the redirect URI's host is 127.0.0.1. + fill_in 'Redirect URI', with: 'http://127.0.0.1' + + check 'View Issues' + click_button 'Create' + end + + assert app = Doorkeeper::Application.find_by_name('Oauth Test') + + find 'h2', visible: true, text: /Oauth Test/ + find 'p code', visible: true, text: app.uid + find 'p strong', visible: true, text: /will not be shown again/ + find 'p code', visible: true, text: /View Issues/ + + # scrape the clear text secret from the page + app_secret = all(:css, 'p code')[1].text + + click_link 'Sign out' + + # + # regular user authorizes the application + # + client = OAuth2::Client.new(app.uid, app_secret, site: "http://127.0.0.1:#{test_port}/") + + # set up a dummy http listener to handle the redirect + port = rand 10000..20000 + redirect_uri = "http://127.0.0.1:#{port}" + # the request handler below will set this to the auth token + token = nil + + # launches webrick, listening for the redirect with the auth code. + launch_client_app(port: port) do |req, res| + # get access code from code url param + if code = req.query['code'].presence + # exchange it for token + token = client.auth_code.get_token(code, redirect_uri: redirect_uri) + res.body = "

Authorization succeeded, you may close this window now.

" + end + end + + log_user 'jsmith', 'jsmith' + with_settings rest_api_enabled: 1 do + visit '/my/account' + click_link 'Authorized applications' + find 'p.nodata', visible: true + + # an oauth client would send the user to this url to request permission + url = client.auth_code.authorize_url redirect_uri: redirect_uri, scope: 'view_issues view_project' + uri = URI.parse url + visit uri.path + '?' + uri.query + + find 'h2', visible: true, text: 'Authorization required' + find 'p', visible: true, text: /Authorize Oauth Test/ + find '.oauth-permissions', visible: true, text: /View Issues/ + find '.oauth-permissions', visible: true, text: /View project/ + + click_button 'Authorize' + + assert grant = app.access_grants.last + assert_equal 'view_issues view_project', grant.scopes.to_s + + # check for output defined above in the request handler + find 'p', visible: true, text: /Authorization succeeded/ + assert token.present? + + visit '/my/account' + click_link 'Authorized applications' + find 'td', visible: true, text: /Oauth Test/ + click_link 'Sign out' + + # Now, use the token for some API requests + assert_raise(RestClient::Unauthorized) do + RestClient.get "http://localhost:#{test_port}/projects/onlinestore/issues.json" + end + + headers = { 'Authorization' => "Bearer #{token.token}" } + r = RestClient.get "http://localhost:#{test_port}/projects/onlinestore/issues.json", headers + issues = JSON.parse(r.body)['issues'] + assert issues.any? + + # time entries access is not part of the granted scopes + assert_raise(RestClient::Forbidden) do + RestClient.get "http://localhost:#{test_port}/projects/onlinestore/time_entries.json", headers + end + end + end + + private + + def launch_client_app(port: 12345, path: '/', &block) + server = WEBrick::HTTPServer.new Port: port + trap('INT') { server.shutdown } + server.mount_proc(path, block) + Thread.new { server.start } + port + end + + def test_port + Capybara.current_session.server.port + end +end diff --git a/test/unit/role_test.rb b/test/unit/role_test.rb index 21103919f..1d0d39d7e 100644 --- a/test/unit/role_test.rb +++ b/test/unit/role_test.rb @@ -175,6 +175,32 @@ class RoleTest < ActiveSupport::TestCase assert_equal false, role.permissions_tracker_ids?(:view_issues, 1) end + def test_allowed_to_with_symbol + role = Role.create!(:name => 'Test', :permissions => [:view_issues]) + assert_equal true, role.allowed_to?(:view_issues) + assert_equal false, role.allowed_to?(:add_issues) + end + + def test_allowed_to_with_symbol_and_scope + role = Role.create!(:name => 'Test', :permissions => [:view_issues, :delete_issues]) + assert_equal true, role.allowed_to?(:view_issues, [:view_issues, :add_issues]) + assert_equal false, role.allowed_to?(:add_issues, [:view_issues, :add_issues]) + assert_equal false, role.allowed_to?(:delete_issues, [:view_issues, :add_issues]) + end + + def test_allowed_to_with_hash + role = Role.create!(:name => 'Test', :permissions => [:view_issues]) + assert_equal true, role.allowed_to?(:controller => 'issues', :action => 'show') + assert_equal false, role.allowed_to?(:controller => 'issues', :action => 'create') + end + + def test_allowed_to_with_hash_and_scope + role = Role.create!(:name => 'Test', :permissions => [:view_issues, :delete_issues]) + assert_equal true, role.allowed_to?({:controller => 'issues', :action => 'show'}, [:view_issues, :add_issues]) + assert_equal false, role.allowed_to?({:controller => 'issues', :action => 'create'}, [:view_issues, :add_issues]) + assert_equal false, role.allowed_to?({:controller => 'issues', :action => 'destroy'}, [:view_issues, :add_issues]) + end + def test_has_permission_without_permissions role = Role.create!(:name => 'Test') assert_equal false, role.has_permission?(:delete_issues) diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index 8474e174b..967771c87 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -1398,6 +1398,67 @@ class UserTest < ActiveSupport::TestCase end end + def test_should_recognize_authorized_by_oauth + u = User.find 2 + assert_not u.authorized_by_oauth? + u.oauth_scope = [:add_issues, :view_issues] + assert u.authorized_by_oauth? + end + + def test_admin_should_be_limited_by_oauth_scope + u = User.find_by_admin(true) + assert u.admin? + + u.oauth_scope = [:add_issues, :view_issues] + assert_not u.admin? + + u.oauth_scope = [:add_issues, :view_issues, :admin] + assert u.admin? + + u = User.find_by_admin(false) + assert_not u.admin? + u.oauth_scope = [:add_issues, :view_issues, :admin] + assert_not u.admin? + end + + def test_oauth_scope_should_limit_global_user_permissions + admin = User.find 1 + user = User.find 2 + [admin, user].each do |u| + assert u.allowed_to?(:add_issues, nil, global: true) + assert u.allowed_to?(:view_issues, nil, global: true) + u.oauth_scope = [:view_issues] + assert_not u.allowed_to?(:add_issues, nil, global: true) + assert u.allowed_to?(:view_issues, nil, global: true) + end + end + + def test_oauth_scope_should_limit_project_user_permissions + admin = User.find 1 + project = Project.find 5 + assert admin.allowed_to?(:add_issues, project) + assert admin.allowed_to?(:view_issues, project) + admin.oauth_scope = [:view_issues] + assert_not admin.allowed_to?(:add_issues, project) + assert admin.allowed_to?(:view_issues, project) + + admin.oauth_scope = [:view_issues, :admin] + assert admin.allowed_to?(:add_issues, project) + assert admin.allowed_to?(:view_issues, project) + + user = User.find 2 + project = Project.find 1 + assert user.allowed_to?(:add_issues, project) + assert user.allowed_to?(:view_issues, project) + user.oauth_scope = [:view_issues] + assert_not user.allowed_to?(:add_issues, project) + assert user.allowed_to?(:view_issues, project) + + user.oauth_scope = [:view_issues, :admin] + assert_not user.allowed_to?(:add_issues, project) + assert user.allowed_to?(:view_issues, project) + end + def test_destroy_should_delete_associated_reactions users(:users_004).reactions.create!( [ -- 2.39.5 (Apple Git-154)