Migrate from shared secret to client credential for publisher api#511
Migrate from shared secret to client credential for publisher api#511
Conversation
📝 WalkthroughWalkthroughMigrates evaluation score publisher authentication from a static API key to OAuth2 client credentials flow. Removes Changes
Sequence DiagramsequenceDiagram
actor EvalJob as Evaluation Job
participant IDP as Identity Provider (Thunder)
participant AgentMgr as Agent Manager
participant JWKS as JWKS Endpoint
EvalJob->>IDP: POST /token (client_id, client_secret)
IDP-->>EvalJob: access_token (short-lived)
EvalJob->>AgentMgr: POST /api/v1/publisher/scores<br/>(Authorization: Bearer token)
AgentMgr->>JWKS: Fetch JWKS (for verification)
JWKS-->>AgentMgr: Public keys
AgentMgr->>AgentMgr: Validate JWT signature<br/>against JWKS
alt Token Valid
AgentMgr-->>EvalJob: 200 OK (scores published)
else Token Invalid
AgentMgr-->>EvalJob: 401 Unauthorized
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agent-manager-service/api/app.go (1)
68-79:⚠️ Potential issue | 🔴 CriticalAdd publisher-specific scope validation to enforce least-privilege access.
The publisher routes reuse
params.AuthMiddleware, which only validates the JWT's issuer and audience globally. Without an additional check for a publisher-specific scope (e.g.,scores:publish) in the controller or middleware, any token valid for the general API can publish scores, defeating the least-privilege model. Either add a scope check inPublishScoresusingjwtassertion.HasAllScopes(ctx, []string{"scores:publish"})or create a publisher-specific auth middleware wrapper.Also note: the comment in
monitor_publisher_routes.goline 27 claims "API key authentication" but the code applies JWT middleware; clarify or correct the comment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agent-manager-service/api/app.go` around lines 68 - 79, The publisher routes currently use the generic params.AuthMiddleware (registered via RegisterMonitorPublisherRoutes) which only checks issuer/audience; add enforcement of a publisher-specific scope by either (A) inserting a scope-check in the PublishScores handler using jwtassertion.HasAllScopes(ctx, []string{"scores:publish"}) at the start of PublishScores or (B) creating a thin publisher-specific middleware wrapper (e.g., PublisherAuthMiddleware) that calls params.AuthMiddleware then verifies jwtassertion.HasAllScopes(ctx, []string{"scores:publish"}) before registering it in app.go for the publisherMux; also update the stale comment in monitor_publisher_routes.go that mentions "API key authentication" to reflect that JWT middleware is used.
🧹 Nitpick comments (2)
deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yaml (1)
256-316: Extract the repeated “ensure application” flow into one reusable helper.This is now the third copy of the same curl/grep/create-or-update script. Any future change to application bootstrap behavior will have to be kept in sync across all three blocks, which is easy to miss in auth-sensitive code. A parameterized helper/template would make this safer to maintain.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yaml` around lines 256 - 316, Extract the repeated create-or-update logic into a single reusable helper (e.g., a template or script like "amp-app-bootstrap.sh" or a Helm named template) and replace the three inline copies with calls to that helper, passing parameters for name, clientId, clientSecret, description, authFlowGraphId, grantTypes, tokenEndpointAuthMethod, pkceRequired, publicClient, accessTokenValidityPeriod, thunderUrl and $curlInsecure; move the APP_PAYLOAD construction and the existing_apps/app_id detection (currently in 53-amp-publisher-client.sh: APP_PAYLOAD, existing_apps, app_id and the curl PUT/POST blocks) into the helper so updates are centralized, and update the three callers to invoke the helper with their specific values.evaluation-job/test_main.py (1)
405-410: Add direct tests forOAuth2TokenManager.All of the updated publish tests stub
get_token(), so the new cache/refresh path never runs under test. Please add focused coverage for initial token fetch, cache hit, refresh after expiry, and token-endpoint failures.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@evaluation-job/test_main.py` around lines 405 - 410, Add focused unit tests for OAuth2TokenManager covering: (1) initial token fetch by invoking OAuth2TokenManager.get_token() with the token endpoint mocked to return a valid access_token and expiry, (2) cache hit where a second get_token() call before expiry returns the same token without calling the token endpoint again, (3) refresh after expiry by advancing/patching the time source (or setting token expiry to past) and asserting get_token() triggers a new token endpoint call and returns a different token, and (4) token-endpoint failure where the mocked token endpoint returns an error/invalid response and get_token() raises or returns the expected error; reference the OAuth2TokenManager class and its get_token() method and use your existing test helpers/mocking framework to stub HTTP requests and time behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@deployments/helm-charts/wso2-amp-evaluation-extension/templates/workflow-templates/workflow-monitor-evaluation.yaml`:
- Around line 77-82: The workflow currently injects the IdP client secret as
plaintext via the env var name "idp-client-secret" using {{
.Values.ampEvaluation.publisher.clientSecret }}, which exposes it in the
Workflow CR; change this to read from a Kubernetes Secret instead by creating a
Secret containing the client secret and replacing the env entry to use
valueFrom.secretKeyRef (reference the same env name "idp-client-secret" but use
valueFrom.secretKeyRef with the Secret name and key), and update any Helm values
to accept a secret name/key rather than the raw secret string so production
deployments no longer embed the secret in the manifest.
In
`@deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yaml`:
- Around line 270-292: The APP_PAYLOAD currently embeds
.Values.thunder.bootstrap.ampPublisherClient.clientSecret into the ConfigMap
which leaks the secret; stop templating client_secret and instead read it from
an environment variable injected from a Kubernetes Secret (e.g.
AMP_PUBLISHER_CLIENT_SECRET) at runtime. Update the bootstrap Job/Pod spec to
mount a Kubernetes Secret as an env var named AMP_PUBLISHER_CLIENT_SECRET,
remove any usage of ampPublisherClient.clientSecret from the template, and
change the script that builds APP_PAYLOAD to insert the client_secret value from
the AMP_PUBLISHER_CLIENT_SECRET environment variable (refer to APP_PAYLOAD and
AMP_PUBLISHER_CLIENT_SECRET to find the locations to change). Ensure no secret
value remains in the rendered ConfigMap or Helm values.
In `@deployments/helm-charts/wso2-amp-thunder-extension/values.yaml`:
- Around line 135-137: Remove the hard-coded default for
ampPublisherClient.clientSecret in values.yaml and require it to be provided
from a Kubernetes Secret or explicitly in values; set
ampPublisherClient.clientSecret to null (or remove it) and update the chart
templates that reference .Values.ampPublisherClient.clientSecret (e.g., the
secret/credentials consumer templates) to either read the value from a
Kubernetes Secret or use Helm's required() function to fail the install with a
clear message like "ampPublisherClient.clientSecret is required" if the value is
not supplied.
In `@evaluation-job/main.py`:
- Around line 62-80: OAuth2TokenManager currently omits an explicit scope in the
client_credentials exchange; add a scope parameter to the OAuth2TokenManager
(constructor and stored property) and include it in the token request payload in
get_token, thread that scope into main() by reading an environment variable
(e.g., IDP_SCOPE) and passing it when constructing OAuth2TokenManager, and
update the Thunder registration by adding the same scope to the
ampPublisherClient entry in values.yaml and the Helm template so the published
token matches the resource-server authorization config (ensure the
resource-server defines the identical scope, e.g., scores:publish).
In `@website/docs/getting-started/managed-cluster.mdx`:
- Around line 564-573: Replace the inline --set usage that embeds a literal
ampEvaluation.publisher.clientSecret (and clientId/idpTokenUrl) with guidance
and an example that uses a values file or Kubernetes Secret (e.g., reference
publisher-auth-values.yaml) so users supply a placeholder and mount a Secret
instead of pasting secrets into shell/CI; update the helm install block to
remove --set ampEvaluation.publisher.clientSecret and instead reference a values
file, add a short note that the Thunder pre-registered OAuth client
(ampPublisherClient) must exist beforehand, and show the clientId/idpTokenUrl as
placeholders to be populated from the values/Secret.
---
Outside diff comments:
In `@agent-manager-service/api/app.go`:
- Around line 68-79: The publisher routes currently use the generic
params.AuthMiddleware (registered via RegisterMonitorPublisherRoutes) which only
checks issuer/audience; add enforcement of a publisher-specific scope by either
(A) inserting a scope-check in the PublishScores handler using
jwtassertion.HasAllScopes(ctx, []string{"scores:publish"}) at the start of
PublishScores or (B) creating a thin publisher-specific middleware wrapper
(e.g., PublisherAuthMiddleware) that calls params.AuthMiddleware then verifies
jwtassertion.HasAllScopes(ctx, []string{"scores:publish"}) before registering it
in app.go for the publisherMux; also update the stale comment in
monitor_publisher_routes.go that mentions "API key authentication" to reflect
that JWT middleware is used.
---
Nitpick comments:
In
`@deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yaml`:
- Around line 256-316: Extract the repeated create-or-update logic into a single
reusable helper (e.g., a template or script like "amp-app-bootstrap.sh" or a
Helm named template) and replace the three inline copies with calls to that
helper, passing parameters for name, clientId, clientSecret, description,
authFlowGraphId, grantTypes, tokenEndpointAuthMethod, pkceRequired,
publicClient, accessTokenValidityPeriod, thunderUrl and $curlInsecure; move the
APP_PAYLOAD construction and the existing_apps/app_id detection (currently in
53-amp-publisher-client.sh: APP_PAYLOAD, existing_apps, app_id and the curl
PUT/POST blocks) into the helper so updates are centralized, and update the
three callers to invoke the helper with their specific values.
In `@evaluation-job/test_main.py`:
- Around line 405-410: Add focused unit tests for OAuth2TokenManager covering:
(1) initial token fetch by invoking OAuth2TokenManager.get_token() with the
token endpoint mocked to return a valid access_token and expiry, (2) cache hit
where a second get_token() call before expiry returns the same token without
calling the token endpoint again, (3) refresh after expiry by advancing/patching
the time source (or setting token expiry to past) and asserting get_token()
triggers a new token endpoint call and returns a different token, and (4)
token-endpoint failure where the mocked token endpoint returns an error/invalid
response and get_token() raises or returns the expected error; reference the
OAuth2TokenManager class and its get_token() method and use your existing test
helpers/mocking framework to stub HTTP requests and time behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2202798d-d4b9-4c02-90f7-ed7b1ee59eb8
📒 Files selected for processing (22)
.github/workflows/agent-manager-service-pr-checks.yamlagent-manager-service/.env.exampleagent-manager-service/api/app.goagent-manager-service/config/config.goagent-manager-service/config/config_loader.goagent-manager-service/controllers/monitor_scores_publisher_controller.goagent-manager-service/wiring/wire_gen.godeployments/helm-charts/wso2-agent-manager/templates/agent-manager-service/deployment.yamldeployments/helm-charts/wso2-agent-manager/templates/agent-manager-service/secret.yamldeployments/helm-charts/wso2-agent-manager/values.yamldeployments/helm-charts/wso2-amp-evaluation-extension/templates/workflow-templates/cluster-workflow-monitor-evaluation.yamldeployments/helm-charts/wso2-amp-evaluation-extension/templates/workflow-templates/workflow-monitor-evaluation.yamldeployments/helm-charts/wso2-amp-evaluation-extension/values.yamldeployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yamldeployments/helm-charts/wso2-amp-thunder-extension/values.yamldeployments/scripts/setup-openchoreo.shdocs/install/managed-cluster.mddocs/install/self-hosted-cluster.mdevaluation-job/main.pyevaluation-job/test_main.pywebsite/docs/getting-started/managed-cluster.mdxwebsite/docs/getting-started/self-hosted-cluster.mdx
💤 Files with no reviewable changes (7)
- agent-manager-service/config/config_loader.go
- agent-manager-service/config/config.go
- deployments/helm-charts/wso2-agent-manager/values.yaml
- agent-manager-service/controllers/monitor_scores_publisher_controller.go
- agent-manager-service/.env.example
- deployments/helm-charts/wso2-agent-manager/templates/agent-manager-service/deployment.yaml
- .github/workflows/agent-manager-service-pr-checks.yaml
| - name: idp-token-url | ||
| value: {{ .Values.ampEvaluation.publisher.idpTokenUrl | quote }} | ||
| - name: idp-client-id | ||
| value: {{ .Values.ampEvaluation.publisher.clientId | quote }} | ||
| - name: idp-client-secret | ||
| value: {{ .Values.ampEvaluation.publisher.clientSecret | quote }} |
There was a problem hiding this comment.
Security concern: Client secret passed as plaintext in workflow arguments.
The idp-client-secret is passed as a plaintext value in the workflow arguments. When the Workflow CR is created, this secret will be visible in the Kubernetes API (e.g., via kubectl get workflow -o yaml). Consider using a Kubernetes Secret reference instead, or ensure the workflow namespace has appropriate RBAC restrictions.
This may be acceptable for development environments, but for production deployments, consider:
- Storing credentials in a Kubernetes Secret
- Referencing the secret via
valueFrom.secretKeyRefin the container env
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@deployments/helm-charts/wso2-amp-evaluation-extension/templates/workflow-templates/workflow-monitor-evaluation.yaml`
around lines 77 - 82, The workflow currently injects the IdP client secret as
plaintext via the env var name "idp-client-secret" using {{
.Values.ampEvaluation.publisher.clientSecret }}, which exposes it in the
Workflow CR; change this to read from a Kubernetes Secret instead by creating a
Secret containing the client secret and replacing the env entry to use
valueFrom.secretKeyRef (reference the same env name "idp-client-secret" but use
valueFrom.secretKeyRef with the Secret name and key), and update any Helm values
to accept a secret name/key rather than the raw secret string so production
deployments no longer embed the secret in the manifest.
| APP_PAYLOAD='{ | ||
| "name": "{{ .Values.thunder.bootstrap.ampPublisherClient.name }}", | ||
| "description": "{{ .Values.thunder.bootstrap.ampPublisherClient.description }}", | ||
| "auth_flow_graph_id": "{{ .Values.thunder.bootstrap.ampPublisherClient.authFlowGraphId }}", | ||
| "inbound_auth_config": [ | ||
| { | ||
| "type": "oauth2", | ||
| "config": { | ||
| "client_id": "{{ .Values.thunder.bootstrap.ampPublisherClient.clientId }}", | ||
| "client_secret": "{{ .Values.thunder.bootstrap.ampPublisherClient.clientSecret }}", | ||
| "grant_types": {{ .Values.thunder.bootstrap.ampPublisherClient.grantTypes | toJson }}, | ||
| "token_endpoint_auth_method": "{{ .Values.thunder.bootstrap.ampPublisherClient.tokenEndpointAuthMethod }}", | ||
| "pkce_required": {{ .Values.thunder.bootstrap.ampPublisherClient.pkceRequired }}, | ||
| "public_client": {{ .Values.thunder.bootstrap.ampPublisherClient.publicClient }}, | ||
| "token": { | ||
| "access_token": { | ||
| "validity_period": {{ .Values.thunder.bootstrap.ampPublisherClient.accessTokenValidityPeriod }} | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| }' |
There was a problem hiding this comment.
Do not embed the OAuth client secret in this ConfigMap-backed script.
Because this payload is rendered into a ConfigMap, the publisher credential becomes readable through ConfigMap access and Helm manifests. That is a direct secret leak for the new machine-to-machine auth path. Please inject the secret from a Kubernetes Secret at runtime and build the payload from that value instead of templating it into the script.
Possible direction
- APP_PAYLOAD='{
+ APP_PAYLOAD=$(cat <<EOF
+ {
"name": "{{ .Values.thunder.bootstrap.ampPublisherClient.name }}",
"description": "{{ .Values.thunder.bootstrap.ampPublisherClient.description }}",
"auth_flow_graph_id": "{{ .Values.thunder.bootstrap.ampPublisherClient.authFlowGraphId }}",
"inbound_auth_config": [
{
"type": "oauth2",
"config": {
"client_id": "{{ .Values.thunder.bootstrap.ampPublisherClient.clientId }}",
- "client_secret": "{{ .Values.thunder.bootstrap.ampPublisherClient.clientSecret }}",
+ "client_secret": "${AMP_PUBLISHER_CLIENT_SECRET}",
"grant_types": {{ .Values.thunder.bootstrap.ampPublisherClient.grantTypes | toJson }},
"token_endpoint_auth_method": "{{ .Values.thunder.bootstrap.ampPublisherClient.tokenEndpointAuthMethod }}",
"pkce_required": {{ .Values.thunder.bootstrap.ampPublisherClient.pkceRequired }},
"public_client": {{ .Values.thunder.bootstrap.ampPublisherClient.publicClient }},
"token": {
"access_token": {
"validity_period": {{ .Values.thunder.bootstrap.ampPublisherClient.accessTokenValidityPeriod }}
}
}
}
}
]
- }'
+ }
+EOF
+ )This still needs the bootstrap job/pod to source AMP_PUBLISHER_CLIENT_SECRET from a Kubernetes Secret.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yaml`
around lines 270 - 292, The APP_PAYLOAD currently embeds
.Values.thunder.bootstrap.ampPublisherClient.clientSecret into the ConfigMap
which leaks the secret; stop templating client_secret and instead read it from
an environment variable injected from a Kubernetes Secret (e.g.
AMP_PUBLISHER_CLIENT_SECRET) at runtime. Update the bootstrap Job/Pod spec to
mount a Kubernetes Secret as an env var named AMP_PUBLISHER_CLIENT_SECRET,
remove any usage of ampPublisherClient.clientSecret from the template, and
change the script that builds APP_PAYLOAD to insert the client_secret value from
the AMP_PUBLISHER_CLIENT_SECRET environment variable (refer to APP_PAYLOAD and
AMP_PUBLISHER_CLIENT_SECRET to find the locations to change). Ensure no secret
value remains in the rendered ConfigMap or Helm values.
| ampPublisherClient: | ||
| clientId: "amp-publisher-client" | ||
| clientSecret: "amp-publisher-client-secret" |
There was a problem hiding this comment.
Avoid shipping a well-known default for the publisher client secret.
This reintroduces a predictable shared credential if the chart is installed without an override, which undercuts the goal of replacing the static publisher key with per-service credentials. Please make this value come from a Kubernetes Secret or fail the install when it is not explicitly provided, rather than committing a usable default here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deployments/helm-charts/wso2-amp-thunder-extension/values.yaml` around lines
135 - 137, Remove the hard-coded default for ampPublisherClient.clientSecret in
values.yaml and require it to be provided from a Kubernetes Secret or explicitly
in values; set ampPublisherClient.clientSecret to null (or remove it) and update
the chart templates that reference .Values.ampPublisherClient.clientSecret
(e.g., the secret/credentials consumer templates) to either read the value from
a Kubernetes Secret or use Helm's required() function to fail the install with a
clear message like "ampPublisherClient.clientSecret is required" if the value is
not supplied.
| def __init__(self, token_url: str, client_id: str, client_secret: str): | ||
| self.token_url = token_url | ||
| self.client_id = client_id | ||
| self.client_secret = client_secret | ||
| self._token: str | None = None | ||
| self._expires_at: float = 0 | ||
|
|
||
| def get_token(self) -> str: | ||
| """Get a valid access token, refreshing if needed.""" | ||
| if self._token and time.time() < self._expires_at - 30: | ||
| return self._token | ||
|
|
||
| response = requests.post( | ||
| self.token_url, | ||
| data={ | ||
| "grant_type": "client_credentials", | ||
| "client_id": self.client_id, | ||
| "client_secret": self.client_secret, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 '\bIDP_SCOPE\b|\bpublisher\.scope\b|\bscores:publish\b|\bampPublisherClient\b|\bscopes?\b' .Repository: wso2/agent-manager
Length of output: 50377
🏁 Script executed:
#!/bin/bash
# Find evaluation-job main.py and check OAuth2TokenManager implementation
find . -name "main.py" -path "*/evaluation-job/*" | head -5Repository: wso2/agent-manager
Length of output: 86
🏁 Script executed:
#!/bin/bash
# Search for scores:publish scope specifically
rg -i 'scores.*publish|publish.*scope' --type py --type yaml --type json -nRepository: wso2/agent-manager
Length of output: 551
🏁 Script executed:
#!/bin/bash
# Check Thunder bootstrap for scope configuration in ampPublisherClient
rg -A 20 'ampPublisherClient' ./deployments/helm-charts/wso2-amp-thunder-extension/values.yamlRepository: wso2/agent-manager
Length of output: 850
🏁 Script executed:
#!/bin/bash
# Search for IDP_SCOPE in evaluation-job specifically
rg 'IDP_' evaluation-job/ -n -A 2Repository: wso2/agent-manager
Length of output: 3749
🏁 Script executed:
#!/bin/bash
cat -n evaluation-job/main.py | head -90 | tail -35Repository: wso2/agent-manager
Length of output: 1436
🏁 Script executed:
#!/bin/bash
# Check Thunder bootstrap template to see if scopes are included in the API payload
rg -A 30 'APP_PAYLOAD=' ./deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yaml | head -50Repository: wso2/agent-manager
Length of output: 2880
🏁 Script executed:
#!/bin/bash
# Find the ampPublisherClient APP_PAYLOAD specifically
rg -B 5 -A 30 '"{{ .Values.thunder.bootstrap.ampPublisherClient' ./deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yaml | head -60Repository: wso2/agent-manager
Length of output: 194
🏁 Script executed:
#!/bin/bash
# Check if Thunder API accepts scopes field in client registration
rg -i 'scope' ./deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yamlRepository: wso2/agent-manager
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search for ampPublisherClient in bootstrap template (literal string)
rg 'ampPublisherClient' -B 3 -A 30 ./deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yamlRepository: wso2/agent-manager
Length of output: 3126
🏁 Script executed:
#!/bin/bash
# Look for any "scopes" or "scope" field in client configs
rg -i '"scopes"|"scope"' ./deployments/helm-charts/wso2-amp-thunder-extension/templates/amp-thunder-bootstrap.yamlRepository: wso2/agent-manager
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check Thunder API documentation or comments for supported fields
rg -i 'scope' ./deployments/helm-charts/wso2-amp-thunder-extension/values.yamlRepository: wso2/agent-manager
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check if there's any authorization/resource server scope config in the codebase
rg -i 'resource.*server|authorization.*scope|api.*scope' ./agent-manager-service/ --type go -n | head -30Repository: wso2/agent-manager
Length of output: 3076
🏁 Script executed:
#!/bin/bash
# Look for any existing scope patterns in the entire deployments/helm-charts
find ./deployments/helm-charts -name "*.yaml" -o -name "*.yml" | xargs grep -l -i 'scope' 2>/dev/null | head -10Repository: wso2/agent-manager
Length of output: 539
🏁 Script executed:
#!/bin/bash
# Check how the evaluation job publishes scores to understand required permissions
rg -B 5 -A 10 'def publish_scores|def _publish_' evaluation-job/main.py | head -40Repository: wso2/agent-manager
Length of output: 421
🏁 Script executed:
#!/bin/bash
# Get more of the publish_scores function to see the endpoint
rg -A 50 'def publish_scores' evaluation-job/main.py | head -70Repository: wso2/agent-manager
Length of output: 1850
🏁 Script executed:
#!/bin/bash
# Search for any scope/permission references in resource server or API definitions
find ./agent-manager-service -name "*.go" | xargs grep -l 'scope' | head -5Repository: wso2/agent-manager
Length of output: 339
🏁 Script executed:
#!/bin/bash
# Check if there are any authorization checks in the publish endpoint
rg -B 5 -A 15 'PublishScores|publish.*score' ./agent-manager-service --type go -A 10 | head -50Repository: wso2/agent-manager
Length of output: 4664
🏁 Script executed:
#!/bin/bash
# Check for authorization middleware or scope validation in publish endpoint
rg -B 10 -A 20 'func.*PublishScores' ./agent-manager-service/services/monitor_scores_service.go | head -40Repository: wso2/agent-manager
Length of output: 1049
🏁 Script executed:
#!/bin/bash
# Search for any middleware or scope validation in the API routes
find ./agent-manager-service -name "*.go" -type f | xargs grep -l 'PublishScores\|/publish' | head -5Repository: wso2/agent-manager
Length of output: 326
🏁 Script executed:
#!/bin/bash
# Check the routes/handlers file for scope authorization
rg -B 5 -A 10 'PublishMonitorScores|/scores.*publish' ./agent-manager-service --type go | head -50Repository: wso2/agent-manager
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check the monitor_publisher_routes.go file
cat -n ./agent-manager-service/api/monitor_publisher_routes.goRepository: wso2/agent-manager
Length of output: 1571
🏁 Script executed:
#!/bin/bash
# Check the monitor_scores_publisher_controller.go file
cat -n ./agent-manager-service/controllers/monitor_scores_publisher_controller.go | head -80Repository: wso2/agent-manager
Length of output: 3446
🏁 Script executed:
#!/bin/bash
# Check what middleware.HandleFuncWithValidation does for authentication
rg -B 5 -A 20 'func HandleFuncWithValidation|type.*Middleware' ./agent-manager-service/middleware/*.go | head -60Repository: wso2/agent-manager
Length of output: 2343
🏁 Script executed:
#!/bin/bash
# Look for API key or OAuth middleware
ls -la ./agent-manager-service/middleware/Repository: wso2/agent-manager
Length of output: 553
🏁 Script executed:
#!/bin/bash
# Check how the evaluation job authenticates - is it using OAuth or API key?
rg -B 5 -A 5 'Authorization|Bearer|api.key' evaluation-job/main.py | head -40Repository: wso2/agent-manager
Length of output: 416
🏁 Script executed:
#!/bin/bash
# Check if there's JWT assertion middleware being used elsewhere
rg -B 5 -A 10 'jwtassertion|JWT' ./agent-manager-service/api/ --type go | head -50Repository: wso2/agent-manager
Length of output: 4503
🏁 Script executed:
#!/bin/bash
# Look for any OAuth scope validation in the codebase
rg -i 'scope.*validation|validate.*scope|required.*scope' ./agent-manager-service --type goRepository: wso2/agent-manager
Length of output: 370
🏁 Script executed:
#!/an/bash
# Check the HasAllScopes function implementation
rg -B 5 -A 15 'HasAllScopes' ./agent-manager-service/middleware/jwtassertion/auth.goRepository: wso2/agent-manager
Length of output: 533
🏁 Script executed:
#!/bin/bash
# Check if JWT assertion middleware is applied to monitor publisher routes
rg -B 10 -A 10 'RegisterMonitorPublisherRoutes\|monitor.*publisher' ./agent-manager-service/api/app.goRepository: wso2/agent-manager
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Search for where jwtassertion middleware is actually used/applied
rg 'jwtassertion\|WithJWTAssertion\|JWT.*Assertion' ./agent-manager-service --type go -B 3 -A 3 | head -60Repository: wso2/agent-manager
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check how monitor_publisher routes are registered in app.go
rg -B 5 -A 5 'monitor_publisher\|MonitorPublisher' ./agent-manager-service/api/app.go -iRepository: wso2/agent-manager
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check if there's any API key authentication being applied
rg -i 'api.*key\|apikey' ./agent-manager-service/middleware/ --type goRepository: wso2/agent-manager
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check the full app.go setup to see route registration
cat -n ./agent-manager-service/api/app.go | grep -A 50 'func NewApp'Repository: wso2/agent-manager
Length of output: 44
🏁 Script executed:
#!/bin/bash
# Check the structure of app.go
wc -l ./agent-manager-service/api/app.go
head -150 ./agent-manager-service/api/app.goRepository: wso2/agent-manager
Length of output: 5165
🏁 Script executed:
#!/bin/bash
# Search for all route registrations in app.go
grep -n 'Register.*Routes\|HandleFunc' ./agent-manager-service/api/app.goRepository: wso2/agent-manager
Length of output: 875
🏁 Script executed:
#!/bin/bash
# Check if HasAllScopes is being used anywhere in the publisher controller
rg 'HasAllScopes|scope' ./agent-manager-service/controllers/monitor_scores_publisher_controller.goRepository: wso2/agent-manager
Length of output: 44
🏁 Script executed:
#!/antml:bash
# Check what params.AuthMiddleware actually does
find ./agent-manager-service -name "*.go" | xargs grep -l 'AuthMiddleware\|auth.*middleware' | head -5Repository: wso2/agent-manager
Length of output: 282
🏁 Script executed:
#!/bin/bash
# Check the complete jwtassertion/auth.go file
cat -n ./agent-manager-service/middleware/jwtassertion/auth.goRepository: wso2/agent-manager
Length of output: 12214
Add explicit scope to OAuth2 client-credentials token exchange.
The OAuth2TokenManager never requests a scope, leaving token permissions to IdP defaults. While the codebase has scope-validation infrastructure (HasAllScopes), the publisher endpoint doesn't enforce any required scopes. This gaps the least-privilege boundary and will break if scope enforcement is added to the endpoint later.
Make the scope explicit in both the token request and the Thunder ampPublisherClient registration:
Suggested changes
In evaluation-job/main.py, add scope parameter to OAuth2TokenManager:
- def __init__(self, token_url: str, client_id: str, client_secret: str):
+ def __init__(self, token_url: str, client_id: str, client_secret: str, scope: str | None = None):
self.token_url = token_url
self.client_id = client_id
self.client_secret = client_secret
+ self.scope = scope
self._token: str | None = None
self._expires_at: float = 0
response = requests.post(
self.token_url,
data={
"grant_type": "client_credentials",
"client_id": self.client_id,
"client_secret": self.client_secret,
+ **({"scope": self.scope} if self.scope else {}),
},
timeout=10,
)In evaluation-job/main.py, thread the scope through the main() function as an environment variable (e.g., IDP_SCOPE).
In Thunder bootstrap values (deployments/helm-charts/wso2-amp-thunder-extension/values.yaml), add scope to ampPublisherClient:
ampPublisherClient:
clientId: "amp-publisher-client"
clientSecret: "amp-publisher-client-secret"
name: "AMP Publisher Client"
description: "Client application for evaluation job to publish scores"
+ scopes:
+ - "scores:publish"Then wire that scope through the Helm template when registering the client with Thunder.
Verify that the same publisher scope (e.g., scores:publish) is defined in the Thunder resource-server authorization config so the evaluation job's token is recognized as authorized to publish scores.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@evaluation-job/main.py` around lines 62 - 80, OAuth2TokenManager currently
omits an explicit scope in the client_credentials exchange; add a scope
parameter to the OAuth2TokenManager (constructor and stored property) and
include it in the token request payload in get_token, thread that scope into
main() by reading an environment variable (e.g., IDP_SCOPE) and passing it when
constructing OAuth2TokenManager, and update the Thunder registration by adding
the same scope to the ampPublisherClient entry in values.yaml and the Helm
template so the published token matches the resource-server authorization config
(ensure the resource-server defines the identical scope, e.g., scores:publish).
| **Note:** The Evaluation Extension uses OAuth2 client credentials (via the AMP Publisher Client created by Thunder) to authenticate when publishing scores to the Agent Manager. The default credentials match the Thunder bootstrap configuration. If you customized the Thunder bootstrap `ampPublisherClient` settings, override them here: | ||
|
|
||
| ```bash | ||
| helm install amp-evaluation-extension \ | ||
| oci://${HELM_CHART_REGISTRY}/wso2-amp-evaluation-extension \ | ||
| --version 0.8.0 \ | ||
| --namespace ${BUILD_CI_NS} \ | ||
| --set ampEvaluation.publisher.apiKey="your-custom-key" \ | ||
| --set ampEvaluation.publisher.idpTokenUrl="http://thunder.amp-thunder.svc.cluster.local:8090/oauth2/token" \ | ||
| --set ampEvaluation.publisher.clientId="amp-publisher-client" \ | ||
| --set ampEvaluation.publisher.clientSecret="amp-publisher-client-secret" \ |
There was a problem hiding this comment.
Avoid publishing a literal client secret in the install docs.
This replaces the old API key, but it still tells readers to reuse a known secret and paste it into helm --set. That recreates the “well-known shared secret” problem and leaks the secret into shell history / CI logs. Please switch this to a placeholder plus Secret/values-file based wiring, and call out that Thunder (or another pre-registered OAuth client) must exist first.
Suggested docs change
-**Note:** The Evaluation Extension uses OAuth2 client credentials (via the AMP Publisher Client created by Thunder) to authenticate when publishing scores to the Agent Manager. The default credentials match the Thunder bootstrap configuration. If you customized the Thunder bootstrap `ampPublisherClient` settings, override them here:
+**Note:** The Evaluation Extension uses OAuth2 client credentials (via a publisher client created by Thunder or another IDP) to authenticate when publishing scores to the Agent Manager. Create a unique client secret per environment and pass it via a values file or Kubernetes Secret. Avoid committing or pasting a literal secret into the command line.
```bash
helm install amp-evaluation-extension \
oci://${HELM_CHART_REGISTRY}/wso2-amp-evaluation-extension \
--version 0.8.0 \
--namespace ${BUILD_CI_NS} \
- --set ampEvaluation.publisher.idpTokenUrl="http://thunder.amp-thunder.svc.cluster.local:8090/oauth2/token" \
- --set ampEvaluation.publisher.clientId="amp-publisher-client" \
- --set ampEvaluation.publisher.clientSecret="amp-publisher-client-secret" \
+ --values publisher-auth-values.yaml \
--timeout 1800s</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @website/docs/getting-started/managed-cluster.mdx around lines 564 - 573,
Replace the inline --set usage that embeds a literal
ampEvaluation.publisher.clientSecret (and clientId/idpTokenUrl) with guidance
and an example that uses a values file or Kubernetes Secret (e.g., reference
publisher-auth-values.yaml) so users supply a placeholder and mount a Secret
instead of pasting secrets into shell/CI; update the helm install block to
remove --set ampEvaluation.publisher.clientSecret and instead reference a values
file, add a short note that the Thunder pre-registered OAuth client
(ampPublisherClient) must exist beforehand, and show the clientId/idpTokenUrl as
placeholders to be populated from the values/Secret.
</details>
<!-- fingerprinting:phantom:medusa:grasshopper -->
<!-- This is an auto-generated comment by CodeRabbit -->
1 participant
Purpose
Resolves: #436
Summary by CodeRabbit
New Features
Documentation
Refactor