Fix scram.*Client.Step so it returns an "ok" flag consistently

scram/rfc5802_consts_test.go

@@ -0,0 +1,50 @@
+// Copyright (c) 2010 IETF Trust and the persons identified as the
+// document authors. All rights reserved. Redistribution and use in source
+// and binary forms, with or without modification, are permitted provided
+// that the following conditions are met:
+//
+//   - Redistributions of source code must retain the above copyright
+//     notice, this list of conditions and the following disclaimer.
+//   - Redistributions in binary form must reproduce the above copyright
+//     notice, this list of conditions and the following disclaimer in
+//     the documentation and/or other materials provided with the
+//     distribution.
+//   - Neither the name of Internet Society, IETF or IETF Trust, nor the
+//     names of specific contributors, may be used to endorse or promote
+//     products derived from this software without specific prior
+//     written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// “AS IS” AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+// FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+// COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+// INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+// BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+// OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+// AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+// DAMAGE.
+
+package scram_test
+
+// Following values are extracted from section 5 of RFC 5802,
+// available at https://datatracker.ietf.org/doc/html/rfc5802#section-5,
+// therefore, the rfc5802_consts_test.go file is licensed under
+// the Revised BSD License (included at the top of this file).
+//
+// Other files are licensed under MIT license as usual in the pq module.
+var (
+	rfc5802User       = "user"
+	rfc5802Pass       = "pencil"
+	rfc5802Nonce      = []byte("fyko+d2lbbFgONRv9qkxdawL")
+	rfc5802ClientMsgs = []string{
+		"n,,n=user,r=fyko+d2lbbFgONRv9qkxdawL",
+		"c=biws,r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,p=v0X8v3Bz2T0CJGbJQyF0X+HI4Ts=",
+	}
+	rfc5802ServerMsgs = [][]byte{
+		[]byte("r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,s=QSXCR+Q6sek8bf92,i=4096"),
+		[]byte("v=rmF9pqV8S7suAoZWja4dJRkFsKQ="),
+	}
+)

scram/scram.go

@@ -25,7 +25,6 @@
 // Package scram implements a SCRAM-{SHA-1,etc} client per RFC5802.
 //
 // http://tools.ietf.org/html/rfc5802
-//
 package scram
 
 import (
@@ -43,17 +42,16 @@ import (
 //
 // A Client may be used within a SASL conversation with logic resembling:
 //
-//    var in []byte
-//    var client = scram.NewClient(sha1.New, user, pass)
-//    for client.Step(in) {
-//            out := client.Out()
-//            // send out to server
-//            in := serverOut
-//    }
-//    if client.Err() != nil {
-//            // auth failed
-//    }
-//
+//	var in []byte // first step requires no input
+//	var client = scram.NewClient(sha256.New, user, pass)
+//	for client.Step(in) { // stop on 1st error or after 3rd step
+//	        out := client.Out()
+//	        // send out to server
+//	        in = serverOut
+//	}
+//	if client.Err() != nil {
+//	        // auth failed
+//	}
 type Client struct {
 	newHash func() hash.Hash
 
@@ -73,8 +71,7 @@ type Client struct {
 //
 // For SCRAM-SHA-256, for example, use:
 //
-//    client := scram.NewClient(sha256.New, user, pass)
-//
+//	client := scram.NewClient(sha256.New, user, pass)
 func NewClient(newHash func() hash.Hash, user, pass string) *Client {
 	c := &Client{
 		newHash: newHash,
@@ -109,8 +106,8 @@ var escaper = strings.NewReplacer("=", "=3D", ",", "=2C")
 
 // Step processes the incoming data from the server and makes the
 // next round of data for the server available via Client.Out.
-// Step returns false if there are no errors and more data is
-// still expected.
+// Step returns true if there are no errors and Step should be called
+// again with upcoming data messages.
 func (c *Client) Step(in []byte) bool {
 	c.out.Reset()
 	if c.step > 2 || c.err != nil {
@@ -125,7 +122,7 @@ func (c *Client) Step(in []byte) bool {
 	case 3:
 		c.err = c.step3(in)
 	}
-	return c.step > 2 || c.err != nil
+	return c.step <= 2 && c.err == nil
 }
 
 func (c *Client) step1(in []byte) error {

scram/scram_test.go

@@ -0,0 +1,35 @@
+// Package scram_test provides functional tests for the scram package.
+package scram_test
+
+import (
+	"crypto/sha1"
+	"testing"
+
+	"github.com/lib/pq/scram"
+)
+
+func TestScramSHA1(t *testing.T) {
+	s := 0
+	var in []byte // first step requires no input
+	var client = scram.NewClient(sha1.New, rfc5802User, rfc5802Pass)
+	client.SetNonce(rfc5802Nonce)
+	for client.Step(in) {
+		if s >= 2 {
+			t.Fatal("Step didn't stop after 3rd step")
+		}
+		out := client.Out()
+		if cm := rfc5802ClientMsgs[s]; cm != string(out) {
+			t.Fatalf(
+				`Step: %d
+Expected message: %q
+Actual message:   %q`,
+				s+1, cm, out,
+			)
+		}
+		in = rfc5802ServerMsgs[s]
+		s++
+	}
+	if err := client.Err(); err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+}