From 799202d50b275224d7066b3fb6e218d66ce285bf Mon Sep 17 00:00:00 2001 From: Vinoth Kannan Date: Tue, 24 Jan 2023 09:10:24 +0530 Subject: [PATCH] FIX: skip email if blank while syncing SSO attributes. (#19939) Also, return email blank error in `EmailValidator` when the email is blank. --- app/models/discourse_connect.rb | 2 +- config/locales/server.en.yml | 1 + lib/validators/email_validator.rb | 5 ++++- spec/models/user_spec.rb | 5 +++++ spec/requests/admin/users_controller_spec.rb | 20 ++++++++++++++++++++ 5 files changed, 31 insertions(+), 2 deletions(-) diff --git a/app/models/discourse_connect.rb b/app/models/discourse_connect.rb index 850784173f4..0d24cf63e7d 100644 --- a/app/models/discourse_connect.rb +++ b/app/models/discourse_connect.rb @@ -328,7 +328,7 @@ class DiscourseConnect < DiscourseConnectBase def change_external_attributes_and_override(sso_record, user) @email_changed = false - if SiteSetting.auth_overrides_email && user.email != Email.downcase(email) + if SiteSetting.auth_overrides_email && email.present? && user.email != Email.downcase(email) user.email = email user.active = false if require_activation @email_changed = true diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index c5f9d0ff42f..b46bbf1cf72 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2680,6 +2680,7 @@ en: must_not_contain_two_special_chars_in_seq: "must not contain a sequence of 2 or more special chars (.-_)" must_not_end_with_confusing_suffix: "must not end with a confusing suffix like .json or .png etc." email: + blank: "can't be blank." invalid: "is invalid." not_allowed: "is not allowed from that email provider. Please use another email address." blocked: "is not allowed." diff --git a/lib/validators/email_validator.rb b/lib/validators/email_validator.rb index aca5a069507..81badf8e853 100644 --- a/lib/validators/email_validator.rb +++ b/lib/validators/email_validator.rb @@ -2,7 +2,10 @@ class EmailValidator < ActiveModel::EachValidator def validate_each(record, attribute, value) - if !EmailAddressValidator.valid_value?(value) + if value.blank? + record.errors.add(attribute, I18n.t(:"user.email.blank")) + invalid = true + elsif !EmailAddressValidator.valid_value?(value) if Invite === record && attribute == :email record.errors.add(:base, I18n.t(:"invite.invalid_email", email: CGI.escapeHTML(value))) else diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ea8f7b708ba..f700de0bd1a 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -2573,6 +2573,11 @@ RSpec.describe User do expect(User.find(user.id).email).to eq(secondary_email_record.email) expect(user.secondary_emails.count).to eq(0) end + + it "returns error if email is nil" do + user.email = nil + expect { user.save! }.to raise_error(ActiveRecord::RecordInvalid) + end end describe "set_random_avatar" do diff --git a/spec/requests/admin/users_controller_spec.rb b/spec/requests/admin/users_controller_spec.rb index 602f84710ea..87866be1010 100644 --- a/spec/requests/admin/users_controller_spec.rb +++ b/spec/requests/admin/users_controller_spec.rb @@ -1777,6 +1777,26 @@ RSpec.describe Admin::UsersController do expect(user.username).to eq("Hokli") end + it "can sync up with the sso without email" do + sso.name = "Bob The Bob" + sso.username = "bob" + sso.email = "bob@bob.com" + sso.external_id = "1" + + user = + DiscourseConnect.parse( + sso.payload, + secure_session: read_secure_session, + ).lookup_or_create_user + + sso.name = "Bill" + sso.username = "Hokli$$!!" + sso.email = nil + + post "/admin/users/sync_sso.json", params: Rack::Utils.parse_query(sso.payload) + expect(response.status).to eq(200) + end + it "should create new users" do sso.name = "Dr. Claw" sso.username = "dr_claw"