fix: tolerate trailing slash when matching trusted issuers

[telegraphchi]

What

Make trusted-issuer matching tolerant of a trailing slash, so a token whose iss claim and the configured trusted_issuers differ only by a trailing / is no longer rejected.

Why

Issuer matching in credentials/verifier_default.go used an exact string comparison:

if !slices.Contains(r.Issuers, parsedClaims.Issuer) { ... }

This rejects otherwise-valid tokens when the configuration and the token disagree on a trailing slash — e.g. trusted_issuers: ["https://issuer/"] with a token carrying iss: "https://issuer" (and vice versa). That is the symptom reported in #527.

Note on #527: the original 2020 report described Oathkeeper appending a slash to the issuer. That specific behaviour is no longer present — the iss claim is now read verbatim (jwtx.ParseMapStringInterfaceClaims → mapx.GetStringDefault). What remains, and what this PR addresses, is that exact matching still fails on a trailing-slash mismatch between config and token, producing the same "valid tokens are failed" symptom.

How

A small helper trims a single trailing slash from both the token issuer and each trusted issuer before comparing. Genuinely different issuers are still rejected.

Tests

Added three cases to TestVerifierDefault:

• trusted issuer has a trailing slash, token does not → passes
• token has a trailing slash, trusted issuer does not → passes
• genuinely different issuer (despite normalization) → still rejected

go test ./credentials/ ./pipeline/authn/ passes; gofmt and go vet are clean.

Closes #527

Summary by CodeRabbit

• Bug Fixes
  • Improved token issuer validation so trusted issuers match even when they differ only by a trailing /.
• Tests
  • Added table-driven test cases covering trailing-slash normalization, including both pass and still-fail scenarios.
• Documentation
  • Updated JSON Patch op field documentation to reflect only add, remove, and replace.
• Chores
  • Adjusted Docker build to copy additional Go module files earlier to improve module download/caching behavior.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6a61ada4-287b-4ebc-99d8-947c022420d7

📥 Commits

Reviewing files that changed from the base of the PR and between 87ad0a7 and 538d08d.

📒 Files selected for processing (1)

• oryx/openapix/jsonpatch.go

✅ Files skipped from review due to trivial changes (1)

• oryx/openapix/jsonpatch.go

---

📝 Walkthrough

Walkthrough

VerifierDefault.Verify now delegates issuer validation to a new matchesTrustedIssuer helper that trims trailing slashes from both the token's iss claim and configured trusted issuers before comparing, with three new test cases covering slash-mismatch scenarios. The JSONPatch Op field documentation is narrowed to document only the three supported operations, and the From field is clarified as unused. Docker build configuration is updated to copy additional Go module files for improved caching.

Changes

Issuer trailing-slash normalization

Layer / File(s)	Summary
matchesTrustedIssuer helper, Verify call-site, and tests credentials/verifier_default.go, credentials/verifier_default_test.go	matchesTrustedIssuer is added to trim trailing slashes from both the token issuer and each configured trusted issuer before comparing. Verify's issuer membership check is replaced with a call to this helper. Three new table-driven test cases assert that trusted-vs-token slash differences pass, while a genuinely different issuer still fails.

JSONPatch operation documentation

Layer / File(s)	Summary
Op and From field documentation oryx/openapix/jsonpatch.go	The JSONPatch struct field comments are updated: Op is restricted to document only "add", "remove", and "replace" operations (removing references to RFC 6902 operations "move", "copy", and "test"), and From is clarified as not used by the currently supported operations.

Docker build module file copying

Layer / File(s)	Summary
Builder stage module file dependencies .docker/Dockerfile-build	The builder stage's early module-file copying is updated to include oryx/go.sum plus middleware/rpctest's go.mod and go.sum, providing additional dependency metadata before go mod download for improved caching efficiency.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The Dockerfile changes to copy additional module files appear out-of-scope relative to the issuer validation fix, though the JSONPatch documentation changes are minor clarifications unrelated to the stated objective.	Review the necessity of Dockerfile module file changes; if unrelated to #527, consider separating them into a distinct PR focused on build optimization.
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main change: implementing tolerance for trailing slashes in trusted issuer matching.
Description check	✅ Passed	The description provides comprehensive context including problem statement, root cause, solution approach, and test coverage, though it does not explicitly reference the linked issue #527 in the description text itself.
Linked Issues check	✅ Passed	The changes fully address the objective in #527 by implementing trailing-slash normalization for issuer comparison, allowing tokens to pass validation regardless of trailing-slash formatting differences.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[telegraphchi]

The failing scanners check is unrelated to this PR.

The Docker Image Scanners workflow builds oryd/oathkeeper:<sha> locally (make docker, no --push) and then scans that image twice:

• Anchore / grype passes — it reads the image from the runner's local Docker daemon.
• kubescape (kubescape/github-action@main) fails — it runs inside its own docker run container with no daemon socket, so it can't see the locally-built image and falls back to the registry: MANIFEST_UNKNOWN: unknown tag=<sha> (the short-SHA tag is never pushed).

So the failure reproduces on any PR that relies on the un-pushed local tag, independent of the diff — this change touches no Dockerfile, go.mod, or dependencies. (The workflow is auto-generated from ory/meta, so the actual fix would live in the template, not here.)

Locally go test ./credentials/ ./pipeline/authn/, gofmt, and go vet all pass. Happy to adjust if a maintainer points me at anything specific. 🙏

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

oryx/openapix/jsonpatch.go (1)

36-41: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align From field docs with the supported operation subset.

Line 36 still says From is used with "move", but Line 15 and runtime validation only allow "add", "remove", and "replace". Please update this comment (or mark From as currently unsupported/legacy) to avoid contradictory API docs.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@oryx/openapix/jsonpatch.go` around lines 36 - 41, Update the comment
documentation for the `From` field to accurately reflect the supported JSON
Patch operations. The current comment states that this field is used with the
"move" operation, but based on the operation validation at line 15 and runtime
checks, only "add", "remove", and "replace" operations are actually supported.
Either remove the reference to the "move" operation from the comment or clarify
that the `From` field is currently unsupported or legacy to align the
documentation with the actual implementation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@oryx/openapix/jsonpatch.go`:
- Around line 36-41: Update the comment documentation for the `From` field to
accurately reflect the supported JSON Patch operations. The current comment
states that this field is used with the "move" operation, but based on the
operation validation at line 15 and runtime checks, only "add", "remove", and
"replace" operations are actually supported. Either remove the reference to the
"move" operation from the comment or clarify that the `From` field is currently
unsupported or legacy to align the documentation with the actual implementation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 149e8e4a-b2d3-4cfb-b862-7699d8f688fa

📥 Commits

Reviewing files that changed from the base of the PR and between 74e25a3 and cdee225.

📒 Files selected for processing (1)

• oryx/openapix/jsonpatch.go