Skip to content

feat: add VaultJWTAuthEnabled and VaultOrgIdAsSecretOwnerEnabled feature gates and vault proto fields for JWT auth #1921

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.

Sign up for GitHub

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

Open
prashantkumar1982 wants to merge 3 commits into
base: main
Choose a base branch
from
+100 −8
Open

feat: add VaultJWTAuthEnabled and VaultOrgIdAsSecretOwnerEnabled feature gates and vault proto fields for JWT auth #1921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7ffd1b3
feat: add VaultJWTAuthEnabled feature gate and vault proto fields for…
prashantkumar1982 Mar 24, 2026
077d74c
Merge remote-tracking branch 'origin/main' into feat/vault-jwt-auth-g…
prashantkumar1982 Mar 24, 2026
37b54c3
feat: add VaultOrgIdAsSecretOwnerEnabled feature gate
prashantkumar1982 Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 80 additions & 8 deletions pkg/capabilities/actions/vault/messages.pb.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 8 additions & 0 deletions pkg/capabilities/actions/vault/messages.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ message EncryptedSecret {
message CreateSecretsRequest {
string request_id = 1;
repeated EncryptedSecret encrypted_secrets = 2;
string org_id = 3;
string workflow_owner = 4;
Copy link
Contributor

@cedric-cordenier cedric-cordenier Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmmm -- I'm not following: why do we need the org_id and workflow_owner here? Shouldn't it live on the SecretIdentifier message?

Copy link
Contributor Author

@prashantkumar1982 prashantkumar1982 Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need both till we are in migration phase.
Where would you put them instead?
Inside each SecretIdentifier item as additional field?
We could do that, but seems less elegant.

Also, I am thinking, after the migration, we should move owner field to outside of Identifier.
It is more of a request level attribute than per-secret attribute.

Copy link
Contributor

@cedric-cordenier cedric-cordenier Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens once we introduce policies over secrets though? I think it's useful to distinguish between the owner making the request and the owner of the secret.

Are we expecting that the CLI will provide both values? Also one thing to bear in mind is that these messages are used across multiple layers (plugin, gateway -> node, user -> gateway); maybe this is an opportunity to start separating between what we expect the user to provide and what we add on the way

Copy link
Contributor Author

@prashantkumar1982 prashantkumar1982 Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we introduce policies, I assume more things will change. Like sub-owners, roles.
But even in that model, orgId will still be an overall customer level field, not per secret field.
In any case, I don't want to think about forward compatibility with policies support, since that will be a bigger change anyways.

The workflow_owner we will likely get rid of here in future.

maybe this is an opportunity to start separating between what we expect the user to provide and what we add on the way
Yes, this sounds like a good refactor to do. Basically split the protos/messages we pass between each layer. But that will be more disruptive, and I would like to do it separately in a different set of PRs.
At this point, we need a way to pass trusted orgId and workflowOwner from Vault Gateway handler to the Capability/Plugin.
What do you think? I am open to doing this refactor now too, as a pre-req before other changes. But will likely act on it immediately as it blocks all other work.

Copy link
Contributor Author

@prashantkumar1982 prashantkumar1982 Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we expecting that the CLI will provide both values?
No, the CLI will provide different things:

  1. Allowlist based auth: CLI provides verified workflowOwner only.
  2. jwt based auth: CLI provides orgID, and optionally an untrusted workflowOwner

Our Vault Cap node will derive both OrgID and WorkflowOwner in a trusted manner, and then pass them to the plugin.

}

message CreateSecretResponse {
Expand All @@ -67,6 +69,8 @@ message CreateSecretsResponse {
message UpdateSecretsRequest {
string request_id = 1;
repeated EncryptedSecret encrypted_secrets = 2;
string org_id = 3;
string workflow_owner = 4;
}

message UpdateSecretResponse {
Expand All @@ -82,6 +86,8 @@ message UpdateSecretsResponse {
message DeleteSecretsRequest {
string request_id = 1;
repeated SecretIdentifier ids = 2;
string org_id = 3;
string workflow_owner = 4;
}

message DeleteSecretResponse {
Expand All @@ -98,6 +104,8 @@ message ListSecretIdentifiersRequest {
string request_id = 1;
string owner = 2;
string namespace = 3;
string org_id = 4;
string workflow_owner = 5;
}

message ListSecretIdentifiersResponse {
Expand Down
2 changes: 2 additions & 0 deletions pkg/settings/cresettings/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ flowchart
subgraph handleRequest[httpServer/websocketServer.handleRequest]
GatewayIncomingPayloadSizeLimit{{GatewayIncomingPayloadSizeLimit}}:::bound
%% TODO GatewayVaultManagementEnabled
VaultJWTAuthEnabled[/VaultJWTAuthEnabled\]:::gate
VaultOrgIdAsSecretOwnerEnabled[/VaultOrgIdAsSecretOwnerEnabled\]:::gate
end

subgraph HandleNodeMessage[gatewayHandler.HandleNodeMessage]
Expand Down
2 changes: 2 additions & 0 deletions pkg/settings/cresettings/defaults.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
"WorkflowExecutionConcurrencyLimit": "200",
"GatewayIncomingPayloadSizeLimit": "1mb",
"GatewayVaultManagementEnabled": "true",
"VaultJWTAuthEnabled": "false",
"VaultOrgIdAsSecretOwnerEnabled": "false",
"GatewayHTTPGlobalRate": "500rps:500",
"GatewayHTTPPerNodeRate": "100rps:100",
"TriggerRegistrationStatusUpdateTimeout": "0s",
Expand Down
2 changes: 2 additions & 0 deletions pkg/settings/cresettings/defaults.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ WorkflowLimit = '200'
WorkflowExecutionConcurrencyLimit = '200'
GatewayIncomingPayloadSizeLimit = '1mb'
GatewayVaultManagementEnabled = 'true'
VaultJWTAuthEnabled = 'false'
VaultOrgIdAsSecretOwnerEnabled = 'false'
GatewayHTTPGlobalRate = '500rps:500'
GatewayHTTPPerNodeRate = '100rps:100'
TriggerRegistrationStatusUpdateTimeout = '0s'
Expand Down
4 changes: 4 additions & 0 deletions pkg/settings/cresettings/settings.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ var Default = Schema{
WorkflowExecutionConcurrencyLimit: Int(200),
GatewayIncomingPayloadSizeLimit: Size(1 * config.MByte),
GatewayVaultManagementEnabled: Bool(true),
VaultJWTAuthEnabled: Bool(false),
VaultOrgIdAsSecretOwnerEnabled: Bool(false),
GatewayHTTPGlobalRate: Rate(rate.Limit(500), 500),
GatewayHTTPPerNodeRate: Rate(rate.Limit(100), 100),
TriggerRegistrationStatusUpdateTimeout: Duration(0 * time.Second),
Expand Down Expand Up @@ -216,6 +218,8 @@ type Schema struct {
WorkflowExecutionConcurrencyLimit Setting[int] `unit:"{workflow}"`
GatewayIncomingPayloadSizeLimit Setting[config.Size]
GatewayVaultManagementEnabled Setting[bool]
VaultJWTAuthEnabled Setting[bool]
VaultOrgIdAsSecretOwnerEnabled Setting[bool]
GatewayHTTPGlobalRate Setting[config.Rate]
GatewayHTTPPerNodeRate Setting[config.Rate]
TriggerRegistrationStatusUpdateTimeout Setting[time.Duration]
Expand Down
2 changes: 2 additions & 0 deletions pkg/settings/cresettings/settings_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ func TestSchema_Unmarshal(t *testing.T) {
assert.Equal(t, 500, cfg.WorkflowLimit.DefaultValue)
assert.Equal(t, 14*config.KByte, cfg.GatewayIncomingPayloadSizeLimit.DefaultValue)
assert.Equal(t, true, cfg.GatewayVaultManagementEnabled.DefaultValue)
assert.Equal(t, false, cfg.VaultJWTAuthEnabled.DefaultValue)
assert.Equal(t, false, cfg.VaultOrgIdAsSecretOwnerEnabled.DefaultValue)
assert.Equal(t, 48*time.Hour, cfg.PerOrg.ZeroBalancePruningTimeout.DefaultValue)
assert.Equal(t, 99, cfg.PerOwner.WorkflowExecutionConcurrencyLimit.DefaultValue)
assert.Equal(t, 250*config.MByte, cfg.PerWorkflow.WASMMemoryLimit.DefaultValue)
Expand Down
Loading