pkg/auth: change VivifyPass to *string to reflect the fact that it's

optional.

Previously, omitting the vivify option from the auth config would
result in VivifyPass being set to empty string (which is a valid
password for HTTP Basic authentication).

Change-Id: I51f441705af4b850ad2bdf27ac3b5a4a9dc83721

pkg/auth/auth.go

@@ -106,9 +106,10 @@ func newLocalhostAuth(string) (AuthMode, error) {
 
 func newDevAuth(pw string) (AuthMode, error) {
 	// the vivify mode password is automatically set to "vivi" + Password
+	vp := "vivi" + pw
 	return &DevAuth{
 		Password:   pw,
-		VivifyPass: "vivi" + pw,
+		VivifyPass: &vp,
 	}, nil
 }
 
@@ -126,7 +127,8 @@ func newUserPassAuth(arg string) (AuthMode, error) {
 			mode.OrLocalhost = true
 		case strings.HasPrefix(opt, "vivify="):
 			// optional vivify mode password: "userpass:joe:ponies:vivify=rainbowdash"
-			mode.VivifyPass = strings.Replace(opt, "vivify=", "", -1)
+			vp := strings.Replace(opt, "vivify=", "", -1)
+			mode.VivifyPass = &vp
 		default:
 			return nil, fmt.Errorf("Unknown userpass option %q", opt)
 		}
@@ -200,9 +202,10 @@ func AddMode(am AuthMode) {
 type UserPass struct {
 	Username, Password string
 	OrLocalhost        bool // if true, allow localhost ident auth too
-	// Alternative password used (only) for the vivify operation.
+
+	// VivifyPass, if not nil, is the alternative password used (only) for the vivify operation.
 	// It is checked when uploading, but Password takes precedence.
-	VivifyPass string
+	VivifyPass *string
 }
 
 func (up *UserPass) AllowedAccess(req *http.Request) Operation {
@@ -212,7 +215,7 @@ func (up *UserPass) AllowedAccess(req *http.Request) Operation {
 			if pass == up.Password {
 				return OpAll
 			}
-			if pass == up.VivifyPass {
+			if up.VivifyPass != nil && pass == *up.VivifyPass {
 				return OpVivify
 			}
 		}
@@ -258,7 +261,7 @@ func (Localhost) AllowedAccess(req *http.Request) (out Operation) {
 type DevAuth struct {
 	Password string
 	// Password for the vivify mode, automatically set to "vivi" + Password
-	VivifyPass string
+	VivifyPass *string
 }
 
 func (da *DevAuth) AllowedAccess(req *http.Request) Operation {
@@ -267,7 +270,7 @@ func (da *DevAuth) AllowedAccess(req *http.Request) Operation {
 		if pass == da.Password {
 			return OpAll
 		}
-		if pass == da.VivifyPass {
+		if da.VivifyPass != nil && pass == *da.VivifyPass {
 			return OpVivify
 		}
 	}

pkg/auth/auth_test.go

@@ -24,6 +24,9 @@ import (
 )
 
 func TestFromConfig(t *testing.T) {
+	newString := func(s string) *string {
+		return &s
+	}
 	tests := []struct {
 		in string
 
@@ -34,11 +37,11 @@ func TestFromConfig(t *testing.T) {
 		{in: "slkdjflksdjf", wanterr: `Unknown auth type: "slkdjflksdjf"`},
 		{in: "none", want: None{}},
 		{in: "localhost", want: Localhost{}},
-		{in: "userpass:alice:secret", want: &UserPass{Username: "alice", Password: "secret", OrLocalhost: false, VivifyPass: ""}},
-		{in: "userpass:alice:secret:+localhost", want: &UserPass{Username: "alice", Password: "secret", OrLocalhost: true, VivifyPass: ""}},
-		{in: "userpass:alice:secret:+localhost:vivify=foo", want: &UserPass{Username: "alice", Password: "secret", OrLocalhost: true, VivifyPass: "foo"}},
-		{in: "devauth:port3179", want: &DevAuth{Password: "port3179", VivifyPass: "viviport3179"}},
-		{in: "basic:alice:secret", want: &UserPass{Username: "alice", Password: "secret", OrLocalhost: false, VivifyPass: ""}},
+		{in: "userpass:alice:secret", want: &UserPass{Username: "alice", Password: "secret", OrLocalhost: false, VivifyPass: nil}},
+		{in: "userpass:alice:secret:+localhost", want: &UserPass{Username: "alice", Password: "secret", OrLocalhost: true, VivifyPass: nil}},
+		{in: "userpass:alice:secret:+localhost:vivify=foo", want: &UserPass{Username: "alice", Password: "secret", OrLocalhost: true, VivifyPass: newString("foo")}},
+		{in: "devauth:port3179", want: &DevAuth{Password: "port3179", VivifyPass: newString("viviport3179")}},
+		{in: "basic:alice:secret", want: &UserPass{Username: "alice", Password: "secret", OrLocalhost: false, VivifyPass: nil}},
 		{in: "basic:alice:secret:+localhost", wanterr: `invalid basic auth syntax. got "alice:secret:+localhost", want "username:password"`},
 		{in: "basic:alice:secret:+vivify=foo", wanterr: `invalid basic auth syntax. got "alice:secret:+vivify=foo", want "username:password"`},
 	}
@@ -93,3 +96,43 @@ func TestMultiMode(t *testing.T) {
 	}
 
 }
+
+func TestEmptyPasswords(t *testing.T) {
+	tests := []struct {
+		config           string
+		providedPassword string
+
+		allowed bool
+	}{
+		{config: "foo:", providedPassword: "", allowed: true},
+		{config: "foo:", providedPassword: "bar", allowed: false},
+		{config: "foo:bar", providedPassword: "", allowed: false},
+		{config: "foo:bar", providedPassword: "bar", allowed: true},
+		{config: "foo:bar:vivify=", providedPassword: "", allowed: true},
+		{config: "foo:bar:vivify=", providedPassword: "notbar", allowed: false},
+		{config: "foo:bar:vivify=otherbar", providedPassword: "", allowed: false},
+		{config: "foo:bar:vivify=otherbar", providedPassword: "notbar", allowed: false},
+		{config: "foo:bar:vivify=otherbar", providedPassword: "bar", allowed: true},
+		{config: "foo:bar:vivify=otherbar", providedPassword: "otherbar", allowed: true},
+	}
+
+	for _, test := range tests {
+		req, err := http.NewRequest("GET", "/", nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		(&UserPass{Username: "foo", Password: test.providedPassword}).AddAuthHeader(req)
+
+		auth, err := newUserPassAuth(test.config)
+		if err != nil {
+			t.Errorf(err.Error())
+			continue
+		}
+		SetMode(auth)
+
+		if Allowed(req, OpVivify) != test.allowed {
+			t.Errorf("Allowed(req, OpVivify) = %#v; want %#v (config: %q, providedPassword: %q)",
+				Allowed(req, OpVivify), test.allowed, test.config, test.providedPassword)
+		}
+	}
+}