Skip to content

fix(core): trim trailing slash from the SSO OIDC discovery URL - #9586

Open
simeng-li wants to merge 1 commit into
masterfrom
simeng-fix-sso-issuer-trailing-slash
Open

fix(core): trim trailing slash from the SSO OIDC discovery URL#9586
simeng-li wants to merge 1 commit into
masterfrom
simeng-fix-sso-issuer-trailing-slash

Conversation

@simeng-li

Copy link
Copy Markdown
Contributor

Summary

Closes #9583.

Context

An OIDC enterprise SSO connector whose Issuer ends with a slash builds its discovery request as <issuer>//.well-known/openid-configuration. Identity providers that do not collapse the doubled slash reject it, and every sign-in through that connector fails — getAuthorizationUrl refetches the discovery document on each attempt.

The resulting failure is also hard to act on. koa-connector-error-handler passes the connector error's data straight into the response body and the audit log, and the Console pins any invalid_connector_config onto the Issuer field, so the operator sees the field flagged with a payload that does not say what went wrong.

What changed

  • fetchOidcConfigRaw trims trailing slashes from the issuer when building the discovery URL (packages/core/src/sso/OidcConnector/utils.ts:47). This is request-shaping only — the stored issuer is untouched, because user_sso_identities is keyed on the raw configured issuer and rewriting it would orphan existing SSO identities.
  • Added describeRequestError, used at the five catch blocks in the same file that build an SsoConnectorError payload from a caught outbound-request error. It returns { statusCode, body } for an HTTPError and <name>: <message> otherwise, instead of handing the caught error over verbatim.

Expected result

  • A connector configured with https://idp.example.com/ now requests https://idp.example.com/.well-known/openid-configuration, matching RFC 8414 §3.1.
  • A transport-level failure (DNS, TLS, or a destination blocked by the SSRF protection) now names itself in the API response and the audit log. Previously a plain Error reduced to {}, while a got error serialized its whole request options object.

Reviewer notes

  • The trim changes the requested path for any issuer ending in a slash. An IdP that serves the doubled-slash path but not the single-slash one would regress; no such IdP is known, and the single-slash form is what every other OIDC client sends.
  • fetchOidcConfigRaw's other caller, packages/core/src/saml-application/SamlApplication/index.ts:315, passes Logto's own endpoint-derived issuer, so the trim is inert there.

Testing

Unit tests

Checklist

  • .changeset
  • unit tests
  • integration tests
  • necessary TSDoc comments

Copilot AI lite review requested due to automatic review settings September 11, 2026 02:18
@github-actions

Copy link
Copy Markdown

COMPARE TO master

Total Size Diff 📈 +2.6 KB

Diff by File
Name Diff
.changeset/purple-eels-shave.md 📈 +712 Bytes
packages/core/src/sso/OidcConnector/utils.test.ts 📈 +1.02 KB
packages/core/src/sso/OidcConnector/utils.ts 📈 +911 Bytes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved blocking issues were identified.

Pull request overview

Fixes OIDC discovery failures caused by trailing slashes and improves outbound-request error reporting.

Changes:

  • Normalize discovery URLs without changing stored issuers.
  • Format HTTP and transport errors clearly.
  • Add unit tests and a changeset.
File summaries
File Description
packages/core/src/sso/OidcConnector/utils.ts Normalizes discovery URLs and error payloads.
packages/core/src/sso/OidcConnector/utils.test.ts Tests URL normalization and error formatting.
.changeset/purple-eels-shave.md Documents the bug fix.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/core/src/sso/OidcConnector/utils.ts Outdated
Copilot AI review requested due to automatic review settings September 11, 2026 06:36
@simeng-li
simeng-li force-pushed the simeng-fix-sso-issuer-trailing-slash branch from 97f3206 to 2d4ecd1 Compare September 11, 2026 06:36
@github-actions github-actions Bot added size/s and removed size/s labels Sep 11, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved review comments remain, and all reviewers found it ready for approval.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

bug: SSO connector validation failure for existing connectors after 1.43.0 upgrade

4 participants