Profile updates from providers

app/controllers/profiles_controller.rb

@@ -9,8 +9,6 @@ class ProfilesController < Profiles::ApplicationController
   end
   def update
-    user_params.except!(:email) if @user.external_email?
-
     respond_to do |format|
       result = Users::UpdateService.new(@user, user_params).execute

app/helpers/profiles_helper.rb

 module ProfilesHelper
-  def email_provider_label
-    return unless current_user.external_email?
-
-    current_user.email_provider.present? ? Gitlab::OAuth::Provider.label_for(current_user.email_provider) : "LDAP"
+  def attribute_provider_label(attribute)
+    user_synced_attributes_metadata = current_user.user_synced_attributes_metadata
+    if user_synced_attributes_metadata&.synced?(attribute)
+      if user_synced_attributes_metadata.provider
+        Gitlab::OAuth::Provider.label_for(user_synced_attributes_metadata.provider)
+      else
+        'LDAP'
+      end
+    end
   end
 end

app/models/user.rb

@@ -15,10 +15,12 @@ class User < ActiveRecord::Base
   include IgnorableColumn
   include FeatureGate
   include CreatedAtFilterable
+  include IgnorableColumn
   DEFAULT_NOTIFICATION_LEVEL = :participating
-  ignore_column :authorized_projects_populated
+  ignore_column :external_email
+  ignore_column :email_provider
   add_authentication_token_field :authentication_token
   add_authentication_token_field :incoming_email_token
@@ -85,6 +87,7 @@ class User < ActiveRecord::Base
   has_many :identities, dependent: :destroy, autosave: true # rubocop:disable Cop/ActiveRecordDependent
   has_many :u2f_registrations, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
   has_many :chat_names, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+  has_one :user_synced_attributes_metadata, autosave: true
   # Groups
   has_many :members, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
@@ -161,6 +164,7 @@ class User < ActiveRecord::Base
   after_update :update_emails_with_primary_email, if: :email_changed?
   before_save :ensure_authentication_token, :ensure_incoming_email_token
   before_save :ensure_user_rights_and_limits, if: :external_changed?
+  before_save :skip_reconfirmation!, if: ->(user) { user.email_changed? && user.read_only_attribute?(:email) }
   after_save :ensure_namespace_correct
   after_commit :update_invalid_gpg_signatures, on: :update, if: -> { previous_changes.key?('email') }
   after_initialize :set_projects_limit
@@ -1045,6 +1049,22 @@ class User < ActiveRecord::Base
     self.email == email
   end
+  def sync_attribute?(attribute)
+    return true if ldap_user? && attribute == :email
+
+    attributes = Gitlab.config.omniauth.sync_profile_attributes
+
+    if attributes.is_a?(Array)
+      attributes.include?(attribute.to_s)
+    else
+      attributes
+    end
+  end
+
+  def read_only_attribute?(attribute)
+    user_synced_attributes_metadata&.read_only?(attribute)
+  end
+
   protected
   # override, from Devise::Validatable

app/models/user_synced_attributes_metadata.rb

+class UserSyncedAttributesMetadata < ActiveRecord::Base
+  belongs_to :user
+
+  validates :user, presence: true
+
+  SYNCABLE_ATTRIBUTES = %i[name email location].freeze
+
+  def read_only?(attribute)
+    Gitlab.config.omniauth.sync_profile_from_provider && synced?(attribute)
+  end
+
+  def read_only_attributes
+    return [] unless Gitlab.config.omniauth.sync_profile_from_provider
+
+    SYNCABLE_ATTRIBUTES.select { |key| synced?(key) }
+  end
+
+  def synced?(attribute)
+    read_attribute("#{attribute}_synced")
+  end
+
+  def set_attribute_synced(attribute, value)
+    write_attribute("#{attribute}_synced", value)
+  end
+end

app/services/users/update_service.rb

@@ -34,6 +34,10 @@ module Users
     private
     def assign_attributes(&block)
+      if @user.user_synced_attributes_metadata
+        params.except!(*@user.user_synced_attributes_metadata.read_only_attributes)
+      end
+
       @user.assign_attributes(params) if params.any?
     end
   end

app/views/profiles/show.html.haml

@@ -45,12 +45,15 @@
           Some options are unavailable for LDAP accounts
     .col-lg-8
       .row
-        = f.text_field :name,  required: true, wrapper: { class: 'col-md-9' },
-          help: 'Enter your name, so people you know can recognize you.'
+        - if @user.read_only_attribute?(:name)
+          = f.text_field :name, required: true, readonly: true, wrapper: { class: 'col-md-9' },
+          help: "Your name was automatically set based on your #{ attribute_provider_label(:name) } account, so people you know can recognize you."
+        - else
+          = f.text_field :name, required: true, wrapper: { class: 'col-md-9' }, help: "Enter your name, so people you know can recognize you."
         = f.text_field :id, readonly: true, label: 'User ID', wrapper: { class: 'col-md-3' }
-      - if @user.external_email?
-        = f.text_field :email, required: true, readonly: true, help: "Your email address was automatically set based on your #{email_provider_label} account."
+      - if @user.read_only_attribute?(:email)
+        = f.text_field :email, required: true, readonly: true, help: "Your email address was automatically set based on your #{ attribute_provider_label(:email) } account."
       - else
         = f.text_field :email, required: true, value: (@user.email unless @user.temp_oauth_email?),
           help: user_email_help_text(@user)
@@ -64,7 +67,10 @@
       = f.text_field :linkedin
       = f.text_field :twitter
       = f.text_field :website_url, label: 'Website'
-      = f.text_field :location
+      - if @user.read_only_attribute?(:location)
+        = f.text_field :location, readonly: true, help: "Your location was automatically set based on your #{ attribute_provider_label(:location) } account."
+      - else
+        = f.text_field :location
       = f.text_field :organization
       = f.text_area :bio, rows: 4, maxlength: 250, help: 'Tell us about yourself in fewer than 250 characters.'
       .prepend-top-default.append-bottom-default

changelogs/unreleased/12968-generalize-profile-updates.yml

+---
+title: Generalize profile updates from providers
+merge_request: 12968
+author: Alexandros Keramidas

config/gitlab.yml.example

@@ -372,9 +372,16 @@ production: &base
     # showing GitLab's sign-in page (default: show the GitLab sign-in page)
     # auto_sign_in_with_provider: saml
-    # Sync user's email address from the specified Omniauth provider every time the user logs
-    # in (default: nil). And consequently make this field read-only.
-    # sync_email_from_provider: cas3
+    # Sync user's profile from the specified Omniauth providers every time the user logs in (default: empty).
+    # Define the allowed providers using an array, e.g. ["cas3", "saml", "twitter"],
+    # or as true/false to allow all providers or none.
+    # sync_profile_from_provider: []
+
+    # Select which info to sync from the providers above. (default: email).
+    # Define the synced profile info using an array. Available options are "name", "email" and "location"
+    # e.g. ["name", "email", "location"] or as true to sync all available.
+    # This consequently will make the selected attributes read-only.
+    # sync_profile_attributes: true
     # CAUTION!
     # This allows users to login without having a user account first. Define the allowed providers

config/initializers/1_settings.rb

@@ -173,7 +173,20 @@ Settings.omniauth['external_providers'] = [] if Settings.omniauth['external_prov
 Settings.omniauth['block_auto_created_users'] = true if Settings.omniauth['block_auto_created_users'].nil?
 Settings.omniauth['auto_link_ldap_user'] = false if Settings.omniauth['auto_link_ldap_user'].nil?
 Settings.omniauth['auto_link_saml_user'] = false if Settings.omniauth['auto_link_saml_user'].nil?
-Settings.omniauth['sync_email_from_provider'] ||= nil
+
+Settings.omniauth['sync_profile_from_provider'] = false if Settings.omniauth['sync_profile_from_provider'].nil?
+Settings.omniauth['sync_profile_attributes'] = ['email'] if Settings.omniauth['sync_profile_attributes'].nil?
+
+# Handle backwards compatibility with merge request 11268
+if Settings.omniauth['sync_email_from_provider']
+  if Settings.omniauth['sync_profile_from_provider'].is_a?(Array)
+    Settings.omniauth['sync_profile_from_provider'] |= [Settings.omniauth['sync_email_from_provider']]
+  elsif !Settings.omniauth['sync_profile_from_provider']
+    Settings.omniauth['sync_profile_from_provider'] = [Settings.omniauth['sync_email_from_provider']]
+  end
+
+  Settings.omniauth['sync_profile_attributes'] |= ['email'] unless Settings.omniauth['sync_profile_attributes'] == true
+end
 Settings.omniauth['providers'] ||= []
 Settings.omniauth['cas3'] ||= Settingslogic.new({})

db/migrate/20170820120108_create_user_synced_attributes_metadata.rb

+class CreateUserSyncedAttributesMetadata < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    create_table :user_synced_attributes_metadata do |t|
+      t.boolean :name_synced, default: false
+      t.boolean :email_synced, default: false
+      t.boolean :location_synced, default: false
+      t.references :user, null: false, index: { unique: true }, foreign_key: { on_delete: :cascade }
+      t.string :provider
+    end
+  end
+end

db/migrate/20170828135939_migrate_user_external_mail_data.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class MigrateUserExternalMailData < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class User < ActiveRecord::Base
+    self.table_name = 'users'
+
+    include EachBatch
+  end
+
+  class UserSyncedAttributesMetadata < ActiveRecord::Base
+    self.table_name = 'user_synced_attributes_metadata'
+
+    include EachBatch
+  end
+
+  def up
+    User.each_batch do |batch|
+      start_id, end_id = batch.pluck('MIN(id), MAX(id)').first
+
+      execute <<-EOF
+        INSERT INTO user_synced_attributes_metadata (user_id, provider, email_synced)
+        SELECT id, email_provider, external_email
+        FROM users
+        WHERE external_email = TRUE
+        AND NOT EXISTS (
+          SELECT true
+          FROM user_synced_attributes_metadata
+          WHERE user_id = users.id
+          AND provider = users.email_provider
+        )
+        AND id BETWEEN #{start_id} AND #{end_id}
+      EOF
+    end
+  end
+
+  def down
+    UserSyncedAttributesMetadata.each_batch do |batch|
+      start_id, end_id = batch.pluck('MIN(id), MAX(id)').first
+
+      execute <<-EOF
+        UPDATE users
+        SET users.email_provider = metadata.provider, users.external_email = metadata.email_synced
+        FROM user_synced_attributes_metadata as metadata, users
+        WHERE metadata.email_synced = TRUE
+        AND metadata.user_id = users.id
+        AND id BETWEEN #{start_id} AND #{end_id}
+      EOF
+    end
+  end
+end

db/post_migrate/20170828170502_post_deploy_migrate_user_external_mail_data.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class PostDeployMigrateUserExternalMailData < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class User < ActiveRecord::Base
+    self.table_name = 'users'
+
+    include EachBatch
+  end
+
+  class UserSyncedAttributesMetadata < ActiveRecord::Base
+    self.table_name = 'user_synced_attributes_metadata'
+
+    include EachBatch
+  end
+
+  def up
+    User.each_batch do |batch|
+      start_id, end_id = batch.pluck('MIN(id), MAX(id)').first
+
+      execute <<-EOF
+        INSERT INTO user_synced_attributes_metadata (user_id, provider, email_synced)
+        SELECT id, email_provider, external_email
+        FROM users
+        WHERE external_email = TRUE
+        AND NOT EXISTS (
+          SELECT true
+          FROM user_synced_attributes_metadata
+          WHERE user_id = users.id
+          AND provider = users.email_provider
+        )
+        AND id BETWEEN #{start_id} AND #{end_id}
+      EOF
+    end
+  end
+
+  def down
+    UserSyncedAttributesMetadata.each_batch do |batch|
+      start_id, end_id = batch.pluck('MIN(id), MAX(id)').first
+
+      execute <<-EOF
+        UPDATE users
+        SET users.email_provider = metadata.provider, users.external_email = metadata.email_synced
+        FROM user_synced_attributes_metadata as metadata, users
+        WHERE metadata.email_synced = TRUE
+        AND metadata.user_id = users.id
+        AND id BETWEEN #{start_id} AND #{end_id}
+      EOF
+    end
+  end
+end

db/post_migrate/20170828170513_remove_user_email_provider_column.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveUserEmailProviderColumn < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    remove_column :users, :email_provider, :string
+  end
+end

db/post_migrate/20170828170516_remove_user_external_mail_columns.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveUserExternalMailColumns < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    remove_column :users, :external_email, :boolean
+  end
+end

db/schema.rb

@@ -1539,6 +1539,16 @@ ActiveRecord::Schema.define(version: 20170905112933) do
   add_index "user_agent_details", ["subject_id", "subject_type"], name: "index_user_agent_details_on_subject_id_and_subject_type", using: :btree
+  create_table "user_synced_attributes_metadata", force: :cascade do |t|
+    t.boolean "name_synced", default: false
+    t.boolean "email_synced", default: false
+    t.boolean "location_synced", default: false
+    t.integer "user_id", null: false
+    t.string "provider"
+  end
+
+  add_index "user_synced_attributes_metadata", ["user_id"], name: "index_user_synced_attributes_metadata_on_user_id", unique: true, using: :btree
+
   create_table "users", force: :cascade do |t|
     t.string "email", default: "", null: false
     t.string "encrypted_password", default: "", null: false
@@ -1604,8 +1614,6 @@ ActiveRecord::Schema.define(version: 20170905112933) do
     t.boolean "notified_of_own_activity"
     t.string "preferred_language"
     t.string "rss_token"
-    t.boolean "external_email", default: false, null: false
-    t.string "email_provider"
   end
   add_index "users", ["admin"], name: "index_users_on_admin", using: :btree
@@ -1756,6 +1764,7 @@ ActiveRecord::Schema.define(version: 20170905112933) do
   add_foreign_key "todos", "projects", name: "fk_45054f9c45", on_delete: :cascade
   add_foreign_key "trending_projects", "projects", on_delete: :cascade
   add_foreign_key "u2f_registrations", "users"
+  add_foreign_key "user_synced_attributes_metadata", "users", on_delete: :cascade
   add_foreign_key "users_star_projects", "projects", name: "fk_22cd27ddfc", on_delete: :cascade
   add_foreign_key "web_hook_logs", "web_hooks", on_delete: :cascade
   add_foreign_key "web_hooks", "projects", name: "fk_0c8ca6d9d1", on_delete: :cascade

doc/integration/omniauth.md

@@ -224,3 +224,21 @@ By default Sign In is enabled via all the OAuth Providers that have been configu
 In order to enable/disable an OmniAuth provider, go to Admin Area -> Settings -> Sign-in Restrictions section -> Enabled OAuth Sign-In sources and select the providers you want to enable or disable.
 ![Enabled OAuth Sign-In sources](img/enabled-oauth-sign-in-sources.png)
+
+
+## Keep OmniAuth user profiles up to date
+
+You can enable profile syncing from selected OmniAuth providers and for all or for specific user information.
+ 
+ ```ruby
+   gitlab_rails['sync_profile_from_provider'] = ['twitter', 'google_oauth2']
+   gitlab_rails['sync_profile_attributes'] = ['name', 'email', 'location']
+ ```
+ 
+ **For installations from source**
+ 
+ ```yaml
+   omniauth:
+     sync_profile_from_provider: ['twitter', 'google_oauth2']
+     sync_profile_claims_from_provider: ['email', 'location']
+ ```
\ No newline at end of file

lib/gitlab/ldap/user.rb

@@ -36,7 +36,7 @@ module Gitlab
       end
       def find_by_email
-        ::User.find_by(email: auth_hash.email.downcase) if auth_hash.has_email?
+        ::User.find_by(email: auth_hash.email.downcase) if auth_hash.has_attribute?(:email)
       end
       def update_user_attributes
@@ -60,7 +60,7 @@ module Gitlab
         ldap_config.block_auto_created_users
       end
-      def sync_email_from_provider?
+      def sync_profile_from_provider?
         true
       end

lib/gitlab/o_auth/auth_hash.rb

@@ -32,8 +32,21 @@ module Gitlab
         @password ||= Gitlab::Utils.force_utf8(Devise.friendly_token[0, 8].downcase)
       end
-      def has_email?
-        get_info(:email).present?
+      def location
+        location = get_info(:address)
+        if location.is_a?(Hash)
+          [location.locality.presence, location.country.presence].compact.join(', ')
+        else
+          location
+        end
+      end
+
+      def has_attribute?(attribute)
+        if attribute == :location
+          get_info(:address).present?
+        else
+          get_info(attribute).present?
+        end
       end
       private

lib/gitlab/o_auth/user.rb

@@ -12,7 +12,7 @@ module Gitlab
       def initialize(auth_hash)
         self.auth_hash = auth_hash
-        update_email
+        update_profile if sync_profile_from_provider?
       end
       def persisted?
@@ -184,20 +184,30 @@ module Gitlab
         }
       end
-      def sync_email_from_provider?
-        auth_hash.provider.to_s == Gitlab.config.omniauth.sync_email_from_provider.to_s
+      def sync_profile_from_provider?
+        providers = Gitlab.config.omniauth.sync_profile_from_provider
+
+        if providers.is_a?(Array)
+          providers.include?(auth_hash.provider)
+        else
+          providers
+        end
       end
-      def update_email
-        if auth_hash.has_email? && sync_email_from_provider?
-          if persisted?
-            gl_user.skip_reconfirmation!
-            gl_user.email = auth_hash.email
-          end
-          gl_user.external_email = true
-          gl_user.email_provider = auth_hash.provider
+      def update_profile
+        user_synced_attributes_metadata = gl_user.user_synced_attributes_metadata || gl_user.build_user_synced_attributes_metadata
+        UserSyncedAttributesMetadata::SYNCABLE_ATTRIBUTES.each do |key|
+          if auth_hash.has_attribute?(key) && gl_user.sync_attribute?(key)
+            gl_user[key] = auth_hash.public_send(key) # rubocop:disable GitlabSecurity/PublicSend
+            user_synced_attributes_metadata.set_attribute_synced(key, true)
+          else
+            user_synced_attributes_metadata.set_attribute_synced(key, false)
+          end
         end
+
+        user_synced_attributes_metadata.provider = auth_hash.provider
+        gl_user.user_synced_attributes_metadata = user_synced_attributes_metadata
       end
       def log

lib/gitlab/saml/user.rb

@@ -40,7 +40,7 @@ module Gitlab
       end
       def find_by_email
-        if auth_hash.has_email?
+        if auth_hash.has_attribute?(:email)
           user = ::User.find_by(email: auth_hash.email.downcase)
           user.identities.new(extern_uid: auth_hash.uid, provider: auth_hash.provider) if user
           user