Skip to content

Commit fdb869e

Browse files
eld120claude
andcommitted
Scope the passwordless invite lookup to its organization
create_passwordless reuses an outstanding invite rather than creating a second role for the same person, but it looked the invite up by email alone. An invite at any other organization matched and was returned, so the organization actually being joined never got a role - and the caller got back a role belonging to somewhere else. Saml::AssertionProcessor reads .user off that role, so an SSO login could provision against the wrong organization entirely. The lookup now takes organization_id, and normalizes the email the way the record stores it so a differently-cased invite still matches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 1e49578 commit fdb869e

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)