-
Notifications
You must be signed in to change notification settings - Fork 0
Add headers parameter to OPARemoteClient #1
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
Co-authored-by: becojo <172889+becojo@users.noreply.github.com>
Co-authored-by: becojo <172889+becojo@users.noreply.github.com>
Co-authored-by: becojo <172889+becojo@users.noreply.github.com>
tests/test_opa.py
Outdated
| "--addr", | ||
| "127.0.0.1:8182", | ||
| "--bundle", | ||
| "tests/testdata/rego", |
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.
You're not testing anything new in this test. Create a different bundle that uses OPA's bearer token auth. see https://www.openpolicyagent.org/docs/security#authentication-and-authorization
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.
Updated test to use OPA's bearer token authentication. The new test starts OPA with --authentication=token --authorization=basic, verifies that requests fail without auth headers, and succeed with the correct Authorization: Bearer test-token header. Commit: fb6544e
README.md
Outdated
| ) | ||
| ``` | ||
|
|
||
| #### Custom Headers |
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.
Not needed to modify the readme
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.
Reverted README changes. Commit: fb6544e
Co-authored-by: becojo <172889+becojo@users.noreply.github.com>
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 PR adds support for custom HTTP headers in OPARemoteClient to enable authentication with remote OPA servers, particularly for scenarios requiring bearer tokens or API keys.
Key Changes:
- Added an optional
headersparameter toOPARemoteClientusingField(default_factory=dict)to avoid mutable default issues - Modified the
is_allowed()method to pass headers to the HTTP request - Added comprehensive test coverage for authentication scenarios with an OPA server configured for token-based authentication
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/opadk/opa.py |
Added headers field to OPARemoteClient class and modified is_allowed() to pass headers to HTTP POST request |
tests/test_opa.py |
Added new test test_opa_remote_client_with_headers that verifies authentication behavior with and without bearer tokens |
tests/testdata/rego-auth/system-authz.rego |
New OPA authorization policy for testing token-based authentication |
tests/testdata/rego-auth/policy.rego |
New OPA policy for testing tool/agent authorization decisions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| json = await response.json() | ||
| return OPADKResponse.model_validate(json.get("result")) |
Copilot
AI
Dec 29, 2025
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 method should check the HTTP response status before attempting to parse the response body. When authentication fails (e.g., 401 Unauthorized) or other HTTP errors occur, the method should handle these appropriately. Consider using response.raise_for_status() or checking response.status to provide clearer error messages for authentication and authorization failures.
| json = await response.json() | |
| return OPADKResponse.model_validate(json.get("result")) | |
| # Check HTTP status before attempting to parse the response body | |
| if response.status == 401: | |
| raise Exception("OPA request unauthorized (401): check credentials or tokens.") | |
| if response.status == 403: | |
| raise Exception("OPA request forbidden (403): insufficient permissions.") | |
| # Raise for any other non-successful status codes | |
| response.raise_for_status() | |
| json_body = await response.json() | |
| return OPADKResponse.model_validate(json_body.get("result")) |
| try: | ||
| await client_no_auth.is_allowed( | ||
| scope="tool", | ||
| input={"state": {"tool_allowed": True}}, | ||
| ) | ||
| self.fail("Expected request to fail without authorization") | ||
| except Exception: | ||
| # Expected - request should fail without proper auth | ||
| pass |
Copilot
AI
Dec 29, 2025
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 test catches all exceptions with a bare except Exception: clause and doesn't verify the specific error type or message. This makes the test less precise and could pass even if the failure happens for an unexpected reason. Consider checking for a specific exception type or verifying the HTTP status code (401 Unauthorized) to ensure authentication is actually being enforced.
Enables passing custom HTTP headers to the remote OPA server, primarily for authentication via tokens or API keys.
Changes
headers: dict[str, str]field toOPARemoteClientusingField(default_factory=dict)to avoid mutable default issuesaiohttp.ClientSession.post()inis_allowed()method--authentication=token --authorization=basicflagsAuthorization: BearerheaderExample
Backward compatible—defaults to empty dict.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.