DEV: Introduce Auth::ManagedAuthenticator
A generic implementation of Auth::Authenticator which stores data in the new UserAssociatedAccount model. This should help significantly reduce the duplicated logic across different auth providers.
This commit is contained in:
parent
059e36a6ff
commit
534e1b1b18
|
@ -0,0 +1,24 @@
|
||||||
|
class UserAssociatedAccount < ActiveRecord::Base
|
||||||
|
belongs_to :user
|
||||||
|
end
|
||||||
|
|
||||||
|
# == Schema Information
|
||||||
|
#
|
||||||
|
# Table name: user_associated_accounts
|
||||||
|
#
|
||||||
|
# id :bigint(8) not null, primary key
|
||||||
|
# provider_name :string not null
|
||||||
|
# provider_uid :string not null
|
||||||
|
# user_id :integer not null
|
||||||
|
# last_used :datetime not null
|
||||||
|
# info :jsonb not null
|
||||||
|
# credentials :jsonb not null
|
||||||
|
# extra :jsonb not null
|
||||||
|
# created_at :datetime not null
|
||||||
|
# updated_at :datetime not null
|
||||||
|
#
|
||||||
|
# Indexes
|
||||||
|
#
|
||||||
|
# associated_accounts_provider_uid (provider_name,provider_uid) UNIQUE
|
||||||
|
# associated_accounts_provider_user (provider_name,user_id) UNIQUE
|
||||||
|
#
|
|
@ -0,0 +1,18 @@
|
||||||
|
class CreateUserAssociatedAccounts < ActiveRecord::Migration[5.2]
|
||||||
|
def change
|
||||||
|
create_table :user_associated_accounts do |t|
|
||||||
|
t.string :provider_name, null: false
|
||||||
|
t.string :provider_uid, null: false
|
||||||
|
t.integer :user_id, null: false
|
||||||
|
t.datetime :last_used, null: false, default: -> { "CURRENT_TIMESTAMP" }
|
||||||
|
t.jsonb :info, null: false, default: {}
|
||||||
|
t.jsonb :credentials, null: false, default: {}
|
||||||
|
t.jsonb :extra, null: false, default: {}
|
||||||
|
|
||||||
|
t.timestamps
|
||||||
|
end
|
||||||
|
|
||||||
|
add_index :user_associated_accounts, [:provider_name, :provider_uid], unique: true, name: 'associated_accounts_provider_uid'
|
||||||
|
add_index :user_associated_accounts, [:provider_name, :user_id], unique: true, name: 'associated_accounts_provider_user'
|
||||||
|
end
|
||||||
|
end
|
|
@ -3,6 +3,7 @@ module Auth; end
|
||||||
require_dependency 'auth/auth_provider'
|
require_dependency 'auth/auth_provider'
|
||||||
require_dependency 'auth/result'
|
require_dependency 'auth/result'
|
||||||
require_dependency 'auth/authenticator'
|
require_dependency 'auth/authenticator'
|
||||||
|
require_dependency 'auth/managed_authenticator'
|
||||||
require_dependency 'auth/facebook_authenticator'
|
require_dependency 'auth/facebook_authenticator'
|
||||||
require_dependency 'auth/open_id_authenticator'
|
require_dependency 'auth/open_id_authenticator'
|
||||||
require_dependency 'auth/github_authenticator'
|
require_dependency 'auth/github_authenticator'
|
||||||
|
|
|
@ -0,0 +1,105 @@
|
||||||
|
class Auth::ManagedAuthenticator < Auth::Authenticator
|
||||||
|
def description_for_user(user)
|
||||||
|
info = UserAssociatedAccount.find_by(provider_name: name, user_id: user.id)&.info
|
||||||
|
return "" if info.nil?
|
||||||
|
info["email"] || info["nickname"] || info["name"] || ""
|
||||||
|
end
|
||||||
|
|
||||||
|
# These three methods are designed to be overriden by child classes
|
||||||
|
def match_by_email
|
||||||
|
true
|
||||||
|
end
|
||||||
|
|
||||||
|
def can_revoke?
|
||||||
|
true
|
||||||
|
end
|
||||||
|
|
||||||
|
def can_connect_existing_user?
|
||||||
|
true
|
||||||
|
end
|
||||||
|
|
||||||
|
def revoke(user, skip_remote: false)
|
||||||
|
association = UserAssociatedAccount.find_by(provider_name: name, user_id: user.id)
|
||||||
|
raise Discourse::NotFound if association.nil?
|
||||||
|
association.destroy!
|
||||||
|
true
|
||||||
|
end
|
||||||
|
|
||||||
|
def after_authenticate(auth_token, existing_account: nil)
|
||||||
|
result = Auth::Result.new
|
||||||
|
|
||||||
|
# Store all the metadata for later, in case the `after_create_account` hook is used
|
||||||
|
result.extra_data = {
|
||||||
|
provider: auth_token[:provider],
|
||||||
|
uid: auth_token[:uid],
|
||||||
|
info: auth_token[:info],
|
||||||
|
extra: auth_token[:extra],
|
||||||
|
credentials: auth_token[:credentials]
|
||||||
|
}
|
||||||
|
|
||||||
|
# Build the Auth::Result object
|
||||||
|
info = auth_token[:info]
|
||||||
|
result.email = email = info[:email]
|
||||||
|
result.name = name = "#{info[:first_name]} #{info[:last_name]}"
|
||||||
|
result.username = info[:nickname]
|
||||||
|
|
||||||
|
# Try and find an association for this account
|
||||||
|
association = UserAssociatedAccount.find_by(provider_name: auth_token[:provider], provider_uid: auth_token[:uid])
|
||||||
|
result.user = association&.user
|
||||||
|
|
||||||
|
# Reconnecting to existing account
|
||||||
|
if can_connect_existing_user? && existing_account && (association.nil? || existing_account.id != association.user_id)
|
||||||
|
association.destroy! if association
|
||||||
|
association = nil
|
||||||
|
result.user = existing_account
|
||||||
|
end
|
||||||
|
|
||||||
|
# Matching an account by email
|
||||||
|
if match_by_email && association.nil? && (user = User.find_by_email(email))
|
||||||
|
UserAssociatedAccount.where(user: user, provider_name: auth_token[:provider]).destroy_all # Destroy existing associations for the new user
|
||||||
|
result.user = user
|
||||||
|
end
|
||||||
|
|
||||||
|
# Add the association to the database if it doesn't already exist
|
||||||
|
if association.nil? && result.user
|
||||||
|
association = create_association!(result.extra_data.merge(user: result.user))
|
||||||
|
end
|
||||||
|
|
||||||
|
# Update all the metadata in the association:
|
||||||
|
if association
|
||||||
|
association.update!(
|
||||||
|
info: auth_token[:info] || {},
|
||||||
|
credentials: auth_token[:credentials] || {},
|
||||||
|
extra: auth_token[:extra] || {}
|
||||||
|
)
|
||||||
|
retrieve_avatar(result.user, auth_token[:info][:image])
|
||||||
|
end
|
||||||
|
|
||||||
|
result.email_valid = true if result.email
|
||||||
|
|
||||||
|
result
|
||||||
|
end
|
||||||
|
|
||||||
|
def create_association!(hash)
|
||||||
|
association = UserAssociatedAccount.create!(
|
||||||
|
user: hash[:user],
|
||||||
|
provider_name: hash[:provider],
|
||||||
|
provider_uid: hash[:uid],
|
||||||
|
info: hash[:info] || {},
|
||||||
|
credentials: hash[:credentials] || {},
|
||||||
|
extra: hash[:extra] || {}
|
||||||
|
)
|
||||||
|
end
|
||||||
|
|
||||||
|
def after_create_account(user, auth)
|
||||||
|
data = auth[:extra_data]
|
||||||
|
create_association!(data.merge(user: user))
|
||||||
|
retrieve_avatar(user, data["info"]["image"])
|
||||||
|
end
|
||||||
|
|
||||||
|
def retrieve_avatar(user, url)
|
||||||
|
return unless user && url
|
||||||
|
return if user.user_avatar.try(:custom_upload_id).present?
|
||||||
|
Jobs.enqueue(:download_avatar_from_url, url: url, user_id: user.id, override_gravatar: false)
|
||||||
|
end
|
||||||
|
end
|
|
@ -0,0 +1,148 @@
|
||||||
|
require 'rails_helper'
|
||||||
|
|
||||||
|
describe Auth::ManagedAuthenticator do
|
||||||
|
let(:authenticator) {
|
||||||
|
Class.new(described_class) do
|
||||||
|
def name; "myauth" end
|
||||||
|
end.new
|
||||||
|
}
|
||||||
|
|
||||||
|
let(:hash) {
|
||||||
|
{
|
||||||
|
provider: "myauth",
|
||||||
|
uid: "1234",
|
||||||
|
info: {
|
||||||
|
name: "Best Display Name",
|
||||||
|
email: "awesome@example.com",
|
||||||
|
nickname: "IAmGroot"
|
||||||
|
},
|
||||||
|
credentials: {
|
||||||
|
token: "supersecrettoken"
|
||||||
|
},
|
||||||
|
extra: {
|
||||||
|
raw_info: {
|
||||||
|
randominfo: "some info"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
describe 'after_authenticate' do
|
||||||
|
it 'can match account from an existing association' do
|
||||||
|
user = Fabricate(:user)
|
||||||
|
associated = UserAssociatedAccount.create!(user: user, provider_name: 'myauth', provider_uid: "1234")
|
||||||
|
result = authenticator.after_authenticate(hash)
|
||||||
|
|
||||||
|
expect(result.user.id).to eq(user.id)
|
||||||
|
associated.reload
|
||||||
|
expect(associated.info["name"]).to eq("Best Display Name")
|
||||||
|
expect(associated.info["email"]).to eq("awesome@example.com")
|
||||||
|
expect(associated.credentials["token"]).to eq("supersecrettoken")
|
||||||
|
expect(associated.extra["raw_info"]["randominfo"]).to eq("some info")
|
||||||
|
end
|
||||||
|
|
||||||
|
describe 'connecting to another user account' do
|
||||||
|
let(:user1) { Fabricate(:user) }
|
||||||
|
let(:user2) { Fabricate(:user) }
|
||||||
|
before { UserAssociatedAccount.create!(user: user1, provider_name: 'myauth', provider_uid: "1234") }
|
||||||
|
|
||||||
|
it 'works by default' do
|
||||||
|
result = authenticator.after_authenticate(hash, existing_account: user2)
|
||||||
|
expect(result.user.id).to eq(user2.id)
|
||||||
|
expect(UserAssociatedAccount.exists?(user_id: user1.id)).to eq(false)
|
||||||
|
expect(UserAssociatedAccount.exists?(user_id: user2.id)).to eq(true)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'does not work when disabled' do
|
||||||
|
authenticator = Class.new(described_class) do
|
||||||
|
def name; "myauth" end
|
||||||
|
def can_connect_existing_user?; false end
|
||||||
|
end.new
|
||||||
|
result = authenticator.after_authenticate(hash, existing_account: user2)
|
||||||
|
expect(result.user.id).to eq(user1.id)
|
||||||
|
expect(UserAssociatedAccount.exists?(user_id: user1.id)).to eq(true)
|
||||||
|
expect(UserAssociatedAccount.exists?(user_id: user2.id)).to eq(false)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe 'match by email' do
|
||||||
|
it 'works normally' do
|
||||||
|
user = Fabricate(:user)
|
||||||
|
result = authenticator.after_authenticate(hash.deep_merge(info: { email: user.email }))
|
||||||
|
expect(result.user.id).to eq(user.id)
|
||||||
|
expect(UserAssociatedAccount.find_by(provider_name: 'myauth', provider_uid: "1234").user_id).to eq(user.id)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'works if there is already an association with the target account' do
|
||||||
|
user = Fabricate(:user, email: "awesome@example.com")
|
||||||
|
result = authenticator.after_authenticate(hash)
|
||||||
|
expect(result.user.id).to eq(user.id)
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'does not match if match_by_email is false' do
|
||||||
|
authenticator = Class.new(described_class) do
|
||||||
|
def name; "myauth" end
|
||||||
|
def match_by_email; false end
|
||||||
|
end.new
|
||||||
|
user = Fabricate(:user, email: "awesome@example.com")
|
||||||
|
result = authenticator.after_authenticate(hash)
|
||||||
|
expect(result.user).to eq(nil)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when no matching user' do
|
||||||
|
it 'returns the correct information' do
|
||||||
|
result = authenticator.after_authenticate(hash)
|
||||||
|
expect(result.user).to eq(nil)
|
||||||
|
expect(result.username).to eq("IAmGroot")
|
||||||
|
expect(result.email).to eq("awesome@example.com")
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'works if there is already an association with the target account' do
|
||||||
|
user = Fabricate(:user, email: "awesome@example.com")
|
||||||
|
result = authenticator.after_authenticate(hash)
|
||||||
|
expect(result.user.id).to eq(user.id)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe 'description_for_user' do
|
||||||
|
let(:user) { Fabricate(:user) }
|
||||||
|
|
||||||
|
it 'returns empty string if no entry for user' do
|
||||||
|
expect(authenticator.description_for_user(user)).to eq("")
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'returns correct information' do
|
||||||
|
association = UserAssociatedAccount.create!(user: user, provider_name: 'myauth', provider_uid: "1234", info: { nickname: "somenickname", email: "test@domain.tld", name: "bestname" })
|
||||||
|
expect(authenticator.description_for_user(user)).to eq('test@domain.tld')
|
||||||
|
association.update(info: { nickname: "somenickname", name: "bestname" })
|
||||||
|
expect(authenticator.description_for_user(user)).to eq('somenickname')
|
||||||
|
association.update(info: { nickname: "bestname" })
|
||||||
|
expect(authenticator.description_for_user(user)).to eq('bestname')
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe 'revoke' do
|
||||||
|
let(:user) { Fabricate(:user) }
|
||||||
|
|
||||||
|
it 'raises exception if no entry for user' do
|
||||||
|
expect { authenticator.revoke(user) }.to raise_error(Discourse::NotFound)
|
||||||
|
end
|
||||||
|
|
||||||
|
context "with valid record" do
|
||||||
|
before do
|
||||||
|
UserAssociatedAccount.create!(user: user, provider_name: 'myauth', provider_uid: "1234", info: { name: "somename" })
|
||||||
|
end
|
||||||
|
|
||||||
|
it 'revokes correctly' do
|
||||||
|
expect(authenticator.description_for_user(user)).to eq("somename")
|
||||||
|
expect(authenticator.can_revoke?).to eq(true)
|
||||||
|
expect(authenticator.revoke(user)).to eq(true)
|
||||||
|
|
||||||
|
expect(authenticator.description_for_user(user)).to eq("")
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
end
|
Loading…
Reference in New Issue