vault: add generic authorizer with jwt auth support#21836
vault: add generic authorizer with jwt auth support#21836prashantkumar1982 wants to merge 1 commit intodevelopfrom
Conversation
|
I see you updated files related to
|
|
✅ No conflicts with other open PRs targeting |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 45f2fc131c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // NewHandler creates the gateway-side Vault handler with internal auth wiring. | ||
| func NewHandler(methodConfig json.RawMessage, donConfig *config.DONConfig, don gwhandlers.DON, capabilitiesRegistry capabilitiesRegistry, workflowRegistrySyncer workflowsyncerv2.WorkflowRegistrySyncer, lggr logger.Logger, clock clockwork.Clock, limitsFactory limits.Factory) (*handler, error) { | ||
| allowListBasedAuth := vaultcap.NewAllowListBasedAuth(lggr, workflowRegistrySyncer) | ||
| jwtBasedAuth, err := vaultcap.NewJWTBasedAuth(vaultcap.JWTBasedAuthConfig{}, limitsFactory, lggr, vaultcap.WithDisabledJWTBasedAuth()) |
There was a problem hiding this comment.
Remove unconditional WithDisabledJWTBasedAuth wiring
NewHandler constructs JWT auth with WithDisabledJWTBasedAuth(), which hard-sets the gate limiter to false (see jwt_based_auth.go) instead of honoring VaultJWTAuthEnabled. As a result, requests that include req.Auth are always rejected as disabled, so enabling the setting cannot actually turn on JWT auth in this runtime path; the same hard-disable pattern is also present in core/capabilities/vault/gw_handler.go. Runtime wiring should use the normal JWT constructor (with real issuer/audience config) and reserve WithDisabledJWTBasedAuth for tests only.
Useful? React with 👍 / 👎.
|




Summary
AllowListBasedAuthandJWTBasedAuthVaultJWTAuthEnabledNotes