clear forbidden chars in names comming from oauth

lib/decidim/civicrm.rb

@@ -119,6 +119,7 @@ def self.allow_unauthorized_path?(path)
     end
 
     def self.unauthorized_url
+      return Civicrm.unauthorized_redirect_url if Civicrm.unauthorized_redirect_url&.starts_with?("http")
       return Civicrm.unauthorized_redirect_url if Civicrm.allow_unauthorized_path?(Civicrm.unauthorized_redirect_url)
 
       "/authorizations/first_login"

lib/omniauth/strategies/civicrm.rb

@@ -6,6 +6,7 @@
 module OmniAuth
   module Strategies
     class Civicrm < OmniAuth::Strategies::OAuth2
+      REGEXP_SANITIZER = /[<>?%&\^*#@()\[\]=+:;"{}\\|]/
       args [:client_id, :client_secret, :site]
 
       option :name, "civicrm" # do not symbolize this
@@ -20,7 +21,8 @@ class Civicrm < OmniAuth::Strategies::OAuth2
 
       info do
         {
-          name: extra[:contact][:display_name],
+          # sometimes the name can have special characters that are not allowed in the name validator
+          name: extra[:contact][:display_name].gsub(REGEXP_SANITIZER, ""),
           nickname: sanitized_nickname,
           email: raw_info["email"],
           image: raw_info["picture"]
@@ -57,7 +59,6 @@ def civicrm_info
       end
 
       def sanitized_nickname
-        # TODO: restrict nicknamize to the current organization
         Decidim::UserBaseEntity.nicknamize(raw_info["preferred_username"] || raw_info["email"])
       end
     end

spec/fixtures/files/find_user_valid_response.json

@@ -16,7 +16,7 @@
         "id": 9999,
         "values": [
           {
-            "display_name": "Arthur Dent"
+            "display_name": "(Arthur) Dent"
           }
         ]
       },

spec/forms/admin/event_meeting_form_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   module Admin

spec/forms/verifications/civicrm_groups_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   module Verifications

spec/forms/verifications/civicrm_membership_types_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   module Verifications

spec/forms/verifications/civicrm_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   module Verifications

spec/jobs/auto_verification_job_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   describe AutoVerificationJob do

spec/jobs/event_sync_job_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   describe EventSyncJob do

spec/jobs/rebuild_verifications_job_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   describe RebuildVerificationsJob do

spec/jobs/sync_all_events_job_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   describe SyncAllEventsJob do

spec/jobs/sync_all_groups_job_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   describe SyncAllGroupsJob do

spec/jobs/sync_event_registrations_job_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   describe SyncEventRegistrationsJob do

spec/jobs/sync_group_members_job_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   describe SyncGroupMembersJob do

spec/jobs/sync_membership_types_job_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   describe SyncMembershipTypesJob do

spec/lib/civicrm/api/base/request_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim
   describe Civicrm::Api::Base::Request, type: :class do

spec/lib/civicrm/api/find_contact_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim
   describe Civicrm::Api::FindContact, type: :class do

spec/lib/civicrm/api/find_event_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim
   describe Civicrm::Api::FindEvent, type: :class do

spec/lib/civicrm/api/find_group_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim
   describe Civicrm::Api::FindGroup, type: :class do

spec/lib/civicrm/api/find_user_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim
   describe Civicrm::Api::FindUser, type: :class do

spec/lib/civicrm/api/list_contact_groups_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim
   describe Civicrm::Api::ListContactGroups, type: :class do

spec/lib/civicrm/api/list_contact_memberships_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim
   describe Civicrm::Api::ListContactMemberships, type: :class do

spec/lib/civicrm/api/list_contacts_in_group_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim
   describe Civicrm::Api::ListContactsInGroup, type: :class do

spec/lib/civicrm/api/list_events_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim
   describe Civicrm::Api::ListEvents, type: :class do

spec/lib/civicrm/api/list_groups_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim
   describe Civicrm::Api::ListGroups, type: :class do

spec/lib/civicrm/api/list_membership_types_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim
   describe Civicrm::Api::ListMembershipTypes, type: :class do

spec/lib/civicrm_spec.rb

@@ -39,5 +39,13 @@ module Decidim
         end
       end
     end
+
+    context "when using a full url" do
+      let(:unauthorized_redirect_url) { "https://example.com/authorizations/first_login" }
+
+      it "uses the specified url" do
+        expect(Civicrm.unauthorized_url).to eq(unauthorized_redirect_url)
+      end
+    end
   end
 end

spec/lib/event_parsers/event_meeting_parser_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   describe EventParsers::EventMeetingParser, type: :class do

spec/lib/event_parsers/event_registration_parser_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 require "spec_helper"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 module Decidim::Civicrm
   describe EventParsers::EventRegistrationParser, type: :class do

spec/omni_auth/strategies/civicrm_spec.rb

@@ -3,7 +3,7 @@
 require "spec_helper"
 require "omniauth"
 require "omniauth/test"
-require "decidim/civicrm/test/shared_contexts"
+require "shared/shared_contexts"
 
 RSpec.configure do |config|
   config.extend OmniAuth::Test::StrategyMacros, type: :strategy
@@ -98,5 +98,13 @@
         expect(subject.info[:nickname]).to eq("foo_bar_2")
       end
     end
+
+    context "when the same user exists" do
+      let!(:existing_user) { create :user, nickname: "foo_bar", email: "[email protected]" }
+
+      it "returns a new valid nickname" do
+        expect(subject.info[:nickname]).to eq("foo_bar_2")
+      end
+    end
   end
 end

spec/system/login_data_spec.rb

@@ -32,7 +32,8 @@
 
   let(:last_user) { Decidim::User.last }
   let(:authorization) { Decidim::Authorization.find_by(name: handler) }
-  let(:user) { create(:user, :confirmed, name: "My Name", email: "[email protected]", organization: organization) }
+  let!(:existing_user) { create(:user, :confirmed, name: "Existing Name", nickname: "civicrm_user", email: "[email protected]", organization: organization) }
+  let(:user) { create(:user, :confirmed, name: "My Name", nickname: "civicrm_user", email: "[email protected]", organization: organization) }
   let!(:identity) { create(:identity, user: user, provider: Decidim::Civicrm::OMNIAUTH_PROVIDER_NAME, uid: "12345") }
   let!(:group) { create :civicrm_group, organization: organization, civicrm_group_id: 3 }
   let!(:membership_type) { create :civicrm_membership_type, organization: organization, civicrm_membership_type_id: 3 }