diff --git a/app/assets/javascripts/discourse/models/login-method.js.es6 b/app/assets/javascripts/discourse/models/login-method.js.es6 index da5282a4ab8..819abeb5bbe 100644 --- a/app/assets/javascripts/discourse/models/login-method.js.es6 +++ b/app/assets/javascripts/discourse/models/login-method.js.es6 @@ -117,7 +117,7 @@ export function findAll(siteSettings, capabilities, isMobileDevice) { params.displayPopup = true; } - if (["facebook"].includes(name)) { + if (["facebook", "google_oauth2"].includes(name)) { params.canConnect = true; } diff --git a/lib/auth/google_oauth2_authenticator.rb b/lib/auth/google_oauth2_authenticator.rb index 3434c6b4b4c..6498715ffc8 100644 --- a/lib/auth/google_oauth2_authenticator.rb +++ b/lib/auth/google_oauth2_authenticator.rb @@ -13,7 +13,26 @@ class Auth::GoogleOAuth2Authenticator < Auth::Authenticator info&.email || info&.name || "" end - def after_authenticate(auth_hash) + def can_revoke? + true + end + + def revoke(user, skip_remote: false) + info = GoogleUserInfo.find_by(user_id: user.id) + raise Discourse::NotFound if info.nil? + + # We get a temporary token from google upon login but do not need it, and do not store it. + # Therefore we do not have any way to revoke the token automatically on google's end + + info.destroy! + true + end + + def can_connect_existing_user? + true + end + + def after_authenticate(auth_hash, existing_account: nil) session_info = parse_hash(auth_hash) google_hash = session_info[:google] @@ -25,7 +44,14 @@ class Auth::GoogleOAuth2Authenticator < Auth::Authenticator result.extra_data = google_hash user_info = ::GoogleUserInfo.find_by(google_user_id: google_hash[:google_user_id]) - result.user = user_info.try(:user) + + if existing_account && (user_info.nil? || existing_account.id != user_info.user_id) + user_info.destroy! if user_info + result.user = existing_account + user_info = GoogleUserInfo.create!({ user_id: result.user.id }.merge(google_hash)) + else + result.user = user_info&.user + end if !result.user && !result.email.blank? && result.email_valid result.user = User.find_by_email(result.email) diff --git a/spec/components/auth/google_oauth2_authenticator_spec.rb b/spec/components/auth/google_oauth2_authenticator_spec.rb index 525d65f9aa9..06962d0c15b 100644 --- a/spec/components/auth/google_oauth2_authenticator_spec.rb +++ b/spec/components/auth/google_oauth2_authenticator_spec.rb @@ -58,6 +58,35 @@ describe Auth::GoogleOAuth2Authenticator do expect(result.user.id).to eq(user.id) end + it 'can connect to a different existing user account' do + authenticator = Auth::GoogleOAuth2Authenticator.new + user1 = Fabricate(:user) + user2 = Fabricate(:user) + + GoogleUserInfo.create!(user_id: user1.id, google_user_id: 100) + + hash = { + uid: "100", + info: { + name: "John Doe", + email: user1.email + }, + extra: { + raw_info: { + email: user1.email, + email_verified: true, + name: "John Doe" + } + } + } + + result = authenticator.after_authenticate(hash, existing_account: user2) + + expect(result.user.id).to eq(user2.id) + expect(GoogleUserInfo.exists?(user_id: user1.id)).to eq(false) + expect(GoogleUserInfo.exists?(user_id: user2.id)).to eq(true) + end + it 'can create a proper result for non existing users' do hash = { uid: "123456789", @@ -81,4 +110,21 @@ describe Auth::GoogleOAuth2Authenticator do expect(result.extra_data[:name]).to eq("Jane Doe") end end + + context 'revoke' do + let(:user) { Fabricate(:user) } + let(:authenticator) { Auth::GoogleOAuth2Authenticator.new } + + it 'raises exception if no entry for user' do + expect { authenticator.revoke(user) }.to raise_error(Discourse::NotFound) + end + + it 'revokes correctly' do + GoogleUserInfo.create!(user_id: user.id, google_user_id: 12345, email: 'someuser@somedomain.tld') + 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