From df7024121ca83b334e12e2014d4be03a0b32e050 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 4 Jun 2014 12:41:32 +0100 Subject: [PATCH 01/11] based openid plugin on general auth plugin --- lib/open_project/openid_connect/engine.rb | 28 ++++++++--------------- openproject-openid_connect.gemspec | 1 + 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/lib/open_project/openid_connect/engine.rb b/lib/open_project/openid_connect/engine.rb index fc5398f106..5f580a9d3b 100644 --- a/lib/open_project/openid_connect/engine.rb +++ b/lib/open_project/openid_connect/engine.rb @@ -19,7 +19,7 @@ module OpenProject::OpenIDConnect openid_connect/auth_provider-google.png ) - initializer "openid_connect.middleware" do |app| + def init_auth # Loading OpenID providers manually since rails doesn't do it automatically, # possibly due to non trivially module-name-convertible paths. require 'omniauth/openid_connect/provider' @@ -36,28 +36,20 @@ module OpenProject::OpenIDConnect end OmniAuth::OpenIDConnect::Provider.load_generic_providers + end - app.config.middleware.use OmniAuth::Builder do - OmniAuth::OpenIDConnect::Provider.all.each do |pro| - p = pro.new - settings_available = if pro.available? - "settings available" - else - "settings missing" - end + def omniauth_strategies + [:openid_connect] + end - Rails.logger.info "[OpenID Connect] Registering provider for #{p.name} (#{settings_available})" - provider :openid_connect, :name => p.name, :setup => lambda { |env| - Rails.logger.info "[OpenID Connect] Trying dynamic provider #{p.name}" - opt = env['omniauth.strategy'].options - p.to_hash.each do |key, value| - opt[key] = value - end - } - end + def providers_for_strategy(strategy) + if strategy == :openid_connect + OmniAuth::OpenIDConnect::Provider.available.map(&:new) end end + include OpenProject::Plugins::AuthPlugin + initializer 'openid_connect.register_hooks' do require 'open_project/openid_connect/hooks' end diff --git a/openproject-openid_connect.gemspec b/openproject-openid_connect.gemspec index ed71cddccb..e3605f6351 100644 --- a/openproject-openid_connect.gemspec +++ b/openproject-openid_connect.gemspec @@ -17,6 +17,7 @@ Gem::Specification.new do |s| s.add_dependency "rails", "~> 3.2.14" s.add_dependency "openproject-plugins", "~> 1.0" + s.add_dependency "openproject-auth_plugins" s.add_dependency "omniauth" s.add_development_dependency "rspec", "~> 2.14" From ee431fb68b51e14a4784be3b484774fc1f71dba1 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 4 Jun 2014 13:36:04 +0100 Subject: [PATCH 02/11] use strategy DSL --- lib/open_project/openid_connect/engine.rb | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/open_project/openid_connect/engine.rb b/lib/open_project/openid_connect/engine.rb index 5f580a9d3b..6d2357078b 100644 --- a/lib/open_project/openid_connect/engine.rb +++ b/lib/open_project/openid_connect/engine.rb @@ -7,6 +7,7 @@ module OpenProject::OpenIDConnect engine_name :openproject_openid_connect include OpenProject::Plugins::ActsAsOpEngine + extend OpenProject::Plugins::AuthPlugin register 'openproject-openid_connect', :author_url => 'http://finn.de', @@ -19,7 +20,7 @@ module OpenProject::OpenIDConnect openid_connect/auth_provider-google.png ) - def init_auth + register_auth_providers do # Loading OpenID providers manually since rails doesn't do it automatically, # possibly due to non trivially module-name-convertible paths. require 'omniauth/openid_connect/provider' @@ -36,20 +37,12 @@ module OpenProject::OpenIDConnect end OmniAuth::OpenIDConnect::Provider.load_generic_providers - end - - def omniauth_strategies - [:openid_connect] - end - def providers_for_strategy(strategy) - if strategy == :openid_connect + strategy :openid_connect do OmniAuth::OpenIDConnect::Provider.available.map(&:new) end end - include OpenProject::Plugins::AuthPlugin - initializer 'openid_connect.register_hooks' do require 'open_project/openid_connect/hooks' end From a8450b911d5d0d90dd13bb909dd91279fc2b4e17 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 4 Jun 2014 23:20:31 +0100 Subject: [PATCH 03/11] base plugin on openproject-auth_plugins --- CHANGELOG.md | 6 ++++ .../openid_connect/openid_connect.css.sass | 31 ------------------- app/views/hooks/login/_providers.html.erb | 14 --------- lib/open_project/openid_connect/engine.rb | 10 ++---- lib/open_project/openid_connect/version.rb | 2 +- 5 files changed, 10 insertions(+), 53 deletions(-) delete mode 100644 app/assets/stylesheets/openid_connect/openid_connect.css.sass delete mode 100644 app/views/hooks/login/_providers.html.erb diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ad4b82f8e..2e9f5f6271 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ # Changelog +## 0.1.0 + +* use openproject-auth_plugins as basis + +## 0.0.1 + * `#5555` Multi-Provider login screens diff --git a/app/assets/stylesheets/openid_connect/openid_connect.css.sass b/app/assets/stylesheets/openid_connect/openid_connect.css.sass deleted file mode 100644 index ff43dacd1a..0000000000 --- a/app/assets/stylesheets/openid_connect/openid_connect.css.sass +++ /dev/null @@ -1,31 +0,0 @@ -/*-- copyright - * OpenProject is a project management system. - * Copyright (C) 2012-2014 the OpenProject Foundation (OPF) - * - * This program is free software; you can redistribute it and/or - * modify it under the terms of the GNU General Public License version 3. - * - * OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: - * Copyright (C) 2006-2013 Jean-Philippe Lang - * Copyright (C) 2010-2013 the ChiliProject Team - * - * 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. - * - * See doc/COPYRIGHT.rdoc for more details. ++ - */ - -#content, #top-menu #nav-login-content - .login-auth-providers a.auth-provider.auth-provider-google - background-image: url(image-path('openid_connect/auth_provider-google.png')) diff --git a/app/views/hooks/login/_providers.html.erb b/app/views/hooks/login/_providers.html.erb deleted file mode 100644 index 92a166e5e9..0000000000 --- a/app/views/hooks/login/_providers.html.erb +++ /dev/null @@ -1,14 +0,0 @@ -<% OmniAuth::OpenIDConnect::Provider.available.each do |pro| %> - <% - opts = { - :controller => '/auth', - :action => pro.provider_name - } - if params["back_url"] - opts[:origin] = params["back_url"] - end - %> - - <%= pro.provider_name.camelize %> - -<% end %> diff --git a/lib/open_project/openid_connect/engine.rb b/lib/open_project/openid_connect/engine.rb index 6d2357078b..db3f265510 100644 --- a/lib/open_project/openid_connect/engine.rb +++ b/lib/open_project/openid_connect/engine.rb @@ -12,11 +12,9 @@ module OpenProject::OpenIDConnect register 'openproject-openid_connect', :author_url => 'http://finn.de', :requires_openproject => '>= 3.1.0pre1', - :global_assets => { css: 'openid_connect/openid_connect.css' }, :settings => { 'default' => { 'providers' => {} } } assets %w( - openid_connect/openid_connect.css openid_connect/auth_provider-google.png ) @@ -39,12 +37,10 @@ module OpenProject::OpenIDConnect OmniAuth::OpenIDConnect::Provider.load_generic_providers strategy :openid_connect do - OmniAuth::OpenIDConnect::Provider.available.map(&:new) + OmniAuth::OpenIDConnect::Provider.available.map do |p| + p.new.to_hash.merge({icon: 'openid_connect/auth_provider-google.png'}) + end end end - - initializer 'openid_connect.register_hooks' do - require 'open_project/openid_connect/hooks' - end end end diff --git a/lib/open_project/openid_connect/version.rb b/lib/open_project/openid_connect/version.rb index b1e686206e..a95cea2b52 100644 --- a/lib/open_project/openid_connect/version.rb +++ b/lib/open_project/openid_connect/version.rb @@ -1,5 +1,5 @@ module OpenProject module OpenIDConnect - VERSION = "0.0.1" + VERSION = "0.1.0" end end From 7767f2686d2639dac85b442ada735acaead19202 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 4 Jun 2014 23:22:23 +0100 Subject: [PATCH 04/11] list all necessary Gemfile entries --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 20610bcb5b..34ae308d7c 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,8 @@ Adds support for OmniAuth OpenID Connect strategy providers, most importantly Go You will have to add the following lines to your OpenProject's _Gemfile.plugins_ for the time being: + gem "openproject-plugins", :git => "git@github.com:opf/openproject-plugins.git", :branch => "dev" + gem "openproject-auth_plugins", :git => 'git@github.com:finnlabs/openproject-auth_plugins, :branch => 'dev' gem 'omniauth-openid-connect', :git => 'git@github.com:finnlabs/omniauth-openid-connect.git', :branch => 'master' gem 'openproject-openid_connect', :git => 'git@github.com:finnlabs/openproject-openid_connect.git', :branch => 'dev' From 0d679b6daab5bbd10d0af943c8636ec75bd6c78e Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 6 Jun 2014 10:36:05 +0100 Subject: [PATCH 05/11] specify used gem versions --- openproject-openid_connect.gemspec | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openproject-openid_connect.gemspec b/openproject-openid_connect.gemspec index e3605f6351..d10340a650 100644 --- a/openproject-openid_connect.gemspec +++ b/openproject-openid_connect.gemspec @@ -17,8 +17,8 @@ Gem::Specification.new do |s| s.add_dependency "rails", "~> 3.2.14" s.add_dependency "openproject-plugins", "~> 1.0" - s.add_dependency "openproject-auth_plugins" - s.add_dependency "omniauth" + s.add_dependency "openproject-auth_plugins", "~> 0.1" + s.add_dependency "omniauth", "~> 1.0" s.add_development_dependency "rspec", "~> 2.14" s.add_development_dependency "rspec-steps", "~> 0.4.0" From 7fe9d3949ba1a5b0dd0b5c4d6e5479e48415e157 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 6 Jun 2014 10:36:13 +0100 Subject: [PATCH 06/11] fixed spec --- spec/requests/openid_connect_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index f4b0947156..c058c03a63 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -142,7 +142,7 @@ describe "OpenID Connect" do get "/login" expect(response.body).not_to include "Google" - expect{click_on_signin("google")}.to raise_error(ArgumentError) + expect{click_on_signin("google")}.to raise_error(ActionController::RoutingError) end it "should make providers that have been configured through settings available without requiring a restart" do From 3973ed94c65fb1ee1d13506f70208af437cfefac Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 6 Jun 2014 11:00:51 +0100 Subject: [PATCH 07/11] made provider icons and labels configurable --- README.md | 9 +++++++++ lib/omniauth/openid_connect/provider.rb | 6 ++++-- lib/open_project/openid_connect/engine.rb | 4 +--- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 34ae308d7c..37c29bd8b0 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,15 @@ Example configuration: google: identifier: "9295222hfbiu2btgu3b4i.apps.googleusercontent.com" secret: "4z389thugh334t8h" + icon: "openid_connect/auth_provider-google.png" + display_name: "Google" + +The last two attributes are commonly available for all providers. +They are used to change a provider's look. + +Note that `openid_connect/auth_provider-google.png` is the one custom provider icon this plugin has out of the box. Other icons you will have to add yourself. + +`display_name` changes a provider's label shown to the user. ### Settings diff --git a/lib/omniauth/openid_connect/provider.rb b/lib/omniauth/openid_connect/provider.rb index 891a02d4d3..dbfed311fd 100644 --- a/lib/omniauth/openid_connect/provider.rb +++ b/lib/omniauth/openid_connect/provider.rb @@ -108,10 +108,12 @@ module OmniAuth { :name => name, :scope => [:openid, :email, :profile], - :client_options => client_options.merge( # override with settings from configuration.yml + :icon => self.class.config["icon"], + :display_name => self.class.config["display_name"], + :client_options => client_options.merge( # override with configuration Hash[ self.class.config.reject do |key, value| - ["identifier", "secret"].include? key + ["identifier", "secret", "icon", "display_name"].include? key end.map do |key, value| [key.to_sym, value] end diff --git a/lib/open_project/openid_connect/engine.rb b/lib/open_project/openid_connect/engine.rb index db3f265510..48d95c7eb4 100644 --- a/lib/open_project/openid_connect/engine.rb +++ b/lib/open_project/openid_connect/engine.rb @@ -37,9 +37,7 @@ module OpenProject::OpenIDConnect OmniAuth::OpenIDConnect::Provider.load_generic_providers strategy :openid_connect do - OmniAuth::OpenIDConnect::Provider.available.map do |p| - p.new.to_hash.merge({icon: 'openid_connect/auth_provider-google.png'}) - end + OmniAuth::OpenIDConnect::Provider.available.map { |p| p.new.to_hash } end end end From fa8a40f1898ccb7ad7110fbeb8697e02cfa91c96 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 6 Jun 2014 11:01:21 +0100 Subject: [PATCH 08/11] deep merge settings --- lib/omniauth/openid_connect/provider.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/omniauth/openid_connect/provider.rb b/lib/omniauth/openid_connect/provider.rb index dbfed311fd..c62740de04 100644 --- a/lib/omniauth/openid_connect/provider.rb +++ b/lib/omniauth/openid_connect/provider.rb @@ -93,7 +93,7 @@ module OmniAuth {} end # Settings override configuration.yml - Hash(OpenProject::Configuration["openid_connect"]).merge(from_settings) + Hash(OpenProject::Configuration["openid_connect"]).deep_merge(from_settings) end def to_hash From 7d9660b6d9c933f01e42b948bb89c81102fcd9fc Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 6 Jun 2014 13:14:32 +0100 Subject: [PATCH 09/11] reference auth plugin ticket --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e9f5f6271..5f82f18c24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## 0.1.0 -* use openproject-auth_plugins as basis +* `#5558` use openproject-auth_plugins as basis ## 0.0.1 From 6ddeb8a6c61b410e56773ede6681b8c2fba63845 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 6 Jun 2014 14:09:17 +0100 Subject: [PATCH 10/11] discover new providers at runtime --- lib/open_project/openid_connect/engine.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/open_project/openid_connect/engine.rb b/lib/open_project/openid_connect/engine.rb index 48d95c7eb4..365300ee1b 100644 --- a/lib/open_project/openid_connect/engine.rb +++ b/lib/open_project/openid_connect/engine.rb @@ -37,6 +37,7 @@ module OpenProject::OpenIDConnect OmniAuth::OpenIDConnect::Provider.load_generic_providers strategy :openid_connect do + OmniAuth::OpenIDConnect::Provider.load_generic_providers # discover new providers OmniAuth::OpenIDConnect::Provider.available.map { |p| p.new.to_hash } end end From e8d0a7e3727924c22f901c24453f55754dd3d297 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 6 Jun 2014 14:13:23 +0100 Subject: [PATCH 11/11] it's not necessary to load the providers right away --- lib/open_project/openid_connect/engine.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/open_project/openid_connect/engine.rb b/lib/open_project/openid_connect/engine.rb index 365300ee1b..9920aa2217 100644 --- a/lib/open_project/openid_connect/engine.rb +++ b/lib/open_project/openid_connect/engine.rb @@ -34,10 +34,8 @@ module OpenProject::OpenIDConnect config.ssl_config.set_default_paths end - OmniAuth::OpenIDConnect::Provider.load_generic_providers - strategy :openid_connect do - OmniAuth::OpenIDConnect::Provider.load_generic_providers # discover new providers + OmniAuth::OpenIDConnect::Provider.load_generic_providers OmniAuth::OpenIDConnect::Provider.available.map { |p| p.new.to_hash } end end