mirror of
https://github.com/we-promise/sure.git
synced 2026-05-12 23:25:00 +00:00
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>
This commit is contained in:
@@ -111,10 +111,21 @@ export default class extends Controller {
|
|||||||
if (response.ok) {
|
if (response.ok) {
|
||||||
const data = await response.json()
|
const data = await response.json()
|
||||||
if (data.issuer) {
|
if (data.issuer) {
|
||||||
// Valid OIDC discovery endpoint
|
if (data.issuer === issuer) {
|
||||||
issuerInput.classList.remove('border-yellow-300', 'border-red-300')
|
issuerInput.classList.remove('border-yellow-300', 'border-red-300', 'border-amber-300')
|
||||||
issuerInput.classList.add('border-green-300')
|
issuerInput.classList.add('border-green-300')
|
||||||
this.showValidationMessage(issuerInput, 'Valid OIDC issuer', 'success')
|
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 {
|
} else {
|
||||||
throw new Error('Invalid discovery response')
|
throw new Error('Invalid discovery response')
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -63,11 +63,14 @@ class SsoProviderTester
|
|||||||
)
|
)
|
||||||
end
|
end
|
||||||
|
|
||||||
# Check if issuer matches
|
# Check if issuer matches exactly. OIDC discovery requires the configured
|
||||||
if discovery["issuer"] != provider.issuer && discovery["issuer"] != provider.issuer.chomp("/")
|
# 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(
|
return Result.new(
|
||||||
success?: false,
|
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"] }
|
details: { expected: provider.issuer, actual: discovery["issuer"] }
|
||||||
)
|
)
|
||||||
end
|
end
|
||||||
@@ -204,4 +207,10 @@ class SsoProviderTester
|
|||||||
def faraday_client
|
def faraday_client
|
||||||
@faraday_client ||= Faraday.new(ssl: self.class.faraday_ssl_options)
|
@faraday_client ||= Faraday.new(ssl: self.class.faraday_ssl_options)
|
||||||
end
|
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
|
end
|
||||||
|
|||||||
34
test/models/sso_provider_tester_test.rb
Normal file
34
test/models/sso_provider_tester_test.rb
Normal file
@@ -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
|
||||||
Reference in New Issue
Block a user