Refactor the authentication logic to support ASP-only users

Added test coverage for more service configurations involving 2FA.

backend/backend.go

@@ -31,7 +31,12 @@ type AppSpecificPassword struct {
 
 // Has2FA returns true if the user supports any 2FA method.
 func (u *User) Has2FA() bool {
-	return u.HasU2F() || u.HasOTP()
+	return u.HasU2F() || u.HasOTP() || u.HasASPs()
+}
+
+// HasASPs returns true if the user has app-specific passwords.
+func (u *User) HasASPs() bool {
+	return len(u.AppSpecificPasswords) > 0
 }
 
 // HasOTP returns true if the user supports (T)OTP.

server/authserver.go

@@ -326,6 +326,11 @@ func (s *Server) Authenticate(ctx context.Context, req *auth.Request) *auth.Resp
 var (
 	errServiceUnknown = errors.New("unknown service")
 	errUserUnknown    = errors.New("unknown user")
+	errNoMechanisms   = errors.New("no authentication mechanisms available")
+	errBadPassword    = errors.New("wrong password")
+	errBadASP         = errors.New("wrong app-specific password")
+	errBadU2F         = errors.New("bad U2F response")
+	errBadOTP         = errors.New("invalid OTP")
 )
 
 // Function with the actual authentication API logic - we return both
@@ -384,18 +389,33 @@ func (s *Server) authenticateUser(req *auth.Request, svc *service, user *backend
 	// Verify different credentials depending on whether the user
 	// has 2FA enabled or not, and on whether the service itself
 	// supports challenge-response authentication.
-	if !svc.ignore2FA && (svc.enforce2FA || user.Has2FA()) {
-		if svc.challengeResponse {
+	err = errNoMechanisms
+
+	if svc.ignore2FA {
+		resp, err = s.authenticateUserWithPassword(user, req)
+	} else if svc.challengeResponse {
+		if svc.enforce2FA || user.Has2FA() {
 			resp, err = s.authenticateUserWith2FA(user, req)
-		} else {
+		} else if user.HasASPs() {
+			// It doesn't make much sense to support ASPs
+			// for a challenge-response service, but we
+			// still do it for completeness.
+			//
 			// Rewrite the 'service' for app-specific
 			// password matching, if necessary.
 			if svc.aspService != "" {
 				req.Service = svc.aspService
 			}
 			resp, err = s.authenticateUserWithASP(user, req)
+		} else {
+			resp, err = s.authenticateUserWithPassword(user, req)
 		}
-	} else {
+	} else if svc.enforce2FA || user.HasASPs() {
+		if svc.aspService != "" {
+			req.Service = svc.aspService
+		}
+		resp, err = s.authenticateUserWithASP(user, req)
+	} else if !user.Has2FA() {
 		resp, err = s.authenticateUserWithPassword(user, req)
 	}
 	if err != nil {
@@ -429,7 +449,7 @@ func (s *Server) authenticateUserWithPassword(user *backend.User, req *auth.Requ
 	if checkPassword(req.Password, user.EncryptedPassword) {
 		return newOK(), nil
 	}
-	return nil, errors.New("wrong password")
+	return nil, errBadPassword
 }
 
 func (s *Server) authenticateUserWithASP(user *backend.User, req *auth.Request) (*auth.Response, error) {
@@ -438,13 +458,13 @@ func (s *Server) authenticateUserWithASP(user *backend.User, req *auth.Request)
 			return newOK(), nil
 		}
 	}
-	return nil, errors.New("wrong app-specific password")
+	return nil, errBadASP
 }
 
 func (s *Server) authenticateUserWith2FA(user *backend.User, req *auth.Request) (*auth.Response, error) {
 	// First of all verify the password.
 	if !checkPassword(req.Password, user.EncryptedPassword) {
-		return nil, errors.New("wrong password")
+		return nil, errBadPassword
 	}
 
 	// If the request contains one of the 2FA attributes, verify
@@ -456,7 +476,7 @@ func (s *Server) authenticateUserWith2FA(user *backend.User, req *auth.Request)
 		if user.HasU2F() && s.checkU2F(user, req.U2FResponse) {
 			return newOK(), nil
 		}
-		return nil, errors.New("bad U2F response")
+		return nil, errBadU2F
 	case req.OTP != "":
 		if user.HasOTP() && s.checkOTP(user, req.OTP, user.TOTPSecret) {
 			// Save the token for replay protection.
@@ -465,7 +485,7 @@ func (s *Server) authenticateUserWith2FA(user *backend.User, req *auth.Request)
 			}
 			return newOK(), nil
 		}
-		return nil, errors.New("bad OTP")
+		return nil, errBadOTP
 	default:
 		resp := &auth.Response{
 			Status: auth.StatusInsufficientCredentials,

server/authserver_test.go

@@ -80,6 +80,16 @@ var (
   u2f_registrations:
     - key_handle: "JcolXA6KaoihO8VuxSugtCT5jyh-6lFuWXLkFAPe8s9qszxTMvDAtJn8gmYg9uGO-kmjgap1h0llchlqqjCpKw"
       public_key: "0498ee4565cd348031cf36ee3549b63b5ea23b5e7ea6f297e7cccaeba99983d185110fb94fa6455c82d3e5c8d0be10be71308d76062fb5fa50d3ea8228048f0037"
+
+- name: aspuser
+  email: aspuser@example.com
+  shard: 42
+  password: "$s$16384$8$1$c479e8eb722f1b071efea7826ccf9c20$96d63ebed0c64afb746026f56f71b2a1f8796c73141d2d6b1958d4ea26c60a0b"
+  app_specific_passwords:
+    - service: test
+      password: "$s$16384$8$1$5b8b0dc22ec2bc1029b06d4856a8b471$6c7257652e3982d8729b760885887b18d4a28966f9b2364041ddada8bea297db"
+    - service: force2fa
+      password: "$s$16384$8$1$5b8b0dc22ec2bc1029b06d4856a8b471$6c7257652e3982d8729b760885887b18d4a28966f9b2364041ddada8bea297db"
 `
 
 	testConfigStr = `---
@@ -89,6 +99,18 @@ services:
       - backend: file
         params:
           src: users.yml
+  no2fa:
+    ignore_2fa: true
+    backends:
+      - backend: file
+        params:
+          src: users.yml
+  force2fa:
+    enforce_2fa: true
+    backends:
+      - backend: file
+        params:
+          src: users.yml
   interactive:
     challenge_response: true
     backends:
@@ -123,6 +145,39 @@ rate_limits:
 `
 )
 
+func runBackendTest(t *testing.T, srv *Server) {
+	testdata := []struct {
+		username, service                         string
+		expectGroups                              []string
+		expectHasOTP, expectHasU2F, expectHasASPs bool
+	}{
+		{"testuser", "interactive", nil, false, false, false},
+		{"2fauser", "interactive", nil, true, true, false},
+		{"aspuser", "interactive", nil, false, false, true},
+	}
+	for _, td := range testdata {
+		svc, ok := srv.getService(td.service)
+		if !ok {
+			t.Errorf("no such service: %s", td.service)
+			continue
+		}
+		user, ok := srv.getUser(context.Background(), svc, td.username)
+		if !ok {
+			t.Errorf("no such user: %s", td.username)
+			continue
+		}
+		if b := user.HasOTP(); b != td.expectHasOTP {
+			t.Errorf("user %s has_otp=%v, expected=%v", td.username, b, td.expectHasOTP)
+		}
+		if b := user.HasU2F(); b != td.expectHasU2F {
+			t.Errorf("user %s has_u2f=%v, expected=%v", td.username, b, td.expectHasU2F)
+		}
+		if b := user.HasASPs(); b != td.expectHasASPs {
+			t.Errorf("user %s has_asp=%v, expected=%v", td.username, b, td.expectHasASPs)
+		}
+	}
+}
+
 func runAuthenticationTest(t *testing.T, client client.Client) {
 	// Test a number of simple password logins.
 	testdata := []struct {
@@ -134,6 +189,17 @@ func runAuthenticationTest(t *testing.T, client client.Client) {
 		{"test", "bad_user", "password", auth.StatusError},
 		{"test", "testuser", "bad_password", auth.StatusError},
 		{"test", "2fauser", "password", auth.StatusError},
+		{"test", "aspuser", "password", auth.StatusError},
+		{"test", "aspuser", "password2", auth.StatusOK},
+
+		{"no2fa", "testuser", "password", auth.StatusOK},
+		{"no2fa", "2fauser", "password", auth.StatusOK},
+		{"no2fa", "aspuser", "password", auth.StatusOK},
+
+		{"force2fa", "testuser", "password", auth.StatusError},
+		{"force2fa", "2fauser", "password", auth.StatusError},
+		{"force2fa", "aspuser", "password", auth.StatusError},
+		{"force2fa", "aspuser", "password2", auth.StatusOK},
 	}
 	for _, td := range testdata {
 		resp, err := client.Authenticate(context.Background(), &auth.Request{
@@ -194,6 +260,15 @@ func TestAuthServer(t *testing.T) {
 	runAuthenticationTest(t, &clientAdapter{s.srv})
 }
 
+func TestAuthServer_Backend(t *testing.T) {
+	s := createTestServer(t, map[string]string{
+		"users.yml":  testUsersFileStr,
+		"config.yml": testConfigStr,
+	})
+	defer s.Close()
+	runBackendTest(t, s.srv)
+}
+
 func TestAuthServer_Blacklist(t *testing.T) {
 	s := createTestServer(t, map[string]string{
 		"users.yml":  testUsersFileStr,

server/sql_test.go

@@ -38,6 +38,23 @@ services:
         params:
           queries:
             get_user: "SELECT email, password, totp_secret, '' AS shard FROM users WHERE name = ?"
+            get_user_asp: "SELECT service, password FROM asps WHERE name = ?"
+  no2fa:
+    ignore_2fa: true
+    backends:
+      - backend: sql
+        params:
+          queries:
+            get_user: "SELECT email, password, totp_secret, '' AS shard FROM users WHERE name = ?"
+            get_user_asp: "SELECT service, password FROM asps WHERE name = ?"
+  force2fa:
+    enforce_2fa: true
+    backends:
+      - backend: sql
+        params:
+          queries:
+            get_user: "SELECT email, password, totp_secret, '' AS shard FROM users WHERE name = ?"
+            get_user_asp: "SELECT service, password FROM asps WHERE name = ?"
   interactive:
     challenge_response: true
     backends:
@@ -62,8 +79,9 @@ func withTestDB(t testing.TB, schema string) (func(), string) {
 	if err != nil {
 		t.Fatalf("sql.Open: %v", err)
 	}
-	_, err = db.Exec(schema)
-	if err != nil {
+	tx, _ := db.Begin()
+	tx.Exec(schema)
+	if err := tx.Commit(); err != nil {
 		t.Fatalf("sql error: %v", err)
 	}
 	db.Close()
@@ -73,7 +91,7 @@ func withTestDB(t testing.TB, schema string) (func(), string) {
 	}, dbPath
 }
 
-func TestBackend_SQL_SimpleSchema(t *testing.T) {
+func TestAuthServer_SQL_SimpleSchema(t *testing.T) {
 	// Test a minimal database schema.
 	cleanup, dbPath := withTestDB(t, `
 CREATE TABLE users (
@@ -107,7 +125,7 @@ INSERT INTO users (email, password) VALUES (
 	}
 }
 
-func TestBackend_SQL(t *testing.T) {
+func TestAuthServer_SQL(t *testing.T) {
 	// Full schema that can run standard authentication tests.
 	cleanup, dbPath := withTestDB(t, `
 CREATE TABLE users (
@@ -136,12 +154,16 @@ CREATE TABLE asps (
 CREATE INDEX asp_idx ON asps(name);
 INSERT INTO users (name, email, totp_secret, password) VALUES (
     'testuser', 'testuser@example.com', NULL, '$s$16384$8$1$c479e8eb722f1b071efea7826ccf9c20$96d63ebed0c64afb746026f56f71b2a1f8796c73141d2d6b1958d4ea26c60a0b'), (
-    '2fauser', '2fauser@example.com', 'O32OBVS5BL5EAPB5', '$s$16384$8$1$c479e8eb722f1b071efea7826ccf9c20$96d63ebed0c64afb746026f56f71b2a1f8796c73141d2d6b1958d4ea26c60a0b');
+    '2fauser', '2fauser@example.com', 'O32OBVS5BL5EAPB5', '$s$16384$8$1$c479e8eb722f1b071efea7826ccf9c20$96d63ebed0c64afb746026f56f71b2a1f8796c73141d2d6b1958d4ea26c60a0b'), (
+    'aspuser', 'aspuser@example.com', NULL, '$s$16384$8$1$c479e8eb722f1b071efea7826ccf9c20$96d63ebed0c64afb746026f56f71b2a1f8796c73141d2d6b1958d4ea26c60a0b');
 INSERT INTO group_membership (name, group_name) VALUES (
     'testuser', 'group1'), (
     '2fauser', 'group2');
 INSERT INTO u2f_registrations (name, key_handle, public_key) VALUES (
     '2fauser', X'25ca255c0e8a6a88a13bc56ec52ba0b424f98f287eea516e5972e41403def2cf6ab33c5332f0c0b499fc826620f6e18efa49a381aa7587496572196aaa30a92b', X'0498ee4565cd348031cf36ee3549b63b5ea23b5e7ea6f297e7cccaeba99983d185110fb94fa6455c82d3e5c8d0be10be71308d76062fb5fa50d3ea8228048f0037');
+INSERT INTO asps (name, service, password) VALUES (
+    'aspuser', 'test', '$s$16384$8$1$5b8b0dc22ec2bc1029b06d4856a8b471$6c7257652e3982d8729b760885887b18d4a28966f9b2364041ddada8bea297db'), (
+    'aspuser', 'force2fa', '$s$16384$8$1$5b8b0dc22ec2bc1029b06d4856a8b471$6c7257652e3982d8729b760885887b18d4a28966f9b2364041ddada8bea297db');
 `)
 	defer cleanup()
 
@@ -151,5 +173,6 @@ INSERT INTO u2f_registrations (name, key_handle, public_key) VALUES (
 	})
 	defer s.Close()
 
+	runBackendTest(t, s.srv)
 	runAuthenticationTest(t, &clientAdapter{s.srv})
 }