Skip to content

Commit 253a5ce

Browse files
authored
Scope the passwordless invite lookup to its organization (#4025)
`OrganizationRole.create_passwordless` reuses an outstanding invite instead of creating a second role for the same person — `CallbackJobs::AfterUserCreateJob` can reach it for a user who was already invited. But it found that invite with `find_by_invited_email`, no organization in the lookup, so an invite at *any* organization matched: the org being joined silently got no role, and the caller was handed a role belonging to somewhere else. `Saml::AssertionProcessor#provision_user` reads `.user` off the return value, so an SSO login for a previously-invited email provisions against the wrong org. - **The lookup takes `organization_id`.** All three callers already pass it. - **It normalizes the email the way `set_calculated_attributes` stores it**, so an invite recorded as `Person@Example.com` still matches and isn't duplicated.
1 parent 804ed24 commit 253a5ce

2 files changed

Lines changed: 38 additions & 2 deletions

File tree

app/models/organization_role.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,10 @@ def self.role_types
5858
def self.create_passwordless(**create_attrs)
5959
new_passwordless_attrs = {skip_processing: true, role: "member"}
6060
if create_attrs[:invited_email].present? # This should always be present...
61-
# We need to check for existing organization_roles because the CallbackJobs::AfterUserCreateJob calls this
62-
existing_organization_role = OrganizationRole.find_by_invited_email(create_attrs[:invited_email])
61+
# We need to check for existing organization_roles because the CallbackJobs::AfterUserCreateJob calls this.
62+
# Scoped to the organization - an invite to a different one says nothing about this one.
63+
existing_organization_role = find_by(organization_id: create_attrs[:organization_id],
64+
invited_email: EmailNormalizer.normalize(create_attrs[:invited_email]))
6365
return existing_organization_role if existing_organization_role.present?
6466
end
6567
organization_role = create!(new_passwordless_attrs.merge(create_attrs))

spec/models/organization_role_spec.rb

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,40 @@
4444
end
4545
end
4646

47+
describe ".create_passwordless" do
48+
let(:invited_email) { "student@sso.edu" }
49+
let(:organization) do
50+
FactoryBot.create(:organization_with_organization_features, enabled_feature_slugs: "passwordless_users")
51+
end
52+
let(:create_passwordless) { OrganizationRole.create_passwordless(organization_id: organization.id, invited_email:) }
53+
54+
it "creates the role and its passwordless user" do
55+
expect(create_passwordless.organization_id).to eq organization.id
56+
expect(create_passwordless.user.email).to eq invited_email
57+
expect(create_passwordless.user.passwordless_user?).to be_truthy
58+
end
59+
60+
it "returns the existing role rather than a second one" do
61+
existing = create_passwordless
62+
expect {
63+
expect(OrganizationRole.create_passwordless(organization_id: organization.id, invited_email: "Student@SSO.edu "))
64+
.to eq existing
65+
}.to_not change(OrganizationRole, :count)
66+
end
67+
68+
context "invited to a different organization" do
69+
let!(:other_organization_role) do
70+
FactoryBot.create(:organization_role, invited_email:,
71+
organization: FactoryBot.create(:organization))
72+
end
73+
74+
it "creates the role for this organization anyway" do
75+
expect(create_passwordless.organization_id).to eq organization.id
76+
expect(create_passwordless.id).to_not eq other_organization_role.id
77+
end
78+
end
79+
end
80+
4781
describe "admin?" do
4882
context "admin" do
4983
it "returns true" do

0 commit comments

Comments
 (0)