Skip to content

[Server] Oauth2 based on middleware#221

Open
sveneld wants to merge 12 commits intomodelcontextprotocol:mainfrom
sveneld:oauth_middleware
Open

[Server] Oauth2 based on middleware#221
sveneld wants to merge 12 commits intomodelcontextprotocol:mainfrom
sveneld:oauth_middleware

Conversation

@sveneld
Copy link
Contributor

@sveneld sveneld commented Jan 12, 2026 •
edited by chr-hertel
Loading

Motivation and Context

Implementation of OAuth based on middlewares from #218

How Has This Been Tested?

Breaking Changes

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Fix #214

@sveneld sveneld mentioned this pull request Jan 12, 2026
Merged
9 tasks
@chr-hertel
Copy link
Member

chr-hertel commented Jan 18, 2026

Thanks @sveneld - it will take some time for me to give some proper feedback here, just to let you know - needs quite some focus & attention - still really appreciated!

@chr-hertel chr-hertel changed the title Oauth2 based on middleware [Server] Oauth2 based on middleware Jan 23, 2026
@chr-hertel chr-hertel added Server Issues & PRs related to the Server component auth Issues and PRs related to Authentication / OAuth labels Jan 23, 2026
chr-hertel
chr-hertel requested changes Jan 26, 2026
View reviewed changes
Copy link
Member

@chr-hertel chr-hertel left a comment

Choose a reason for hiding this comment

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

Hi again @sveneld! This is great already - went to a first round of very detailed review directly. Thanks again for working on this! 🙏

Didn't manage to go through all, but dropping the first round of comments.

examples/server/oauth-keycloak/README.md Outdated

5. **Use with MCP Inspector:**

The MCP Inspector doesn't support OAuth out of the box, but you can test using curl or build a custom client.
Copy link
Member

@chr-hertel chr-hertel Jan 26, 2026

Choose a reason for hiding this comment

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

This was a bit unexpected to me since I saw this in inspector:
image

examples/server/oauth-keycloak/README.md
Copy link
Member

@chr-hertel chr-hertel Jan 26, 2026

Choose a reason for hiding this comment

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

Works right away 💪 👍

@sveneld
Copy link
Contributor Author

sveneld commented Jan 28, 2026

@chr-hertel I updated pull request

@chr-hertel chr-hertel mentioned this pull request Feb 8, 2026
Closed
6 tasks
@Nyholm
Copy link
Contributor

Nyholm commented Feb 11, 2026

Can you please rebase this now when the Middleware Handler is merged?

@sveneld
OAuth Implementation based on middleware
eb2a422 
Signed-off-by: Volodymyr Panivko <sveneld300@gmail.com>
@sveneld
Copy link
Contributor Author

sveneld commented Feb 11, 2026

Can you please rebase this now when the Middleware Handler is merged?

Done.

Nyholm
Nyholm reviewed Feb 11, 2026
View reviewed changes
Copy link
Contributor

@Nyholm Nyholm left a comment

Choose a reason for hiding this comment

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

Great. Looks good.

I only have small things to comment at. I have not tested it out.

@sveneld
Copy link
Contributor Author

sveneld commented Feb 16, 2026

I have updated the pull request, but I am not satisfied with the result. I need more time to rethink the flow and improve the implementation.

@chr-hertel
Copy link
Member

chr-hertel commented Feb 17, 2026

@sveneld anything in particular you would need feedback here?
can tell you about nit picks already, e.g. using Mcp\Exception\*Exception classes instead of built-in ones

@sveneld
Copy link
Contributor Author

sveneld commented Feb 18, 2026

Thank you, no feedback needed.

@sveneld
Copy link
Contributor Author

sveneld commented Feb 22, 2026

@chr-hertel take a look.

This was referenced Feb 22, 2026
Open
Closed
@sveneld
Fix body retrieval in OAuthRequestMetaMiddleware for proper string co…
1096f8e 
…nversion

Signed-off-by: Volodymyr Panivko <sveneld300@gmail.com>
@chr-hertel
Copy link
Member

chr-hertel commented Feb 26, 2026

@chr-hertel take a look.

@sveneld will do so this weekend - thanks! 👍

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

Reviewers

@Nyholm Nyholm Nyholm left review comments

@chr-hertel chr-hertel chr-hertel requested changes

@CodeWithKyrian CodeWithKyrian Awaiting requested review from CodeWithKyrian CodeWithKyrian is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

auth Issues and PRs related to Authentication / OAuth Server Issues & PRs related to the Server component

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Server] Implement OAuth Flow for Client Authentication

3 participants

@sveneld @chr-hertel @Nyholm