From 4df2eee4d9b8a880cbe72e7e899c9444687c6193 Mon Sep 17 00:00:00 2001
From: Dan Milne <d@nmilne.com>
Date: Sun, 9 Nov 2025 12:22:41 +1100
Subject: [PATCH] Bug fix for domain names with empty string instead of null.
 Form errors and some security fixes

---
 app/controllers/application_controller.rb       |  3 +++
 app/controllers/oidc_controller.rb              |  4 +---
 app/models/application.rb                       |  5 ++++-
 app/views/shared/_form_errors.html.erb          |  4 ++--
 .../20251109011443_fix_empty_domain_patterns.rb | 17 +++++++++++++++++
 db/schema.rb                                    |  2 +-
 6 files changed, 28 insertions(+), 7 deletions(-)
 create mode 100644 db/migrate/20251109011443_fix_empty_domain_patterns.rb

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 5f38f02..5b17219 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -5,4 +5,7 @@ class ApplicationController < ActionController::Base
 
   # Changes to the importmap will invalidate the etag for HTML responses
   stale_when_importmap_changes
+
+  # CSRF protection
+  protect_from_forgery with: :exception
 end
diff --git a/app/controllers/oidc_controller.rb b/app/controllers/oidc_controller.rb
index 3ed9393..b3f36a1 100644
--- a/app/controllers/oidc_controller.rb
+++ b/app/controllers/oidc_controller.rb
@@ -408,9 +408,7 @@ class OidcController < ApplicationController
                         when "plain"
                           code_verifier
                         when "S256"
-                          Digest::SHA256.base64digest(code_verifier)
-                            .tr("+/", "-_")
-                            .tr("=", "")
+                          Base64.urlsafe_encode64(Digest::SHA256.digest(code_verifier), padding: false)
                         else
                           return {
                             valid: false,
diff --git a/app/models/application.rb b/app/models/application.rb
index c7c1d60..fd1ef60 100644
--- a/app/models/application.rb
+++ b/app/models/application.rb
@@ -18,7 +18,10 @@ class Application < ApplicationRecord
   validates :landing_url, format: { with: URI::regexp(%w[http https]), allow_nil: true, message: "must be a valid URL" }
 
   normalizes :slug, with: ->(slug) { slug.strip.downcase }
-  normalizes :domain_pattern, with: ->(pattern) { pattern&.strip&.downcase }
+  normalizes :domain_pattern, with: ->(pattern) {
+    normalized = pattern&.strip&.downcase
+    normalized.blank? ? nil : normalized
+  }
 
   before_validation :generate_client_credentials, on: :create, if: :oidc?
 
diff --git a/app/views/shared/_form_errors.html.erb b/app/views/shared/_form_errors.html.erb
index 097cfd8..8b6a99c 100644
--- a/app/views/shared/_form_errors.html.erb
+++ b/app/views/shared/_form_errors.html.erb
@@ -1,5 +1,5 @@
-<%# Usage: <%= render "shared/form_errors", object: @user %> %>
-<%# Usage: <%= render "shared/form_errors", form: form %> %>
+<%# Usage: render "shared/form_errors", object: @user %>
+<%# Usage: render "shared/form_errors", form: form %>
 
 <% form_object = form.respond_to?(:object) ? form.object : (object || form) %>
 <% if form_object&.errors&.any? %>
diff --git a/db/migrate/20251109011443_fix_empty_domain_patterns.rb b/db/migrate/20251109011443_fix_empty_domain_patterns.rb
new file mode 100644
index 0000000..6cc8672
--- /dev/null
+++ b/db/migrate/20251109011443_fix_empty_domain_patterns.rb
@@ -0,0 +1,17 @@
+class FixEmptyDomainPatterns < ActiveRecord::Migration[8.1]
+  def up
+    # Convert empty string domain_patterns to NULL
+    # This fixes a unique constraint issue where multiple OIDC apps
+    # had empty string domain_patterns, causing uniqueness violations
+    execute <<-SQL
+      UPDATE applications
+      SET domain_pattern = NULL
+      WHERE domain_pattern = ''
+    SQL
+  end
+
+  def down
+    # No need to reverse this - empty strings and NULL are functionally equivalent
+    # for OIDC applications where domain_pattern is not used
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index ddaba2d..4b67cd6 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.1].define(version: 2025_11_08_090123) do
+ActiveRecord::Schema[8.1].define(version: 2025_11_09_011443) do
   create_table "application_groups", force: :cascade do |t|
     t.integer "application_id", null: false
     t.datetime "created_at", null: false