fix(auth): reconcile smbCloud user by email when the auth_user id differs
find_or_create_from_smbcloud keyed only on smbcloud_id, so when smbCloud's auth_user id for an email differed from an older local record (e.g. a GitHub social login resolved to a different auth_user than the user's existing email/password record), it tried to create a duplicate User and failed on the unique email — surfacing as "An unexpected error occurred" on the GitHub callback (and would wedge email/password sign-in too). Now, when the smbcloud_id is unseen but the authenticated email already belongs to a local user, adopt that record and move it onto the new smbcloud_id (smbCloud is the identity authority). Adds model specs for the reconcile path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Seto Elkahfi committed
Jun 30, 2026 at 21:16 UTC
1b415d7ee06f0c3641d1859cfde9058e1116c0de
2 files changed
+61
app/models/user.rb
+15
@@ -98,6 +98,21 @@ class User < ApplicationRecord
98
99
user = find_or_initialize_by(smbcloud_id: smbcloud_id)
100
101
+ # smbCloud is the identity authority. If we've never seen this smbcloud_id
102
+ # but the authenticated email already belongs to a local user, adopt that
103
+ # record and move it onto the new smbcloud_id rather than failing on the
104
+ # unique email constraint. The smbCloud auth_user id for an email can change
105
+ # (e.g. the account was recreated, or a social login resolved to a different
106
+ # auth_user than an older local record), and this can otherwise wedge both
107
+ # GitHub and email/password sign-in for that user.
108
+ if user.new_record? && email.present?
109
+ existing = find_by(email: email)
110
+ if existing
111
+ user = existing
112
+ user.smbcloud_id = smbcloud_id
113
+ end
114
+ end
115
+
116
# Always sync email and token in case they changed on the auth server.
117
user.email = email if email.present?
118
user.access_token = access_token if access_token.present?
spec/models/user_smbcloud_spec.rb
new
+46
@@ -0,0 +1,46 @@
1
+# frozen_string_literal: true
2
+
3
+require "rails_helper"
4
+
5
+RSpec.describe User, ".find_or_create_from_smbcloud" do
6
+ it "creates a new user when the smbcloud_id is unseen and the email is free" do
7
+ expect do
8
+ user = User.find_or_create_from_smbcloud({ id: 100, email: "new@example.com" })
9
+ expect(user.smbcloud_id).to eq(100)
10
+ expect(user.email).to eq("new@example.com")
11
+ end.to change(User, :count).by(1)
12
+ end
13
+
14
+ it "returns the same record for a known smbcloud_id" do
15
+ existing = User.create!(smbcloud_id: 200, email: "known@example.com", username: "known")
16
+ expect do
17
+ user = User.find_or_create_from_smbcloud({ id: 200, email: "known@example.com" })
18
+ expect(user.id).to eq(existing.id)
19
+ end.not_to change(User, :count)
20
+ end
21
+
22
+ # The regression: smbCloud's auth_user id for an email can differ from the id
23
+ # stored on an older local record (e.g. a social login resolves to a different
24
+ # auth_user). Keying only on smbcloud_id would try to create a duplicate and
25
+ # fail on the unique email, wedging sign-in.
26
+ it "adopts an existing user by email and moves it to the new smbcloud_id" do
27
+ existing = User.create!(smbcloud_id: 18, email: "person@example.com", username: "person")
28
+
29
+ user = nil
30
+ expect do
31
+ user = User.find_or_create_from_smbcloud({ id: 86, email: "person@example.com" })
32
+ end.not_to change(User, :count)
33
+
34
+ expect(user.id).to eq(existing.id)
35
+ expect(user.smbcloud_id).to eq(86)
36
+ expect(user.username).to eq("person") # username preserved
37
+ expect(existing.reload.smbcloud_id).to eq(86)
38
+ end
39
+
40
+ it "matches the existing email case-insensitively" do
41
+ User.create!(smbcloud_id: 18, email: "mixed@example.com", username: "mixed")
42
+ user = User.find_or_create_from_smbcloud({ id: 99, email: "MIXED@example.com" })
43
+ expect(user.smbcloud_id).to eq(99)
44
+ expect(User.where("lower(email) = ?", "mixed@example.com").count).to eq(1)
45
+ end
46
+end