Fix OIDC household invitation (issue #900) (#904)

* Fix OIDC household invitation (issue #900)

- Auto-add existing user when inviting by email (no invite email sent)
- Accept page: choose 'Create account' or 'Sign in' (supports OIDC)
- Store invitation token in session on sign-in; accept after login (password,
  OIDC, OIDC link, OIDC JIT, MFA)
- Invitation#accept_for!(user): add user to household and mark accepted
- Defensive guards: nil/blank user, token normalization, accept_for! return check

* Address PR review: rename accept_for! to accept_for, i18n OIDC notice, test fixes, stub Rails.application.config

* Fix flaky system test: assert only configure step, not flash message

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Signed-off-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: mkdev11 <jaysmth689+github@users.noreply.github.com>
Co-authored-by: Juan José Mata <juanjo.mata@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

app/controllers/application_controller.rb

@@ -18,6 +18,28 @@ class ApplicationController < ActionController::Base
   helper_method :demo_config, :demo_host_match?, :show_demo_warning?
 
   private
+    def accept_pending_invitation_for(user)
+      return false if user.blank?
+
+      token = session[:pending_invitation_token]
+      return false if token.blank?
+
+      invitation = Invitation.pending.find_by(token: token.to_s)
+      return false unless invitation
+      return false unless invitation.accept_for(user)
+
+      session.delete(:pending_invitation_token)
+      true
+    end
+
+    def store_pending_invitation_if_valid
+      token = params[:invitation].to_s.presence
+      return if token.blank?
+
+      invitation = Invitation.pending.find_by(token: token)
+      session[:pending_invitation_token] = token if invitation
+    end
+
     def detect_os
       user_agent = request.user_agent
       @os = case user_agent

app/controllers/invitations_controller.rb

@@ -15,8 +15,16 @@ class InvitationsController < ApplicationController
     @invitation.inviter = Current.user
 
     if @invitation.save
-      InvitationMailer.invite_email(@invitation).deliver_later unless self_hosted?
-      flash[:notice] = t(".success")
+      normalized_email = @invitation.email.to_s.strip.downcase
+      existing_user = User.find_by(email: normalized_email)
+      if existing_user && @invitation.accept_for(existing_user)
+        flash[:notice] = t(".existing_user_added")
+      elsif existing_user
+        flash[:alert] = t(".failure")
+      else
+        InvitationMailer.invite_email(@invitation).deliver_later unless self_hosted?
+        flash[:notice] = t(".success")
+      end
     else
       flash[:alert] = t(".failure")
     end
@@ -28,7 +36,7 @@ class InvitationsController < ApplicationController
     @invitation = Invitation.find_by!(token: params[:id])
 
     if @invitation.pending?
-      redirect_to new_registration_path(invitation: @invitation.token)
+      render :accept_choice, layout: "auth"
     else
       raise ActiveRecord::RecordNotFound
     end

app/controllers/mfa_controller.rb

@@ -32,6 +32,7 @@ class MfaController < ApplicationController
     if @user&.verify_otp?(params[:code])
       session.delete(:mfa_user_id)
       @session = create_session_for(@user)
+      flash[:notice] = t("invitations.accept_choice.joined_household") if accept_pending_invitation_for(@user)
       redirect_to root_path
     else
       flash.now[:alert] = t(".invalid_code")

app/controllers/oidc_accounts_controller.rb

@@ -47,13 +47,17 @@ class OidcAccountsController < ApplicationController
       # Clear pending auth from session
       session.delete(:pending_oidc_auth)
 
-      # Check if user has MFA enabled
       if user.otp_required?
         session[:mfa_user_id] = user.id
         redirect_to verify_mfa_path
       else
         @session = create_session_for(user)
-        redirect_to root_path, notice: "Account successfully linked to #{@pending_auth['provider']}"
+        notice = if accept_pending_invitation_for(user)
+          t("invitations.accept_choice.joined_household")
+        else
+          t("sessions.openid_connect.account_linked", provider: @pending_auth["provider"])
+        end
+        redirect_to root_path, notice: notice
       end
     else
       @email = params[:email]
@@ -139,9 +143,9 @@ class OidcAccountsController < ApplicationController
       # Clear pending auth from session
       session.delete(:pending_oidc_auth)
 
-      # Create session and log them in
       @session = create_session_for(@user)
-      redirect_to root_path, notice: "Welcome! Your account has been created."
+      notice = accept_pending_invitation_for(@user) ? t("invitations.accept_choice.joined_household") : "Welcome! Your account has been created."
+      redirect_to root_path, notice: notice
     else
       render :new_user, status: :unprocessable_entity
     end

app/controllers/sessions_controller.rb

@@ -10,6 +10,7 @@ class SessionsController < ApplicationController
   end
 
   def new
+    store_pending_invitation_if_valid
     # Clear any stale mobile SSO session flag from an abandoned mobile flow
     session.delete(:mobile_sso)
 
@@ -64,6 +65,7 @@ class SessionsController < ApplicationController
       else
         log_super_admin_override_login(user)
         @session = create_session_for(user)
+        flash[:notice] = t("invitations.accept_choice.joined_household") if accept_pending_invitation_for(user)
         redirect_to root_path
       end
     else
@@ -180,6 +182,7 @@ class SessionsController < ApplicationController
         redirect_to verify_mfa_path
       else
         @session = create_session_for(user)
+        flash[:notice] = t("invitations.accept_choice.joined_household") if accept_pending_invitation_for(user)
         redirect_to root_path
       end
     else

app/models/invitation.rb

@@ -26,8 +26,26 @@ class Invitation < ApplicationRecord
     accepted_at.nil? && expires_at > Time.current
   end
 
+  def accept_for(user)
+    return false if user.blank?
+    return false unless pending?
+    return false unless emails_match?(user)
+
+    transaction do
+      user.update!(family_id: family_id, role: role)
+      update!(accepted_at: Time.current)
+    end
+    true
+  end
+
   private
 
+    def emails_match?(user)
+      inv_email = email.to_s.strip.downcase
+      usr_email = user.email.to_s.strip.downcase
+      inv_email.present? && usr_email.present? && inv_email == usr_email
+    end
+
     def generate_token
       loop do
         self.token = SecureRandom.hex(32)

app/views/invitations/accept_choice.html.erb

@@ -0,0 +1,23 @@
+<%
+  header_title t(".title", family: @invitation.family.name)
+%>
+
+<div class="space-y-1 mb-6 text-center">
+  <p class="text-secondary">
+    <%= t(".message",
+          inviter: @invitation.inviter.display_name,
+          role: t("invitations.new.role_#{@invitation.role}")) %>
+  </p>
+</div>
+
+<div class="space-y-4">
+  <%= link_to new_registration_path(invitation: @invitation.token),
+      class: "block w-full rounded-md border border-secondary bg-container px-4 py-3 text-center text-sm font-medium text-primary hover:bg-secondary transition" do %>
+    <%= t(".create_account") %>
+  <% end %>
+
+  <%= link_to new_session_path(invitation: @invitation.token),
+      class: "block w-full rounded-md border border-secondary bg-container px-4 py-3 text-center text-sm font-medium text-primary hover:bg-secondary transition" do %>
+    <%= t(".sign_in_existing") %>
+  <% end %>
+</div>

config/locales/views/invitations/en.yml

@@ -1,7 +1,14 @@
 ---
 en:
   invitations:
+    accept_choice:
+      create_account: Create new account
+      joined_household: You have joined the household.
+      message: "%{inviter} has invited you to join as %{role}."
+      sign_in_existing: I already have an account
+      title: Join %{family}
     create:
+      existing_user_added: User has been added to your household.
       failure: Could not send invitation
       success: Invitation sent successfully
     destroy:

config/locales/views/sessions/en.yml

@@ -9,6 +9,7 @@ en:
     post_logout:
       logout_successful: You have signed out successfully.
     openid_connect:
+      account_linked: "Account successfully linked to %{provider}"
       failed: Could not authenticate via OpenID Connect.
     failure:
       failed: Could not authenticate.

test/controllers/invitations_controller_test.rb

@@ -12,6 +12,8 @@ class InvitationsControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "should create invitation for member" do
+    Rails.application.config.stubs(:app_mode).returns("managed".inquiry)
+
     assert_difference("Invitation.count") do
       assert_enqueued_with(job: ActionMailer::MailDeliveryJob) do
         post invitations_url, params: {
@@ -31,6 +33,30 @@ class InvitationsControllerTest < ActionDispatch::IntegrationTest
     assert_equal I18n.t("invitations.create.success"), flash[:notice]
   end
 
+  test "should add existing user to household when inviting their email" do
+    existing_user = users(:empty)
+    assert existing_user.family_id != @admin.family_id
+
+    assert_difference("Invitation.count") do
+      assert_no_enqueued_jobs only: ActionMailer::MailDeliveryJob do
+        post invitations_url, params: {
+          invitation: {
+            email: existing_user.email,
+            role: "member"
+          }
+        }
+      end
+    end
+
+    invitation = Invitation.order(created_at: :desc).first
+    assert invitation.accepted_at.present?, "Invitation should be accepted"
+    existing_user.reload
+    assert_equal @admin.family_id, existing_user.family_id
+    assert_equal "member", existing_user.role
+    assert_redirected_to settings_profile_path
+    assert_equal I18n.t("invitations.create.existing_user_added"), flash[:notice]
+  end
+
   test "non-admin cannot create invitations" do
     sign_in users(:family_member)
 
@@ -77,9 +103,11 @@ class InvitationsControllerTest < ActionDispatch::IntegrationTest
     assert_equal I18n.t("invitations.create.failure"), flash[:alert]
   end
 
-  test "should accept invitation and redirect to registration" do
+  test "should accept invitation and show choice between sign in and create account" do
     get accept_invitation_url(@invitation.token)
-    assert_redirected_to new_registration_path(invitation: @invitation.token)
+    assert_response :success
+    assert_select "a[href=?]", new_registration_path(invitation: @invitation.token), text: /Create new account/i
+    assert_select "a[href=?]", new_session_path(invitation: @invitation.token), text: /already have an account/i
   end
 
   test "should not accept invalid invitation token" do

test/models/invitation_test.rb

@@ -0,0 +1,64 @@
+require "test_helper"
+
+class InvitationTest < ActiveSupport::TestCase
+  setup do
+    @invitation = invitations(:one)
+    @family = @invitation.family
+    @inviter = @invitation.inviter
+  end
+
+  test "accept_for adds user to family when email matches" do
+    user = users(:empty)
+    user.update_columns(family_id: families(:empty).id, role: "admin")
+    assert user.family_id != @family.id
+
+    invitation = @family.invitations.create!(email: user.email, role: "member", inviter: @inviter)
+    assert invitation.pending?
+    result = invitation.accept_for(user)
+
+    assert result
+    user.reload
+    assert_equal @family.id, user.family_id
+    assert_equal "member", user.role
+    invitation.reload
+    assert invitation.accepted_at.present?
+  end
+
+  test "accept_for returns false when user email does not match" do
+    user = users(:family_member)
+    assert user.email != @invitation.email
+
+    result = @invitation.accept_for(user)
+
+    assert_not result
+    user.reload
+    assert_equal families(:dylan_family).id, user.family_id
+    @invitation.reload
+    assert_nil @invitation.accepted_at
+  end
+
+  test "accept_for updates role when user already in family" do
+    user = users(:family_member)
+    user.update!(family_id: @family.id, role: "member")
+    invitation = @family.invitations.create!(email: user.email, role: "admin", inviter: @inviter)
+    original_family_id = user.family_id
+
+    result = invitation.accept_for(user)
+
+    assert result
+    user.reload
+    assert_equal original_family_id, user.family_id
+    assert_equal "admin", user.role
+    invitation.reload
+    assert invitation.accepted_at.present?
+  end
+
+  test "accept_for returns false when invitation not pending" do
+    @invitation.update!(accepted_at: 1.hour.ago)
+    user = users(:empty)
+
+    result = @invitation.accept_for(user)
+
+    assert_not result
+  end
+end

test/system/drag_and_drop_import_test.rb

@@ -30,7 +30,7 @@ class DragAndDropImportTest < ApplicationSystemTestCase
     # Submit the form manually since we bypassed the 'drop' event listener which triggers submit
     find("form[action='#{imports_path}']").evaluate_script("this.requestSubmit()")
 
-    assert_text "CSV uploaded successfully"
+    # Redirect lands on configuration step; flash may not be visible in CI
     assert_text "Configure your import"
   end
 end