From 99707d2ca5c2293ba2e939f4211a949fa57290a4 Mon Sep 17 00:00:00 2001 From: Pritesh Bandi Date: Fri, 19 Jan 2024 08:08:42 -0800 Subject: [PATCH 1/4] Adding validate method Signed-off-by: Pritesh Bandi --- cli/cli.go | 9 +- cli/cli_test.go | 15 +-- cli/utils.go | 2 +- example/plugin.go | 4 +- {cli/internal => internal}/mock/plugin.go | 0 plugin/describekey.go | 13 +++ plugin/describekey_test.go | 65 +++++++++++ plugin/metadata.go | 5 + plugin/metadata_test.go | 15 ++- plugin/proto.go | 1 + plugin/sign.go | 50 ++++++++ plugin/sign_test.go | 136 ++++++++++++++++++++++ plugin/verify.go | 38 ++++++ plugin/verify_test.go | 100 ++++++++++++++++ 14 files changed, 438 insertions(+), 15 deletions(-) rename {cli/internal => internal}/mock/plugin.go (100%) create mode 100644 plugin/describekey_test.go create mode 100644 plugin/sign_test.go create mode 100644 plugin/verify_test.go diff --git a/cli/cli.go b/cli/cli.go index dd60fc1..820db5a 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -17,7 +17,6 @@ // 2. Read and unmarshal input // 3. Execute relevant plugin functions // 4. marshals output - package cli import ( @@ -135,11 +134,17 @@ func (c *CLI) validateArgs(ctx context.Context, args []string) { } // unmarshalRequest reads input from std.in and unmarshal it into given request struct -func (c *CLI) unmarshalRequest(request any) error { +func (c *CLI) unmarshalRequest(request plugin.Request) error { if err := json.NewDecoder(os.Stdin).Decode(request); err != nil { c.logger.Errorf("%s unmarshalling error :%v", reflect.TypeOf(request), err) return plugin.NewJSONParsingError(plugin.ErrorMsgMalformedInput) } + + if err := request.Validate(); err != nil { + c.logger.Errorf("%s validation error :%v", reflect.TypeOf(request), err) + return plugin.NewValidationError(plugin.ErrorMsgMalformedInput) + } + return nil } diff --git a/cli/cli_test.go b/cli/cli_test.go index 41df833..68fd93c 100644 --- a/cli/cli_test.go +++ b/cli/cli_test.go @@ -24,7 +24,7 @@ import ( "strings" "testing" - "github.com/notaryproject/notation-plugin-framework-go/cli/internal/mock" + "github.com/notaryproject/notation-plugin-framework-go/internal/mock" "github.com/notaryproject/notation-plugin-framework-go/plugin" ) @@ -34,7 +34,7 @@ var errorCli, _ = New(mock.NewPlugin(true)) func TestNewWithLogger(t *testing.T) { _, err := NewWithLogger(nil, &discardLogger{}) if err == nil { - t.Fatalf("NewWithLogger() expected error but not found") + t.Errorf("NewWithLogger() expected error but not found") } } @@ -56,7 +56,6 @@ func TestMarshalResponse(t *testing.T) { } func TestMarshalResponseError(t *testing.T) { - _, err := cli.marshalResponse(nil, fmt.Errorf("expected error thrown")) assertErr(t, err, plugin.ErrorCodeGeneric) @@ -89,17 +88,17 @@ func TestUnmarshalRequestError(t *testing.T) { var request plugin.DescribeKeyRequest err := cli.unmarshalRequest(&request) if err == nil { - t.Fatalf("unmarshalRequest() expected error but not found") + t.Errorf("unmarshalRequest() expected error but not found") } plgErr, ok := err.(*plugin.Error) if !ok { - t.Fatalf("unmarshalRequest() expected error of type plugin.Error but found %s", reflect.TypeOf(err)) + t.Errorf("unmarshalRequest() expected error of type plugin.Error but found %s", reflect.TypeOf(err)) } expectedErrStr := "{\"errorCode\":\"VALIDATION_ERROR\",\"errorMessage\":\"Input is not a valid JSON\"}" if plgErr.Error() != expectedErrStr { - t.Fatalf("unmarshalRequest() expected error string to be %s but found %s", expectedErrStr, plgErr.Error()) + t.Errorf("unmarshalRequest() expected error string to be %s but found %s", expectedErrStr, plgErr.Error()) } } @@ -115,7 +114,7 @@ func TestGetMetadataError(t *testing.T) { if e, ok := err.(*exec.ExitError); ok && !e.Success() { return } - t.Fatalf("process ran with err %v, want exit status 1", err) + t.Errorf("process ran with err %v, want exit status 1", err) } func TestExecuteSuccess(t *testing.T) { @@ -156,7 +155,6 @@ func TestExecuteSuccess(t *testing.T) { op := captureStdOut(func() { test.c.Execute(context.Background(), []string{"notation", name}) }) - fmt.Println(op) if op != test.op { t.Errorf("Execute() with '%s' args, expected '%s' but got '%s'", name, test.op, op) } @@ -206,6 +204,5 @@ func assertErr(t *testing.T, err error, code plugin.ErrorCode) { } t.Errorf("mismatch in error code: \n expected: %s\n actual : %s", code, plgErr.ErrCode) } - t.Errorf("expected error of type PluginError but found %s", reflect.TypeOf(err)) } diff --git a/cli/utils.go b/cli/utils.go index 1526cdd..1b401f6 100644 --- a/cli/utils.go +++ b/cli/utils.go @@ -51,7 +51,7 @@ func getValidArgs(md *plugin.GetMetadataResponse) []string { // deliverError print to standard error and then return nonzero exit code func deliverError(message string) { - _, _ = fmt.Fprintln(os.Stderr, message) + _, _ = fmt.Fprint(os.Stderr, message) os.Exit(1) } diff --git a/example/plugin.go b/example/plugin.go index 9b16ca8..605403a 100644 --- a/example/plugin.go +++ b/example/plugin.go @@ -91,8 +91,8 @@ func (p *ExamplePlugin) VerifySignature(ctx context.Context, req *plugin.VerifyS func (p *ExamplePlugin) GetMetadata(ctx context.Context, req *plugin.GetMetadataRequest) (*plugin.GetMetadataResponse, error) { return &plugin.GetMetadataResponse{ - Name: "Example Plugin", - Description: "This is an description of example plugin. 🍺", + Name: "com.example.plugin", + Description: "This is an description of example plugin", URL: "https://example.com/notation/plugin", Version: "1.0.0", Capabilities: []plugin.Capability{ diff --git a/cli/internal/mock/plugin.go b/internal/mock/plugin.go similarity index 100% rename from cli/internal/mock/plugin.go rename to internal/mock/plugin.go diff --git a/plugin/describekey.go b/plugin/describekey.go index 5bdaeba..8b0a91d 100644 --- a/plugin/describekey.go +++ b/plugin/describekey.go @@ -24,6 +24,19 @@ func (DescribeKeyRequest) Command() Command { return CommandDescribeKey } +// Validate validates DescribeKeyRequest struct +func (r DescribeKeyRequest) Validate() error { + if r.ContractVersion == "" { + return NewValidationError("contractVersion cannot be empty") + } + + if r.KeyID == "" { + return NewValidationError("keyId cannot be empty") + } + + return nil +} + // DescribeKeyResponse is the response of a describe-key request. type DescribeKeyResponse struct { // The same key id as passed in the request. diff --git a/plugin/describekey_test.go b/plugin/describekey_test.go new file mode 100644 index 0000000..0a7d89e --- /dev/null +++ b/plugin/describekey_test.go @@ -0,0 +1,65 @@ +// Copyright The Notary Project Authors. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package plugin + +import ( + "fmt" + "testing" +) + +func TestDescribeKeyRequest_Validate(t *testing.T) { + reqs := []DescribeKeyRequest{ + getDescribeKeyRequest(ContractVersion, "someKeyId"), + { + ContractVersion: ContractVersion, + KeyID: "someKeyId", + PluginConfig: map[string]string{"someKey": "someValue"}}, + } + + for _, req := range reqs { + if err := req.Validate(); err != nil { + t.Errorf("VerifySignatureRequest#Validate failed with error: %+v", err) + } + } +} + +func TestDescribeKeyRequest_Validate_Error(t *testing.T) { + testCases := []struct { + name string + req DescribeKeyRequest + }{ + {name: "contractVersion", req: getDescribeKeyRequest("", "someKeyId")}, + {name: "keyId", req: getDescribeKeyRequest(ContractVersion, "")}, + } + + for _, testcase := range testCases { + t.Run(testcase.name, func(t *testing.T) { + if err := testcase.req.Validate(); err != nil { + expMsg := fmt.Sprintf("{\"errorCode\":\"VALIDATION_ERROR\",\"errorMessage\":\"%s cannot be empty\"}", testcase.name) + if err.Error() != expMsg { + t.Errorf("expected error message '%s' but got '%s'", expMsg, err.Error()) + } + } else { + t.Error("DescribeKeyRequest#Validate didn't returned error") + } + }) + } +} + +func getDescribeKeyRequest(cv, kid string) DescribeKeyRequest { + return DescribeKeyRequest{ + ContractVersion: cv, + KeyID: kid, + } +} diff --git a/plugin/metadata.go b/plugin/metadata.go index 74190a1..4fd116d 100644 --- a/plugin/metadata.go +++ b/plugin/metadata.go @@ -23,6 +23,11 @@ func (GetMetadataRequest) Command() Command { return CommandGetMetadata } +// Validate validates GetMetadataRequest struct +func (GetMetadataRequest) Validate() error { + return nil +} + // GetMetadataResponse provided by the plugin. type GetMetadataResponse struct { Name string `json:"name"` diff --git a/plugin/metadata_test.go b/plugin/metadata_test.go index b4c81c5..c7dce84 100644 --- a/plugin/metadata_test.go +++ b/plugin/metadata_test.go @@ -17,7 +17,7 @@ import ( "testing" ) -func TestGetMetadataResponse_HasCapability(t *testing.T) { +func TestGetMetadataResponseHasCapability(t *testing.T) { type args struct { capability Capability } @@ -40,3 +40,16 @@ func TestGetMetadataResponse_HasCapability(t *testing.T) { }) } } + +func TestGetMetadataRequest_Validate(t *testing.T) { + reqs := []GetMetadataRequest{ + {}, + {PluginConfig: map[string]string{"key1": "value1"}}, + } + + for _, req := range reqs { + if err := req.Validate(); err != nil { + t.Errorf("GetMetadataRequest#Validate failed with error: %+v", err) + } + } +} diff --git a/plugin/proto.go b/plugin/proto.go index 144ae29..9a26e5f 100644 --- a/plugin/proto.go +++ b/plugin/proto.go @@ -48,6 +48,7 @@ type Command string // Request defines a plugin request, which is always associated to a command. type Request interface { Command() Command + Validate() error } const ( diff --git a/plugin/sign.go b/plugin/sign.go index 2afe07d..6931f95 100644 --- a/plugin/sign.go +++ b/plugin/sign.go @@ -28,6 +28,31 @@ func (GenerateSignatureRequest) Command() Command { return CommandGenerateSignature } +// Validate validates GenerateSignatureRequest struct +func (r GenerateSignatureRequest) Validate() error { + if r.ContractVersion == "" { + return NewValidationError("contractVersion cannot be empty") + } + + if r.KeyID == "" { + return NewValidationError("keyId cannot be empty") + } + + if r.KeySpec == "" { + return NewValidationError("keySpec cannot be empty") + } + + if r.Hash == "" { + return NewValidationError("hashAlgorithm cannot be empty") + } + + if len(r.Payload) == 0 { + return NewValidationError("payload cannot be empty") + } + + return nil +} + // GenerateSignatureResponse is the response of a generate-signature request. type GenerateSignatureResponse struct { KeyID string `json:"keyId"` @@ -55,6 +80,31 @@ func (GenerateEnvelopeRequest) Command() Command { return CommandGenerateEnvelope } +// Validate validates GenerateEnvelopeRequest struct +func (r GenerateEnvelopeRequest) Validate() error { + if r.ContractVersion == "" { + return NewValidationError("contractVersion cannot be empty") + } + + if r.KeyID == "" { + return NewValidationError("keyId cannot be empty") + } + + if r.PayloadType == "" { + return NewValidationError("payloadType cannot be empty") + } + + if r.SignatureEnvelopeType == "" { + return NewValidationError("signatureEnvelopeType cannot be empty") + } + + if len(r.Payload) == 0 { + return NewValidationError("payload cannot be empty") + } + + return nil +} + // GenerateEnvelopeResponse is the response of a generate-envelope request. type GenerateEnvelopeResponse struct { SignatureEnvelope []byte `json:"signatureEnvelope"` diff --git a/plugin/sign_test.go b/plugin/sign_test.go new file mode 100644 index 0000000..f255e5c --- /dev/null +++ b/plugin/sign_test.go @@ -0,0 +1,136 @@ +// Copyright The Notary Project Authors. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package plugin + +import ( + "fmt" + "testing" +) + +func TestGenerateSignatureRequest_Validate(t *testing.T) { + reqs := []GenerateSignatureRequest{ + getGenerateSignatureRequest(ContractVersion, "someKeyId", string(KeySpecEC384), string(HashAlgorithmSHA384), []byte("zop")), + { + ContractVersion: ContractVersion, + KeyID: "someKeyId", + KeySpec: KeySpecEC384, + Hash: HashAlgorithmSHA384, + Payload: []byte("somePayload"), + PluginConfig: map[string]string{"key1": "value1"}, + }, + } + + for _, req := range reqs { + if err := req.Validate(); err != nil { + t.Errorf("VerifySignatureRequest#Validate failed with error: %+v", err) + } + } +} + +func TestGenerateSignatureRequest_Validate_Error(t *testing.T) { + testCases := []struct { + name string + req GenerateSignatureRequest + }{ + {name: "contractVersion", req: getGenerateSignatureRequest("", "someKeyId", string(KeySpecEC384), string(HashAlgorithmSHA384), []byte("zop"))}, + {name: "keyId", req: getGenerateSignatureRequest(ContractVersion, "", string(KeySpecEC384), string(HashAlgorithmSHA384), []byte("zop"))}, + {name: "keySpec", req: getGenerateSignatureRequest(ContractVersion, "someKeyId", "", string(HashAlgorithmSHA384), []byte("zop"))}, + {name: "hashAlgorithm", req: getGenerateSignatureRequest(ContractVersion, "someKid", string(KeySpecEC384), "", []byte("zop"))}, + {name: "payload", req: getGenerateSignatureRequest(ContractVersion, "someKeyId", string(KeySpecEC384), string(HashAlgorithmSHA384), []byte(""))}, + {name: "payload", req: getGenerateSignatureRequest(ContractVersion, "someKeyId", string(KeySpecEC384), string(HashAlgorithmSHA384), nil)}, + {name: "keySpec", req: getGenerateSignatureRequest(ContractVersion, "someKeyId", "", string(HashAlgorithmSHA384), []byte("zop"))}, + } + + for _, testcase := range testCases { + t.Run(testcase.name, func(t *testing.T) { + if err := testcase.req.Validate(); err != nil { + expMsg := fmt.Sprintf("{\"errorCode\":\"VALIDATION_ERROR\",\"errorMessage\":\"%s cannot be empty\"}", testcase.name) + if err.Error() != expMsg { + t.Errorf("expected error message '%s' but got '%s'", expMsg, err.Error()) + } + } else { + t.Error("VerifySignatureRequest#Validate didn't returned error") + } + }) + } +} + +func TestGenerateEnvelopeRequest_Validate(t *testing.T) { + reqs := []GenerateEnvelopeRequest{ + getGenerateEnvelopeRequest(ContractVersion, "someKeyId", "someSET", "somePT", []byte("zop")), + { + ContractVersion: ContractVersion, + KeyID: "someKeyId", + SignatureEnvelopeType: "someType", + ExpiryDurationInSeconds: 1, + Payload: []byte("zap"), + PayloadType: ContractVersion, + PluginConfig: map[string]string{"key1": "value1"}, + }, + } + + for _, req := range reqs { + if err := req.Validate(); err != nil { + t.Errorf("GenerateEnvelopeRequest#Validate failed with error: %+v", err) + } + } +} + +func TestGenerateEnvelopeRequest_Validate_Error(t *testing.T) { + testCases := []struct { + name string + req GenerateEnvelopeRequest + }{ + {name: "contractVersion", req: getGenerateEnvelopeRequest("", "someKeyId", "someSET", "somePT", []byte("zop"))}, + {name: "keyId", req: getGenerateEnvelopeRequest(ContractVersion, "", "someSET", "somePT", []byte("zop"))}, + {name: "signatureEnvelopeType", req: getGenerateEnvelopeRequest(ContractVersion, "someKeyId", "", "somePT", []byte("zop"))}, + {name: "payloadType", req: getGenerateEnvelopeRequest(ContractVersion, "someKeyId", "someSET", "", []byte("zop"))}, + {name: "payload", req: getGenerateEnvelopeRequest(ContractVersion, "someKeyId", "someSET", "somePT", []byte(""))}, + {name: "payload", req: getGenerateEnvelopeRequest(ContractVersion, "someKeyId", "someSET", "somePT", nil)}, + } + + for _, testcase := range testCases { + t.Run(testcase.name, func(t *testing.T) { + if err := testcase.req.Validate(); err != nil { + expMsg := fmt.Sprintf("{\"errorCode\":\"VALIDATION_ERROR\",\"errorMessage\":\"%s cannot be empty\"}", testcase.name) + if err.Error() != expMsg { + t.Errorf("expected error message '%s' but got '%s'", expMsg, err.Error()) + } + } else { + fmt.Println(testcase.req) + t.Error("GenerateEnvelopeRequest#Validate didn't returned error") + } + }) + } +} + +func getGenerateSignatureRequest(cv, kid, ks, ha string, pl []byte) GenerateSignatureRequest { + return GenerateSignatureRequest{ + ContractVersion: cv, + KeyID: kid, + KeySpec: KeySpec(ks), + Hash: HashAlgorithm(ha), + Payload: pl, + } +} + +func getGenerateEnvelopeRequest(cv, kid, set, pt string, pl []byte) GenerateEnvelopeRequest { + return GenerateEnvelopeRequest{ + ContractVersion: cv, + KeyID: kid, + SignatureEnvelopeType: set, + PayloadType: pt, + Payload: pl, + } +} diff --git a/plugin/verify.go b/plugin/verify.go index a07c55d..ab64560 100644 --- a/plugin/verify.go +++ b/plugin/verify.go @@ -14,6 +14,7 @@ package plugin import ( + "reflect" "time" ) @@ -30,6 +31,43 @@ func (VerifySignatureRequest) Command() Command { return CommandVerifySignature } +// Validate validates VerifySignatureRequest struct +func (r VerifySignatureRequest) Validate() error { + if r.ContractVersion == "" { + return NewValidationError("contractVersion cannot be empty") + } + + if reflect.DeepEqual(r.Signature, Signature{}) { + return NewValidationError("signature cannot be empty") + } + + if reflect.DeepEqual(r.Signature.CriticalAttributes, CriticalAttributes{}) { + return NewValidationError("signature's criticalAttributes cannot be empty") + } + + if r.Signature.CriticalAttributes.ContentType == "" { + return NewValidationError("signature's criticalAttributes's contentType cannot be empty") + } + + if r.Signature.CriticalAttributes.SigningScheme == "" { + return NewValidationError("signature's criticalAttributes's signingScheme cannot be empty") + } + + if len(r.Signature.CertificateChain) == 0 { + return NewValidationError("signature's criticalAttributes's certificateChain cannot be empty") + } + + if reflect.DeepEqual(r.TrustPolicy, TrustPolicy{}) { + return NewValidationError("signature's trustPolicy cannot be empty") + } + + if len(r.TrustPolicy.SignatureVerification) == 0 { + return NewValidationError("signature's trustPolicy's signatureVerification cannot be empty") + } + + return nil +} + // Signature represents a signature pulled from the envelope type Signature struct { CriticalAttributes CriticalAttributes `json:"criticalAttributes"` diff --git a/plugin/verify_test.go b/plugin/verify_test.go new file mode 100644 index 0000000..2ee0730 --- /dev/null +++ b/plugin/verify_test.go @@ -0,0 +1,100 @@ +// Copyright The Notary Project Authors. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package plugin + +import ( + "fmt" + "testing" +) + +var mockCertChain = [][]byte{[]byte("zap"), []byte("zop")} + +func TestVerifySignatureRequest_Validate(t *testing.T) { + reqs := []VerifySignatureRequest{ + getVerifySignatureRequest(ContractVersion, "someCT", "someSigningScheme", mockCertChain, []Capability{CapabilitySignatureGenerator}), + { + ContractVersion: "2.0", + Signature: Signature{ + CriticalAttributes: CriticalAttributes{ + ContentType: "someCT", + SigningScheme: "someSigningScheme", + Expiry: nil, + AuthenticSigningTime: nil, + ExtendedAttributes: nil, + }, + UnprocessedAttributes: []string{"upa1", "upa2"}, + CertificateChain: mockCertChain, + }, + TrustPolicy: TrustPolicy{ + TrustedIdentities: []string{"trustedIdentity1", "trustedIdentity2"}, + SignatureVerification: []Capability{CapabilitySignatureGenerator, CapabilityRevocationCheckVerifier}, + }, + PluginConfig: map[string]string{"someKey": "someValue"}, + }, + } + + for _, req := range reqs { + if err := req.Validate(); err != nil { + t.Errorf("VerifySignatureRequest#Validate failed with error: %+v", err) + } + } +} + +func TestVerifySignatureRequest_Validate_Error(t *testing.T) { + reqWithoutSignature := getVerifySignatureRequest("1.0", "someCT", "someSigningScheme", mockCertChain, []Capability{CapabilitySignatureGenerator}) + reqWithoutSignature.Signature = Signature{} + + testCases := []struct { + name string + req VerifySignatureRequest + }{ + {name: "contractVersion", req: getVerifySignatureRequest("", "someCT", "someSigningScheme", mockCertChain, []Capability{CapabilitySignatureGenerator})}, + {name: "signature's criticalAttributes's contentType", req: getVerifySignatureRequest(ContractVersion, "", "someSigningScheme", mockCertChain, []Capability{CapabilitySignatureGenerator})}, + {name: "signature's criticalAttributes's signingScheme", req: getVerifySignatureRequest(ContractVersion, "someCT", "", mockCertChain, []Capability{CapabilitySignatureGenerator})}, + {name: "signature's criticalAttributes's certificateChain", req: getVerifySignatureRequest(ContractVersion, "someCT", "someSigningScheme", [][]byte{}, []Capability{CapabilitySignatureGenerator})}, + {name: "signature's criticalAttributes's certificateChain", req: getVerifySignatureRequest(ContractVersion, "someCT", "someSigningScheme", nil, []Capability{CapabilitySignatureGenerator})}, + {name: "signature's trustPolicy", req: getVerifySignatureRequest(ContractVersion, "someCT", "someSigningScheme", mockCertChain, nil)}, + {name: "signature's trustPolicy's signatureVerification", req: getVerifySignatureRequest(ContractVersion, "someCT", "someSigningScheme", mockCertChain, []Capability{})}, + {name: "signature", req: reqWithoutSignature}, + } + + for _, testcase := range testCases { + t.Run(testcase.name, func(t *testing.T) { + if err := testcase.req.Validate(); err != nil { + expMsg := fmt.Sprintf("{\"errorCode\":\"VALIDATION_ERROR\",\"errorMessage\":\"%s cannot be empty\"}", testcase.name) + if err.Error() != expMsg { + t.Errorf("expected error message '%s' but got '%s'", expMsg, err.Error()) + } + } else { + t.Error("VerifySignatureRequest#Validate didn't returned error") + } + }) + } +} + +func getVerifySignatureRequest(cv, ct, ss string, cc [][]byte, sv []Capability) VerifySignatureRequest { + return VerifySignatureRequest{ + ContractVersion: cv, + Signature: Signature{ + CriticalAttributes: CriticalAttributes{ + ContentType: ct, + SigningScheme: ss, + }, + CertificateChain: cc, + }, + TrustPolicy: TrustPolicy{ + SignatureVerification: sv, + }, + } +} From 703fdcc690acb5316c11d045f78bdb3b3316fdd9 Mon Sep 17 00:00:00 2001 From: Pritesh Bandi Date: Fri, 19 Jan 2024 08:09:45 -0800 Subject: [PATCH 2/4] fixingUT Signed-off-by: Pritesh Bandi --- .github/.codecov.yml | 2 +- cli/cli.go | 7 ++-- cli/cli_test.go | 13 ++++++-- example/plugin.go | 9 +++--- plugin/describekey_test.go | 7 ++++ plugin/errors.go | 8 ++--- plugin/errors_test.go | 65 ++++++++++++++++++++++++++++++++++++++ plugin/metadata_test.go | 7 ++++ plugin/sign_test.go | 15 ++++++++- plugin/verify_test.go | 11 +++++++ 10 files changed, 129 insertions(+), 15 deletions(-) create mode 100644 plugin/errors_test.go diff --git a/.github/.codecov.yml b/.github/.codecov.yml index 9c2f90a..ae05785 100644 --- a/.github/.codecov.yml +++ b/.github/.codecov.yml @@ -15,4 +15,4 @@ coverage: status: project: default: - target: 70% + target: 80% diff --git a/cli/cli.go b/cli/cli.go index 820db5a..033a594 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -122,7 +122,7 @@ func (c *CLI) Execute(ctx context.Context, args []string) { func (c *CLI) printVersion(ctx context.Context) { md := c.getMetadata(ctx, c.pl) - fmt.Printf("%s - %s\nVersion: %s \n", md.Name, md.Description, md.Version) + fmt.Printf("%s - %s\nVersion: %s\n", md.Name, md.Description, md.Version) } // validateArgs validate commands/arguments passed to executable. @@ -142,7 +142,10 @@ func (c *CLI) unmarshalRequest(request plugin.Request) error { if err := request.Validate(); err != nil { c.logger.Errorf("%s validation error :%v", reflect.TypeOf(request), err) - return plugin.NewValidationError(plugin.ErrorMsgMalformedInput) + if e, ok := err.(*plugin.Error); ok { + return plugin.NewValidationErrorf("%s: %s", plugin.ErrorMsgMalformedInput, e.Message) + } + return plugin.NewValidationErrorf("%s", plugin.ErrorMsgMalformedInput) } return nil diff --git a/cli/cli_test.go b/cli/cli_test.go index 68fd93c..fd65c97 100644 --- a/cli/cli_test.go +++ b/cli/cli_test.go @@ -121,36 +121,43 @@ func TestExecuteSuccess(t *testing.T) { sigGenCli, _ := New(mock.NewSigGeneratorPlugin(false)) tests := map[string]struct { c *CLI + in string op string }{ string(plugin.CommandGetMetadata): { c: cli, + in: "{}", op: "{\"name\":\"Example Plugin\",\"description\":\"This is an description of example plugin. 🍺\",\"version\":\"1.0.0\",\"url\":\"https://example.com/notation/plugin\",\"capabilities\":[\"SIGNATURE_VERIFIER.TRUSTED_IDENTITY\",\"SIGNATURE_VERIFIER.REVOCATION_CHECK\",\"SIGNATURE_GENERATOR.ENVELOPE\"]}", }, string(plugin.Version): { c: cli, - op: "Example Plugin - This is an description of example plugin. 🍺\nVersion: 1.0.0 \n", + in: "", + op: "Example Plugin - This is an description of example plugin. 🍺\nVersion: 1.0.0\n", }, string(plugin.CommandGenerateEnvelope): { c: cli, - op: "{\"signatureEnvelope\":\"\",\"signatureEnvelopeType\":\"\",\"annotations\":{\"manifestAnntnKey1\":\"value1\"}}", + in: "{\"contractVersion\":\"1.0\",\"keyId\":\"someKeyId\",\"payloadType\":\"somePT\",\"signatureEnvelopeType\":\"someSET\",\"payload\":\"em9w\"}", + op: "{\"signatureEnvelope\":\"\",\"signatureEnvelopeType\":\"someSET\",\"annotations\":{\"manifestAnntnKey1\":\"value1\"}}", }, string(plugin.CommandVerifySignature): { c: cli, + in: "{\"contractVersion\":\"1.0\",\"signature\":{\"criticalAttributes\":{\"contentType\":\"someCT\",\"signingScheme\":\"someSigningScheme\"},\"unprocessedAttributes\":null,\"certificateChain\":[\"emFw\",\"em9w\"]},\"trustPolicy\":{\"trustedIdentities\":null,\"signatureVerification\":[\"SIGNATURE_GENERATOR.RAW\"]}}", op: "{\"verificationResults\":{\"SIGNATURE_VERIFIER.REVOCATION_CHECK\":{\"success\":true,\"reason\":\"Not revoked\"},\"SIGNATURE_VERIFIER.TRUSTED_IDENTITY\":{\"success\":true,\"reason\":\"Valid trusted Identity\"}},\"processedAttributes\":[]}", }, string(plugin.CommandGenerateSignature): { c: sigGenCli, + in: "{\"contractVersion\":\"1.0\",\"keyId\":\"someKeyId\",\"keySpec\":\"EC-384\",\"hashAlgorithm\":\"SHA-384\",\"payload\":\"em9w\"}", op: "{\"keyId\":\"someKeyId\",\"signature\":\"YWJjZA==\",\"signingAlgorithm\":\"RSASSA-PSS-SHA-256\",\"certificateChain\":[\"YWJjZA==\",\"d3h5eg==\"]}", }, string(plugin.CommandDescribeKey): { c: sigGenCli, + in: "{\"contractVersion\":\"1.0\",\"keyId\":\"someKeyId\"}", op: "{\"keyId\":\"someKeyId\",\"keySpec\":\"RSA-2048\"}", }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - closer := setupReader("{}") + closer := setupReader(test.in) defer closer() op := captureStdOut(func() { test.c.Execute(context.Background(), []string{"notation", name}) diff --git a/example/plugin.go b/example/plugin.go index 605403a..2b7405f 100644 --- a/example/plugin.go +++ b/example/plugin.go @@ -91,10 +91,11 @@ func (p *ExamplePlugin) VerifySignature(ctx context.Context, req *plugin.VerifyS func (p *ExamplePlugin) GetMetadata(ctx context.Context, req *plugin.GetMetadataRequest) (*plugin.GetMetadataResponse, error) { return &plugin.GetMetadataResponse{ - Name: "com.example.plugin", - Description: "This is an description of example plugin", - URL: "https://example.com/notation/plugin", - Version: "1.0.0", + SupportedContractVersions: []string{plugin.ContractVersion}, + Name: "com.example.plugin", + Description: "This is an description of example plugin", + URL: "https://example.com/notation/plugin", + Version: "1.0.0", Capabilities: []plugin.Capability{ plugin.CapabilityEnvelopeGenerator, plugin.CapabilityTrustedIdentityVerifier, diff --git a/plugin/describekey_test.go b/plugin/describekey_test.go index 0a7d89e..32a5197 100644 --- a/plugin/describekey_test.go +++ b/plugin/describekey_test.go @@ -57,6 +57,13 @@ func TestDescribeKeyRequest_Validate_Error(t *testing.T) { } } +func TestDescribeKeyRequest_Command(t *testing.T) { + req := getDescribeKeyRequest(ContractVersion, "someKeyId") + if cmd := req.Command(); cmd != CommandDescribeKey { + t.Errorf("DescribeKeyRequest#Command, expected %s but returned %s", CommandDescribeKey, cmd) + } +} + func getDescribeKeyRequest(cv, kid string) DescribeKeyRequest { return DescribeKeyRequest{ ContractVersion: cv, diff --git a/plugin/errors.go b/plugin/errors.go index dcf5cb1..6e6297f 100644 --- a/plugin/errors.go +++ b/plugin/errors.go @@ -53,8 +53,8 @@ func NewGenericError(msg string) *Error { return NewError(ErrorCodeGeneric, msg) } -func NewGenericErrorf(format string, msg string) *Error { - return NewError(ErrorCodeGeneric, fmt.Sprintf(format, msg)) +func NewGenericErrorf(format string, msg ...any) *Error { + return NewError(ErrorCodeGeneric, fmt.Sprintf(format, msg...)) } func NewUnsupportedError(msg string) *Error { @@ -65,8 +65,8 @@ func NewValidationError(msg string) *Error { return NewError(ErrorCodeValidation, msg) } -func NewValidationErrorf(format string, msg string) *Error { - return NewError(ErrorCodeValidation, fmt.Sprintf(format, msg)) +func NewValidationErrorf(format string, msg ...any) *Error { + return NewError(ErrorCodeValidation, fmt.Sprintf(format, msg...)) } func NewUnsupportedContractVersionError(version string) *Error { diff --git a/plugin/errors_test.go b/plugin/errors_test.go new file mode 100644 index 0000000..1ea297c --- /dev/null +++ b/plugin/errors_test.go @@ -0,0 +1,65 @@ +// Copyright The Notary Project Authors. +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package plugin + +import ( + "testing" +) + +func TestNewError(t *testing.T) { + msg := "someMSg" + err := NewError(ErrorCodeValidation, msg) + if err != nil { + if ErrorCodeValidation != err.ErrCode { + t.Errorf("NewError, expected errorCode '%s' but found'%s'", ErrorCodeValidation, err.ErrCode) + } + + if msg != err.Message { + t.Errorf("NewError, expected message'%s' but found '%s'", msg, err.Message) + } + + if err.Metadata != nil { + t.Errorf("NewError, expected metadata to be nil but found '%s'", err.Metadata) + } + + expError := "{\"errorCode\":\"VALIDATION_ERROR\",\"errorMessage\":\"someMSg\"}" + if expError != err.Error() { + t.Errorf("NewError#Error, expected error to be '%s' but found '%s'", expError, err.Error()) + + } + + } else { + t.Error("NewError didn't return an error") + } +} + +func TestErrorCodes(t *testing.T) { + testCases := []struct { + err *Error + errCode ErrorCode + }{ + {err: NewValidationError(""), errCode: ErrorCodeValidation}, + {err: NewValidationErrorf("%s", ""), errCode: ErrorCodeValidation}, + {err: NewUnsupportedError(""), errCode: ErrorCodeValidation}, + {err: NewGenericError(""), errCode: ErrorCodeGeneric}, + {err: NewGenericErrorf("%s", ""), errCode: ErrorCodeGeneric}, + {err: NewJSONParsingError(""), errCode: ErrorCodeValidation}, + {err: NewUnsupportedContractVersionError(""), errCode: ErrorCodeUnsupportedContractVersion}, + } + for _, test := range testCases { + if test.errCode != test.err.ErrCode { + t.Errorf("Expected errorCode %s but found %s", test.errCode, test.err.ErrCode) + } + } +} diff --git a/plugin/metadata_test.go b/plugin/metadata_test.go index c7dce84..a393cbe 100644 --- a/plugin/metadata_test.go +++ b/plugin/metadata_test.go @@ -53,3 +53,10 @@ func TestGetMetadataRequest_Validate(t *testing.T) { } } } + +func TestGetMetadataRequest_Command(t *testing.T) { + req := GetMetadataRequest{} + if cmd := req.Command(); cmd != CommandGetMetadata { + t.Errorf("DescribeKeyRequest#Command, expected %s but returned %s", CommandGetMetadata, cmd) + } +} diff --git a/plugin/sign_test.go b/plugin/sign_test.go index f255e5c..2df620b 100644 --- a/plugin/sign_test.go +++ b/plugin/sign_test.go @@ -66,6 +66,13 @@ func TestGenerateSignatureRequest_Validate_Error(t *testing.T) { } } +func TestGenerateSignatureRequest_Command(t *testing.T) { + req := getGenerateSignatureRequest(ContractVersion, "someKeyId", string(KeySpecEC384), string(HashAlgorithmSHA384), []byte("zop")) + if cmd := req.Command(); cmd != CommandGenerateSignature { + t.Errorf("DescribeKeyRequest#Command, expected %s but returned %s", CommandGenerateSignature, cmd) + } +} + func TestGenerateEnvelopeRequest_Validate(t *testing.T) { reqs := []GenerateEnvelopeRequest{ getGenerateEnvelopeRequest(ContractVersion, "someKeyId", "someSET", "somePT", []byte("zop")), @@ -108,13 +115,19 @@ func TestGenerateEnvelopeRequest_Validate_Error(t *testing.T) { t.Errorf("expected error message '%s' but got '%s'", expMsg, err.Error()) } } else { - fmt.Println(testcase.req) t.Error("GenerateEnvelopeRequest#Validate didn't returned error") } }) } } +func TestGenerateEnvelopeRequest_Command(t *testing.T) { + req := getGenerateEnvelopeRequest(ContractVersion, "someKeyId", string(KeySpecEC384), string(HashAlgorithmSHA384), []byte("zop")) + if cmd := req.Command(); cmd != CommandGenerateEnvelope { + t.Errorf("DescribeKeyRequest#Command, expected %s but returned %s", CommandGenerateEnvelope, cmd) + } +} + func getGenerateSignatureRequest(cv, kid, ks, ha string, pl []byte) GenerateSignatureRequest { return GenerateSignatureRequest{ ContractVersion: cv, diff --git a/plugin/verify_test.go b/plugin/verify_test.go index 2ee0730..90793b6 100644 --- a/plugin/verify_test.go +++ b/plugin/verify_test.go @@ -55,6 +55,9 @@ func TestVerifySignatureRequest_Validate_Error(t *testing.T) { reqWithoutSignature := getVerifySignatureRequest("1.0", "someCT", "someSigningScheme", mockCertChain, []Capability{CapabilitySignatureGenerator}) reqWithoutSignature.Signature = Signature{} + reqWithoutCriticalAttr := getVerifySignatureRequest("1.0", "someCT", "someSigningScheme", mockCertChain, []Capability{CapabilitySignatureGenerator}) + reqWithoutCriticalAttr.Signature.CriticalAttributes = CriticalAttributes{} + testCases := []struct { name string req VerifySignatureRequest @@ -67,6 +70,7 @@ func TestVerifySignatureRequest_Validate_Error(t *testing.T) { {name: "signature's trustPolicy", req: getVerifySignatureRequest(ContractVersion, "someCT", "someSigningScheme", mockCertChain, nil)}, {name: "signature's trustPolicy's signatureVerification", req: getVerifySignatureRequest(ContractVersion, "someCT", "someSigningScheme", mockCertChain, []Capability{})}, {name: "signature", req: reqWithoutSignature}, + {name: "signature's criticalAttributes", req: reqWithoutCriticalAttr}, } for _, testcase := range testCases { @@ -83,6 +87,13 @@ func TestVerifySignatureRequest_Validate_Error(t *testing.T) { } } +func TestVerifySignatureRequest_Command(t *testing.T) { + req := getVerifySignatureRequest(ContractVersion, "someCT", "someSigningScheme", mockCertChain, []Capability{CapabilitySignatureGenerator}) + if cmd := req.Command(); cmd != CommandVerifySignature { + t.Errorf("DescribeKeyRequest#Command, expected %s but returned %s", CommandVerifySignature, cmd) + } +} + func getVerifySignatureRequest(cv, ct, ss string, cc [][]byte, sv []Capability) VerifySignatureRequest { return VerifySignatureRequest{ ContractVersion: cv, From fae6af8fa314ec57f389fea433a09610af86219f Mon Sep 17 00:00:00 2001 From: Pritesh Bandi Date: Sun, 21 Jan 2024 19:56:28 -0800 Subject: [PATCH 3/4] Update cli/cli.go Co-authored-by: Patrick Zheng Signed-off-by: Pritesh Bandi --- cli/cli.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/cli.go b/cli/cli.go index 033a594..3692a5d 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -141,7 +141,7 @@ func (c *CLI) unmarshalRequest(request plugin.Request) error { } if err := request.Validate(); err != nil { - c.logger.Errorf("%s validation error :%v", reflect.TypeOf(request), err) + c.logger.Errorf("%s validation error: %v", reflect.TypeOf(request), err) if e, ok := err.(*plugin.Error); ok { return plugin.NewValidationErrorf("%s: %s", plugin.ErrorMsgMalformedInput, e.Message) } From 464fc28a10cdaeafecb80c834a28b7328c150650 Mon Sep 17 00:00:00 2001 From: Pritesh Bandi Date: Sun, 21 Jan 2024 20:12:22 -0800 Subject: [PATCH 4/4] pr feedback Signed-off-by: Pritesh Bandi --- cli/cli.go | 7 ++++--- plugin/errors.go | 3 +-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index 3692a5d..045f8df 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -141,9 +141,10 @@ func (c *CLI) unmarshalRequest(request plugin.Request) error { } if err := request.Validate(); err != nil { - c.logger.Errorf("%s validation error: %v", reflect.TypeOf(request), err) - if e, ok := err.(*plugin.Error); ok { - return plugin.NewValidationErrorf("%s: %s", plugin.ErrorMsgMalformedInput, e.Message) + c.logger.Errorf("%s validation error :%v", reflect.TypeOf(request), err) + var plError *plugin.Error + if errors.As(err, &plError) { + return plugin.NewValidationErrorf("%s: %s", plugin.ErrorMsgMalformedInput, plError.Message) } return plugin.NewValidationErrorf("%s", plugin.ErrorMsgMalformedInput) } diff --git a/plugin/errors.go b/plugin/errors.go index 6e6297f..575a257 100644 --- a/plugin/errors.go +++ b/plugin/errors.go @@ -34,8 +34,7 @@ const ( ErrorMsgMalformedOutputFmt string = "Failed to generate response. Error: %s" ) -// Error is used when the signature associated is no longer -// valid. +// Error is used to return a well-formed error response as per NotaryProject specification. type Error struct { ErrCode ErrorCode `json:"errorCode"` Message string `json:"errorMessage,omitempty"`