-
Notifications
You must be signed in to change notification settings - Fork 3.4k
DRAFT docs: ch 3.13 simple auth #398
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
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.
Pull Request Overview
This pull request introduces sample code and documentation for an advanced chapter (12) in the "Getting Started" series, focusing on MCP (Model Context Protocol) authentication and tool implementations. The changes include both basic MCP examples and OAuth-based authentication examples.
Key changes:
- Adds comprehensive MCP server and client examples with shopping cart functionality
- Implements OAuth 2.0 authentication examples with token introspection
- Creates test utilities and middleware for authentication validation
Reviewed Changes
Copilot reviewed 12 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
03-GettingStarted/12-advanced/sample/python/server.py | Core MCP server with product and cart management tools |
03-GettingStarted/12-advanced/sample/python/client_llm.py | LLM-integrated client using GitHub's AI models |
03-GettingStarted/12-advanced/sample/python/client.py | Basic MCP client for direct tool interaction |
03-GettingStarted/12-advanced/sample/python/README.md | Usage instructions for running the sample |
03-GettingStarted/12-advanced/code/auth-example/python/token_verifier.py | OAuth token verification implementation |
03-GettingStarted/12-advanced/code/auth-example/python/server.py | MCP server with OAuth authentication |
03-GettingStarted/12-advanced/code/auth-example/python/client.py | OAuth client with PKCE flow |
03-GettingStarted/12-advanced/code/auth-example/python/auth-server.py | Minimal OAuth authorization server |
03-GettingStarted/12-advanced/code/01-auth/python/test_server.py | Test server with custom middleware |
03-GettingStarted/12-advanced/code/01-auth/python/test_client.py | Simple HTTP client for testing |
03-GettingStarted/12-advanced/code/01-auth/python/server.py | MCP server with custom authentication middleware |
03-GettingStarted/12-advanced/code/01-auth/python/client.py | Streamable HTTP client with authentication |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
"""Add product to cart""" | ||
product = next((p for p in products if p["name"] == product_name), None) | ||
if not product: | ||
return {"type": "text", "name": f"Product [{product_name}] not found"} |
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.
The function return type annotation indicates CartItem
but this error case returns a dictionary. This will cause a type mismatch. Consider raising an exception or returning a proper error structure.
return {"type": "text", "name": f"Product [{product_name}] not found"} | |
raise ValueError(f"Product [{product_name}] not found") |
Copilot uses AI. Check for mistakes.
by OAuth authentication. User must be authenticated to access it. | ||
""" | ||
|
||
now = datetime.datetime.now() |
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.
Missing import for datetime
module. Add import datetime
at the top of the file.
Copilot uses AI. Check for mistakes.
logger.info("Starting client...") | ||
async with streamablehttp_client( | ||
url = f"http://localhost:{port}/mcp", | ||
headers = {"Authorization": "Bearer secret-token1"} |
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.
The token secret-token1
doesn't match the expected token secret-token
in the server's valid_token
function. This will cause authentication to fail.
headers = {"Authorization": "Bearer secret-token1"} | |
headers = {"Authorization": "Bearer secret-token"} |
Copilot uses AI. Check for mistakes.
if state: | ||
params["state"] = state | ||
url = f"{redirect_uri}?{urlencode(params)}" | ||
return RedirectResponse(url=url) |
Check warning
Code scanning / CodeQL
URL redirection from remote source Medium
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 16 days ago
To resolve the issue, we must ensure that only authorized, "safe" redirect URIs are used. This can be achieved by maintaining a whitelist of valid redirect URIs and checking that the incoming value for redirect_uri
matches one of the allowed entries before performing the redirect. Alternatively, if the list of allowed URIs is unknown or dynamic, we can at least verify that the URI is a relative path (not containing a remote hostname/scheme), which is much safer than allowing redirection to any arbitrary URL.
To fix the issue in this file:
- Before redirecting using the user-supplied
redirect_uri
, validate that it either matches a pre-approved list or is a local relative path. - For demonstration purposes, we can define a simple whitelist of acceptable URIs (e.g.,
"http://localhost:8000/callback"
), or implement a check that makes sure the redirect URI doesn't contain a scheme or netloc. - Use Python's
urllib.parse.urlparse
to confirm that the user input doesn't specify an external domain or scheme. - If the supplied URI fails validation, redirect to a default safe location (e.g.,
/
or simply refuse).
We also need to import urlparse
from urllib.parse
(this is probably already imported, but verify in the code region provided).
-
Copy modified line R48 -
Copy modified lines R50-R67
@@ -45,12 +45,26 @@ | ||
print("Authorize request received") | ||
|
||
code = "authcode-123" | ||
# Validate redirect_uri before redirection | ||
if redirect_uri: | ||
params = {"code": code} | ||
if state: | ||
params["state"] = state | ||
url = f"{redirect_uri}?{urlencode(params)}" | ||
return RedirectResponse(url=url) | ||
from urllib.parse import urlparse | ||
# Accept only relative paths or approved hostnames (for demo: allow only localhost callback) | ||
WHITELISTED_URIS = ["http://localhost:8000/callback"] | ||
safe = False | ||
uri_strip = redirect_uri.replace("\\", "") | ||
# Allow if matches whitelist, or is strictly a relative path | ||
if uri_strip in WHITELISTED_URIS: | ||
safe = True | ||
elif not urlparse(uri_strip).netloc and not urlparse(uri_strip).scheme: | ||
safe = True | ||
if safe: | ||
params = {"code": code} | ||
if state: | ||
params["state"] = state | ||
url = f"{uri_strip}?{urlencode(params)}" | ||
return RedirectResponse(url=url) | ||
# Unsafe URI: redirect to home page or refuse | ||
return RedirectResponse(url="/") | ||
return JSONResponse({"code": code, "state": state}) | ||
|
||
|
Purpose
Starting with OAuth 2.1 can be intimidating as there are many moving parts. An easier start could be to just pass a secret through a middleware. Also, how can we handle role based access control?
Does this introduce a breaking change?
When developers merge from main and run the server, azd up, or azd deploy, will this produce an error?
If you're not sure, try it out on an old environment.
Does this require changes to learn.microsoft.com docs or modules?
which includes deployment, settings and usage instructions.
Type of change