Repository navigation
fix(oidc): validate provider URLs and endpoint aliases - #162
Open
cariandrum22 wants to merge 1 commit into
Open
cariandrum22 wants to merge 1 commit into
cariandrum22 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The compliance traceability is incomplete and the specification incorrectly claims the TLS fixture requires public DNS.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds provider URL and mTLS alias validation across Federation metadata, ordinary Discovery, caches, and upstream operations.
Changes:
- Introduces shared URL and alias validation.
- Revalidates retained and policy-derived metadata before use.
- Adds TLS-backed integration fixtures and regression coverage.
| File | Description |
|---|---|
spec/runtime-link-manifest.json |
Refreshes runtime-link metadata. |
spec/compliance-matrix.yaml |
Records the new compliance requirement. |
docs/specs/openid-federation-spec.md |
Documents URL admission behavior. |
crates/server/src/oidc/provider_urls.rs |
Implements URL and alias validation. |
crates/server/src/oidc/provider_urls/test_contract.rs |
Defines shared validation fixtures. |
crates/server/src/oidc/mod.rs |
Registers the validation module. |
crates/server/src/federation/metadata.rs |
Validates signed metadata URLs. |
crates/server/src/federation/tests/url_metadata.rs |
Tests Federation URL admission. |
crates/server/src/federation/tests/repositories/individual_admission.rs |
Covers repository admission failures. |
crates/server/src/federation/tests/endorsed_keys.rs |
Registers URL metadata tests. |
crates/server/src/web/upstream_metadata.rs |
Supports fixture-scoped HTTP clients. |
crates/server/src/web/upstream_metadata/discovery.rs |
Validates raw Discovery URLs. |
crates/server/src/web/upstream_metadata/federation.rs |
Revalidates typed and resolved metadata. |
crates/server/src/web/upstream_refresh/exchange.rs |
Uses the state-based HTTP client. |
crates/server/src/web/upstream_callback_exchange.rs |
Uses the state-based HTTP client. |
crates/server/src/web/upstream_authorize/discovery.rs |
Uses the state-based HTTP client. |
crates/server/src/web/state.rs |
Adds test-only transport injection. |
crates/server/src/web/test_support.rs |
Initializes the test transport field. |
crates/server/src/web/management/tests/upstream_metadata.rs |
Registers URL tests and updates scope notes. |
crates/server/src/web/management/tests/upstream_metadata/urls.rs |
Adds operation and cache regressions. |
crates/server/src/web/management/tests/upstream_metadata/signing_capabilities.rs |
Migrates raw-fetch tests to TLS. |
crates/server/src/web/management/tests/upstream_metadata/capabilities.rs |
Aligns capability tests with URL validation. |
crates/server/src/web/management/tests/upstream_metadata/harness.rs |
Integrates the TLS relay fixture. |
crates/server/src/web/management/tests/upstream_metadata/harness/tls.rs |
Implements the scoped TLS relay. |
crates/server/src/web/management/tests/bootstrap_and_profiles/common_and_runtime.rs |
Initializes the test transport field. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+285
to
+286
| run unchanged; the live fixture still requires public DNS resolution of | ||
| `example.com`. Finite fixture results do not attest an external provider, |
| - Finite signed, individual, typed, flat-policy, fresh/cache and loopback tests do not establish full imported schemas, locale syntax, DPoP asymmetric classification, final companion conditions, routed DB/Redis/external HTTPS, sibling composition, whole Federation conformance or formal discharge. | ||
|
|
||
| - id: fed-rp-018 | ||
| section: '3 (Discovery errata2); 2 (RFC 8414); 5 (RFC 8705); 4 (RFC 8628); 5 (RFC 9126); 2.1 (RP-Initiated Logout)' |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validate supplied provider URLs and known mTLS endpoint aliases in signed OP/AS metadata and ordinary Discovery. Reject malformed originals before replacement and recheck policy-derived and retained cache values before authorization state, redirects or token credentials can be used. Preserve partial metadata, informational URI schemes, endpoint queries at schema admission and opaque alias extensions. Field contracts follow OIDC Discovery errata set 2 section 3, RFC 8414 section 2, RFC 8705 section 5 and the endpoint extensions documented in
docs/specs/openid-federation-spec.md.Validation
Notes
Known URL/alias null refusal and lexical/userinfo/fragment restrictions include explicit local consumer policy. Existing outbound TLS/SSRF, exact-match and no-query restrictions remain separate. Unknown-only raw aliases may project to an empty typed object; no new mTLS selection is added. No dependencies, settings or migrations change. Finite local fixture results do not attest complete schemas, external providers, DB/Redis routing, correctness of combined changes or formal assurance.