-
Notifications
You must be signed in to change notification settings - Fork 29
feat(auth): allow login via reverse-proxy email header #1685
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
6a6fa1d
1557b9c
ae33a7c
d562aa5
2fe76f8
ff56081
a672b93
dc4a4c3
3ecc44b
4eeab4f
8c8ec17
ac053ed
1344550
748b0cd
afc3199
3778578
e8d008b
c4f99e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| module Authentication | ||
| extend ActiveSupport::Concern | ||
|
|
||
| REMOTE_HEADER_SSO_PROVIDER = "remote_user_header" | ||
|
|
||
| included do | ||
| before_action :set_request_details | ||
| before_action :authenticate_user! | ||
|
|
@@ -16,7 +18,17 @@ def skip_authentication(**options) | |
|
|
||
| private | ||
| def authenticate_user! | ||
| if session_record = find_session_by_cookie | ||
| cookie_session = find_session_by_cookie | ||
|
|
||
| if cookie_session && cookie_session_disagrees_with_header?(cookie_session) | ||
| cookie_session.destroy | ||
| cookies.delete(:session_token) | ||
| cookie_session = nil | ||
| end | ||
|
|
||
| if cookie_session | ||
| Current.session = cookie_session | ||
| elsif session_record = create_session_by_remote_header | ||
| Current.session = session_record | ||
| else | ||
| if self_hosted_first_login? | ||
|
|
@@ -27,6 +39,85 @@ def authenticate_user! | |
| end | ||
| end | ||
|
|
||
| def cookie_session_disagrees_with_header?(session) | ||
| email = trusted_remote_user_email | ||
| email.present? && session.user.email != email | ||
| end | ||
|
|
||
| def create_session_by_remote_header | ||
| return unless user_email = trusted_remote_user_email | ||
|
|
||
| user, created = find_or_create_remote_header_user(user_email) | ||
| if created | ||
| SsoAuditLog.log_jit_account_created!( | ||
| user: user, | ||
| provider: REMOTE_HEADER_SSO_PROVIDER, | ||
| request: request | ||
| ) | ||
| end | ||
| SsoAuditLog.log_login!( | ||
| user: user, | ||
| provider: REMOTE_HEADER_SSO_PROVIDER, | ||
| request: request | ||
| ) | ||
| create_session_for(user) | ||
| end | ||
|
|
||
| # Returns the email asserted by the upstream proxy, but only when the | ||
| # request passes all configured trust gates: self-hosted mode, header | ||
| # set, source IP in the trusted-proxies allowlist, shared-secret match | ||
| # (if configured), and email shape is valid. | ||
| def trusted_remote_user_email | ||
| return nil unless Rails.application.config.app_mode.self_hosted? | ||
|
|
||
| header_name = Rails.application.config.remote_user_header_email | ||
| return nil if header_name.blank? | ||
| return nil unless remote_user_proxy_trusted? | ||
| return nil unless remote_user_secret_valid? | ||
|
|
||
| email = request.headers[header_name]&.strip&.downcase | ||
| return nil if email.blank? | ||
| return nil unless URI::MailTo::EMAIL_REGEXP.match?(email) | ||
|
|
||
| end | ||
|
|
||
| def remote_user_proxy_trusted? | ||
| trusted = Rails.application.config.remote_user_trusted_proxies | ||
| peer_ip = IPAddr.new(request.env["REMOTE_ADDR"]) | ||
| trusted.any? { |range| range.include?(peer_ip) } | ||
| rescue IPAddr::Error | ||
| false | ||
| end | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| def remote_user_secret_valid? | ||
| expected = Rails.application.config.remote_user_shared_secret | ||
| return true if expected.blank? | ||
|
|
||
| provided = request.headers[Rails.application.config.remote_user_shared_secret_header].to_s | ||
| ActiveSupport::SecurityUtils.secure_compare(expected, provided) | ||
| end | ||
|
|
||
| def find_or_create_remote_header_user(user_email) | ||
| if user = User.find_by(email: user_email) | ||
| [ user, false ] | ||
| else | ||
| # Leave password_digest nil so the user can't fall back to local | ||
| # password login or password reset; the proxy is the only path in. | ||
| user = User.new | ||
| user.email = user_email | ||
| user.skip_password_validation = true | ||
| user.family = Family.new | ||
|
Comment on lines
+108
to
+110
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This JIT path creates users with Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Independently confirmed against head 40286d8. Generated by Claude Code |
||
| user.role = User.role_for_new_family_creator(fallback_role: :admin) | ||
| begin | ||
| user.save! | ||
| [ user, true ] | ||
| rescue ActiveRecord::RecordNotUnique | ||
| [ User.find_by!(email: user_email), false ] | ||
| end | ||
| end | ||
| end | ||
|
|
||
| def find_session_by_cookie | ||
| cookie_value = cookies.signed[:session_token] | ||
|
|
||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Asked about this in our Discord here. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,4 +87,42 @@ class PasswordResetsControllerTest < ActionDispatch::IntegrationTest | |
| sso_user.reload | ||
| assert_nil sso_user.password_digest, "SSO-only user should still have nil password_digest" | ||
| end | ||
|
|
||
| # Security: users provisioned with no local password (e.g. via the | ||
| # remote-header proxy auth path) must not be able to acquire a | ||
| # password through reset, even when they have no OIDC identity. | ||
| test "create does not send email for password-less user without OIDC identity" do | ||
| jit_user = create_jit_user | ||
|
|
||
| assert_no_enqueued_emails do | ||
| post password_reset_path, params: { email: jit_user.email } | ||
| end | ||
|
|
||
| assert_redirected_to new_password_reset_url(step: "pending") | ||
| end | ||
|
|
||
| test "update blocks password setting for password-less user without OIDC identity" do | ||
| jit_user = create_jit_user | ||
| token = jit_user.generate_token_for(:password_reset) | ||
|
|
||
| patch password_reset_path(token: token), | ||
| params: { user: { password: "NewSecure1!", password_confirmation: "NewSecure1!" } } | ||
|
|
||
| assert_redirected_to new_session_path | ||
| assert_match(/SSO/, flash[:alert]) | ||
| assert_match(/authentication/, flash[:alert]) | ||
| assert_match(/contact your administrator/, flash[:alert]) | ||
| jit_user.reload | ||
| assert_nil jit_user.password_digest, "password-less user should still have nil password_digest after update attempt" | ||
| end | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| private | ||
| def create_jit_user(email: "[email protected]") | ||
| User.create!( | ||
| email: email, | ||
| family: Family.create!, | ||
| role: :admin, | ||
| skip_password_validation: true | ||
| ) | ||
| end | ||
| end | ||
Uh oh!
There was an error while loading. Please reload this page.