-
Notifications
You must be signed in to change notification settings - Fork 1
feat: Support direct token in Authorization header #29
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
Conversation
- Add _get_token_from_request() to retrieve tokens from Authorization header - Support both direct token (Method 1) and FastMCP OAuth flow (Method 2) - Enable external token management for advanced scenarios - Add Context parameter to tool functions for HTTP request access
- Document Method 1 (FastMCP OAuth flow) and Method 2 (direct token) - Simplify structure and remove redundant notes - Add Azure CLI example for token acquisition - Add Japanese documentation (setup_ja.md)
|
/gemini review |
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.
Code Review
このプルリクエストは、Authorization: Bearer <token>ヘッダーを介して直接アクセストークンを渡す機能を追加し、認証の柔軟性を高めるものです。変更内容は明確で、既存のOAuthフローとの後方互換性も維持されており、素晴らしい改善だと思います。
主な変更点である_get_token_from_request関数について、1点フィードバックがあります。例外処理の範囲をより限定的にすることで、コードの堅牢性をさらに高めることができると考えます。
ドキュメントの更新も適切に行われており、新しい認証方法の使い方が分かりやすく説明されています。
- Use specific exceptions (RuntimeError, AttributeError) instead of broad Exception - Use try-except-else pattern for better code clarity - More precise error handling for HTTP context access
- Test token retrieval from Authorization header - Test token retrieval from FastMCP OAuth context - Test priority (header > OAuth context) - Test error handling (RuntimeError, AttributeError) - Test edge cases (empty header, missing Bearer prefix, case sensitivity) - All 9 tests passing, no regressions in existing tests (49 total)
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
素晴らしい機能追加です!Authorizationヘッダー経由での直接トークン渡しをサポートすることで、テストやカスタム統合のシナリオに柔軟に対応できるようになりました。既存のOAuthフローとの後方互換性を維持しながら、新しい認証方法を追加する設計は評価できます。
主な変更点は以下の通りです:
_get_token_from_request()関数を追加し、Authorizationヘッダーとget_access_token()の両方からトークンを取得可能に- ツール関数に
Contextパラメータを追加し、HTTPリクエストへのアクセスを実現 - 2つのトークン取得方法を明確に説明するドキュメントを更新
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/server.py | Authorizationヘッダーからのトークン取得機能を追加、_get_token_from_request()関数を実装、ツール関数にContextパラメータを追加 |
| docs/setup_ja.md | 2つのトークン取得方法(FastMCP OAuthフローとAuthorizationヘッダー経由)を説明するセクションを追加、使用例を記載 |
| docs/setup.md | 日本語版と同様に2つのトークン取得方法の説明を追加(英語版) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code improvements: - Separate RuntimeError and AttributeError handling with appropriate log levels - Add strip() and empty token validation for Authorization header - Improve error messages and code clarity Documentation improvements: - Clarify Method 2 is HTTP transport only (not available in stdio mode) - Add note that standard MCP clients (Claude Desktop) should use Method 1 - Emphasize this is primarily for testing and custom HTTP integrations - Update both English and Japanese documentation
|
@k-ibaraki |
|
/gemini review |
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.
Code Review
このプルリクエストは、Authorizationヘッダーを介して直接アクセストークンを渡す機能を追加するもので、素晴らしい改善です。これにより、既存のトークン管理システムを持つユーザーの柔軟性が向上し、テストも容易になります。
コードの変更は概ね良好で、新しいトークン取得ロジックは_get_token_from_request関数にうまくカプセル化されています。また、この新機能に対する単体テストが追加されている点も評価できます。
レビューコメントとして、HTTP仕様(RFC 7235)に準拠するため、Bearerスキームのチェックを大文字・小文字を区別しないように修正する提案をしました。これに合わせて、関連するテストケースの修正も提案しています。
全体として、後方互換性を維持しつつ機能を拡張する、よく考えられた変更だと思います。
Update Authorization header parsing to accept Bearer scheme in any case (Bearer, bearer, BEARER, etc.) as required by RFC 7235. This improves interoperability with various HTTP clients. Changes: - Use case-insensitive check for "bearer " prefix in src/server.py - Update test to verify lowercase "bearer" is accepted - Rename test from test_bearer_prefix_case_sensitive to test_bearer_prefix_case_insensitive to reflect new behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
概要
Authorizationヘッダー経由で直接アクセストークンを渡せるようにし、高度なトークン管理シナリオに対応します。
変更内容
_get_token_from_request()関数を追加し、Authorizationヘッダーからトークンを取得Authorization: Bearer <token>ヘッダー経由で直接トークンを渡すsharepoint_docs_search,sharepoint_docs_download)にContextパラメータを追加メリット
get_http_request()を使用)テスト