diff --git a/Gemfile b/Gemfile index 0b95b32fd..88a4a8c28 100644 --- a/Gemfile +++ b/Gemfile @@ -81,7 +81,7 @@ gem 'rotp' gem 'grpc', '~> 1.67' gem 'tucana', '0.0.76' -gem 'code0-identities', '~> 0.0.3' +gem 'code0-identities', '~> 0.0.6' gem 'pry', '~> 0.16.0' gem 'pry-byebug', '~> 3.10' diff --git a/Gemfile.lock b/Gemfile.lock index 9c61ae663..2ac494c4e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -89,7 +89,7 @@ GEM builder (3.3.0) byebug (13.0.0) reline (>= 0.6.0) - code0-identities (0.0.3) + code0-identities (0.0.6) base64 (~> 0.3.0) httparty (~> 0.22) ruby-saml (~> 1.18) @@ -102,7 +102,7 @@ GEM concurrent-ruby (1.3.8) connection_pool (3.0.2) crass (1.0.7) - csv (3.3.5) + csv (3.3.6) database_cleaner-active_record (2.2.2) activerecord (>= 5.a) database_cleaner-core (~> 2.0) @@ -151,7 +151,7 @@ GEM grpc (1.83.0) google-protobuf (>= 3.25, < 5.0) googleapis-common-protos-types (~> 1.0) - httparty (0.23.2) + httparty (0.24.2) csv mini_mime (>= 1.0.0) multi_xml (>= 0.5.2) @@ -196,8 +196,8 @@ GEM drb (~> 2.0) prism (~> 1.5) msgpack (1.8.1) - multi_xml (0.7.2) - bigdecimal (~> 3.1) + multi_xml (0.9.1) + bigdecimal (>= 3.1, < 5) net-imap (0.6.3) date net-protocol @@ -625,7 +625,7 @@ DEPENDENCIES bcrypt (~> 3.1.7) benchmark bootsnap - code0-identities (~> 0.0.3) + code0-identities (~> 0.0.6) code0-license (~> 0.3.0) code0-zero_track (= 0.0.7) database_cleaner-active_record (~> 2.1) diff --git a/app/graphql/types/identity_provider_type.rb b/app/graphql/types/identity_provider_type.rb index 9e5a74a16..85c216d09 100644 --- a/app/graphql/types/identity_provider_type.rb +++ b/app/graphql/types/identity_provider_type.rb @@ -8,7 +8,7 @@ class IdentityProviderType < IdentityProviderBasicType description: 'Configuration details of the identity provider.' def config - object.config.merge(type: object.type) + object[:config].merge(type: object[:type]) end end end diff --git a/app/graphql/types/input/identity_provider_config_input_type.rb b/app/graphql/types/input/identity_provider_config_input_type.rb index 7401f8fb0..338f7219a 100644 --- a/app/graphql/types/input/identity_provider_config_input_type.rb +++ b/app/graphql/types/input/identity_provider_config_input_type.rb @@ -11,6 +11,8 @@ class IdentityProviderConfigInputType < Types::BaseInputObject argument :client_id, String, required: false, description: 'The client ID for the OIDC identity provider' argument :client_secret, String, required: false, description: 'The client secret for the OIDC identity provider' argument :redirect_uri, String, required: false, description: 'The redirect URI for the OIDC identity provider' + argument :token_url, String, required: false, + description: 'The token URL for the OIDC identity provider' argument :user_details_url, String, required: false, description: 'The user details URL for the OIDC identity provider' diff --git a/app/graphql/types/oidc_identity_provider_config_type.rb b/app/graphql/types/oidc_identity_provider_config_type.rb index 88f81655a..55bc0a687 100644 --- a/app/graphql/types/oidc_identity_provider_config_type.rb +++ b/app/graphql/types/oidc_identity_provider_config_type.rb @@ -27,15 +27,23 @@ class OidcIdentityProviderConfigType < Types::BaseObject description: 'The name of the OIDC identity provider' field :user_details_url, String, - null: false, + null: true, description: 'The user details URL for the OIDC identity provider' field :authorization_url, String, - null: false, + null: true, description: 'The authorization URL for the OIDC identity provider' + field :token_url, String, + null: true, + description: 'The token URL for the OIDC identity provider' + field :attribute_statements, GraphQL::Types::JSON, - null: false, + null: true, description: 'List of attribute statements for the OIDC identity provider' + + def provider_name + object[:provider_name] || object[:type].downcase + end end end diff --git a/app/graphql/types/saml_identity_provider_config_type.rb b/app/graphql/types/saml_identity_provider_config_type.rb index 4a1f665e0..086ef5dc9 100644 --- a/app/graphql/types/saml_identity_provider_config_type.rb +++ b/app/graphql/types/saml_identity_provider_config_type.rb @@ -13,19 +13,23 @@ class SamlIdentityProviderConfigType < Types::BaseObject description: 'The name of the SAML identity provider' field :attribute_statements, GraphQL::Types::JSON, - null: false, + null: true, description: 'List of attribute statements for the SAML identity provider' field :settings, GraphQL::Types::JSON, - null: false, + null: true, description: 'The SAML settings for the identity provider' field :response_settings, GraphQL::Types::JSON, - null: false, + null: true, description: 'The SAML response settings for the identity provider' field :metadata_url, String, null: true, description: 'The metadata url to fetch the metadatas (replacement for settings)' + + def provider_name + object[:provider_name] || object[:type].downcase + end end end diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index c71032535..990e44766 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -65,17 +65,14 @@ def validate_identity_providers next errors.add(:value, :id_type_or_config_missing) end - if provider[:type] == 'saml' - allowed_keys = %i[provider_name attribute_statements settings response_settings metadata_url] - errors.add(:value, :invalid_saml_configuration_keys) unless (provider[:config].keys - allowed_keys).empty? - else - required_keys = %i[client_id client_secret redirect_uri user_details_url authorization_url] - allowed_keys = %i[provider_name attribute_statements] + required_keys - - required_keys -= %i[user_details_url authorization_url] unless provider[:type] == 'oidc' - - errors.add(:value, :invalid_oidc_configuration_keys) unless (provider[:config].keys - allowed_keys).empty? - errors.add(:value, :missing_oidc_configuration_keys) unless (required_keys - provider[:config].keys).empty? + provider_instance = Code0::Identities::IdentityProvider.for_type(provider[:type]).new(provider[:config].compact) + + begin + provider_instance.validate_config! + rescue Code0::Identities::MissingConfigurationError + errors.add(:value, :missing_identity_provider_config) + rescue Code0::Identities::InvalidConfigurationError + errors.add(:value, :invalid_identity_provider_config) end end end diff --git a/app/services/users/identity/base_service.rb b/app/services/users/identity/base_service.rb index 49b7a37f4..329d2609a 100644 --- a/app/services/users/identity/base_service.rb +++ b/app/services/users/identity/base_service.rb @@ -7,7 +7,7 @@ def identity_provider identity_provider = Code0::Identities::IdentityProvider.new enabled_providers = ApplicationSetting.current[:identity_providers] enabled_providers.each do |provider| - provider.deep_symbolize_keys! + provider.to_h.deep_symbolize_keys! identity_provider.add_named_provider(provider[:id], provider[:type], -> { provider[:config] }) end identity_provider diff --git a/app/services/users/identity/link_service.rb b/app/services/users/identity/link_service.rb index 74fd2d130..2531db155 100644 --- a/app/services/users/identity/link_service.rb +++ b/app/services/users/identity/link_service.rb @@ -4,6 +4,7 @@ module Users module Identity class LinkService < BaseService include Sagittarius::Database::Transactional + include Code0::ZeroTrack::Loggable attr_reader :current_authentication, :current_user, :provider_id, :args @@ -19,10 +20,13 @@ def execute begin identity = identity_provider.load_identity(provider_id, args) rescue Code0::Identities::Error => e - return ServiceResponse.error(payload: e, message: 'An error occurred while loading external identity') + logger.warn(message: 'Failed to load external identity', provider_id: provider_id, error: e.message, + backtrace: e.backtrace) + return ServiceResponse.error(error_code: :loading_identity_failed, + message: 'An error occurred while loading external identity') end if identity.nil? - return ServiceResponse.error(payload: :invalid_external_identity, message: 'External identity is nil') + return ServiceResponse.error(error_code: :invalid_external_identity, message: 'External identity is nil') end transactional do |t| diff --git a/docs/graphql/input_object/identityproviderconfiginput.md b/docs/graphql/input_object/identityproviderconfiginput.md index e13bd0b05..719d9251d 100644 --- a/docs/graphql/input_object/identityproviderconfiginput.md +++ b/docs/graphql/input_object/identityproviderconfiginput.md @@ -17,4 +17,5 @@ Input for identity provider configuration. Contains fields for both OIDC and SAM | `redirectUri` | [`String`](../scalar/string.md) | The redirect URI for the OIDC identity provider | | `responseSettings` | [`JSON`](../scalar/json.md) | The SAML response settings for the identity provider | | `settings` | [`JSON`](../scalar/json.md) | The SAML settings for the identity provider | +| `tokenUrl` | [`String`](../scalar/string.md) | The token URL for the OIDC identity provider | | `userDetailsUrl` | [`String`](../scalar/string.md) | The user details URL for the OIDC identity provider | diff --git a/docs/graphql/object/oidcidentityproviderconfig.md b/docs/graphql/object/oidcidentityproviderconfig.md index 2a015b46c..7ab7aa987 100644 --- a/docs/graphql/object/oidcidentityproviderconfig.md +++ b/docs/graphql/object/oidcidentityproviderconfig.md @@ -4,16 +4,17 @@ title: OidcIdentityProviderConfig Represents an OIDC identity provider configuration -For more information see: +For more information see: ## Fields without arguments | Name | Type | Description | |------|------|-------------| -| `attributeStatements` | [`JSON!`](../scalar/json.md) | List of attribute statements for the OIDC identity provider | -| `authorizationUrl` | [`String!`](../scalar/string.md) | The authorization URL for the OIDC identity provider | +| `attributeStatements` | [`JSON`](../scalar/json.md) | List of attribute statements for the OIDC identity provider | +| `authorizationUrl` | [`String`](../scalar/string.md) | The authorization URL for the OIDC identity provider | | `clientId` | [`String!`](../scalar/string.md) | The client ID for the OIDC identity provider | | `clientSecret` | [`String!`](../scalar/string.md) | The client secret for the OIDC identity provider | | `providerName` | [`String!`](../scalar/string.md) | The name of the OIDC identity provider | | `redirectUri` | [`String!`](../scalar/string.md) | The redirect URI for the OIDC identity provider | -| `userDetailsUrl` | [`String!`](../scalar/string.md) | The user details URL for the OIDC identity provider | +| `tokenUrl` | [`String`](../scalar/string.md) | The token URL for the OIDC identity provider | +| `userDetailsUrl` | [`String`](../scalar/string.md) | The user details URL for the OIDC identity provider | diff --git a/docs/graphql/object/samlidentityproviderconfig.md b/docs/graphql/object/samlidentityproviderconfig.md index 84e8adb6b..e465c175c 100644 --- a/docs/graphql/object/samlidentityproviderconfig.md +++ b/docs/graphql/object/samlidentityproviderconfig.md @@ -4,14 +4,14 @@ title: SamlIdentityProviderConfig Represents the configuration for a SAML identity provider. -For more information see: +For more information see: ## Fields without arguments | Name | Type | Description | |------|------|-------------| -| `attributeStatements` | [`JSON!`](../scalar/json.md) | List of attribute statements for the SAML identity provider | +| `attributeStatements` | [`JSON`](../scalar/json.md) | List of attribute statements for the SAML identity provider | | `metadataUrl` | [`String`](../scalar/string.md) | The metadata url to fetch the metadatas (replacement for settings) | | `providerName` | [`String!`](../scalar/string.md) | The name of the SAML identity provider | -| `responseSettings` | [`JSON!`](../scalar/json.md) | The SAML response settings for the identity provider | -| `settings` | [`JSON!`](../scalar/json.md) | The SAML settings for the identity provider | +| `responseSettings` | [`JSON`](../scalar/json.md) | The SAML response settings for the identity provider | +| `settings` | [`JSON`](../scalar/json.md) | The SAML settings for the identity provider | diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 596839ec7..b110dd9ed 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -114,7 +114,7 @@ } ]) expect(setting).not_to be_valid - expect(setting.errors.added?(:value, :invalid_saml_configuration_keys)).to be true + expect(setting.errors.added?(:value, :invalid_identity_provider_config)).to be true end it 'is invalid with missing keys in oidc config' do @@ -126,7 +126,7 @@ } ]) expect(setting).not_to be_valid - expect(setting.errors.added?(:value, :missing_oidc_configuration_keys)).to be true + expect(setting.errors.added?(:value, :missing_identity_provider_config)).to be true end it 'expects valid OIDC configuration' do @@ -140,6 +140,7 @@ redirect_uri: 'https://example.com/callback', user_details_url: 'https://example.com/userinfo', authorization_url: 'https://example.com/auth', + token_url: 'https://example.com/token', }, } ]) @@ -175,7 +176,7 @@ } ]) expect(setting).not_to be_valid - expect(setting.errors.added?(:value, :missing_oidc_configuration_keys)).to be true + expect(setting.errors.added?(:value, :missing_identity_provider_config)).to be true end end