diff --git a/.rubocop.yml b/.rubocop.yml index c0ae3d84751..ba28636b6f6 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -175,8 +175,7 @@ RSpec/IndexedLet: # The http verbs in Rack::Test do not accept named parameters (params: params) Rails/HttpPositionalArguments: - Exclude: - - 'spec/requests/api/v3/**/*.rb' + Enabled: false # require_dependency is an obsolete method for Rails applications running in Zeitwerk mode. Rails/RequireDependency: diff --git a/app/controllers/scim_v2/base_controller_actions.rb b/app/controllers/scim_v2/base_controller_actions.rb index 9aca5278ab5..b1778c0ce07 100644 --- a/app/controllers/scim_v2/base_controller_actions.rb +++ b/app/controllers/scim_v2/base_controller_actions.rb @@ -55,13 +55,10 @@ module ScimV2 .limit(pagination_info.limit) .to_a - excluded_attributes = params.fetch(:excludedAttributes, "").split(",") - attributes = storage_class.scim_attributes_map.keys + storage_class.scim_attributes_map.values.find_all { |i| i.is_a? Hash }.flat_map(&:keys) - attributes = attributes.map(&:to_s) super(pagination_info, page_of_results) do |record| record.to_scim( location: url_for(action: :show, id: record.id), - include_attributes: attributes - excluded_attributes + include_attributes: ) end end @@ -69,15 +66,29 @@ module ScimV2 def show super do |record_id| record = storage_scope.find(record_id) - excluded_attributes = params.fetch(:excludedAttributes, "").split(",") - attributes = storage_class.scim_attributes_map.keys + storage_class.scim_attributes_map.values.find_all { |i| i.is_a? Hash }.flat_map(&:keys) - attributes = attributes.map(&:to_s) record.to_scim( location: url_for(action: :show, id: record_id), - include_attributes: attributes - excluded_attributes + include_attributes: ) end end + + private + + def include_attributes + first_level_attrs = storage_class.scim_attributes_map.keys.map(&:to_s) + second_level_attrs = + storage_class + .scim_attributes_map + .find_all { |_, v| v.is_a? Hash } + .flat_map { |parent, childs| childs.map { |child, _| "#{parent}.#{child}" } } + all_possible_attributes = (first_level_attrs + second_level_attrs) + + excluded_attributes = params.fetch(:excludedAttributes, "").split(",") + excluded_parents = excluded_attributes.filter_map { |attr| attr.split(".")[-2] } + + all_possible_attributes - excluded_attributes - excluded_parents + end end end end diff --git a/app/controllers/scim_v2/groups_controller.rb b/app/controllers/scim_v2/groups_controller.rb index dd5c115a3d2..78389fddd36 100644 --- a/app/controllers/scim_v2/groups_controller.rb +++ b/app/controllers/scim_v2/groups_controller.rb @@ -62,7 +62,10 @@ module ScimV2 .on_failure { |call| raise call.message } end - group.to_scim(location: url_for(action: :show, id: group.id)) + group.to_scim( + location: url_for(action: :show, id: group.id), + include_attributes: + ) end end end @@ -77,7 +80,10 @@ module ScimV2 .call(user_ids: scim_resource.members.map(&:value)) .on_failure { |call| raise call.message } group.reload - group.to_scim(location: url_for(action: :show, id: group.id)) + group.to_scim( + location: url_for(action: :show, id: group.id), + include_attributes: + ) end end end @@ -93,7 +99,10 @@ module ScimV2 .call(user_ids:) .on_failure { |call| raise call.message } group.reload - group.to_scim(location: url_for(action: :show, id: group.id)) + group.to_scim( + location: url_for(action: :show, id: group.id), + include_attributes: + ) end end end diff --git a/app/controllers/scim_v2/users_controller.rb b/app/controllers/scim_v2/users_controller.rb index a77792a5fdd..288b37a2491 100644 --- a/app/controllers/scim_v2/users_controller.rb +++ b/app/controllers/scim_v2/users_controller.rb @@ -56,7 +56,10 @@ module ScimV2 end user = call.result - user.to_scim(location: url_for(action: :show, id: user.id)) + user.to_scim( + location: url_for(action: :show, id: user.id), + include_attributes: + ) end end end @@ -70,7 +73,10 @@ module ScimV2 .new(user: User.current, model: user) .call .on_failure { |call| raise call.message } - user.to_scim(location: url_for(action: :show, id: user.id)) + user.to_scim( + location: url_for(action: :show, id: user.id), + include_attributes: + ) end end end @@ -84,7 +90,10 @@ module ScimV2 .new(user: User.current, model: user) .call .on_failure { |call| raise call.message } - user.to_scim(location: url_for(action: :show, id: user.id)) + user.to_scim( + location: url_for(action: :show, id: user.id), + include_attributes: + ) end end end @@ -95,7 +104,19 @@ module ScimV2 Users::DeleteService .new(user: User.current, model: user) .call - .on_failure { |call| raise call.message } + .on_failure do |result| + unauthorized_error = result.errors.find { |e| e.type == :error_unauthorized } + if unauthorized_error.present? + raise Scimitar::ErrorResponse.new( + status: 403, + detail: "User can't be deleted due to permission absence." + ) + else + raise result.message + end + + raise call.message + end end end diff --git a/app/models/group.rb b/app/models/group.rb index c19f999f4c4..a55e37f8ee5 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -69,20 +69,6 @@ class Group < Principal lastname end - def scim_external_id - active_user_auth_provider_link&.external_id - end - - def scim_external_id=(external_id) - oidc_provider = User.current.service_account_association.service.auth_provider - - ::Groups::SetAttributesService - .new(user: User.system, model: self, contract_class: EmptyContract) - .call(identity_url: "#{oidc_provider.slug}:#{external_id}") - .on_failure { |result| raise result.to_s } - external_id - end - def scim_members @scim_members ||= users end @@ -94,20 +80,6 @@ class Group < Principal @scim_members = array end - def scim_active=(is_active) - if is_active - activate - true - else - lock if active? - false - end - end - - def scim_active - active? - end - def self.scim_resource_type Scimitar::Resources::Group end @@ -141,11 +113,6 @@ class Group < Principal } end - def self.scim_mutable_attributes - # Allow mutation of everything with a write accessor - nil - end - def self.scim_queryable_attributes { displayName: { column: :lastname }, @@ -153,13 +120,6 @@ class Group < Principal } end - def self.scim_timestamps_map - { - created: :created_at, - lastModified: :updated_at - } - end - include Scimitar::Resources::Mixin private diff --git a/app/models/principal.rb b/app/models/principal.rb index 7b7537f0e6e..3db8f0429b8 100644 --- a/app/models/principal.rb +++ b/app/models/principal.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -181,7 +183,7 @@ class Principal < ApplicationRecord ## # Allows the API and other sources to determine locking actions # on represented collections of children of Principals. - # Must be overridden by User + # Must be overridden by descendants def lockable? false end @@ -208,6 +210,47 @@ class Principal < ApplicationRecord end end + def scim_active=(is_active) + if is_active + activate + true + else + lock if active? + false + end + end + + def scim_active + active? + end + + def scim_external_id + active_user_auth_provider_link&.external_id + end + + def scim_external_id=(external_id) + oidc_provider = User.current.service_account_association.service.auth_provider + + "::#{self.class}s::SetAttributesService" + .constantize + .new(user: User.current, model: self, contract_class: EmptyContract) + .call(identity_url: "#{oidc_provider.slug}:#{external_id}") + .on_failure { |result| raise result.to_s } + external_id + end + + def self.scim_mutable_attributes + # Allow mutation of everything with a write accessor + nil + end + + def self.scim_timestamps_map + { + created: :created_at, + lastModified: :updated_at + } + end + class << self # Hack to exclude the Users::InexistentUser # from showing up on filters for type. diff --git a/app/models/user.rb b/app/models/user.rb index d90919b7a28..1bfcd5b80fd 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -558,34 +558,6 @@ class User < Principal SystemUser.first end - def scim_external_id - active_user_auth_provider_link&.external_id - end - - def scim_external_id=(external_id) - oidc_provider = User.current.service_account_association.service.auth_provider - - ::Users::SetAttributesService - .new(user: User.system, model: self, contract_class: EmptyContract) - .call(identity_url: "#{oidc_provider.slug}:#{external_id}") - .on_failure { |result| raise result.to_s } - external_id - end - - def scim_active=(is_active) - if is_active - activate - true - else - lock if active? - false - end - end - - def scim_active - active? - end - def scim_emails [ScimEmail.new(mail, true, "work")] end @@ -637,11 +609,6 @@ class User < Principal } end - def self.scim_mutable_attributes - # Allow mutation of everything with a write accessor - nil - end - def self.scim_queryable_attributes { externalId: { column: UserAuthProviderLink.arel_table[:external_id] }, @@ -654,13 +621,6 @@ class User < Principal } end - def self.scim_timestamps_map - { - created: :created_at, - lastModified: :updated_at - } - end - include Scimitar::Resources::Mixin protected diff --git a/app/services/groups/set_attributes_service.rb b/app/services/groups/set_attributes_service.rb index 9393d4fa896..7dbf79e902f 100644 --- a/app/services/groups/set_attributes_service.rb +++ b/app/services/groups/set_attributes_service.rb @@ -30,6 +30,8 @@ module Groups class SetAttributesService < ::BaseServices::SetAttributes + include ::UserAuthProviderLinksSetter + private def set_attributes(params) @@ -51,27 +53,6 @@ module Groups mark_outdated_users existing_user_ids - user_ids end - def set_user_auth_provider_links(identity_url) - if identity_url.present? - slug, external_id = identity_url.split(":", 2) - if slug.present? && external_id.present? - auth_provider_id = AuthProvider.where(slug:).pick(:id) - if auth_provider_id.present? - link = model.user_auth_provider_links - .find_or_initialize_by(auth_provider_id:) - link.assign_attributes(external_id:, principal: model) - if link.changed? && link.persisted? - link.save! - model.user_auth_provider_links.reload - model.user_auth_provider_links.find { |l| l.id == link.id }.external_id_will_change! - end - else - raise ActiveRecord::RecordNotFound, "AuthProvider with slug: \"#{slug}\" has been not found" - end - end - end - end - def build_new_users(new_user_ids) new_user_ids.each do |id| model.group_users.build(user_id: id) diff --git a/app/services/user_auth_provider_links_setter.rb b/app/services/user_auth_provider_links_setter.rb new file mode 100644 index 00000000000..7fdd3280b47 --- /dev/null +++ b/app/services/user_auth_provider_links_setter.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# 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 COPYRIGHT and LICENSE files for more details. +#++ + +module UserAuthProviderLinksSetter + private + + def set_user_auth_provider_links(identity_url) + if identity_url.present? + slug, external_id = identity_url.split(":", 2) + if slug.present? && external_id.present? + auth_provider_id = AuthProvider.where(slug:).pick(:id) + if auth_provider_id.present? + link = model.user_auth_provider_links + .find_or_initialize_by(auth_provider_id:) + link.assign_attributes(external_id:, principal: model) + if link.changed? && link.persisted? + link.save! + model.user_auth_provider_links.reload + model.user_auth_provider_links.find { |l| l.id == link.id }.external_id_will_change! + end + else + raise ActiveRecord::RecordNotFound, "AuthProvider with slug: \"#{slug}\" has been not found" + end + end + end + end +end diff --git a/app/services/users/set_attributes_service.rb b/app/services/users/set_attributes_service.rb index 79409a0573a..f6dec00dd82 100644 --- a/app/services/users/set_attributes_service.rb +++ b/app/services/users/set_attributes_service.rb @@ -31,6 +31,7 @@ module Users class SetAttributesService < ::BaseServices::SetAttributes include ::HookHelper + include ::UserAuthProviderLinksSetter private @@ -63,27 +64,6 @@ module Users .call(pref) end - def set_user_auth_provider_links(identity_url) - if identity_url.present? - slug, external_id = identity_url.split(":", 2) - if slug.present? && external_id.present? - auth_provider_id = AuthProvider.where(slug:).pick(:id) - if auth_provider_id.present? - link = model.user_auth_provider_links - .find_or_initialize_by(auth_provider_id:) - link.assign_attributes(external_id:, principal: model) - if link.changed? && link.persisted? - link.save! - model.user_auth_provider_links.reload - model.user_auth_provider_links.find { |l| l.id == link.id }.external_id_will_change! - end - else - raise ActiveRecord::RecordNotFound, "AuthProvider with slug: \"#{slug}\" has been not found" - end - end - end - end - # rubocop:disable Metrics/AbcSize def assign_name_attributes_from_mail(params) placeholder = placeholder_name(params[:mail]) diff --git a/config/initializers/scimitar.rb b/config/initializers/scimitar.rb index cf6bfd5713f..a50550655fb 100644 --- a/config/initializers/scimitar.rb +++ b/config/initializers/scimitar.rb @@ -30,7 +30,8 @@ Rails.application.config.to_prepare do Scimitar.service_provider_configuration = Scimitar::ServiceProviderConfiguration.new( - patch: Scimitar::Supportable.supported + patch: Scimitar::Supportable.supported, + authenticationSchemes: [Scimitar::AuthenticationScheme.bearer] ) Scimitar.engine_configuration = Scimitar::EngineConfiguration.new( application_controller_mixin: ScimV2::ScimControllerMixins @@ -38,7 +39,10 @@ Rails.application.config.to_prepare do module ScimitarSchemaExtension def scim_attributes - super + [Scimitar::Schema::Attribute.new(name: "externalId", type: "string")] + super + [Scimitar::Schema::Attribute.new(name: "externalId", + type: "string", + caseExact: true, + required: true)] end end diff --git a/spec/requests/scim_v2/authentication_spec.rb b/spec/requests/scim_v2/authentication_spec.rb new file mode 100644 index 00000000000..d765b3b3a85 --- /dev/null +++ b/spec/requests/scim_v2/authentication_spec.rb @@ -0,0 +1,166 @@ +# frozen_string_literal: true + +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) the OpenProject GmbH +# +# 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 COPYRIGHT and LICENSE files for more details. +#++ + +require "spec_helper" + +RSpec.describe "SCIM API Authentication" do + let(:oidc_provider_slug) { "keycloak" } + let(:oidc_provider) { create(:oidc_provider, slug: oidc_provider_slug) } + let(:headers) { { "CONTENT_TYPE" => "application/scim+json", "HTTP_AUTHORIZATION" => "Bearer #{token}" } } + let(:service_account) { create(:service_account, service: scim_client) } + let(:scim_client) { create(:scim_client, authentication_method: :oauth2_token, auth_provider_id: oidc_provider.id) } + + describe "GET /scim_v2/ServiceProviderConfig" do + context "with the feature flag enabled", with_flag: { scim_api: true } do + context "with static token" do + let(:oauth_access_token) { create(:oauth_access_token, resource_owner: service_account, scopes: ["scim_v2"]) } + let!(:token) { oauth_access_token.plaintext_token } + + it do + get "/scim_v2/ServiceProviderConfig", {}, headers + expect(last_response).to have_http_status(200) + end + end + + context "with JWT token", :webmock do + let(:jwk) { JWT::JWK.new(OpenSSL::PKey::RSA.new(2048), kid: "my-kid", use: "sig", alg: "RS256") } + let(:payload) do + { + "exp" => token_exp.to_i, + "iat" => 1721283370, + "jti" => "c526b435-991f-474a-ad1b-c371456d1fd0", + "iss" => token_issuer, + "aud" => token_aud, + "sub" => token_sub, + "typ" => "Bearer", + "azp" => "https://openproject.local", + "session_state" => "eb235240-0b47-48fa-8b3e-f3b310d352e3", + "acr" => "1", + "allowed-origins" => ["https://openproject.local"], + "realm_access" => { "roles" => ["create-realm", "default-roles-master", "offline_access", "admin", + "uma_authorization"] }, + "resource_access" => + { "master-realm" => + { "roles" => + ["view-realm", + "view-identity-providers", + "manage-identity-providers", + "impersonation", + "create-client", + "manage-users", + "query-realms", + "view-authorization", + "query-clients", + "query-users", + "manage-events", + "manage-realm", + "view-events", + "view-users", + "view-clients", + "manage-authorization", + "manage-clients", + "query-groups"] }, + "account" => { "roles" => ["manage-account", "manage-account-links", "view-profile"] } }, + "scope" => token_scope, + "sid" => "eb235240-0b47-48fa-8b3e-f3b310d352e3", + "email_verified" => false, + "preferred_username" => "admin" + } + end + let(:token) { JWT.encode(payload, jwk.signing_key, jwk[:alg], { kid: jwk[:kid] }) } + let(:token_exp) { 5.minutes.from_now } + let(:token_sub) { "b70e2fbf-ea68-420c-a7a5-0a287cb689c6" } + let(:token_aud) { ["https://openproject.local", "master-realm", "account"] } + let(:token_issuer) { "https://keycloak.local/realms/master" } + let(:token_scope) { "scim_v2" } + let(:expected_message) { "You did not provide the correct credentials." } + let(:keys_request_stub) do + stub_request(:get, "https://keycloak.local/realms/master/protocol/openid-connect/certs") + .to_return(status: 200, body: JWT::JWK::Set.new(jwk_response).export.to_json, headers: {}) + end + let(:jwk_response) { jwk } + + before do + service_account.user_auth_provider_links.create!( + external_id: token_sub, + auth_provider: oidc_provider + ) + keys_request_stub + + header "Authorization", "Bearer #{token}" + end + + it "" do + get "/scim_v2/ServiceProviderConfig", {}, headers + + expect(last_response).to have_http_status(200) + end + + context "when scim_v2 scope is missing in token" do + let(:token_scope) { "api_v3" } + + it "" do + get "/scim_v2/ServiceProviderConfig", {}, headers + expect(last_response).to have_http_status(401) + expect(last_response.body).to eq("insufficient_scope") + expect(last_response.headers["WWW-Authenticate"]).to eq("Bearer realm=\"OpenProject API\", error=\"insufficient_scope\", error_description=\"Requires scope scim_v2 to access this resource.\"") + end + end + + context "when token_sub does not match a service_account" do + before { service_account.user_auth_provider_links.delete_all } + + it "" do + get "/scim_v2/ServiceProviderConfig", {}, headers + + expect(last_response).to have_http_status(401) + expect(last_response.body).to eq("invalid_token") + expect(last_response.headers["WWW-Authenticate"]).to eq("Bearer realm=\"OpenProject API\", error=\"invalid_token\", error_description=\"The user identified by the token is not known\"") + end + end + end + end + + context "with the feature flag disabled", with_flag: { scim_api: false } do + let(:oauth_access_token) { create(:oauth_access_token, resource_owner: service_account, scopes: ["scim_v2"]) } + let!(:token) { oauth_access_token.plaintext_token } + + it do + get "/scim_v2/ServiceProviderConfig", {}, headers + + response_body = JSON.parse(last_response.body) + expect(response_body).to eq( + { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], + "status" => "401" } + ) + end + end + end +end diff --git a/spec/requests/scim_v2/groups_spec.rb b/spec/requests/scim_v2/groups_spec.rb index 45b2a182d6e..f34015b096f 100644 --- a/spec/requests/scim_v2/groups_spec.rb +++ b/spec/requests/scim_v2/groups_spec.rb @@ -57,32 +57,30 @@ RSpec.describe "SCIM API Groups" do context "with the feature flag enabled", with_flag: { scim_api: true } do before { group } - it do + it "responds with group list" do get "/scim_v2/Groups", {}, headers response_body = JSON.parse(last_response.body) - expect(response_body).to match({ "Resources" => match_array([{ "displayName" => group.name, - "externalId" => external_group_id, - "id" => group.id.to_s, - "members" => [{ "value" => user.id.to_s }], - "meta" => { "location" => "http://test.host/scim_v2/Groups/#{group.id}", - "created" => group.created_at.iso8601, - "lastModified" => group.updated_at.iso8601, - "resourceType" => "Group" }, - "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] }, - { "displayName" => group_without_external_id.name, - "id" => group_without_external_id.id.to_s, - "members" => [{ "value" => user.id.to_s }], - "meta" => { "location" => "http://test.host/scim_v2/Groups/#{group_without_external_id.id}", - "created" => group_without_external_id.created_at.iso8601, - "lastModified" => group_without_external_id.updated_at.iso8601, - "resourceType" => "Group" }, - "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] } - ]), - "itemsPerPage" => 100, - "schemas" => ["urn:ietf:params:scim:api:messages:2.0:ListResponse"], - "startIndex" => 1, - "totalResults" => 2 }) + expect(response_body).to match({ "Resources" => contain_exactly({ "displayName" => group.name, + "externalId" => external_group_id, + "id" => group.id.to_s, + "members" => [{ "value" => user.id.to_s }], + "meta" => { "location" => "http://test.host/scim_v2/Groups/#{group.id}", + "created" => group.created_at.iso8601, + "lastModified" => group.updated_at.iso8601, + "resourceType" => "Group" }, + "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] }, { "displayName" => group_without_external_id.name, + "id" => group_without_external_id.id.to_s, + "members" => [{ "value" => user.id.to_s }], + "meta" => { "location" => "http://test.host/scim_v2/Groups/#{group_without_external_id.id}", + "created" => group_without_external_id.created_at.iso8601, + "lastModified" => group_without_external_id.updated_at.iso8601, + "resourceType" => "Group" }, + "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] }), + "itemsPerPage" => 100, + "schemas" => ["urn:ietf:params:scim:api:messages:2.0:ListResponse"], + "startIndex" => 1, + "totalResults" => 2 }) end it "filters results" do @@ -125,13 +123,14 @@ RSpec.describe "SCIM API Groups" do { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "status" => "401" } ) + expect(last_response).to have_http_status(401) end end end describe "GET /scim_v2/Groups/:id" do context "with the feature flag enabled", with_flag: { scim_api: true } do - it do + it "responds with specific group data" do group get "/scim_v2/Groups/#{group.id}", {}, headers @@ -172,6 +171,7 @@ RSpec.describe "SCIM API Groups" do { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "status" => "401" } ) + expect(last_response).to have_http_status(401) end end end @@ -180,15 +180,15 @@ RSpec.describe "SCIM API Groups" do context "with the feature flag enabled", with_flag: { scim_api: true } do let(:group_name) { "Group 123" } - it do + it "creates a group with members" do user request_body = { "displayName" => group_name, "externalId" => external_group_id, "members" => [{ "value" => user.id.to_s }], "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] } - expect { + expect do post "/scim_v2/Groups/", request_body.to_json, headers - }.to change(Group, :count).by(1) + end.to change(Group, :count).by(1) response_body = JSON.parse(last_response.body) group = Group.find_by(name: group_name) @@ -201,17 +201,16 @@ RSpec.describe "SCIM API Groups" do "lastModified" => group.updated_at.iso8601, "resourceType" => "Group" }, "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] }) - end - it "memberless request" do + it "creates group without members specified" do user request_body = { "displayName" => "Group 123", "externalId" => external_group_id, "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] } - expect { + expect do post "/scim_v2/Groups/", request_body.to_json, headers - }.to change(Group, :count).by(1) + end.to change(Group, :count).by(1) response_body = JSON.parse(last_response.body) group = Group.find_by(name: group_name) @@ -224,7 +223,6 @@ RSpec.describe "SCIM API Groups" do "lastModified" => group.updated_at.iso8601, "resourceType" => "Group" }, "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] }) - end end @@ -237,13 +235,14 @@ RSpec.describe "SCIM API Groups" do { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "status" => "401" } ) + expect(last_response).to have_http_status(401) end end end describe "DELETE /scim_v2/Groups/:id" do context "with the feature flag enabled", with_flag: { scim_api: true } do - it do + it "deletes specific group" do group delete "/scim_v2/Groups/#{group.id}", "", headers @@ -286,13 +285,14 @@ RSpec.describe "SCIM API Groups" do { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "status" => "401" } ) + expect(last_response).to have_http_status(401) end end end - describe "PUT /scim_v2/Users/:id" do + describe "PUT /scim_v2/Groups/:id" do context "with the feature flag enabled", with_flag: { scim_api: true } do - it do + it "updates specific group by replacing it with newly provided data" do admin group new_external_group_id = "new_idp_group_id_123asdqwe12345" @@ -319,10 +319,7 @@ RSpec.describe "SCIM API Groups" do "created" => group.created_at.iso8601, "lastModified" => group.updated_at.iso8601, "resourceType" => "Group" }, - "members" => match_array([ - { "value" => user.id.to_s }, - { "value" => admin.id.to_s } - ]), + "members" => contain_exactly({ "value" => user.id.to_s }, { "value" => admin.id.to_s }), "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] }) end end @@ -336,13 +333,14 @@ RSpec.describe "SCIM API Groups" do { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "status" => "401" } ) + expect(last_response).to have_http_status(401) end end end describe "PATCH /scim_v2/Groups/:id" do context "with the feature flag enabled", with_flag: { scim_api: true } do - it "changes external_id" do + it "supports external_id replacing" do group new_external_group_id = "new_idp_user_id_123asdqwe12345" request_body = { @@ -369,7 +367,7 @@ RSpec.describe "SCIM API Groups" do "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] }) end - it "replaces members" do + it "supports replacing of members" do group user2 @@ -377,12 +375,12 @@ RSpec.describe "SCIM API Groups" do "schemas" => ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], "Operations" => [{ - "op"=> "replace", - "path"=> "members", - "value"=> [{ - "value"=> user2.id.to_s - }] - }] + "op" => "replace", + "path" => "members", + "value" => [{ + "value" => user2.id.to_s + }] + }] } patch "/scim_v2/Groups/#{group.id}", request_body.to_json, headers @@ -399,7 +397,7 @@ RSpec.describe "SCIM API Groups" do "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] }) end - it "adds a member" do + it "supports adding of a member" do group user2 @@ -407,10 +405,10 @@ RSpec.describe "SCIM API Groups" do "schemas" => ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], "Operations" => [{ - "op"=> "add", - "path"=> "members", - "value"=> [{"value"=> user2.id.to_s},] - }] + "op" => "add", + "path" => "members", + "value" => [{ "value" => user2.id.to_s }] + }] } patch "/scim_v2/Groups/#{group.id}", request_body.to_json, headers @@ -428,17 +426,17 @@ RSpec.describe "SCIM API Groups" do "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] }) end - it "removes a member" do + it "supports removal of a member" do group request_body = { "schemas" => ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], "Operations" => [{ - "op"=> "remove", - "path"=> "members", - "value"=> [{"value"=> user1.id.to_s},] - }] + "op" => "remove", + "path" => "members", + "value" => [{ "value" => user1.id.to_s }] + }] } patch "/scim_v2/Groups/#{group.id}", request_body.to_json, headers @@ -454,6 +452,32 @@ RSpec.describe "SCIM API Groups" do "resourceType" => "Group" }, "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] }) end + + it "supports removal of a member with exclusion of members list from the response" do + group + + request_body = { + "schemas" => + ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + "Operations" => [{ + "op" => "remove", + "path" => "members", + "value" => [{ "value" => user1.id.to_s }] + }] + } + patch "/scim_v2/Groups/#{group.id}?excludedAttributes=members", request_body.to_json, headers + + response_body = JSON.parse(last_response.body) + group.reload + expect(response_body).to eq({ "displayName" => group.name, + "externalId" => external_group_id, + "id" => group.id.to_s, + "meta" => { "location" => "http://test.host/scim_v2/Groups/#{group.id}", + "created" => group.created_at.iso8601, + "lastModified" => group.updated_at.iso8601, + "resourceType" => "Group" }, + "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:Group"] }) + end end context "with the feature flag disabled", with_flag: { scim_api: false } do diff --git a/spec/requests/scim_v2/schemas_spec.rb b/spec/requests/scim_v2/schemas_spec.rb index 2fdc9e40806..fd9bbe27afd 100644 --- a/spec/requests/scim_v2/schemas_spec.rb +++ b/spec/requests/scim_v2/schemas_spec.rb @@ -30,7 +30,7 @@ require "spec_helper" -RSpec.describe "SCIM API ServiceProviderConfig" do +RSpec.describe "SCIM API Schemas" do let(:oidc_provider_slug) { "keycloak" } let(:oidc_provider) { create(:oidc_provider, slug: oidc_provider_slug) } let(:headers) { { "CONTENT_TYPE" => "application/scim+json", "HTTP_AUTHORIZATION" => "Bearer #{token.plaintext_token}" } } @@ -51,6 +51,16 @@ RSpec.describe "SCIM API ServiceProviderConfig" do expect(response_body["schemas"]).to eq(["urn:ietf:params:scim:api:messages:2.0:ListResponse"]) group_schema = response_body["Resources"].find { |r| r["name"] == "Group" } user_schema = response_body["Resources"].find { |r| r["name"] == "User" } + external_id_schema = { "multiValued" => false, + "required" => true, + "caseExact" => true, + "mutability" => "readWrite", + "uniqueness" => "none", + "returned" => "default", + "name" => "externalId", + "type" => "string" } + expect(group_schema["attributes"]).to include(external_id_schema) + expect(user_schema["attributes"]).to include(external_id_schema) end end diff --git a/spec/requests/scim_v2/service_provider_config_spec.rb b/spec/requests/scim_v2/service_provider_config_spec.rb index 45e0834d078..3f61ea29cbe 100644 --- a/spec/requests/scim_v2/service_provider_config_spec.rb +++ b/spec/requests/scim_v2/service_provider_config_spec.rb @@ -42,14 +42,11 @@ RSpec.describe "SCIM API ServiceProviderConfig" do describe "GET /scim_v2/ServiceProviderConfig" do context "with the feature flag enabled", with_flag: { scim_api: true } do - it do + it "responds with ServiceProviderConfig information" do get "/scim_v2/ServiceProviderConfig", {}, headers response_body = JSON.parse(last_response.body) - expect(response_body).to include({ "authenticationSchemes" => [{ "description" => "Authentication scheme using the HTTP Basic Standard", - "name" => "HTTP Basic", - "type" => "httpbasic" }, - { "description" => "Authentication scheme using the OAuth Bearer Token Standard", + expect(response_body).to include({ "authenticationSchemes" => [{ "description" => "Authentication scheme using the OAuth Bearer Token Standard", "name" => "OAuth Bearer Token", "type" => "oauthbearertoken" }], "bulk" => { "supported" => false }, @@ -72,6 +69,7 @@ RSpec.describe "SCIM API ServiceProviderConfig" do { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "status" => "401" } ) + expect(last_response).to have_http_status(401) end end end diff --git a/spec/requests/scim_v2/users_spec.rb b/spec/requests/scim_v2/users_spec.rb index c304b3f8e43..87fac55e617 100644 --- a/spec/requests/scim_v2/users_spec.rb +++ b/spec/requests/scim_v2/users_spec.rb @@ -53,7 +53,7 @@ RSpec.describe "SCIM API Users" do group end - it do + it "responds with users list" do get "/scim_v2/Users", {}, headers response_body = JSON.parse(last_response.body) @@ -175,13 +175,14 @@ RSpec.describe "SCIM API Users" do { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "status" => "401" } ) + expect(last_response).to have_http_status(401) end end end describe "GET /scim_v2/Users/:id" do context "with the feature flag enabled", with_flag: { scim_api: true } do - it do + it "returns specific user data" do group get "/scim_v2/Users/#{user.id}", {}, headers @@ -213,6 +214,7 @@ RSpec.describe "SCIM API Users" do { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "status" => "401" } ) + expect(last_response).to have_http_status(401) end end end @@ -222,7 +224,7 @@ RSpec.describe "SCIM API Users" do context "with the feature flag enabled", with_flag: { scim_api: true } do context "when user with userName has already exists" do - it do + it "responds with uniqueness error" do group request_body = { "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:User"], @@ -242,8 +244,7 @@ RSpec.describe "SCIM API Users" do post "/scim_v2/Users/", request_body.to_json, headers - - expect(last_response.status).to eq(409) + expect(last_response).to have_http_status(409) response_body = JSON.parse(last_response.body) expect(response_body).to eq({ "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "detail" => "Operation failed due to a uniqueness constraint: Username has already been taken.", @@ -252,7 +253,43 @@ RSpec.describe "SCIM API Users" do end end - it do + it "creates user with provided data and excludes some attributes" do + request_body = { + "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:User"], + "externalId" => external_user_id, + "userName" => "jdoe", + "name" => { + "givenName" => "John", + "familyName" => "Doe" + }, + "active" => true, + "emails" => [ + { + "value" => "jdoe@example.com", + "type" => "work", + "primary" => true + } + ] + } + post "/scim_v2/Users/?excludedAttributes=emails,name.givenName", request_body.to_json, headers + + response_body = JSON.parse(last_response.body) + created_user = User.find_by(login: "jdoe") + expect(created_user).to be_present + expect(response_body).to eq({ "active" => true, + "externalId" => external_user_id, + "groups" => [], + "id" => created_user.id.to_s, + "meta" => { "created" => created_user.created_at.iso8601, + "lastModified" => created_user.updated_at.iso8601, + "location" => "http://test.host/scim_v2/Users/#{created_user.id}", + "resourceType" => "User" }, + "name" => { "familyName" => "Doe" }, + "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:User"], + "userName" => "jdoe" }) + end + + it "creates user with provided data" do request_body = { "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:User"], "externalId" => external_user_id, @@ -292,7 +329,7 @@ RSpec.describe "SCIM API Users" do "userName" => "jdoe" }) end - it "microsoft style" do + it "creates user with any email type string provided" do request_body = { "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:User"], "externalId" => external_user_id, @@ -340,50 +377,66 @@ RSpec.describe "SCIM API Users" do { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "status" => "401" } ) + expect(last_response).to have_http_status(401) end end end - describe "DELETE /scim_v2/Users/:id", with_settings: { users_deletable_by_admins: true } do + describe "DELETE /scim_v2/Users/:id" do context "with the feature flag enabled", with_flag: { scim_api: true } do - it do - group + context "when users_deletable_by_admins is enabled", with_settings: { users_deletable_by_admins: true } do + it do + group - delete "/scim_v2/Users/#{user.id}", "", headers + delete "/scim_v2/Users/#{user.id}", "", headers - expect(last_response.body).to eq("") - expect(last_response).to have_http_status(204) + expect(last_response.body).to eq("") + expect(last_response).to have_http_status(204) - get "/scim_v2/Users/#{user.id}", "", headers + get "/scim_v2/Users/#{user.id}", "", headers - response_body = JSON.parse(last_response.body) - expect(response_body).to eq({ "active" => false, - "emails" => [{ "primary" => true, - "type" => "work", - "value" => user.mail }], - "externalId" => external_user_id, - "groups" => [{ "value" => group.id.to_s }], - "id" => user.id.to_s, - "meta" => { "created" => user.created_at.iso8601, - "lastModified" => user.updated_at.iso8601, - "location" => "http://test.host/scim_v2/Users/#{user.id}", - "resourceType" => "User" }, - "name" => { "familyName" => user.lastname, - "givenName" => user.firstname }, - "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:User"], - "userName" => user.login }) + response_body = JSON.parse(last_response.body) + expect(response_body).to eq({ "active" => false, + "emails" => [{ "primary" => true, + "type" => "work", + "value" => user.mail }], + "externalId" => external_user_id, + "groups" => [{ "value" => group.id.to_s }], + "id" => user.id.to_s, + "meta" => { "created" => user.created_at.iso8601, + "lastModified" => user.updated_at.iso8601, + "location" => "http://test.host/scim_v2/Users/#{user.id}", + "resourceType" => "User" }, + "name" => { "familyName" => user.lastname, + "givenName" => user.firstname }, + "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:User"], + "userName" => user.login }) - perform_enqueued_jobs - assert_performed_jobs 1 + perform_enqueued_jobs + assert_performed_jobs 1 - get "/scim_v2/Users/#{user.id}", "", headers + get "/scim_v2/Users/#{user.id}", "", headers - response_body = JSON.parse(last_response.body) - expect(response_body).to eq( - { "detail" => "Resource \"#{user.id}\" not found", - "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], - "status" => "404" } - ) + response_body = JSON.parse(last_response.body) + expect(response_body).to eq( + { "detail" => "Resource \"#{user.id}\" not found", + "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], + "status" => "404" } + ) + end + end + + context "when users_deletable_by_admins is disabled", with_settings: { users_deletable_by_admins: false } do + it "responds with 403 error" do + group + delete "/scim_v2/Users/#{user.id}", "", headers + + response_body = JSON.parse(last_response.body) + expect(response_body).to eq({ "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], + "detail" => "User can't be deleted due to permission absence.", + "status" => "403" }) + expect(last_response).to have_http_status(403) + end end end @@ -396,6 +449,7 @@ RSpec.describe "SCIM API Users" do { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "status" => "401" } ) + expect(last_response).to have_http_status(401) end end end @@ -406,7 +460,7 @@ RSpec.describe "SCIM API Users" do context "with the feature flag enabled", with_flag: { scim_api: true } do let(:new_external_user_id) { "new_idp_user_id_123asdqwe12345" } - it do + it "updates existing user by replacing with newly provided data" do request_body = { "schemas" => ["urn:ietf:params:scim:schemas:core:2.0:User"], "externalId" => new_external_user_id, @@ -449,7 +503,6 @@ RSpec.describe "SCIM API Users" do context "with the feature flag disabled", with_flag: { scim_api: false } do it do - headers = { "CONTENT_TYPE" => "application/scim+json", "HTTP_AUTHORIZATION" => "Bearer access_token" } put "/scim_v2/Users/123", "", headers response_body = JSON.parse(last_response.body) @@ -457,6 +510,7 @@ RSpec.describe "SCIM API Users" do { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "status" => "401" } ) + expect(last_response).to have_http_status(401) end end end @@ -545,6 +599,7 @@ RSpec.describe "SCIM API Users" do { "detail" => "Requires authentication", "schemas" => ["urn:ietf:params:scim:api:messages:2.0:Error"], "status" => "401" } ) + expect(last_response).to have_http_status(401) end end end