Browse Source

Fixes #4889 - Third-party authorization handling is not using the unique identifier correctly (uid).

Dominik Klein 1 year ago
parent
commit
07e8af44d6

+ 3 - 0
.gitignore

@@ -13,6 +13,9 @@
 # database (copy from config/database/database.yml, or use `rails bs:init`)
 /config/database.yml
 
+# storage (copy from config/zammad/storage.yml.dist)
+/config/zammad/storage.yml
+
 # local backup config file
 /contrib/backup/config
 

+ 13 - 17
app/models/authorization.rb

@@ -62,38 +62,32 @@ class Authorization < ApplicationModel
 
   def self.create_from_hash(hash, user = nil)
 
-    if !user && Setting.get('auth_third_party_auto_link_at_inital_login') && hash['info'] && hash['info']['email'].present?
-      user = User.find_by(email: hash['info']['email'].downcase)
-    end
-
-    if !user
-      user = User.create_from_hash!(hash)
-    end
+    auth_provider = "#{PROVIDER_CLASS_PREFIX}#{hash['provider'].camelize}".constantize.new(hash, user)
 
     # save/update avatar
     if hash['info'].present? && hash['info']['image'].present?
       avatar = Avatar.add(
         object:        'User',
-        o_id:          user.id,
+        o_id:          auth_provider.user.id,
         url:           hash['info']['image'],
-        source:        hash['provider'],
+        source:        auth_provider.name,
         deletable:     true,
-        updated_by_id: user.id,
-        created_by_id: user.id,
+        updated_by_id: auth_provider.user.id,
+        created_by_id: auth_provider.user.id,
       )
 
       # update user link
-      if avatar && user.image != avatar.store_hash
-        user.image = avatar.store_hash
-        user.save
+      if avatar && auth_provider.user.image != avatar.store_hash
+        auth_provider.user.image = avatar.store_hash
+        auth_provider.user.save
       end
     end
 
     Authorization.create!(
-      user:     user,
-      uid:      hash['uid'],
+      user:     auth_provider.user,
+      uid:      auth_provider.uid,
       username: hash['info']['nickname'] || hash['info']['username'] || hash['info']['name'] || hash['info']['email'] || hash['username'],
-      provider: hash['provider'],
+      provider: auth_provider.name,
       token:    hash['credentials']['token'],
       secret:   hash['credentials']['secret']
     )
@@ -101,6 +95,8 @@ class Authorization < ApplicationModel
 
   private
 
+  PROVIDER_CLASS_PREFIX = 'Authorization::Provider::'.freeze
+
   def delete_user_cache
     return if !user
 

+ 37 - 0
app/models/authorization/provider.rb

@@ -0,0 +1,37 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Authorization::Provider
+  include Mixin::RequiredSubPaths
+
+  attr_reader :auth_hash, :user, :info, :uid
+
+  def initialize(auth_hash, user = nil)
+    @auth_hash = auth_hash
+    @uid = auth_hash['uid']
+    @info = auth_hash['info'] || {}
+
+    @user = user.presence || fetch_user
+  end
+
+  def name
+    self.class.name.demodulize.downcase
+  end
+
+  private
+
+  def fetch_user
+    if Setting.get('auth_third_party_auto_link_at_inital_login')
+      user = find_user
+
+      return user if user.present?
+    end
+
+    User.create_from_hash!(auth_hash)
+  end
+
+  def find_user
+    return if info['email'].nil?
+
+    User.find_by(email: info['email'].downcase)
+  end
+end

+ 4 - 0
app/models/authorization/provider/facebook.rb

@@ -0,0 +1,4 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Authorization::Provider::Facebook < Authorization::Provider
+end

+ 4 - 0
app/models/authorization/provider/github.rb

@@ -0,0 +1,4 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Authorization::Provider::GitHub < Authorization::Provider
+end

+ 4 - 0
app/models/authorization/provider/gitlab.rb

@@ -0,0 +1,4 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Authorization::Provider::GitLab < Authorization::Provider
+end

+ 4 - 0
app/models/authorization/provider/google_oauth2.rb

@@ -0,0 +1,4 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Authorization::Provider::GoogleOauth2 < Authorization::Provider
+end

+ 4 - 0
app/models/authorization/provider/linkedin.rb

@@ -0,0 +1,4 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Authorization::Provider::Linkedin < Authorization::Provider
+end

+ 4 - 0
app/models/authorization/provider/microsoft_office365.rb

@@ -0,0 +1,4 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Authorization::Provider::MicrosoftOffice365 < Authorization::Provider
+end

+ 15 - 0
app/models/authorization/provider/saml.rb

@@ -0,0 +1,15 @@
+# Copyright (C) 2012-2023 Zammad Foundation, https://zammad-foundation.org/
+
+class Authorization::Provider::Saml < Authorization::Provider
+  private
+
+  def find_user
+    user = User.find_by(login: uid)
+
+    if !user && !Setting.get('user_email_multiple_use') && info['email'].present?
+      user = User.find_by(email: info['email'].downcase)
+    end
+
+    user
+  end
+end

Some files were not shown because too many files changed in this diff