Compare revisions

Isaac Janzen · dependabot[bot] · dependabot[bot] · Alan Guo Xiang Tan · Natalie Tay · Davide Depau
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -9,7 +9,7 @@ GEM
    prettier_print (1.2.0)
    rainbow (3.1.1)
    regexp_parser (2.6.0)
-    rexml (3.2.5)
+    rexml (3.2.6)
    rubocop (1.36.0)
      json (~> 2.3)
      parallel (~> 1.10)

--- a/lib/saml_authenticator.rb
+++ b/lib/saml_authenticator.rb
@@ -40,11 +40,11 @@ class SamlAuthenticator < ::Auth::ManagedAuthenticator
    statements
      .split("|")
      .map do |statement|
-        attrs = statement.split(":", 2)
-        next if attrs.count != 2
-        (result[attrs[0]] ||= []) << attrs[1].split(",")
-        result[attrs[0]].flatten!
-      end
+      attrs = statement.split(":", 2)
+      next if attrs.count != 2
+      (result[attrs[0]] ||= []) << attrs[1].split(",")
+      result[attrs[0]].flatten!
+    end

    result
  end
@@ -78,7 +78,7 @@ class SamlAuthenticator < ::Auth::ManagedAuthenticator
        want_assertions_signed: !!setting(:want_assertions_signed),
        logout_requests_signed: !!setting(:logout_requests_signed),
        logout_responses_signed: !!setting(:logout_responses_signed),
-        signature_method: XMLSecurity::Document::RSA_SHA1,
+        signature_method: XMLSecurity::Document::RSA_SHA256,
      },
      idp_slo_session_destroy:
        proc do |env, session|
@@ -91,9 +91,28 @@ class SamlAuthenticator < ::Auth::ManagedAuthenticator
    )
  end

+  # Only match by the NameID
+  def match_by_email
+    false
+  end
+
+  def match_by_username
+    false
+  end
+
+  def is_anonymous?(email)
+    email.start_with?("anonymous+") && email.end_with?("@rev.ng")
+  end
+
  def primary_email_verified?(auth_token)
    attributes = OneLogin::RubySaml::Attributes.new(auth_token.extra&.[](:raw_info) || {})

+    email = attributes.single("email")
+    return false if is_anonymous?(email)
+
+    email_verified = attributes.single("emailVerified")
+    return email_verified == "true" if attributes.include?("emailVerified")
+
    group_attribute = setting(:groups_attribute)
    if setting(:validate_email_fields).present? && attributes.multi(group_attribute).present?
      validate_email_fields = setting(:validate_email_fields).split("|").map(&:downcase)
@@ -110,12 +129,16 @@ class SamlAuthenticator < ::Auth::ManagedAuthenticator
    extra_data = auth.extra || {}
    attributes = extra_data[:raw_info] || OneLogin::RubySaml::Attributes.new

+    log("after_authenticate: auth: #{auth.inspect}")
+    log("after_authenticate: attributes: #{attributes.inspect}")
+    log("after_authenticate: extra_data: #{extra_data.inspect}")
+    log("after_authenticate: uid: #{attributes.single("uid")}, #{auth[:uid]}")
+
    auth[:uid] = attributes.single("uid") || auth[:uid] if setting(:use_attributes_uid)
    uid = auth[:uid]

-    auth.info[:email] ||= uid if uid.to_s&.include?("@")
-
-    auth.info[:nickname] = uid.to_s if uid && setting(:use_attributes_uid)
+    auth.info[:username] = attributes.single("username")
+    auth.info[:nickname] = attributes.single("username")

    auth.extra = { "raw_info" => attributes.attributes }
    result = super
@@ -133,22 +156,31 @@ class SamlAuthenticator < ::Auth::ManagedAuthenticator

    result.skip_email_validation = true if setting(:skip_email_validation)

-    if result.user.blank?
-      result.username = "" if setting(:clear_username)
-      result.user = auto_create_account(result, uid) if setting(:auto_create_account) &&
-        result.email_valid
+    email = attributes.single("email")
+
+    if not is_anonymous?(email)
+      if result.user.blank?
+        result.username = "" if setting(:clear_username)
+        result.user = auto_create_account(result, uid) if setting(:auto_create_account) &&
+          result.email_valid
+      else
+        user = result.user
+        sync_groups(user, attributes, info)
+        sync_custom_fields(user, attributes, info)
+        sync_moderator(user, attributes)
+        sync_admin(user, attributes)
+        sync_trust_level(user, attributes)
+        sync_locale(user, attributes)
+      end
    else
-      user = result.user
-      sync_groups(user, attributes, info)
-      sync_custom_fields(user, attributes, info)
-      sync_moderator(user, attributes)
-      sync_admin(user, attributes)
-      sync_trust_level(user, attributes)
-      sync_locale(user, attributes)
+      result.failed = true
+      result.failed_reason = "Anonymous users cannot access Discourse. " +
+        "Convert your account to a regular account to continue."
    end

    result.overrides_username = setting(:omit_username)
    result.overrides_email = setting(:sync_email)
+    result.overrides_name = true

    result
  end
@@ -267,19 +299,19 @@ class SamlAuthenticator < ::Auth::ManagedAuthenticator
    statements
      .split("|")
      .each do |statement|
-        key, field_id = statement.split(":")
-        next if key.blank? || field_id.blank?
+      key, field_id = statement.split(":")
+      next if key.blank? || field_id.blank?

-        val = info[key] || attributes.multi(key)&.join(",")
-        user.custom_fields["user_field_#{field_id}"] = val if val.present?
-      end
+      val = info[key] || attributes.multi(key)&.join(",")
+      user.custom_fields["user_field_#{field_id}"] = val if val.present?
+    end
  end

  def sync_moderator(user, attributes)
    return unless setting(:sync_moderator)

-    is_moderator_attribute = setting(:moderator_attribute) || "isModerator"
-    is_moderator = %w[1 true].include?(attributes.single(is_moderator_attribute).to_s.downcase)
+    roles = attributes.multi("roles") || []
+    is_moderator = roles.include?("discourse-moderator") or roles.include?("discourse-admin")

    return if user.moderator == is_moderator

@@ -290,8 +322,8 @@ class SamlAuthenticator < ::Auth::ManagedAuthenticator
  def sync_admin(user, attributes)
    return unless setting(:sync_admin)

-    is_admin_attribute = setting(:admin_attribute) || "isAdmin"
-    is_admin = %w[1 true].include?(attributes.single(is_admin_attribute).to_s.downcase)
+    roles = attributes.multi("roles") || []
+    is_admin = roles.include?("discourse-admin")

    return if user.admin == is_admin

@@ -369,10 +401,6 @@ class SamlAuthenticator < ::Auth::ManagedAuthenticator
  end

  def resolve_username(username, name, email, uid)
-    suggester_input = [username, name]
-    suggester_input << email if SiteSetting.use_email_for_username_and_name_suggestions
-    suggester_input << uid
-
-    UserNameSuggester.suggest(*suggester_input)
+    username
  end
 end
--- a/package.json
+++ b/package.json
@@ -5,6 +5,6 @@
  "author": "Discourse",
  "license": "MIT",
  "devDependencies": {
-    "eslint-config-discourse": "^3.1.0"
+    "eslint-config-discourse": "^3.4.0"
  }
 }
--- a/plugin.rb
+++ b/plugin.rb
@@ -9,7 +9,7 @@

 gem "macaddr", "1.0.0"
 gem "uuid", "2.3.7"
-gem "rexml", "3.2.5"
+gem "rexml", "3.2.6"
 gem "ruby-saml", "1.13.0"
 gem "omniauth-saml", "1.9.0"


--- a/yarn.lock
+++ b/yarn.lock
@@ -1145,7 +1145,7 @@ escape-string-regexp@^4.0.0:
  resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-4.0.0.tgz#14ba83a5d373e3d311e5afca29cf5bfad965bf34"
  integrity sha512-TtpcNJ3XAzx3Gq8sWRzJaVajRs0uVxA2YAkdb1jm2YkPz4G6egUFAyA3n5vtEIZefPk5Wa4UXbKuS5fKkJWdgA==

-eslint-config-discourse@^3.1.0:
+eslint-config-discourse@^3.4.0:
  version "3.4.0"
  resolved "https://registry.yarnpkg.com/eslint-config-discourse/-/eslint-config-discourse-3.4.0.tgz#636a1824bca48c90aeac5bee2f8d7b993609191f"
  integrity sha512-9jwu8GQPDOxAO0ByV6RbInu5r39HrFvbAHQRJ8YoGg2fuvHcX+p7fYcxEWj64LhmF4qD55cAGhN0Gmj10RVjoQ==
@@ -2535,14 +2535,14 @@ safe-regex-test@^1.0.0:
    is-regex "^1.1.4"

 semver@^6.1.0, semver@^6.3.0:
-  version "6.3.0"
-  resolved "https://registry.yarnpkg.com/semver/-/semver-6.3.0.tgz#ee0a64c8af5e8ceea67687b133761e1becbd1d3d"
-  integrity sha512-b39TBaTSfV6yBrapU89p5fKekE2m/NwnDocOVruQFS1/veMgdzuPcnOM34M6CwxW8jH/lxEa5rBoDeUwu5HHTw==
+  version "6.3.1"
+  resolved "https://registry.yarnpkg.com/semver/-/semver-6.3.1.tgz#556d2ef8689146e46dcea4bfdd095f3434dffcb4"
+  integrity sha512-BR7VvDCVHO+q2xBEWskxS6DJE1qRnb7DxzUrogb71CWoSficBxYsiAGd+Kl0mmq/MprG9yArRkyrQxTO6XjMzA==

 semver@^7.3.2, semver@^7.3.4:
-  version "7.3.8"
-  resolved "https://registry.yarnpkg.com/semver/-/semver-7.3.8.tgz#07a78feafb3f7b32347d725e33de7e2a2df67798"
-  integrity sha512-NB1ctGL5rlHrPJtFDVIVzTyQylMLu9N9VICA6HSFJo8MCGVTMW6gfpicwKmmK/dAjTOrqu5l63JJOpDSrAis3A==
+  version "7.5.4"
+  resolved "https://registry.yarnpkg.com/semver/-/semver-7.5.4.tgz#483986ec4ed38e1c6c48c34894a9182dbff68a6e"
+  integrity sha512-1bCSESV6Pv+i21Hvpxp3Dx+pSD8lIPt8uVjRrxAUt/nbswYc+tK6Y2btiULjd4+fnq15PX+nqQDC7Oft7WkwcA==
  dependencies:
    lru-cache "^6.0.0"

@@ -2912,9 +2912,9 @@ which@^2.0.1:
    isexe "^2.0.0"

 word-wrap@^1.2.3:
-  version "1.2.3"
-  resolved "https://registry.yarnpkg.com/word-wrap/-/word-wrap-1.2.3.tgz#610636f6b1f703891bd34771ccb17fb93b47079c"
-  integrity sha512-Hz/mrNwitNRh/HUAtM/VT/5VH+ygD6DV7mYKZAtHOrbs8U7lvPS6xf7EJKMF0uW1KJCl0H701g3ZGus+muE5vQ==
+  version "1.2.4"
+  resolved "https://registry.yarnpkg.com/word-wrap/-/word-wrap-1.2.4.tgz#cb4b50ec9aca570abd1f52f33cd45b6c61739a9f"
+  integrity sha512-2V81OA4ugVo5pRo46hAoD2ivUJx8jXmWXfUkY4KFNw0hEptvN0QfH3K4nHiwzGeKl5rFKedV48QVoqYavy4YpA==

 workerpool@^6.1.5:
  version "6.3.1"
No results found