From d96a864436ec4a6bcb82f7921e3902642fa0a2df Mon Sep 17 00:00:00 2001
From: Dan Milne <d@nmilne.com>
Date: Wed, 29 Oct 2025 10:19:51 +1100
Subject: [PATCH] Improve finding the requested host's domain for setting the
 domain cookie

---
 Gemfile                                    |  3 ++
 Gemfile.lock                               |  1 +
 README.md                                  |  4 +-
 app/controllers/concerns/authentication.rb | 57 ++++++++++++----------
 4 files changed, 38 insertions(+), 27 deletions(-)

diff --git a/Gemfile b/Gemfile
index 6fab3d2..0360f9b 100644
--- a/Gemfile
+++ b/Gemfile
@@ -31,6 +31,9 @@ gem "rqrcode", "~> 3.1"
 # JWT for OIDC ID tokens
 gem "jwt", "~> 3.1"
 
+# Public Suffix List for domain parsing
+gem "public_suffix", "~> 6.0"
+
 # Windows does not include zoneinfo files, so bundle the tzinfo-data gem
 gem "tzinfo-data", platforms: %i[ windows jruby ]
 
diff --git a/Gemfile.lock b/Gemfile.lock
index 57f0586..3bcd83a 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -413,6 +413,7 @@ DEPENDENCIES
   kamal
   letter_opener
   propshaft
+  public_suffix (~> 6.0)
   puma (>= 5.0)
   rails (~> 8.1.0)
   rotp (~> 6.3)
diff --git a/README.md b/README.md
index 7572bc1..5b2a20a 100644
--- a/README.md
+++ b/README.md
@@ -28,8 +28,8 @@ Clinch sits in a sweet spot between two excellent open-source identity solutions
 ### Sign In
 [![Sign In](docs/screenshots/thumbs/1-signin.png)](docs/screenshots/1-signin.png)
 
-### Sign In with Error
-[![Sign In with Error](docs/screenshots/thumbs/2-signin.png)](docs/screenshots/2-signin.png)
+### Sign In with 2FA
+[![Sign In with 2FA](docs/screenshots/thumbs/2-signin.png)](docs/screenshots/2-signin.png)
 
 ### Users Management
 [![Users Management](docs/screenshots/thumbs/3-users.png)](docs/screenshots/3-users.png)
diff --git a/app/controllers/concerns/authentication.rb b/app/controllers/concerns/authentication.rb
index 5ef05a3..0f9874a 100644
--- a/app/controllers/concerns/authentication.rb
+++ b/app/controllers/concerns/authentication.rb
@@ -1,4 +1,5 @@
 require 'uri'
+require 'public_suffix'
 
 module Authentication
   extend ActiveSupport::Concern
@@ -73,37 +74,43 @@ module Authentication
       cookies.delete(:session_id)
     end
 
-    # Extract root domain for cross-subdomain cookies
+    # Extract root domain for cross-subdomain cookies in SSO forward_auth system.
+    #
+    # PURPOSE: Enables a single authentication session to work across multiple subdomains
+    # by setting cookies with the domain parameter (e.g., .example.com allows access from
+    # both app.example.com and api.example.com).
+    #
+    # CRITICAL: Returns nil for IP addresses and localhost - this is intentional!
+    # When accessing services by IP, there are no subdomains to share cookies with,
+    # and setting a domain cookie would break authentication.
+    #
+    # Uses the Public Suffix List (industry standard maintained by Mozilla) to
+    # correctly handle complex domain patterns like co.uk, com.au, appspot.com, etc.
+    #
     # Examples:
-    # - clinch.aapamilne.com -> .aapamilne.com
-    # - app.example.co.uk -> .example.co.uk
-    # - localhost -> nil (no domain setting for local development)
+    # - app.example.com -> .example.com (enables cross-subdomain SSO)
+    # - api.example.co.uk -> .example.co.uk (handles complex TLDs)
+    # - myapp.appspot.com -> .myapp.appspot.com (handles platform domains)
+    # - localhost -> nil (local development, no domain cookie)
+    # - 192.168.1.1 -> nil (IP access, no domain cookie - prevents SSO breakage)
+    #
+    # @param host [String] The request host (may include port)
+    # @return [String, nil] Root domain with leading dot for cookies, or nil for no domain setting
     def extract_root_domain(host)
       return nil if host.blank? || host.match?(/^(localhost|127\.0\.0\.1|::1)$/)
 
-      # Split hostname into parts
-      parts = host.split('.')
+      # Strip port number for domain parsing
+      host_without_port = host.split(':').first
 
-      # For normal domains like example.com, we need at least 2 parts
-      # For complex domains like co.uk, we need at least 3 parts
-      return nil if parts.length < 2
+      # Check if it's an IP address - if so, don't set domain cookie
+      return nil if host_without_port.match?(/^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$/)
 
-      # Extract root domain with leading dot for cross-subdomain cookies
-      if parts.length >= 3
-        # Check if it's a known complex TLD
-        complex_tlds = %w[co.uk com.au co.nz co.za co.jp]
-        second_level = "#{parts[-2]}.#{parts[-1]}"
-
-        if complex_tlds.include?(second_level)
-          # For complex TLDs, include more parts: app.example.co.uk -> .example.co.uk
-          root_parts = parts[-3..-1]
-          return ".#{root_parts.join('.')}"
-        end
-      end
-
-      # For regular domains: app.example.com -> .example.com
-      root_parts = parts[-2..-1]
-      ".#{root_parts.join('.')}"
+      # Use Public Suffix List for accurate domain parsing
+      domain = PublicSuffix.parse(host_without_port)
+      ".#{domain.domain}"
+    rescue PublicSuffix::DomainInvalid
+      # Fallback for invalid domains or IPs
+      nil
     end
 
     # Create a one-time token for forward auth to handle the race condition