Updates & tests for json serialized urls in config

[rholinshead]

Summary

There are a number of places in the config (auth/oauth) where values are AnyHttpUrl type. This type is not serializable to yaml via default yaml.dump and results in an invalid tag which would then product an error when deserializing the yaml with yaml.load/safe_load:

yaml.constructor.ConstructorError: could not determine a constructor for the tag 'tag:yaml.org,2002:python/object:pydantic.networks.AnyHttpUrl'

This currently affects cloud deployments for apps using oauth tools. We've landed a fix in the backend to use mode="json" for yaml dumping, which serializes the URLs as strings, but it also adds a trailing slash "/". This PR makes sure trailing slash in the config values is supported.

This should also unblock #601 to use mode="json" when materializing the settings during deployment

Testing

Updated tests here, make tests succeeds.

Also ran both the examples/oauth/pre_authorize and examples/oauth/interactive_tool examples successfully

Summary by CodeRabbit

• Bug Fixes

  • Improved OAuth robustness by normalizing URLs to handle trailing slashes consistently across authorization URL construction, server selection, and token verification.

• Tests

  • Added tests covering URL normalization and related OAuth/token verification scenarios, including authorization URL formation and issuer/audience matching.

[coderabbitai]

Walkthrough

The PR normalizes URL handling by stripping trailing slashes when constructing authorization URLs, selecting authorization servers, and validating token issuer and audiences to avoid mismatches and double-slash issues.

Changes

Cohort / File(s)	Summary
OAuth flow & metadata src/mcp_agent/oauth/flow.py, src/mcp_agent/oauth/metadata.py	Authorization URL construction now strips trailing slashes before appending query strings; authorization-server selection compares normalized URLs (trailing slashes removed) and falls back to first candidate with a warning if no match.
Token verification src/mcp_agent/server/token_verifier.py	Issuer equality and audience checks normalize values by stripping trailing slashes before comparing or building sets; logs and payload reporting use sanitized values.
Tests tests/test_oauth_utils.py, tests/test_token_verifier.py	Added tests covering serialization/deserialization impacts, trailing-slash handling for server selection and authorization URL construction, OAuth callback URL formation, and issuer comparison across trailing-slash variations (mocks/AsyncMock used for network interactions).

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Flow as OAuth Flow
    participant Meta as Metadata Selector
    participant Verifier as Token Verifier
    Client->>Flow: request authorization URL
    Flow->>Flow: normalize auth_endpoint (strip trailing slash)
    Flow->>Meta: select_authorization_server(preferred?)
    Meta->>Meta: normalize candidate URLs (strip trailing slash)
    Meta-->>Flow: chosen server URL
    Flow-->>Client: constructed auth URL (no double-slash)
    Note over Verifier: Token validation path
    Client->>Verifier: present token
    Verifier->>Verifier: normalize configured issuer/aud & token iss/aud (strip trailing slashes)
    Verifier-->>Client: validation result (accept/reject)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Attention areas:
  • src/mcp_agent/oauth/metadata.py: confirm fallback behavior and logged warning semantics when preferred server not matched.
  • src/mcp_agent/server/token_verifier.py: verify normalization covers all audience types (single vs list) and logging shows sanitized values.
  • Tests: ensure AsyncMock/http mocks and JSON-mode serialization tests exercise both normalized and non-normalized cases.

Poem

🐰 I hopped through URLs, trailing slashes in tow,

I trimmed them away so the queries would flow.
No doubles, no mismatches, neat paths all around,
Now OAuth dances on firm, tidy ground. ✨

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title refers to a real aspect of the changes (JSON serialization and URL handling in config), but it is overly broad and doesn't highlight the main change—which is normalizing trailing slashes in URL validation/comparison logic.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/handle-config-url-trailing-slash

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67f7790 and c2c791d.

📒 Files selected for processing (1)

• tests/test_token_verifier.py (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• tests/test_token_verifier.py

---

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.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 27b2a4d and 67f7790.

📒 Files selected for processing (5)

• src/mcp_agent/oauth/flow.py (1 hunks)
• src/mcp_agent/oauth/metadata.py (1 hunks)
• src/mcp_agent/server/token_verifier.py (2 hunks)
• tests/test_oauth_utils.py (4 hunks)
• tests/test_token_verifier.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (3)

tests/test_oauth_utils.py (3)

src/mcp_agent/config.py (2)

• MCPOAuthClientSettings (88-140)
• OAuthSettings (161-179)

src/mcp_agent/oauth/metadata.py (1)

• select_authorization_server (62-85)

src/mcp_agent/oauth/flow.py (2)

• AuthorizationFlowCoordinator (40-297)
• authorize (47-297)

tests/test_token_verifier.py (2)

src/mcp_agent/config.py (1)

• MCPAuthorizationServerSettings (30-85)

src/mcp_agent/server/token_verifier.py (3)

• MCPAgentTokenVerifier (22-270)
• _introspect (113-209)
• aclose (263-264)

src/mcp_agent/oauth/metadata.py (2)

src/mcp_agent/core/context.py (1)

• logger (215-232)

src/mcp_agent/app.py (1)

• logger (227-244)

🔇 Additional comments (9)

src/mcp_agent/oauth/flow.py (1)

138-142: LGTM - Correct URL construction with trailing slash normalization.

The change strips trailing slashes from the authorization endpoint before appending the query string, preventing malformed URLs like authorize/?param=value when the endpoint includes a trailing slash.

src/mcp_agent/oauth/metadata.py (1)

72-84: LGTM - Proper normalization for authorization server selection.

The implementation correctly normalizes URLs by stripping trailing slashes for comparison while preserving the original candidate values. The fallback behavior with warning provides helpful diagnostics when the preferred server isn't found.

src/mcp_agent/server/token_verifier.py (2)

157-168: LGTM - Issuer comparison properly normalized.

Both the configured issuer and token's issuer claim are normalized by stripping trailing slashes before comparison, ensuring compatibility regardless of how URLs are formatted in configuration or tokens.

---

246-249: LGTM - Audience validation properly normalized.

Both the configured expected audiences and token audiences are normalized by stripping trailing slashes before performing the intersection check, ensuring robust validation across different URL formats.

tests/test_oauth_utils.py (4)

79-113: LGTM - Comprehensive test for JSON serialization impact.

This test effectively validates that authorization server selection works correctly after config is serialized with mode="json", which adds trailing slashes to AnyHttpUrl fields.

---

115-136: LGTM - Thorough trailing slash mismatch coverage.

The test validates both scenarios: preferred with trailing slash vs. candidates without, and vice versa. This ensures the normalization logic handles all combinations correctly.

---

162-181: LGTM - Validates callback URL construction after serialization.

This test ensures the callback URL construction pattern (using rstrip("/")) works correctly when callback_base_url has a trailing slash after JSON serialization.

---

196-269: LGTM - Integration test validates authorization URL construction.

This test effectively validates that the authorization URL is constructed correctly when the endpoint has a trailing slash, ensuring no double slashes appear before the query string.

tests/test_token_verifier.py (1)

860-910: LGTM - Validates issuer comparison with trailing slash normalization.

This test confirms that issuer comparison succeeds when the configuration has a trailing slash (from JSON serialization) but the token's issuer claim doesn't, thanks to the normalization logic.