Skip to content

Add a blog post about secure MCP SSE server #2284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sberyozkin
Copy link
Member

No description provided.

Copy link

github-actions bot commented Apr 17, 2025

🎊 PR Preview 2b23014 has been successfully built and deployed to https://quarkus-site-pr-2284-preview.surge.sh

  • Images of blog posts older than 3 months are not available.
  • Newsletters older than 3 months are not available.

@sberyozkin sberyozkin force-pushed the secure-mcp-http-server branch from ade0346 to 55ec40c Compare April 17, 2025 17:46
@geoand geoand requested a review from maxandersen April 22, 2025 06:58
Copy link
Member

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

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

It's a great post. I thing we can clarify a few things:

  1. Explains where the authentication happens in the MCP protocol (in which message the header must be set), even diving a bit in the MCP protocol a bit
  2. I would actually start with curl and then having a dev ui demo (maybe even a quick video)

@sberyozkin sberyozkin force-pushed the secure-mcp-http-server branch 3 times, most recently from a3b0b6f to 015bb69 Compare April 22, 2025 13:06
@sberyozkin sberyozkin closed this Apr 23, 2025
@sberyozkin sberyozkin reopened this Apr 23, 2025
@sberyozkin sberyozkin force-pushed the secure-mcp-http-server branch from de4f14e to 6da0f63 Compare April 23, 2025 17:41
@sberyozkin
Copy link
Member Author

@maxandersen @cescoffier Thanks very much for the thoughtful review, I know I was not quite getting along, but as always, on reflection, I know the updated PR should be better.

One thing I found difficult to update was to move curl as the first example. MCP Dev UI is good as the first one because both MCP and OIDC cards are aligned and I describe how to get OIDC token, and use Dev UI, and then in the follow up section I just refer to the Dev UI section for users to see how to get the token. Starting from curl or the inspector requires to repeat the same info re the OIDC token acqusitiuon.

I honestly don't mind if it goes first, but not sure how to make it work neatly if curl goes first, Clement, if you'd like please give it a try.

Please follow up tomorrow morning with suggestions, especially with respect to some further technical clarifications and we should be ready to merge soon.

Cheers

@sberyozkin
Copy link
Member Author

I've resolved all the comments though I may not have addressed them as you were expecting - please create new suggestions

@sberyozkin
Copy link
Member Author

May be I can have a dedicated section for getting the token and then refer to it from all other sections, that might work, will try tomorrow

Copy link
Member

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

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

Way better! I made a few comments and suggestions.

The main thing I think would be to use prod mode for the inspector and curl demo. Do you think it's possible?

@sberyozkin
Copy link
Member Author

sberyozkin commented Apr 24, 2025

@cescoffier Thanks, sure, I'll have a look at having it run with jbang, I'll apply all the suggestions except those related to the versions and then let's try to do one more review round

@sberyozkin
Copy link
Member Author

Hi @cescoffier, actually, unfortunately, supporting the prod mode will make it more complex.

We'd still not be able to plug it in into Claude right now, until it starts supporting the MCP server user login, so that would leave the only other option, which I actually used to have but dropped:

  • We'd require users to register a GitHub application which may be a bit of a problem for some users
  • We'd have to provide a JAX-RS callback endpoint, once GitHub authentication is done, which would return GitHub token for the user to copy it and pass into MCP inspector - but it does not look great IMHO - users won't do it in prod

I suggest we just keep it in Dev Mode for this one, this is 100% not the last article about MCP security, and we'll do something interesting for prod once Claude and other MCP clients actually start supporting logging in users

@sberyozkin sberyozkin force-pushed the secure-mcp-http-server branch 2 times, most recently from cea4bba to e862da9 Compare April 24, 2025 18:20
@sberyozkin sberyozkin marked this pull request as ready for review April 24, 2025 18:22
@sberyozkin sberyozkin force-pushed the secure-mcp-http-server branch 3 times, most recently from f1f4b08 to cfd9fbd Compare April 24, 2025 22:59
@cescoffier
Copy link
Member

I've another long train journey tomorrow. I will do another review tomorrow.

@sberyozkin sberyozkin force-pushed the secure-mcp-http-server branch from cfd9fbd to c51c1f3 Compare April 25, 2025 09:05
Copy link
Member

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

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

a few minor nitpicks for clarity but otherwise really good stuff!

@sberyozkin sberyozkin force-pushed the secure-mcp-http-server branch from 25e347b to e907c77 Compare April 25, 2025 12:20
@sberyozkin
Copy link
Member Author

sberyozkin commented Apr 25, 2025

Thanks @maxandersen, I've applied your suggestions with minor tweaks, please add more suggestions if you'd like.

Clement would also like to have another look tomorrow, so I guess I'll update the publish date to Monday Apr 28th

@sberyozkin sberyozkin force-pushed the secure-mcp-http-server branch from e907c77 to 38a38ce Compare April 25, 2025 14:21
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.

3 participants