Skip to content

Commit 70d73f2

Browse files
prashantkumar1982prashantkumar1982
committed
[codex] Fix Vault JWT topology and owner responses
1 parent f14a237 commit 70d73f2

3 files changed

Lines changed: 140 additions & 15 deletions

File tree

‎.github/workflows/cre-system-tests.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ jobs:
8686
8787
# Add list of tests with certain topologies
8888
PER_TEST_TOPOLOGIES_JSON=${PER_TEST_TOPOLOGIES_JSON:-'{
89+
"Test_CRE_V2_Suite_Bucket_B": [
90+
{"topology":"workflow-gateway-capabilities-vault-flags-enabled","configs":"configs/workflow-gateway-capabilities-don-vault-flags-enabled.toml"}
91+
],
8992
"Test_CRE_V2_Aptos_Suite": [
9093
{"topology":"workflow-gateway-aptos","configs":"configs/workflow-gateway-don-aptos.toml"}
9194
],

‎core/services/ocr2/plugins/vault/plugin.go‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,18 @@ func (r *ReportingPlugin) orgIDAsSecretOwnerEnabled(ctx context.Context) bool {
390390
return r.cfg.OrgIDAsSecretOwnerEnabled.AllowErr(ctx) == nil
391391
}
392392

393+
func (r *ReportingPlugin) canonicalResponseID(ctx context.Context, id *vaultcommon.SecretIdentifier, orgID string) *vaultcommon.SecretIdentifier {
394+
if id == nil || orgID == "" || !r.orgIDAsSecretOwnerEnabled(ctx) {
395+
return id
396+
}
397+
398+
return &vaultcommon.SecretIdentifier{
399+
Key: id.Key,
400+
Namespace: id.Namespace,
401+
Owner: orgID,
402+
}
403+
}
404+
393405
type pendingQueueStore interface {
394406
WritePendingQueue(ctx context.Context, pending []*vaultcommon.StoredPendingQueueItem) error
395407
}
@@ -1911,7 +1923,7 @@ func (r *ReportingPlugin) stateTransitionCreateSecrets(ctx context.Context, stor
19111923
})
19121924
continue
19131925
}
1914-
resp, err := r.stateTransitionCreateSecretsRequest(ctx, store, req, resp)
1926+
resp, err := r.stateTransitionCreateSecretsRequest(ctx, store, req, resp, first.GetCreateSecretsRequest().OrgId)
19151927
if err != nil {
19161928
logUserErrorAware(r.lggr, "failed to handle create secret request", err, "id", req.Id, "requestID", reqID)
19171929
errorMsg := userFacingError(err, "failed to handle create secret request")
@@ -1933,7 +1945,7 @@ func (r *ReportingPlugin) stateTransitionCreateSecrets(ctx context.Context, stor
19331945
}
19341946
}
19351947

1936-
func (r *ReportingPlugin) stateTransitionCreateSecretsRequest(ctx context.Context, store WriteKVStore, req *vaultcommon.EncryptedSecret, resp *vaultcommon.CreateSecretResponse) (*vaultcommon.CreateSecretResponse, error) {
1948+
func (r *ReportingPlugin) stateTransitionCreateSecretsRequest(ctx context.Context, store WriteKVStore, req *vaultcommon.EncryptedSecret, resp *vaultcommon.CreateSecretResponse, orgID string) (*vaultcommon.CreateSecretResponse, error) {
19371949
if resp.GetError() != "" {
19381950
return resp, newUserError(resp.GetError())
19391951
}
@@ -1974,7 +1986,7 @@ func (r *ReportingPlugin) stateTransitionCreateSecretsRequest(ctx context.Contex
19741986
}
19751987

19761988
return &vaultcommon.CreateSecretResponse{
1977-
Id: req.Id,
1989+
Id: r.canonicalResponseID(ctx, req.Id, orgID),
19781990
Success: true,
19791991
Error: "",
19801992
}, nil
@@ -2029,7 +2041,7 @@ func (r *ReportingPlugin) stateTransitionUpdateSecrets(ctx context.Context, stor
20292041
})
20302042
continue
20312043
}
2032-
resp, err := r.stateTransitionUpdateSecretsRequest(ctx, store, req, resp)
2044+
resp, err := r.stateTransitionUpdateSecretsRequest(ctx, store, req, resp, first.GetUpdateSecretsRequest().OrgId)
20332045
if err != nil {
20342046
logUserErrorAware(r.lggr, "failed to handle update secret request", err, "id", req.Id, "requestID", reqID)
20352047
errorMsg := userFacingError(err, "failed to handle update secret request")
@@ -2051,7 +2063,7 @@ func (r *ReportingPlugin) stateTransitionUpdateSecrets(ctx context.Context, stor
20512063
}
20522064
}
20532065

2054-
func (r *ReportingPlugin) stateTransitionUpdateSecretsRequest(ctx context.Context, store WriteKVStore, req *vaultcommon.EncryptedSecret, resp *vaultcommon.UpdateSecretResponse) (*vaultcommon.UpdateSecretResponse, error) {
2066+
func (r *ReportingPlugin) stateTransitionUpdateSecretsRequest(ctx context.Context, store WriteKVStore, req *vaultcommon.EncryptedSecret, resp *vaultcommon.UpdateSecretResponse, orgID string) (*vaultcommon.UpdateSecretResponse, error) {
20552067
if resp.GetError() != "" {
20562068
return resp, newUserError(resp.GetError())
20572069
}
@@ -2078,7 +2090,7 @@ func (r *ReportingPlugin) stateTransitionUpdateSecretsRequest(ctx context.Contex
20782090
}
20792091

20802092
return &vaultcommon.UpdateSecretResponse{
2081-
Id: req.Id,
2093+
Id: r.canonicalResponseID(ctx, req.Id, orgID),
20822094
Success: true,
20832095
Error: "",
20842096
}, nil
@@ -2133,7 +2145,7 @@ func (r *ReportingPlugin) stateTransitionDeleteSecrets(ctx context.Context, stor
21332145
})
21342146
continue
21352147
}
2136-
resp, err := r.stateTransitionDeleteSecretsRequest(ctx, store, req, resp)
2148+
resp, err := r.stateTransitionDeleteSecretsRequest(ctx, store, req, resp, first.GetDeleteSecretsRequest().OrgId)
21372149
if err != nil {
21382150
logUserErrorAware(r.lggr, "failed to handle delete secret request", err, "id", id, "requestId", reqID)
21392151
errorMsg := userFacingError(err, "failed to handle delete secret request")
@@ -2155,7 +2167,7 @@ func (r *ReportingPlugin) stateTransitionDeleteSecrets(ctx context.Context, stor
21552167
}
21562168
}
21572169

2158-
func (r *ReportingPlugin) stateTransitionDeleteSecretsRequest(ctx context.Context, store WriteKVStore, id *vaultcommon.SecretIdentifier, resp *vaultcommon.DeleteSecretResponse) (*vaultcommon.DeleteSecretResponse, error) {
2170+
func (r *ReportingPlugin) stateTransitionDeleteSecretsRequest(ctx context.Context, store WriteKVStore, id *vaultcommon.SecretIdentifier, resp *vaultcommon.DeleteSecretResponse, orgID string) (*vaultcommon.DeleteSecretResponse, error) {
21592171
if resp.GetError() != "" {
21602172
return resp, newUserError(resp.GetError())
21612173
}
@@ -2166,7 +2178,7 @@ func (r *ReportingPlugin) stateTransitionDeleteSecretsRequest(ctx context.Contex
21662178
}
21672179

21682180
return &vaultcommon.DeleteSecretResponse{
2169-
Id: id,
2181+
Id: r.canonicalResponseID(ctx, id, orgID),
21702182
Success: true,
21712183
Error: "",
21722184
}, nil

‎core/services/ocr2/plugins/vault/plugin_test.go‎

Lines changed: 116 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3723,6 +3723,114 @@ func TestPlugin_StateTransition_CreateSecretsRequest_UsesWorkflowOwnerMetadataWh
37233723
assert.Nil(t, ss)
37243724
}
37253725

3726+
func TestPlugin_StateTransition_CreateSecretsRequest_RewritesResponseOwnerToOrgIDWhenGateEnabled(t *testing.T) {
3727+
lggr, observed := logger.TestLoggerObserved(t, zapcore.DebugLevel)
3728+
store := requests.NewStore[*vaulttypes.Request]()
3729+
_, pk, shares, err := tdh2easy.GenerateKeys(1, 3)
3730+
require.NoError(t, err)
3731+
cfg := makeReportingPluginConfig(
3732+
t,
3733+
10,
3734+
pk,
3735+
shares[0],
3736+
5,
3737+
1024,
3738+
100,
3739+
100,
3740+
100,
3741+
10,
3742+
)
3743+
cfg.OrgIDAsSecretOwnerEnabled = limits.NewGateLimiter(true)
3744+
r := &ReportingPlugin{
3745+
lggr: lggr,
3746+
onchainCfg: ocr3types.ReportingPluginConfig{
3747+
N: 4,
3748+
F: 1,
3749+
},
3750+
store: store,
3751+
metrics: newTestMetrics(t),
3752+
cfg: cfg,
3753+
}
3754+
3755+
const (
3756+
orgID = "org-create-success"
3757+
workflowOwner = "0x5555555555555555555555555555555555555555"
3758+
)
3759+
3760+
requestID := &vaultcommon.SecretIdentifier{
3761+
Owner: workflowOwner,
3762+
Namespace: "main",
3763+
Key: "secret",
3764+
}
3765+
canonicalID := &vaultcommon.SecretIdentifier{
3766+
Owner: orgID,
3767+
Namespace: "main",
3768+
Key: "secret",
3769+
}
3770+
3771+
value := []byte("encrypted-value")
3772+
req := &vaultcommon.CreateSecretsRequest{
3773+
RequestId: "request-id",
3774+
EncryptedSecrets: []*vaultcommon.EncryptedSecret{
3775+
{
3776+
Id: requestID,
3777+
EncryptedValue: hex.EncodeToString(value),
3778+
},
3779+
},
3780+
OrgId: orgID,
3781+
WorkflowOwner: workflowOwner,
3782+
}
3783+
resp := &vaultcommon.CreateSecretsResponse{
3784+
Responses: []*vaultcommon.CreateSecretResponse{
3785+
{
3786+
Id: requestID,
3787+
Success: false,
3788+
Error: "",
3789+
},
3790+
},
3791+
}
3792+
3793+
kv := &kv{m: make(map[string]response)}
3794+
rs := newTestReadStore(t, kv)
3795+
obsb := marshalObservations(t, observation{requestID, req, resp})
3796+
reportPrecursor, err := r.StateTransition(
3797+
t.Context(),
3798+
1,
3799+
types.AttributedQuery{},
3800+
[]types.AttributedObservation{
3801+
{Observer: 0, Observation: types.Observation(obsb)},
3802+
{Observer: 1, Observation: types.Observation(obsb)},
3803+
{Observer: 2, Observation: types.Observation(obsb)},
3804+
},
3805+
kv,
3806+
nil,
3807+
)
3808+
require.NoError(t, err)
3809+
3810+
os := &vaultcommon.Outcomes{}
3811+
require.NoError(t, proto.Unmarshal(reportPrecursor, os))
3812+
require.Len(t, os.Outcomes, 1)
3813+
3814+
o := os.Outcomes[0]
3815+
assert.True(t, proto.Equal(req, o.GetCreateSecretsRequest()))
3816+
expectedResp := &vaultcommon.CreateSecretsResponse{
3817+
Responses: []*vaultcommon.CreateSecretResponse{
3818+
{
3819+
Id: canonicalID,
3820+
Success: true,
3821+
Error: "",
3822+
},
3823+
},
3824+
}
3825+
assert.True(t, proto.Equal(expectedResp, o.GetCreateSecretsResponse()), o.GetCreateSecretsResponse())
3826+
3827+
ss, err := rs.GetSecret(t.Context(), canonicalID)
3828+
require.NoError(t, err)
3829+
assert.Equal(t, []byte("encrypted-value"), ss.EncryptedSecret)
3830+
3831+
assert.Equal(t, 1, observed.FilterMessage("sufficient observations for sha").Len())
3832+
}
3833+
37263834
func TestPlugin_Reports(t *testing.T) {
37273835
value := "encrypted-value"
37283836
id := &vaultcommon.SecretIdentifier{
@@ -4533,7 +4641,7 @@ func TestPlugin_StateTransition_UpdateSecretsRequest_MigratesWorkflowOwnerSecret
45334641
RequestId: "request-id",
45344642
EncryptedSecrets: []*vaultcommon.EncryptedSecret{
45354643
{
4536-
Id: id,
4644+
Id: legacyID,
45374645
EncryptedValue: hex.EncodeToString([]byte("encrypted-value")),
45384646
},
45394647
},
@@ -4543,14 +4651,14 @@ func TestPlugin_StateTransition_UpdateSecretsRequest_MigratesWorkflowOwnerSecret
45434651
resp := &vaultcommon.UpdateSecretsResponse{
45444652
Responses: []*vaultcommon.UpdateSecretResponse{
45454653
{
4546-
Id: id,
4654+
Id: legacyID,
45474655
Success: false,
45484656
Error: "",
45494657
},
45504658
},
45514659
}
45524660

4553-
obsb := marshalObservations(t, observation{id, req, resp})
4661+
obsb := marshalObservations(t, observation{legacyID, req, resp})
45544662
reportPrecursor, err := r.StateTransition(
45554663
t.Context(),
45564664
1,
@@ -4573,6 +4681,7 @@ func TestPlugin_StateTransition_UpdateSecretsRequest_MigratesWorkflowOwnerSecret
45734681
assert.True(t, proto.Equal(req, o.GetUpdateSecretsRequest()), o.GetUpdateSecretsRequest())
45744682
require.Len(t, o.GetUpdateSecretsResponse().Responses, 1)
45754683
assert.True(t, o.GetUpdateSecretsResponse().Responses[0].Success)
4684+
assert.Equal(t, orgID, o.GetUpdateSecretsResponse().Responses[0].Id.Owner)
45764685

45774686
ss, err := rs.GetSecret(t.Context(), id)
45784687
require.NoError(t, err)
@@ -5059,21 +5168,21 @@ func TestPlugin_StateTransition_DeleteSecretsRequest_DeletesWorkflowOwnerSecretW
50595168
}
50605169
req := &vaultcommon.DeleteSecretsRequest{
50615170
RequestId: "request-id",
5062-
Ids: []*vaultcommon.SecretIdentifier{id},
5171+
Ids: []*vaultcommon.SecretIdentifier{legacyID},
50635172
OrgId: orgID,
50645173
WorkflowOwner: workflowOwner,
50655174
}
50665175
resp := &vaultcommon.DeleteSecretsResponse{
50675176
Responses: []*vaultcommon.DeleteSecretResponse{
50685177
{
5069-
Id: id,
5178+
Id: legacyID,
50705179
Success: false,
50715180
Error: "",
50725181
},
50735182
},
50745183
}
50755184

5076-
obsb := marshalObservations(t, observation{id, req, resp})
5185+
obsb := marshalObservations(t, observation{legacyID, req, resp})
50775186
reportPrecursor, err := r.StateTransition(
50785187
t.Context(),
50795188
1,
@@ -5096,6 +5205,7 @@ func TestPlugin_StateTransition_DeleteSecretsRequest_DeletesWorkflowOwnerSecretW
50965205
assert.True(t, proto.Equal(req, o.GetDeleteSecretsRequest()), o.GetDeleteSecretsRequest())
50975206
require.Len(t, o.GetDeleteSecretsResponse().Responses, 1)
50985207
assert.True(t, o.GetDeleteSecretsResponse().Responses[0].Success)
5208+
assert.True(t, proto.Equal(id, o.GetDeleteSecretsResponse().Responses[0].Id), o.GetDeleteSecretsResponse().Responses[0].Id)
50995209

51005210
ss, err := rs.GetSecret(t.Context(), legacyID)
51015211
require.NoError(t, err)

0 commit comments

Comments
 (0)