From 0954200ad43ca4e7aec7a5504aee6bc71f3ffe7a Mon Sep 17 00:00:00 2001
From: "Sure Admin (bot)" <sure-admin@splashblot.com>
Date: Tue, 5 May 2026 00:47:45 +0200
Subject: [PATCH] fix(auth): surface exact OIDC issuer mismatches (#1666)

* fix(auth): surface exact OIDC issuer mismatches

* fix(auth): align issuer mismatch hint with tests

---------

Co-authored-by: SureBot <sure-bot@we-promise.com>
---
 .../controllers/admin_sso_form_controller.js  | 19 ++++++++---
 app/models/sso_provider_tester.rb             | 15 ++++++--
 test/models/sso_provider_tester_test.rb       | 34 +++++++++++++++++++
 3 files changed, 61 insertions(+), 7 deletions(-)
 create mode 100644 test/models/sso_provider_tester_test.rb

diff --git a/app/javascript/controllers/admin_sso_form_controller.js b/app/javascript/controllers/admin_sso_form_controller.js
index 2344b8b63..2ade19da5 100644
--- a/app/javascript/controllers/admin_sso_form_controller.js
+++ b/app/javascript/controllers/admin_sso_form_controller.js
@@ -111,10 +111,21 @@ export default class extends Controller {
       if (response.ok) {
         const data = await response.json()
         if (data.issuer) {
-          // Valid OIDC discovery endpoint
-          issuerInput.classList.remove('border-yellow-300', 'border-red-300')
-          issuerInput.classList.add('border-green-300')
-          this.showValidationMessage(issuerInput, 'Valid OIDC issuer', 'success')
+          if (data.issuer === issuer) {
+            issuerInput.classList.remove('border-yellow-300', 'border-red-300', 'border-amber-300')
+            issuerInput.classList.add('border-green-300')
+            this.showValidationMessage(issuerInput, 'Valid OIDC issuer', 'success')
+          } else {
+            issuerInput.classList.remove('border-yellow-300', 'border-green-300')
+            issuerInput.classList.add('border-amber-300')
+
+            const trailingSlashOnly = data.issuer.replace(/\/$/, '') === issuer.replace(/\/$/, '')
+            const message = trailingSlashOnly
+              ? `Issuer mismatch: discovery returned ${data.issuer}. This is usually a trailing slash mismatch, so copy the issuer exactly as returned.`
+              : `Issuer mismatch: discovery returned ${data.issuer}. Copy the issuer exactly as returned by the provider.`
+
+            this.showValidationMessage(issuerInput, message, 'warning')
+          }
         } else {
           throw new Error('Invalid discovery response')
         }
diff --git a/app/models/sso_provider_tester.rb b/app/models/sso_provider_tester.rb
index 7b27f08b7..9a66cc003 100644
--- a/app/models/sso_provider_tester.rb
+++ b/app/models/sso_provider_tester.rb
@@ -63,11 +63,14 @@ class SsoProviderTester
           )
         end
 
-        # Check if issuer matches
-        if discovery["issuer"] != provider.issuer && discovery["issuer"] != provider.issuer.chomp("/")
+        # Check if issuer matches exactly. OIDC discovery requires the configured
+        # issuer string to be identical to the issuer returned by the provider.
+        if discovery["issuer"] != provider.issuer
+          hint = trailing_slash_hint(provider.issuer, discovery["issuer"])
+
           return Result.new(
             success?: false,
-            message: "Issuer mismatch: expected #{provider.issuer}, got #{discovery["issuer"]}",
+            message: [ "Issuer mismatch: expected #{provider.issuer}, got #{discovery["issuer"]}", hint ].compact.join(". "),
             details: { expected: provider.issuer, actual: discovery["issuer"] }
           )
         end
@@ -204,4 +207,10 @@ class SsoProviderTester
     def faraday_client
       @faraday_client ||= Faraday.new(ssl: self.class.faraday_ssl_options)
     end
+
+    def trailing_slash_hint(expected, actual)
+      return unless expected.to_s.chomp("/") == actual.to_s.chomp("/")
+
+      "trailing slash mismatch. This usually means the issuer URL differs only by a trailing slash. Update the configured issuer to exactly match the discovery document"
+    end
 end
diff --git a/test/models/sso_provider_tester_test.rb b/test/models/sso_provider_tester_test.rb
new file mode 100644
index 000000000..e01685d62
--- /dev/null
+++ b/test/models/sso_provider_tester_test.rb
@@ -0,0 +1,34 @@
+require "test_helper"
+
+class SsoProviderTesterTest < ActiveSupport::TestCase
+  test "oidc discovery requires exact issuer match" do
+    provider = SsoProvider.new(
+      strategy: "openid_connect",
+      name: "pocket_id",
+      label: "Pocket ID",
+      issuer: "https://pocketid.example.com/",
+      client_id: "client-id",
+      client_secret: "secret"
+    )
+
+    response = stub(status: 200, success?: true, body: {
+      issuer: "https://pocketid.example.com",
+      authorization_endpoint: "https://pocketid.example.com/authorize",
+      token_endpoint: "https://pocketid.example.com/api/oidc/token"
+    }.to_json)
+
+    client = stub
+    client.stubs(:get).returns(response)
+
+    tester = SsoProviderTester.new(provider)
+    tester.stubs(:faraday_client).returns(client)
+
+    result = tester.test!
+
+    assert_not result.success?
+    assert_includes result.message, "Issuer mismatch"
+    assert_includes result.message, "trailing slash mismatch"
+    assert_equal "https://pocketid.example.com/", result.details[:expected]
+    assert_equal "https://pocketid.example.com", result.details[:actual]
+  end
+end