Skip to content

Commit dcd7e88

Browse files
committed
Remove redundant return value for verification
1 parent 25accc8 commit dcd7e88

File tree

5 files changed

+39
-56
lines changed

5 files changed

+39
-56
lines changed

client.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
6161
}
6262

6363
if c.verifier != nil {
64-
_, err := VerifyResponse(c.sigName, *c.verifier, res)
64+
err := VerifyResponse(c.sigName, *c.verifier, res)
6565
if err != nil {
6666
return nil, err
6767
}
@@ -70,7 +70,7 @@ func (c *Client) Do(req *http.Request) (*http.Response, error) {
7070
if err != nil {
7171
return nil, err
7272
}
73-
_, err := VerifyResponse(sigName, *verifier, res)
73+
err := VerifyResponse(sigName, *verifier, res)
7474
if err != nil {
7575
return nil, err
7676
}

config.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package httpsign
22

33
import (
44
"fmt"
5+
"log"
56
"net/http"
67
"time"
78
)
@@ -162,7 +163,8 @@ func defaultReqNotVerified(w http.ResponseWriter, _ *http.Request, err error) {
162163
if err == nil { // should not happen
163164
_, _ = fmt.Fprintf(w, "Unknown error")
164165
}
165-
_, _ = fmt.Fprintln(w, "Could not verify request signature: "+err.Error())
166+
log.Println("Could not verify request signature: " + err.Error())
167+
_, _ = fmt.Fprintln(w, "Could not verify request signature")
166168
}
167169

168170
// SetReqNotVerified defines a callback to be called when a request fails to verify. The default

handler.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ func verifyServerRequest(w http.ResponseWriter, r *http.Request, config *Handler
134134
config.reqNotVerified(w, r, fmt.Errorf("could not fetch a verifier, check key ID"))
135135
return false
136136
}
137-
verified, err := VerifyRequest(sigName, *verifier, r)
138-
if !verified {
137+
err := VerifyRequest(sigName, *verifier, r)
138+
if err != nil {
139139
config.reqNotVerified(w, r, err)
140140
return false
141141
}

signatures.go

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -141,18 +141,18 @@ func addPseudoHeaders(message *parsedMessage, config SignConfig) {
141141
}
142142

143143
//
144-
// VerifyRequest verifies a signed HTTP request. Returns true if verification was successful.
144+
// VerifyRequest verifies a signed HTTP request. Returns an error if verification failed for any reason, otherwise nil.
145145
//
146-
func VerifyRequest(signatureName string, verifier Verifier, req *http.Request) (verified bool, err error) {
146+
func VerifyRequest(signatureName string, verifier Verifier, req *http.Request) (err error) {
147147
if req == nil {
148-
return false, fmt.Errorf("nil request")
148+
return fmt.Errorf("nil request")
149149
}
150150
if signatureName == "" {
151-
return false, fmt.Errorf("empty signature name")
151+
return fmt.Errorf("empty signature name")
152152
}
153153
parsedMessage, err := parseRequest(req)
154154
if err != nil {
155-
return false, err
155+
return err
156156
}
157157
return verifyMessage(*verifier.c, signatureName, verifier, *parsedMessage, verifier.f)
158158
}
@@ -216,60 +216,59 @@ func messageKeyID(signatureName string, parsedMessage parsedMessage) (keyID, alg
216216
}
217217

218218
//
219-
// VerifyResponse verifies a signed HTTP response. Returns true if verification was successful.
219+
// VerifyResponse verifies a signed HTTP response. Returns an error if verification failed for any reason, otherwise nil.
220220
//
221-
func VerifyResponse(signatureName string, verifier Verifier, res *http.Response) (verified bool, err error) {
221+
func VerifyResponse(signatureName string, verifier Verifier, res *http.Response) (err error) {
222222
if res == nil {
223-
return false, fmt.Errorf("nil response")
223+
return fmt.Errorf("nil response")
224224
}
225225
if signatureName == "" {
226-
return false, fmt.Errorf("empty signature name")
226+
return fmt.Errorf("empty signature name")
227227
}
228228
parsedMessage, err := parseResponse(res)
229229
if err != nil {
230-
return false, err
230+
return err
231231
}
232232
return verifyMessage(*verifier.c, signatureName, verifier, *parsedMessage, verifier.f)
233233
}
234234

235-
func verifyMessage(config VerifyConfig, name string, verifier Verifier, message parsedMessage, fields Fields) (bool, error) {
235+
func verifyMessage(config VerifyConfig, name string, verifier Verifier, message parsedMessage, fields Fields) error {
236236
wsi, found := message.components[*fromHeaderName("signature-input")]
237237
if !found {
238-
return false, fmt.Errorf("missing \"signature-input\" header")
238+
return fmt.Errorf("missing \"signature-input\" header")
239239
}
240240
wantSignatureInput := wsi[0]
241241
ws, found := message.components[*fromHeaderName("signature")]
242242
if !found {
243-
return false, fmt.Errorf("missing \"signature\" header")
243+
return fmt.Errorf("missing \"signature\" header")
244244
}
245245
wantSignature := ws[0]
246246
delete(message.components, *fromHeaderName("signature-input"))
247247
delete(message.components, *fromHeaderName("signature"))
248248
err := validateFields(fields)
249249
if err != nil {
250-
return false, err
250+
return err
251251
}
252252
wantSigRaw, err := parseWantSignature(wantSignature, name)
253253
if err != nil {
254-
return false, err
254+
return err
255255
}
256256
psiSig, err := parseSignatureInput(wantSignatureInput, name)
257257
if err != nil {
258-
return false, err
258+
return err
259259
}
260260
if !(psiSig.fields.contains(&fields)) {
261-
return false, fmt.Errorf("actual signature does not cover all required fields")
261+
return fmt.Errorf("actual signature does not cover all required fields")
262262
}
263263
err = applyVerificationPolicy(psiSig, config)
264264
if err != nil {
265-
return false, err
265+
return err
266266
}
267267
signatureInput, err := generateSignatureInput(message, psiSig.fields, psiSig.origSigParams)
268268
if err != nil {
269-
return false, err
269+
return err
270270
}
271-
verified, err := verifySignature(verifier, signatureInput, wantSigRaw)
272-
return verified, err
271+
return verifySignature(verifier, signatureInput, wantSigRaw)
273272
}
274273

275274
func applyVerificationPolicy(psi *psiSignature, config VerifyConfig) error {
@@ -327,12 +326,12 @@ func applyVerificationPolicy(psi *psiSignature, config VerifyConfig) error {
327326
return nil
328327
}
329328

330-
func verifySignature(verifier Verifier, input string, signature []byte) (bool, error) {
329+
func verifySignature(verifier Verifier, input string, signature []byte) error {
331330
verified, err := verifier.verify([]byte(input), signature)
332-
if !verified && err == nil {
331+
if !verified && (err == nil) {
333332
err = fmt.Errorf("bad signature, check key or signature value")
334333
}
335-
return verified, err
334+
return err
336335
}
337336

338337
type psiSignature struct {

signatures_test.go

Lines changed: 9 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -545,13 +545,10 @@ func TestSignAndVerifyHMAC(t *testing.T) {
545545
if err != nil {
546546
t.Errorf("could not generate verifier: %s", err)
547547
}
548-
verified, err := VerifyRequest(signatureName, *verifier, req)
548+
err = VerifyRequest(signatureName, *verifier, req)
549549
if err != nil {
550550
t.Errorf("verification error: %s", err)
551551
}
552-
if !verified {
553-
t.Errorf("message did not pass verification")
554-
}
555552
}
556553

557554
func TestCreated(t *testing.T) {
@@ -571,7 +568,7 @@ func TestCreated(t *testing.T) {
571568
if err != nil {
572569
t.Errorf("could not generate verifier: %s", err)
573570
}
574-
_, err = VerifyResponse(signatureName, *verifier, res2)
571+
err = VerifyResponse(signatureName, *verifier, res2)
575572
if wantSuccess && err != nil {
576573
t.Errorf("verification error: %s", err)
577574
}
@@ -603,13 +600,10 @@ func TestSignAndVerifyResponseHMAC(t *testing.T) {
603600
if err != nil {
604601
t.Errorf("could not generate verifier: %s", err)
605602
}
606-
verified, err := VerifyResponse(signatureName, *verifier, res2)
603+
err = VerifyResponse(signatureName, *verifier, res2)
607604
if err != nil {
608605
t.Errorf("verification error: %s", err)
609606
}
610-
if !verified {
611-
t.Errorf("message did not pass verification")
612-
}
613607
}
614608

615609
func TestSignAndVerifyRSAPSS(t *testing.T) {
@@ -633,13 +627,10 @@ func TestSignAndVerifyRSAPSS(t *testing.T) {
633627
if err != nil {
634628
t.Errorf("could not generate verifier: %s", err)
635629
}
636-
verified, err := VerifyRequest(signatureName, *verifier, req)
630+
err = VerifyRequest(signatureName, *verifier, req)
637631
if err != nil {
638632
t.Errorf("verification error: %s", err)
639633
}
640-
if !verified {
641-
t.Errorf("message did not pass verification")
642-
}
643634
}
644635

645636
func TestSignAndVerifyRSA(t *testing.T) {
@@ -663,13 +654,10 @@ func TestSignAndVerifyRSA(t *testing.T) {
663654
if err != nil {
664655
t.Errorf("could not generate verifier: %s", err)
665656
}
666-
verified, err := VerifyRequest(signatureName, *verifier, req)
657+
err = VerifyRequest(signatureName, *verifier, req)
667658
if err != nil {
668659
t.Errorf("verification error: %s", err)
669660
}
670-
if !verified {
671-
t.Errorf("message did not pass verification")
672-
}
673661
}
674662

675663
func TestSignAndVerifyP256(t *testing.T) {
@@ -696,13 +684,10 @@ func TestSignAndVerifyP256(t *testing.T) {
696684
if err != nil {
697685
t.Errorf("could not generate verifier: %s", err)
698686
}
699-
verified, err := VerifyRequest(signatureName, *verifier, req)
687+
err = VerifyRequest(signatureName, *verifier, req)
700688
if err != nil {
701689
t.Errorf("verification error: %s", err)
702690
}
703-
if !verified {
704-
t.Errorf("message did not pass verification")
705-
}
706691
}
707692

708693
func TestSignResponse(t *testing.T) {
@@ -779,8 +764,8 @@ Signature: sig77=:3e9KqLP62NHfHY5OMG4036+U6tvBowZF35ALzTjpsf0=:
779764
780765
`
781766
req, _ := http.ReadRequest(bufio.NewReader(strings.NewReader(reqStr)))
782-
verified, _ := VerifyRequest("sig77", *verifier, req)
783-
fmt.Printf("verified: %t", verified)
767+
err := VerifyRequest("sig77", *verifier, req)
768+
fmt.Printf("verified: %t", err == nil)
784769
// Output: verified: true
785770
}
786771

@@ -880,14 +865,11 @@ func TestVerifyRequest(t *testing.T) {
880865
}
881866
for _, tt := range tests {
882867
t.Run(tt.name, func(t *testing.T) {
883-
got, err := VerifyRequest(tt.args.signatureName, tt.args.verifier, tt.args.req)
868+
err := VerifyRequest(tt.args.signatureName, tt.args.verifier, tt.args.req)
884869
if (err != nil) != tt.wantErr {
885870
t.Errorf("VerifyRequest() error = %v, wantErr %v", err, tt.wantErr)
886871
return
887872
}
888-
if got != tt.want {
889-
t.Errorf("VerifyRequest() got = %v, want %v", got, tt.want)
890-
}
891873
})
892874
}
893875
}

0 commit comments

Comments
 (0)