-
Notifications
You must be signed in to change notification settings - Fork 387
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
base: main
Are you sure you want to change the base?
Conversation
🎊 PR Preview 2b23014 has been successfully built and deployed to https://quarkus-site-pr-2284-preview.surge.sh
|
ade0346
to
55ec40c
Compare
There was a problem hiding this 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:
- 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
- I would actually start with curl and then having a dev ui demo (maybe even a quick video)
a3b0b6f
to
015bb69
Compare
de4f14e
to
6da0f63
Compare
@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 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 |
I've resolved all the comments though I may not have addressed them as you were expecting - please create new suggestions |
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 |
There was a problem hiding this 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?
@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 |
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:
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 |
cea4bba
to
e862da9
Compare
f1f4b08
to
cfd9fbd
Compare
I've another long train journey tomorrow. I will do another review tomorrow. |
cfd9fbd
to
c51c1f3
Compare
There was a problem hiding this 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!
25e347b
to
e907c77
Compare
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 |
e907c77
to
38a38ce
Compare
No description provided.