Skip to content

Feat/MCP Authentication + alignment with REST and SKILL adapters #309

Open
jlouvel wants to merge 9 commits intomainfrom
feat/mcp-authentication
Open

Feat/MCP Authentication + alignment with REST and SKILL adapters #309
jlouvel wants to merge 9 commits intomainfrom
feat/mcp-authentication

Conversation

@jlouvel
Copy link
Copy Markdown
Contributor

@jlouvel jlouvel commented Apr 12, 2026

Related Issue

Closes #294


What does this PR do?

Adds OAuth 2.1 resource server authentication to Naftiko, shared across all three server adapters (REST, MCP, Skill). This is a follow-up to #308 (Restlet transport migration) and builds on the unified Restlet foundation.

Key changes:

  • AuthOAuth2 — new authentication type in the JSON schema (type: oauth2) with properties: authorizationServerUrl, resource, scopes, audience, tokenValidation
  • OAuth2AuthenticationSpec — Java spec class registered in the AuthenticationSpec @JsonSubTypes union
  • OAuth2AuthenticationRestlet — shared JWT/JWKS validation restlet with AS metadata discovery (RFC 8414), JWKS caching, ChallengeRequest-based WWW-Authenticate
  • McpOAuth2Restlet — extends OAuth2AuthenticationRestlet with MCP Protected Resource Metadata endpoint (RFC 9728)
  • ServerAdapter — factored all authentication chain logic (buildServerChain, createOAuth2Restlet, buildChallengeAuthenticator, template/constant helpers) into the base class; MCP overrides createOAuth2RestletMcpOAuth2Restlet
  • ServerSpec — lifted authentication field from McpServerSpec, RestServerSpec, SkillServerSpec into the base class
  • Spectral rules — 3 new rules: naftiko-oauth2-https-authserver (error), naftiko-oauth2-resource-https (warn), naftiko-oauth2-scopes-defined (warn)
  • nimbus-jose-jwt 9.37.3 — new dependency for JWT parsing and JWKS key resolution
  • Blueprint revision — updated mcp-server-authentication.md to reflect Restlet foundation

Factorization: Three rounds of factorization eliminated ~250+ lines of duplicated auth logic across the three adapters. buildServerChain(Restlet) reads authentication from getSpec().getAuthentication() internally, so adapters just call buildServerChain(router) with no wrapper methods.


Tests

  • OAuth2AuthenticationRestletTest — 18 unit tests for JWT validation (missing token, expired, wrong issuer, wrong audience, wrong scope, JWKS key resolution, etc.)
  • McpOAuth2RestletTest — 8 unit tests for MCP Protected Resource Metadata endpoint
  • McpOAuth2IntegrationTest — 5 integration tests with mock authorization server
  • McpAuthenticationIntegrationTest — 4 integration tests for basic/bearer/apikey/digest on MCP
  • ServerAdapterAuthenticationTest — 9 centralized tests for the shared authentication chain across all adapter types
  • Full suite: 512 tests pass, 0 failures, 5 skipped

Checklist

  • CI is green (build, tests, schema validation, security scans)
  • Rebased on latest main
  • Small and focused — one concern per PR
  • Commit messages follow Conventional Commits

Agent Context (optional)

agent_name: GitHub Copilot
llm: Claude Opus 4.6
tool: VS Code Chat
confidence: high
source_event: blueprints/mcp-server-authentication.md
discovery_method: code_review
review_focus: ServerAdapter.java, OAuth2AuthenticationRestlet.java, McpOAuth2Restlet.java, naftiko-schema.json

@jlouvel jlouvel requested a review from eskenazit April 12, 2026 00:38
@jlouvel jlouvel self-assigned this Apr 12, 2026
@jlouvel jlouvel force-pushed the chore/mcp-restlet-transport branch from 22e64b2 to 9fdee47 Compare April 13, 2026 18:04
@jlouvel jlouvel force-pushed the feat/mcp-authentication branch from 813adaa to 80f17bd Compare April 13, 2026 18:05
@jlouvel
Copy link
Copy Markdown
Contributor Author

jlouvel commented Apr 13, 2026

@eskenazit Excellent review. I was able to fix all issues, adding related test cases.

@eskenazit
Copy link
Copy Markdown
Contributor

eskenazit commented Apr 14, 2026

I just realised that this branch is not from main but from
chore/mcp-restlet-transport. Is that intended ?

I think this is why there are conflicts, since the rebase of chore/mcp-restlet-transport makes it see as both main and feat/mcp-authentication are working on the chore/mcp-restlet-transport files.

Base automatically changed from chore/mcp-restlet-transport to main April 14, 2026 08:07
@jlouvel jlouvel force-pushed the feat/mcp-authentication branch from 19305a5 to ec6ed90 Compare April 14, 2026 11:39
@jlouvel
Copy link
Copy Markdown
Contributor Author

jlouvel commented Apr 14, 2026

@eskenazit Yes this was intentional for the branch, but I should have been explicit. I just rebased on main (now that the Restlet transport PR has been merged), which was straightforward (no conflict to resolve)

@jlouvel jlouvel force-pushed the feat/mcp-authentication branch from ec6ed90 to c15a474 Compare April 14, 2026 12:57
jlouvel added 8 commits April 14, 2026 14:57
…r chain

- Add AuthOAuth2 to the shared Authentication union in the JSON schema
- Add OAuth2AuthenticationSpec for YAML deserialization
- Create OAuth2AuthenticationRestlet with JWKS-based JWT validation,
  AS metadata discovery, and token caching
- Create McpOAuth2Restlet extending the shared restlet with MCP
  Protected Resource Metadata (RFC 9728)
- Lift authentication field and buildServerChain into ServerAdapter
  base class, eliminating duplication across MCP, REST, and Skill
- MCP adapter overrides createOAuth2Restlet for McpOAuth2Restlet
- Add nimbus-jose-jwt 9.37.3 dependency for JWT/JWKS handling
- Add 3 Spectral rules for OAuth2 validation
- Add ServerAdapterAuthenticationTest (9 tests) for shared auth chain
- Add OAuth2AuthenticationRestletTest (18 tests) for JWT validation
- Add McpOAuth2RestletTest (8 tests) for MCP metadata extension
- Add McpOAuth2IntegrationTest (5 tests) with mock AS server
validateClaims checked exp but not nbf — a JWT with nbf in the future was accepted. Added nbf check and corresponding unit tests.
@jlouvel jlouvel force-pushed the feat/mcp-authentication branch from c15a474 to 4302ff1 Compare April 14, 2026 18:58
@jlouvel jlouvel requested a review from eskenazit April 14, 2026 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Framework : Add support for authentication in the MCP server adapter

2 participants