Merge pull request #4560 from Shopify/basic-auth-map

Support configuring basic auth credentials as a map of user/password hashes

docs/user-guide/nginx-configuration/annotations.md

@@ -19,6 +19,7 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
 |[nginx.ingress.kubernetes.io/affinity](#session-affinity)|cookie|
 |[nginx.ingress.kubernetes.io/auth-realm](#authentication)|string|
 |[nginx.ingress.kubernetes.io/auth-secret](#authentication)|string|
+|[nginx.ingress.kubernetes.io/auth-secret-type](#authentication)|string|
 |[nginx.ingress.kubernetes.io/auth-type](#authentication)|basic or digest|
 |[nginx.ingress.kubernetes.io/auth-tls-secret](#client-certificate-authentication)|string|
 |[nginx.ingress.kubernetes.io/auth-tls-verify-depth](#client-certificate-authentication)|number|
@@ -166,7 +167,7 @@ The NGINX annotation `nginx.ingress.kubernetes.io/session-cookie-path` defines t
 
 ### Authentication
 
-Is possible to add authentication adding additional annotations in the Ingress rule. The source of the authentication is a secret that contains usernames and passwords inside the key `auth`.
+Is possible to add authentication adding additional annotations in the Ingress rule. The source of the authentication is a secret that contains usernames and passwords.
 
 The annotations are:
 ```
@@ -182,6 +183,15 @@ nginx.ingress.kubernetes.io/auth-secret: secretName
 The name of the Secret that contains the usernames and passwords which are granted access to the `path`s defined in the Ingress rules.
 This annotation also accepts the alternative form "namespace/secretName", in which case the Secret lookup is performed in the referenced namespace instead of the Ingress namespace.
 
+```
+nginx.ingress.kubernetes.io/auth-secret-type: [auth-file|auth-map]
+```
+
+The `auth-secret` can have two forms:
+
+- `auth-file` - default, an htpasswd file in the key `auth` within the secret
+- `auth-map` - the keys of the secret are the usernames, and the values are the hashed passwords
+
 ```
 nginx.ingress.kubernetes.io/auth-realm: "realm string"
 ```

internal/ingress/annotations/auth/main.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io/ioutil"
 	"regexp"
+	"strings"
 
 	"github.com/pkg/errors"
 	api "k8s.io/api/core/v1"
@@ -41,12 +42,13 @@ var (
 
 // Config returns authentication configuration for an Ingress rule
 type Config struct {
-	Type    string `json:"type"`
+	Type       string `json:"type"`
-	Realm   string `json:"realm"`
+	Realm      string `json:"realm"`
-	File    string `json:"file"`
+	File       string `json:"file"`
-	Secured bool   `json:"secured"`
+	Secured    bool   `json:"secured"`
-	FileSHA string `json:"fileSha"`
+	FileSHA    string `json:"fileSha"`
-	Secret  string `json:"secret"`
+	Secret     string `json:"secret"`
+	SecretType string `json:"secretType"`
 }
 
 // Equal tests for equality between two Config types
@@ -102,6 +104,12 @@ func (a auth) Parse(ing *networking.Ingress) (interface{}, error) {
 		return nil, ing_errors.NewLocationDenied("invalid authentication type")
 	}
 
+	var secretType string
+	secretType, err = parser.GetStringAnnotation("auth-secret-type", ing)
+	if err != nil {
+		secretType = "auth-file"
+	}
+
 	s, err := parser.GetStringAnnotation("auth-secret", ing)
 	if err != nil {
 		return nil, ing_errors.LocationDenied{
@@ -131,24 +139,37 @@ func (a auth) Parse(ing *networking.Ingress) (interface{}, error) {
 	realm, _ := parser.GetStringAnnotation("auth-realm", ing)
 
 	passFile := fmt.Sprintf("%v/%v-%v.passwd", a.authDirectory, ing.GetNamespace(), ing.GetName())
-	err = dumpSecret(passFile, secret)
+
-	if err != nil {
+	if secretType == "auth-file" {
-		return nil, err
+		err = dumpSecretAuthFile(passFile, secret)
+		if err != nil {
+			return nil, err
+		}
+	} else if secretType == "auth-map" {
+		err = dumpSecretAuthMap(passFile, secret)
+		if err != nil {
+			return nil, err
+		}
+	} else {
+		return nil, ing_errors.LocationDenied{
+			Reason: errors.Wrap(err, "invalid auth-secret-type in annotation, must be 'auth-file' or 'auth-map'"),
+		}
 	}
 
 	return &Config{
-		Type:    at,
+		Type:       at,
-		Realm:   realm,
+		Realm:      realm,
-		File:    passFile,
+		File:       passFile,
-		Secured: true,
+		Secured:    true,
-		FileSHA: file.SHA1(passFile),
+		FileSHA:    file.SHA1(passFile),
-		Secret:  name,
+		Secret:     name,
+		SecretType: secretType,
 	}, nil
 }
 
 // dumpSecret dumps the content of a secret into a file
 // in the expected format for the specified authorization
-func dumpSecret(filename string, secret *api.Secret) error {
+func dumpSecretAuthFile(filename string, secret *api.Secret) error {
 	val, ok := secret.Data["auth"]
 	if !ok {
 		return ing_errors.LocationDenied{
@@ -165,3 +186,22 @@ func dumpSecret(filename string, secret *api.Secret) error {
 
 	return nil
 }
+
+func dumpSecretAuthMap(filename string, secret *api.Secret) error {
+	builder := &strings.Builder{}
+	for user, pass := range secret.Data {
+		builder.WriteString(user)
+		builder.WriteString(":")
+		builder.WriteString(string(pass))
+		builder.WriteString("\n")
+	}
+
+	err := ioutil.WriteFile(filename, []byte(builder.String()), file.ReadWriteByUser)
+	if err != nil {
+		return ing_errors.LocationDenied{
+			Reason: errors.Wrap(err, "unexpected error creating password file"),
+		}
+	}
+
+	return nil
+}

internal/ingress/annotations/auth/main_test.go

@@ -182,6 +182,25 @@ func TestIngressAuthWithoutSecret(t *testing.T) {
 	}
 }
 
+func TestIngressAuthInvalidSecretKey(t *testing.T) {
+	ing := buildIngress()
+
+	data := map[string]string{}
+	data[parser.GetAnnotationWithPrefix("auth-type")] = "basic"
+	data[parser.GetAnnotationWithPrefix("auth-secret")] = "demo-secret"
+	data[parser.GetAnnotationWithPrefix("auth-secret-type")] = "invalid-type"
+	data[parser.GetAnnotationWithPrefix("auth-realm")] = "-realm-"
+	ing.SetAnnotations(data)
+
+	_, dir, _ := dummySecretContent(t)
+	defer os.RemoveAll(dir)
+
+	_, err := NewParser(dir, mockSecret{}).Parse(ing)
+	if err == nil {
+		t.Errorf("expected an error with invalid secret name")
+	}
+}
+
 func dummySecretContent(t *testing.T) (string, string, *api.Secret) {
 	dir, err := ioutil.TempDir("", fmt.Sprintf("%v", time.Now().Unix()))
 	if err != nil {
@@ -197,20 +216,30 @@ func dummySecretContent(t *testing.T) (string, string, *api.Secret) {
 	return tmpfile.Name(), dir, s
 }
 
-func TestDumpSecret(t *testing.T) {
+func TestDumpSecretAuthFile(t *testing.T) {
 	tmpfile, dir, s := dummySecretContent(t)
 	defer os.RemoveAll(dir)
 
 	sd := s.Data
 	s.Data = nil
 
-	err := dumpSecret(tmpfile, s)
+	err := dumpSecretAuthFile(tmpfile, s)
 	if err == nil {
 		t.Errorf("Expected error with secret without auth")
 	}
 
 	s.Data = sd
-	err = dumpSecret(tmpfile, s)
+	err = dumpSecretAuthFile(tmpfile, s)
+	if err != nil {
+		t.Errorf("Unexpected error creating htpasswd file %v: %v", tmpfile, err)
+	}
+}
+
+func TestDumpSecretAuthMap(t *testing.T) {
+	tmpfile, dir, s := dummySecretContent(t)
+	defer os.RemoveAll(dir)
+
+	err := dumpSecretAuthMap(tmpfile, s)
 	if err != nil {
 		t.Errorf("Unexpected error creating htpasswd file %v: %v", tmpfile, err)
 	}

test/e2e/annotations/auth.go

@@ -183,6 +183,37 @@ var _ = framework.IngressNginxDescribe("Annotations - Auth", func() {
 		Expect(resp.StatusCode).Should(Equal(http.StatusOK))
 	})
 
+	It("should return status code 200 when authentication is configured with a map and Authorization header is sent", func() {
+		host := "auth"
+
+		s := f.EnsureSecret(buildMapSecret("foo", "bar", "test", f.Namespace))
+
+		annotations := map[string]string{
+			"nginx.ingress.kubernetes.io/auth-type":        "basic",
+			"nginx.ingress.kubernetes.io/auth-secret":      s.Name,
+			"nginx.ingress.kubernetes.io/auth-secret-type": "auth-map",
+			"nginx.ingress.kubernetes.io/auth-realm":       "test auth",
+		}
+
+		ing := framework.NewSingleIngress(host, "/", host, f.Namespace, framework.EchoService, 80, &annotations)
+		f.EnsureIngress(ing)
+
+		f.WaitForNginxServer(host,
+			func(server string) bool {
+				return Expect(server).Should(ContainSubstring("server_name auth"))
+			})
+
+		resp, _, errs := gorequest.New().
+			Get(f.GetURL(framework.HTTP)).
+			Retry(10, 1*time.Second, http.StatusNotFound).
+			Set("Host", host).
+			SetBasicAuth("foo", "bar").
+			End()
+
+		Expect(errs).Should(BeEmpty())
+		Expect(resp.StatusCode).Should(Equal(http.StatusOK))
+	})
+
 	It("should return status code 500 when authentication is configured with invalid content and Authorization header is sent", func() {
 		host := "auth"
 
@@ -546,3 +577,20 @@ func buildSecret(username, password, name, namespace string) *corev1.Secret {
 		Type: corev1.SecretTypeOpaque,
 	}
 }
+
+func buildMapSecret(username, password, name, namespace string) *corev1.Secret {
+	out, err := exec.Command("openssl", "passwd", "-crypt", password).CombinedOutput()
+	Expect(err).NotTo(HaveOccurred())
+
+	return &corev1.Secret{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:                       name,
+			Namespace:                  namespace,
+			DeletionGracePeriodSeconds: framework.NewInt64(1),
+		},
+		Data: map[string][]byte{
+			username: []byte(out),
+		},
+		Type: corev1.SecretTypeOpaque,
+	}
+}