diff --git a/app/controllers/api/auth_providers_controller.rb b/app/controllers/api/auth_providers_controller.rb index f83915e9f..a79c0588a 100644 --- a/app/controllers/api/auth_providers_controller.rb +++ b/app/controllers/api/auth_providers_controller.rb @@ -80,14 +80,13 @@ class API::AuthProvidersController < API::ApiController if params['auth_provider']['providable_type'] == DatabaseProvider.name params.require(:auth_provider).permit(:name, :providable_type) elsif params['auth_provider']['providable_type'] == OAuth2Provider.name - params.require(:auth_provider) - .permit(:name, :providable_type, - providable_attributes: [:id, :base_url, :token_endpoint, :authorization_endpoint, :logout_endpoint, - :profile_url, :client_id, :client_secret, :scopes, - o_auth2_mappings_attributes: [:id, :local_model, :local_field, :api_field, - :api_endpoint, :api_data_type, :_destroy, - transformation: [:type, :format, :true_value, - :false_value, mapping: %i[from to]]]]) + params.require(:auth_provider) + .permit(:name, :providable_type, + providable_attributes: %i[id base_url token_endpoint authorization_endpoint logout_endpoint + profile_url client_id client_secret scopes], + auth_provider_mappings_attributes: [:id, :local_model, :local_field, :api_field, :api_endpoint, :api_data_type, + :_destroy, transformation: [:type, :format, :true_value, :false_value, + mapping: %i[from to]]]) end end end diff --git a/app/frontend/src/javascript/controllers/admin/authentications.js b/app/frontend/src/javascript/controllers/admin/authentications.js index 9bbe4a3d3..8288bd19d 100644 --- a/app/frontend/src/javascript/controllers/admin/authentications.js +++ b/app/frontend/src/javascript/controllers/admin/authentications.js @@ -34,7 +34,7 @@ const findIdxById = function (elements, id) { /** * For OAuth2 authentications, mapping the user's ID is mandatory. This function will check that this mapping * is effective and will return false otherwise - * @param mappings {Array} expected: $scope.provider.providable_attributes.o_auth2_mappings_attributes + * @param mappings {Array} expected: $scope.provider.auth_provider_mappings_attributes * @returns {Boolean} true if the mapping is declared */ const check_oauth2_id_is_mapped = function (mappings) { @@ -246,8 +246,8 @@ Application.Controllers.controller('NewAuthenticationController', ['$scope', '$s $scope.updateProvidable = function () { // === OAuth2Provider === if ($scope.provider.providable_type === 'OAuth2Provider') { - if (typeof $scope.provider.providable_attributes.o_auth2_mappings_attributes === 'undefined') { - return $scope.provider.providable_attributes.o_auth2_mappings_attributes = []; + if (typeof $scope.provider.auth_provider_mappings_attributes === 'undefined') { + return $scope.provider.auth_provider_mappings_attributes = []; } } }; @@ -274,7 +274,7 @@ Application.Controllers.controller('NewAuthenticationController', ['$scope', '$s // === OAuth2Provider === } else if ($scope.provider.providable_type === 'OAuth2Provider') { // check the ID mapping - if (!check_oauth2_id_is_mapped($scope.provider.providable_attributes.o_auth2_mappings_attributes)) { + if (!check_oauth2_id_is_mapped($scope.provider.auth_provider_mappings_attributes)) { growl.error(_t('app.admin.authentication_new.it_is_required_to_set_the_matching_between_User.uid_and_the_API_to_add_this_provider')); return false; } @@ -330,7 +330,7 @@ Application.Controllers.controller('EditAuthenticationController', ['$scope', '$ */ $scope.updateProvider = function () { // check the ID mapping - if (!check_oauth2_id_is_mapped($scope.provider.providable_attributes.o_auth2_mappings_attributes)) { + if (!check_oauth2_id_is_mapped($scope.provider.auth_provider_mappings_attributes)) { growl.error(_t('app.admin.authentication_edit.it_is_required_to_set_the_matching_between_User.uid_and_the_API_to_add_this_provider')); return false; } diff --git a/app/frontend/templates/admin/authentications/_oauth2_mapping.html b/app/frontend/templates/admin/authentications/_oauth2_mapping.html index d6d37811c..48a370eb6 100644 --- a/app/frontend/templates/admin/authentications/_oauth2_mapping.html +++ b/app/frontend/templates/admin/authentications/_oauth2_mapping.html @@ -14,7 +14,7 @@ - + {{m.local_model}} {{m.local_field}} {{m.api_endpoint}} @@ -72,7 +72,7 @@ required/> - + diff --git a/app/models/auth_provider.rb b/app/models/auth_provider.rb index 318340048..29fdf0a0f 100644 --- a/app/models/auth_provider.rb +++ b/app/models/auth_provider.rb @@ -18,6 +18,9 @@ class AuthProvider < ApplicationRecord belongs_to :providable, polymorphic: true, dependent: :destroy accepts_nested_attributes_for :providable + has_many :auth_provider_mappings, dependent: :destroy + accepts_nested_attributes_for :auth_provider_mappings, allow_destroy: true + before_create :set_initial_state def build_providable(params) @@ -75,7 +78,11 @@ class AuthProvider < ApplicationRecord ## Return the user's profile fields that are currently managed from the SSO ## @return [Array] def sso_fields - providable.protected_fields + fields = [] + auth_provider_mappings.each do |mapping| + fields.push(mapping.local_model + '.' + mapping.local_field) + end + fields end ## Return the link the user have to follow to edit his profile on the SSO diff --git a/app/models/auth_provider_mapping.rb b/app/models/auth_provider_mapping.rb new file mode 100644 index 000000000..4f4c19d56 --- /dev/null +++ b/app/models/auth_provider_mapping.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +# AuthProviderMapping defines the relationship between a database field (saving user's data) +# and an external API, that is authorized through an external SSO (like oAuth 2.0). +class AuthProviderMapping < ApplicationRecord + belongs_to :auth_provider +end diff --git a/app/models/database_provider.rb b/app/models/database_provider.rb index 6c4c9fb95..6b625603d 100644 --- a/app/models/database_provider.rb +++ b/app/models/database_provider.rb @@ -5,10 +5,6 @@ class DatabaseProvider < ApplicationRecord has_one :auth_provider, as: :providable, dependent: :destroy - def protected_fields - [] - end - def profile_url '/#!/dashboard/profile' end diff --git a/app/models/o_auth2_mapping.rb b/app/models/o_auth2_mapping.rb deleted file mode 100644 index 6f097dea7..000000000 --- a/app/models/o_auth2_mapping.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -# OAuth2Mapping defines a database field, saving user's data, that is mapped to an external API, that is authorized -# through an external SSO of type oAuth 2 -class OAuth2Mapping < ApplicationRecord - belongs_to :o_auth2_provider -end diff --git a/app/models/o_auth2_provider.rb b/app/models/o_auth2_provider.rb index 75f847d7d..a91647b85 100644 --- a/app/models/o_auth2_provider.rb +++ b/app/models/o_auth2_provider.rb @@ -4,18 +4,9 @@ # the oAuth 2.0 protocol. class OAuth2Provider < ApplicationRecord has_one :auth_provider, as: :providable - has_many :o_auth2_mappings, dependent: :destroy - accepts_nested_attributes_for :o_auth2_mappings, allow_destroy: true def domain URI(base_url).scheme + '://' + URI(base_url).host end - def protected_fields - fields = [] - o_auth2_mappings.each do |mapping| - fields.push(mapping.local_model + '.' + mapping.local_field) - end - fields - end end diff --git a/app/models/open_id_connect_provider.rb b/app/models/open_id_connect_provider.rb new file mode 100644 index 000000000..a631a26a7 --- /dev/null +++ b/app/models/open_id_connect_provider.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +# OpenIdConnectProvider is a special type of AuthProvider which provides authentication through an external SSO server using +# the OpenID Connect protocol. +class OpenIdConnectProvider < ApplicationRecord + has_one :auth_provider, as: :providable +end diff --git a/app/views/api/auth_providers/_auth_provider.json.jbuilder b/app/views/api/auth_providers/_auth_provider.json.jbuilder index 325094b72..8889c959d 100644 --- a/app/views/api/auth_providers/_auth_provider.json.jbuilder +++ b/app/views/api/auth_providers/_auth_provider.json.jbuilder @@ -1,3 +1,6 @@ # frozen_string_literal: true json.extract! auth_provider, :id, :name, :status, :providable_type, :strategy_name +json.auth_provider_mappings_attributes @provider.auth_provider_mappings do |m| + json.extract! m, :id, :local_model, :local_field, :api_field, :api_endpoint, :api_data_type, :transformation +end diff --git a/app/views/api/auth_providers/show.json.jbuilder b/app/views/api/auth_providers/show.json.jbuilder index 78e7397b1..183fad96b 100644 --- a/app/views/api/auth_providers/show.json.jbuilder +++ b/app/views/api/auth_providers/show.json.jbuilder @@ -1,3 +1,5 @@ +# frozen_string_literal: true + json.partial! 'api/auth_providers/auth_provider', auth_provider: @provider # OAuth 2.0 @@ -5,8 +7,5 @@ json.partial! 'api/auth_providers/auth_provider', auth_provider: @provider if @provider.providable_type == OAuth2Provider.name json.providable_attributes do json.extract! @provider.providable, :id, :base_url, :token_endpoint, :authorization_endpoint, :profile_url, :client_id, :client_secret, :scopes - json.o_auth2_mappings_attributes @provider.providable.o_auth2_mappings do |m| - json.extract! m, :id, :local_model, :local_field, :api_field, :api_endpoint, :api_data_type, :transformation - end end end diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 95df71d50..32aa13e4a 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -231,7 +231,9 @@ Devise.setup do |config| require_relative '../../lib/omni_auth/omni_auth' active_provider = AuthProvider.active if active_provider.providable_type == OAuth2Provider.name - config.omniauth OmniAuth::Strategies::SsoOauth2Provider.name.to_sym, active_provider.providable.client_id, active_provider.providable.client_secret + config.omniauth OmniAuth::Strategies::SsoOauth2Provider.name.to_sym, + active_provider.providable.client_id, + active_provider.providable.client_secret end # ==> Warden configuration diff --git a/db/migrate/20220328141618_create_open_id_connect_providers.rb b/db/migrate/20220328141618_create_open_id_connect_providers.rb new file mode 100644 index 000000000..9d04d1ce0 --- /dev/null +++ b/db/migrate/20220328141618_create_open_id_connect_providers.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +# This migration allow configuration of OpenID Connect providers +class CreateOpenIdConnectProviders < ActiveRecord::Migration[5.2] + def change + create_table :open_id_connect_providers do |t| + t.string :issuer + t.boolean :discovery + t.string :client_auth_method + t.string :scope + t.string :response_type + t.string :response_type + t.string :response_mode + t.string :display + t.string :prompt + t.boolean :send_scope_to_token_endpoint + t.string :post_logout_redirect_uri + t.string :uid_field + t.string :extra_authorize_params + t.string :allow_authorize_params + t.string :client_identifier + t.string :client_secret + t.string :client_redirect_uri + t.string :client_scheme + t.string :client_host + t.string :client_port + t.string :client_authorization_endpoint + t.string :client_token_endpoint + t.string :client_userinfo_endpoint + t.string :client_jwks_uri + t.string :client_end_session_endpoint + + t.timestamps + end + end +end diff --git a/db/migrate/20220328144305_rename_o_auth2_mappings_to_auth_provider_mappings.rb b/db/migrate/20220328144305_rename_o_auth2_mappings_to_auth_provider_mappings.rb new file mode 100644 index 000000000..f64d47a41 --- /dev/null +++ b/db/migrate/20220328144305_rename_o_auth2_mappings_to_auth_provider_mappings.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +# This migration renames the OAuth2Mappings table to AuthProviderMappings because the +# field mapping is common to all kinds of single-sign-on providers. +class RenameOAuth2MappingsToAuthProviderMappings < ActiveRecord::Migration[5.2] + def change + rename_table :o_auth2_mappings, :auth_provider_mappings + add_reference :auth_provider_mappings, :auth_provider, index: true, foreign_key: true + + end +end diff --git a/db/migrate/20220328145017_migrate_o_auth2_provider_id_from_auth_provider_mappings.rb b/db/migrate/20220328145017_migrate_o_auth2_provider_id_from_auth_provider_mappings.rb new file mode 100644 index 000000000..cc659e48c --- /dev/null +++ b/db/migrate/20220328145017_migrate_o_auth2_provider_id_from_auth_provider_mappings.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +# Previously, the AuthProviderMapping was saving an o_auth2_provider_id. +# This migration migrates that data to bind the mappings directly to an AuthProvider as this table is now protocol-generic. +class MigrateOAuth2ProviderIdFromAuthProviderMappings < ActiveRecord::Migration[5.2] + def up + execute <<~SQL + UPDATE auth_provider_mappings + SET auth_provider_id = auth_providers.id + FROM o_auth2_providers + INNER JOIN auth_providers ON auth_providers.providable_id = o_auth2_providers.id + AND auth_providers.providable_type = 'OAuth2Provider' + WHERE auth_provider_mappings.o_auth2_provider_id = o_auth2_providers.id + SQL + + remove_reference :auth_provider_mappings, :o_auth2_provider, index: true, foreign_key: true + end + + def down + add_reference :auth_provider_mappings, :o_auth2_provider, index: true, foreign_key: true + + execute <<~SQL + UPDATE auth_provider_mappings + SET o_auth2_provider_id = o_auth2_providers.id + FROM o_auth2_providers + INNER JOIN auth_providers ON auth_providers.providable_id = o_auth2_providers.id + AND auth_providers.providable_type = 'OAuth2Provider' + WHERE auth_provider_mappings.auth_provider_id = auth_providers.id + SQL + end +end diff --git a/db/schema.rb b/db/schema.rb index a3ad2f439..ec9e9a11e 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2022_03_22_135836) do +ActiveRecord::Schema.define(version: 2022_03_28_145017) do # These are extensions that must be enabled in order to support this database enable_extension "fuzzystrmatch" @@ -72,6 +72,19 @@ ActiveRecord::Schema.define(version: 2022_03_22_135836) do t.datetime "updated_at" end + create_table "auth_provider_mappings", id: :serial, force: :cascade do |t| + t.string "local_field" + t.string "api_field" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.string "local_model" + t.string "api_endpoint" + t.string "api_data_type" + t.jsonb "transformation" + t.bigint "auth_provider_id" + t.index ["auth_provider_id"], name: "index_auth_provider_mappings_on_auth_provider_id" + end + create_table "auth_providers", id: :serial, force: :cascade do |t| t.string "name" t.string "status" @@ -369,19 +382,6 @@ ActiveRecord::Schema.define(version: 2022_03_22_135836) do t.index ["receiver_id"], name: "index_notifications_on_receiver_id" end - create_table "o_auth2_mappings", id: :serial, force: :cascade do |t| - t.integer "o_auth2_provider_id" - t.string "local_field" - t.string "api_field" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false - t.string "local_model" - t.string "api_endpoint" - t.string "api_data_type" - t.jsonb "transformation" - t.index ["o_auth2_provider_id"], name: "index_o_auth2_mappings_on_o_auth2_provider_id" - end - create_table "o_auth2_providers", id: :serial, force: :cascade do |t| t.string "base_url" t.string "token_endpoint" @@ -411,6 +411,35 @@ ActiveRecord::Schema.define(version: 2022_03_22_135836) do t.datetime "updated_at", null: false end + create_table "open_id_connect_providers", force: :cascade do |t| + t.string "issuer" + t.boolean "discovery" + t.string "client_auth_method" + t.string "scope" + t.string "response_type" + t.string "response_mode" + t.string "display" + t.string "prompt" + t.boolean "send_scope_to_token_endpoint" + t.string "post_logout_redirect_uri" + t.string "uid_field" + t.string "extra_authorize_params" + t.string "allow_authorize_params" + t.string "client_identifier" + t.string "client_secret" + t.string "client_redirect_uri" + t.string "client_scheme" + t.string "client_host" + t.string "client_port" + t.string "client_authorization_endpoint" + t.string "client_token_endpoint" + t.string "client_userinfo_endpoint" + t.string "client_jwks_uri" + t.string "client_end_session_endpoint" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "organizations", id: :serial, force: :cascade do |t| t.string "name" t.datetime "created_at", null: false @@ -982,6 +1011,7 @@ ActiveRecord::Schema.define(version: 2022_03_22_135836) do end add_foreign_key "accounting_periods", "users", column: "closed_by" + add_foreign_key "auth_provider_mappings", "auth_providers" add_foreign_key "availability_tags", "availabilities" add_foreign_key "availability_tags", "tags" add_foreign_key "event_price_categories", "events" @@ -999,7 +1029,6 @@ ActiveRecord::Schema.define(version: 2022_03_22_135836) do add_foreign_key "invoices", "statistic_profiles" add_foreign_key "invoices", "wallet_transactions" add_foreign_key "invoicing_profiles", "users" - add_foreign_key "o_auth2_mappings", "o_auth2_providers" add_foreign_key "organizations", "invoicing_profiles" add_foreign_key "payment_gateway_objects", "payment_gateway_objects" add_foreign_key "payment_schedule_items", "invoices" diff --git a/lib/omni_auth/strategies/sso_oauth2_provider.rb b/lib/omni_auth/strategies/sso_oauth2_provider.rb index af59688ad..8d9cfd9e7 100644 --- a/lib/omni_auth/strategies/sso_oauth2_provider.rb +++ b/lib/omni_auth/strategies/sso_oauth2_provider.rb @@ -58,7 +58,7 @@ module OmniAuth::Strategies @raw_info ||= {} logger.debug "[raw_info] @raw_infos = #{@raw_info&.to_json}" unless @raw_info.size.positive? - OmniAuth::Strategies::SsoOauth2Provider.active_provider.providable.o_auth2_mappings.each do |mapping| + OmniAuth::Strategies::SsoOauth2Provider.active_provider.auth_provider_mappings.each do |mapping| logger.debug "mapping = #{mapping&.to_json}" next if @raw_info.key?(mapping.api_endpoint.to_sym) @@ -78,7 +78,7 @@ module OmniAuth::Strategies @parsed_info ||= {} logger.debug "[parsed_info] @parsed_info = #{@parsed_info.to_json}" unless @parsed_info.size.positive? - OmniAuth::Strategies::SsoOauth2Provider.active_provider.providable.o_auth2_mappings.each do |mapping| + OmniAuth::Strategies::SsoOauth2Provider.active_provider.auth_provider_mappings.each do |mapping| raw_data = ::JsonPath.new(mapping.api_field).on(raw_info[mapping.api_endpoint.to_sym]).first logger.debug "@parsed_info[#{local_sym(mapping)}] mapped from #{raw_data}" diff --git a/test/fixtures/o_auth2_mappings.yml b/test/fixtures/auth_provider_mappings.yml similarity index 100% rename from test/fixtures/o_auth2_mappings.yml rename to test/fixtures/auth_provider_mappings.yml diff --git a/test/integration/auth_providers_test.rb b/test/integration/auth_providers_test.rb index d985ab580..f55afc51d 100644 --- a/test/integration/auth_providers_test.rb +++ b/test/integration/auth_providers_test.rb @@ -22,24 +22,24 @@ class AuthProvidersTest < ActionDispatch::IntegrationTest base_url: 'https://github.com/login/oauth/', profile_url: 'https://github.com/settings/profile', client_id: ENV.fetch('OAUTH_CLIENT_ID') { 'github-oauth-app-id' }, - client_secret: ENV.fetch('OAUTH_CLIENT_SECRET') { 'github-oauth-app-secret' }, - o_auth2_mappings_attributes: [ - { - api_data_type: 'json', - api_endpoint: 'https://api.github.com/user', - api_field: 'id', - local_field: 'uid', - local_model: 'user' - }, - { - api_data_type: 'json', - api_endpoint: 'https://api.github.com/user', - api_field: 'html_url', - local_field: 'github', - local_model: 'profile' - } - ] - } + client_secret: ENV.fetch('OAUTH_CLIENT_SECRET') { 'github-oauth-app-secret' } + }, + auth_provider_mappings_attributes: [ + { + api_data_type: 'json', + api_endpoint: 'https://api.github.com/user', + api_field: 'id', + local_field: 'uid', + local_model: 'user' + }, + { + api_data_type: 'json', + api_endpoint: 'https://api.github.com/user', + api_field: 'html_url', + local_field: 'github', + local_model: 'profile' + } + ] } }.to_json, headers: default_headers @@ -56,7 +56,7 @@ class AuthProvidersTest < ActionDispatch::IntegrationTest assert_equal name, provider[:name] assert_equal db_provider.id, provider[:id] assert_equal 'pending', provider[:status] - assert_equal 2, provider[:providable_attributes][:o_auth2_mappings_attributes].length + assert_equal 2, provider[:auth_provider_mappings_attributes].length # now let's activate this new provider Fablab::Application.load_tasks if Rake::Task.tasks.empty?