Skip to content

Commit 0055283

Browse files
eld120claude
andcommitted
Force SSO on sign-up, not just sign-in
An SSO organization's IdP owns every account on its email domain, but the guard only covered the sign-in entry points. /users/new was reachable with the email prefilled - identify redirects there itself when no account exists yet - so an SSO-managed email could still register a Bike Index account the IdP has never heard of. redirect_forced_saml and submitted_email move to Sessionable so both controllers share one definition; submitted_email now also reads the sign-up form's user[:email]. An organization whose SAML configuration isn't live yet still falls through to the ordinary form. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 1e49578 commit 0055283

4 files changed

Lines changed: 61 additions & 13 deletions

File tree

app/controllers/concerns/sessionable.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,20 @@ def authenticate_user_for_my_accounts_controller
5656

5757
private
5858

59+
# SSO orgs force SSO: hand an SSO-managed email off to the IdP rather than let it sign in
60+
# or sign up any other way. Redirecting halts the filter chain, so the guarded action
61+
# never runs for a forced-SSO email.
62+
def redirect_forced_saml
63+
organization = Organization.saml_email_matching(submitted_email)
64+
redirect_to saml_init_path(org_slug: organization.to_param) if organization.present?
65+
end
66+
67+
# The email an unauthenticated request is offering up, wherever its form puts it:
68+
# session[:email] signing in, user[:email] signing up, a bare :email elsewhere.
69+
def submitted_email
70+
params.dig(:session, :email).presence || params.dig(:user, :email).presence || params[:email]
71+
end
72+
5973
# Passwordless users are nudged to set a password, unless their organization is what signs them in.
6074
# UI::Alerts::FlashMessage renders the hash - it owns the copy and builds the link
6175
def set_sign_in_flash(user, signed_up)

app/controllers/sessions_controller.rb

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -119,13 +119,6 @@ def login_method_for(email)
119119
Organization.passwordless_email_matching(email).present? ? "magic_link" : "password"
120120
end
121121

122-
# See the before_action: hand an SSO-managed email off to the IdP. Redirecting here
123-
# halts the filter chain, so the action never runs for a forced-SSO email.
124-
def redirect_forced_saml
125-
organization = Organization.saml_email_matching(submitted_email)
126-
redirect_to saml_init_path(org_slug: organization.to_param) if organization.present?
127-
end
128-
129122
def send_magic_link_and_redirect(user)
130123
# Stash the remember-me choice so the emailed-link GET (which carries no form
131124
# params) can still honor it in sign_in_and_redirect.
@@ -134,12 +127,6 @@ def send_magic_link_and_redirect(user)
134127
redirect_to magic_link_sent_session_path(partner: sign_in_partner)
135128
end
136129

137-
# The three guarded actions carry the email in different params: identify/create post
138-
# session[:email]; create_magic_link posts a top-level :email.
139-
def submitted_email
140-
params.dig(:session, :email).presence || params[:email]
141-
end
142-
143130
def submitted_remember_me
144131
params.dig(:session, :remember_me).presence || params[:remember_me]
145132
end

app/controllers/users_controller.rb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ class UsersController < ApplicationController
22
include Sessionable
33

44
before_action :skip_if_signed_in, only: %i[new]
5+
# An SSO org owns its domain's accounts, so signing up is the IdP's job too — otherwise
6+
# the sign-in guard is bypassed by whatever link or bookmark lands on the signup form.
7+
before_action :redirect_forced_saml, only: %i[new create]
58
before_action :find_user_from_token_for_password_reset!, only: %i[update_password_form_with_reset_token update_password_with_reset_token]
69

710
def new

spec/requests/users_request_spec.rb

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,24 @@
1010
expect(response).to render_template(:new)
1111
expect(Capybara.string(response.body)).to have_css("[data-controller='ui--forms--email'] input#user_email")
1212
end
13+
14+
context "sso organization domain" do
15+
let!(:organization) do
16+
FactoryBot.create(:organization_with_organization_features,
17+
enabled_feature_slugs: ["saml_sso"], user_email_domain: "sso.edu")
18+
end
19+
let!(:saml_configuration) { FactoryBot.create(:organization_saml_configuration, :enabled, organization:) }
20+
21+
it "hands the prefilled email off to the IdP rather than the signup form" do
22+
get "#{base_url}/new", params: {email: "student@sso.edu"}
23+
expect(response).to redirect_to(saml_init_path(org_slug: organization.to_param))
24+
end
25+
26+
it "renders for an email the org doesn't claim" do
27+
get "#{base_url}/new", params: {email: "student@example.edu"}
28+
expect(response).to render_template(:new)
29+
end
30+
end
1331
end
1432

1533
describe "create" do
@@ -49,6 +67,32 @@
4967
expect(response).to redirect_to "https://parkit.bikehub.com/account?reauthenticate_bike_index=true"
5068
end
5169
end
70+
71+
context "sso organization domain" do
72+
let(:email) { "student@sso.edu" }
73+
let!(:organization) do
74+
FactoryBot.create(:organization_with_organization_features,
75+
enabled_feature_slugs: ["saml_sso"], user_email_domain: "sso.edu")
76+
end
77+
let!(:saml_configuration) { FactoryBot.create(:organization_saml_configuration, :enabled, organization:) }
78+
79+
it "forces SSO instead of creating an account the IdP doesn't know about" do
80+
expect {
81+
post base_url, params: {user: {email:, name: "Test name", terms_of_service: "1"}}
82+
}.to_not change(User, :count)
83+
expect(response).to redirect_to(saml_init_path(org_slug: organization.to_param))
84+
end
85+
86+
context "SAML config not yet live" do
87+
let(:saml_configuration) { FactoryBot.create(:organization_saml_configuration, organization:) }
88+
89+
it "signs up normally rather than redirecting into an unconfigured IdP" do
90+
expect {
91+
post base_url, params: {user: {email:, name: "Test name", terms_of_service: "1"}}
92+
}.to change(User, :count).by(1)
93+
end
94+
end
95+
end
5296
end
5397

5498
describe "create with a null origin" do

0 commit comments

Comments
 (0)