Improve test coverage for account management actions

Add enable/disable OTP tests, and a delete ASP test.

actions.go

@@ -251,7 +251,10 @@ func randomAppSpecificPasswordID() string {
 }
 
 func generateTOTPSecret() (string, error) {
-	key, err := totp.Generate(totp.GenerateOpts{})
+	key, err := totp.Generate(totp.GenerateOpts{
+		Issuer:      "accountserver",
+		AccountName: "placeholder",
+	})
 	if err != nil {
 		return "", err
 	}

actions_user.go

@@ -346,7 +346,7 @@ func (r *EnableOTPRequest) Sanitize() {
 func (r *EnableOTPRequest) Validate(_ *RequestContext) error {
 	var err *ValidationError
 	// Only check if the client-side secret is set, skip otherwise.
-	if r.TOTPSecret == "" && len(r.TOTPSecret) != 16 {
+	if r.TOTPSecret != "" && len(r.TOTPSecret) != 16 {
 		err = newValidationError(err, "totp_secret", "bad value")
 	}
 	return err.orNil()

backend/ldap/model_test.go

@@ -22,23 +22,28 @@ const (
 	testBaseDN = "dc=example,dc=com"
 )
 
-func startServerAndGetUser(t testing.TB) (*ldaptest.TestLDAPServer, as.Backend, *as.RawUser) {
+func startServerAndGetUser(t *testing.T) (*ldaptest.TestLDAPServer, as.Backend, *as.RawUser) {
 	return startServerAndGetUserWithName(t, testUser1)
 }
 
-func startServerAndGetUser2(t testing.TB) (*ldaptest.TestLDAPServer, as.Backend, *as.RawUser) {
+func startServerAndGetUser2(t *testing.T) (*ldaptest.TestLDAPServer, as.Backend, *as.RawUser) {
 	return startServerAndGetUserWithName(t, testUser2)
 }
 
-func startServerAndGetUser3(t testing.TB) (*ldaptest.TestLDAPServer, as.Backend, *as.RawUser) {
+func startServerAndGetUser3(t *testing.T) (*ldaptest.TestLDAPServer, as.Backend, *as.RawUser) {
 	return startServerAndGetUserWithName(t, testUser3)
 }
 
-func startServerAndGetUser4(t testing.TB) (*ldaptest.TestLDAPServer, as.Backend, *as.RawUser) {
+func startServerAndGetUser4(t *testing.T) (*ldaptest.TestLDAPServer, as.Backend, *as.RawUser) {
 	return startServerAndGetUserWithName(t, testUser4)
 }
 
-func startServer(t testing.TB) (*ldaptest.TestLDAPServer, as.Backend) {
+func startServer(t *testing.T) (*ldaptest.TestLDAPServer, as.Backend) {
+	// Tell the test runtime that we can run multiple integration tests in
+	// parallel. This just happens to be a convenient call site for all
+	// integration tests.
+	t.Parallel()
+
 	srv := ldaptest.StartServer(t, &ldaptest.Config{
 		Dir:  "../../ldaptest",
 		Base: "dc=example,dc=com",
@@ -59,7 +64,7 @@ func startServer(t testing.TB) (*ldaptest.TestLDAPServer, as.Backend) {
 	return srv, b
 }
 
-func startServerAndGetUserWithName(t testing.TB, username string) (*ldaptest.TestLDAPServer, as.Backend, *as.RawUser) {
+func startServerAndGetUserWithName(t *testing.T, username string) (*ldaptest.TestLDAPServer, as.Backend, *as.RawUser) {
 	srv, b := startServer(t)
 
 	tx, _ := b.NewTransaction()
@@ -226,6 +231,8 @@ func TestModel_SearchUser(t *testing.T) {
 }
 
 func TestModel_SetResourceStatus(t *testing.T) {
+	t.Parallel()
+
 	srv := ldaptest.StartServer(t, &ldaptest.Config{
 		Dir:   "../../ldaptest",
 		Base:  "dc=example,dc=com",
@@ -258,6 +265,8 @@ func TestModel_SetResourceStatus(t *testing.T) {
 }
 
 func TestModel_HasAnyResource(t *testing.T) {
+	t.Parallel()
+
 	srv := ldaptest.StartServer(t, &ldaptest.Config{
 		Dir:   "../../ldaptest",
 		Base:  "dc=example,dc=com",
@@ -422,6 +431,8 @@ func TestModel_NextUID(t *testing.T) {
 }
 
 func TestSortResources(t *testing.T) {
+	t.Parallel()
+
 	rsrcs := []*as.Resource{
 		&as.Resource{
 			ID:       "id1",
@@ -451,6 +462,8 @@ func TestSortResources(t *testing.T) {
 }
 
 func TestSortResources_ExternalParentID(t *testing.T) {
+	t.Parallel()
+
 	rsrcs := []*as.Resource{
 		&as.Resource{
 			ID:       "id1",

integrationtest/account_mgmt_test.go

@@ -218,7 +218,7 @@ func runAccountRecoveryTest(t *testing.T, username string, enableCache, enableOp
 	return checkUserInvariants(t, be, username, newPw)
 }
 
-func TestIntegration_AppSpecificPassword(t *testing.T) {
+func TestIntegration_AppSpecificPassword_Create(t *testing.T) {
 	stop, _, c := startService(t)
 	defer stop()
 
@@ -265,3 +265,166 @@ func TestIntegration_AppSpecificPassword(t *testing.T) {
 		t.Errorf("could not find the ASPs that was just created: %+v", user)
 	}
 }
+
+func TestIntegration_AppSpecificPassword_Delete(t *testing.T) {
+	stop, _, c := startService(t)
+	defer stop()
+
+	username := "tre@investici.org"
+
+	err := c.request("/api/user/delete_app_specific_password", &as.DeleteApplicationSpecificPasswordRequest{
+		UserRequestBase: as.UserRequestBase{
+			RequestBase: as.RequestBase{
+				SSO: c.ssoTicket(username),
+			},
+			Username: username,
+		},
+		AspID: "id1",
+	}, nil)
+	if err != nil {
+		t.Fatalf("DeleteApplicationSpecificPassword failed: %v", err)
+	}
+
+	var user as.User
+	err = c.request("/api/user/get", &as.GetUserRequest{
+		UserRequestBase: as.UserRequestBase{
+			RequestBase: as.RequestBase{
+				SSO: c.ssoTicket(testAdminUser, testAdminGroup),
+			},
+			Username: username,
+		},
+	}, &user)
+	if err != nil {
+		t.Fatalf("GetUser error: %v", err)
+	}
+	found := false
+	for _, asp := range user.AppSpecificPasswords {
+		if asp.ID == "id1" {
+			found = true
+			break
+		}
+	}
+	if found {
+		t.Error("the ASPs was not deleted")
+	}
+}
+
+func TestIntegration_EnableOTP_WithoutSecret(t *testing.T) {
+	stop, _, c := startService(t)
+	defer stop()
+
+	username := "due@investici.org"
+
+	// Test without providing a secret.
+	var resp as.EnableOTPResponse
+	err := c.request("/api/user/enable_otp", &as.EnableOTPRequest{
+		UserRequestBase: as.UserRequestBase{
+			RequestBase: as.RequestBase{
+				SSO: c.ssoTicket(username),
+			},
+			Username: username,
+		},
+	}, &resp)
+	if err != nil {
+		t.Fatalf("EnableOTP failed: %v", err)
+	}
+	if resp.TOTPSecret == "" {
+		t.Errorf("TOTP secret unset in response")
+	}
+
+	var user as.User
+	err = c.request("/api/user/get", &as.GetUserRequest{
+		UserRequestBase: as.UserRequestBase{
+			RequestBase: as.RequestBase{
+				SSO: c.ssoTicket(testAdminUser, testAdminGroup),
+			},
+			Username: username,
+		},
+	}, &user)
+	if err != nil {
+		t.Fatalf("GetUser error: %v", err)
+	}
+	if !user.Has2FA {
+		t.Errorf("user still has Has2FA=false")
+	}
+}
+
+func TestIntegration_EnableOTP_WithSecret(t *testing.T) {
+	stop, _, c := startService(t)
+	defer stop()
+
+	username := "due@investici.org"
+
+	// Test without providing a secret.
+	secret := "ABCDEF0123456789"
+	var resp as.EnableOTPResponse
+	err := c.request("/api/user/enable_otp", &as.EnableOTPRequest{
+		UserRequestBase: as.UserRequestBase{
+			RequestBase: as.RequestBase{
+				SSO: c.ssoTicket(username),
+			},
+			Username: username,
+		},
+		TOTPSecret: secret,
+	}, &resp)
+	if err != nil {
+		t.Fatalf("EnableOTP failed: %v", err)
+	}
+	if resp.TOTPSecret != secret {
+		t.Errorf("wrong TOTP secret returned in response")
+	}
+
+	var user as.User
+	err = c.request("/api/user/get", &as.GetUserRequest{
+		UserRequestBase: as.UserRequestBase{
+			RequestBase: as.RequestBase{
+				SSO: c.ssoTicket(testAdminUser, testAdminGroup),
+			},
+			Username: username,
+		},
+	}, &user)
+	if err != nil {
+		t.Fatalf("GetUser error: %v", err)
+	}
+	if !user.Has2FA {
+		t.Errorf("user still has Has2FA=false")
+	}
+}
+
+func TestIntegration_DisableOTP(t *testing.T) {
+	stop, _, c := startService(t)
+	defer stop()
+
+	username := "tre@investici.org"
+
+	err := c.request("/api/user/disable_otp", &as.DisableOTPRequest{
+		UserRequestBase: as.UserRequestBase{
+			RequestBase: as.RequestBase{
+				SSO: c.ssoTicket(username),
+			},
+			Username: username,
+		},
+	}, nil)
+	if err != nil {
+		t.Fatalf("DisableOTP failed: %v", err)
+	}
+
+	var user as.User
+	err = c.request("/api/user/get", &as.GetUserRequest{
+		UserRequestBase: as.UserRequestBase{
+			RequestBase: as.RequestBase{
+				SSO: c.ssoTicket(testAdminUser, testAdminGroup),
+			},
+			Username: username,
+		},
+	}, &user)
+	if err != nil {
+		t.Fatalf("GetUser error: %v", err)
+	}
+	if n := len(user.AppSpecificPasswords); n > 0 {
+		t.Errorf("user still has app-specific passwords (%d)", n)
+	}
+	if user.Has2FA {
+		t.Errorf("user still has Has2FA=true")
+	}
+}

integrationtest/integration_test.go

@@ -143,7 +143,12 @@ func (c *testClient) request(uri string, req, out interface{}) error {
 	return json.Unmarshal(data, out)
 }
 
-func startServiceWithConfigAndCache(t testing.TB, svcConfig as.Config, enableCache bool) (func(), as.Backend, *testClient) {
+func startServiceWithConfigAndCache(t *testing.T, svcConfig as.Config, enableCache bool) (func(), as.Backend, *testClient) {
+	// Tell the test runtime that we can run multiple integration tests in
+	// parallel. This just happens to be a convenient call site for all
+	// integration tests.
+	t.Parallel()
+
 	ldapsrv := ldaptest.StartServer(t, &ldaptest.Config{
 		Dir:  "../ldaptest",
 		Base: "dc=example,dc=com",
@@ -248,11 +253,11 @@ webauthn:
 	}, be, c
 }
 
-func startServiceWithConfig(t testing.TB, svcConfig as.Config) (func(), as.Backend, *testClient) {
+func startServiceWithConfig(t *testing.T, svcConfig as.Config) (func(), as.Backend, *testClient) {
 	return startServiceWithConfigAndCache(t, svcConfig, false)
 }
 
-func startService(t testing.TB) (func(), as.Backend, *testClient) {
+func startService(t *testing.T) (func(), as.Backend, *testClient) {
 	return startServiceWithConfig(t, as.Config{})
 }
 

integrationtest/testdata/test4.ldif

@@ -21,7 +21,8 @@ uidNumber: 23801
 userPassword:: JGEyJDQkMzI3NjgkMSQwZDgyMzU1YjQ0Mzg0M2NmZDY4MjU1MzE4ZTVjYTdiZSRmNTQ0ODkxOTFiNWZlYzk2MDRlNWQ2ODZjMDQxZjJkNTFmOTgxOGY4ZTFmM2E4MDYzY2U3ZTEwMTE3OTc2OGI0
 u2fRegistration:: BLbpGj8p0EaIWWbA6DiG4bQSxDPGW6J5U1ZV4C5Al2MIIrDIdMos5yqqvWZGCgl0zn0DgjvILPX5Wqy1uMlRrrbuJtcRvBQ9DEZZJmMP5CJAJqdKLG07kezOPeLQRNTjhKnW0Zixqzc8jIlqMX/+no675UeHYXr7VSmKALYekyVk
 u2fRegistration:: BCCBvjcPNk4xn7Vi2YbJA8alBwIL7pkIkmtdZJwZ9Bcz4EzyE9As/9x43WwvNzaFHvqiB34hncw6IHq/SQrAq/XpdfSnqSm9tYskcbgWcNwsrXhpjTu9Pi9UyWNZtEG4nFGGFRmuNNpjA5C/P2A9V/DIat17nWE4hndFupMU2kVG
-totpSecret: ABCDEF
+appSpecificPassword: id1:email:encryptedpassword:comment
+appSpecificPassword: id2:jabber:encryptedpassword:comment
 host: host2
 originalHost: host2
 status: active

integrationtest/update_user_test.go

@@ -96,6 +96,37 @@ func TestIntegration_UpdateUser_U2F(t *testing.T) {
 	}
 }
 
+func TestIntegration_UpdateUser_DisableU2F(t *testing.T) {
+	stop, _, c := startService(t)
+	defer stop()
+
+	// Removing all hardware tokens should clear 2FA for this user.
+	err := c.request("/api/user/update", &as.UpdateUserRequest{
+		UserRequestBase: as.UserRequestBase{
+			RequestBase: as.RequestBase{
+				SSO: c.ssoTicket("quattro@investici.org"),
+			},
+			Username: "quattro@investici.org",
+		},
+		SetU2FRegistrations: true,
+		U2FRegistrations:    nil,
+	}, nil)
+	if err != nil {
+		t.Fatalf("UpdateUser(): %v", err)
+	}
+
+	user, n := countRegistrations(t, c)
+	if n != 0 {
+		t.Fatalf("didn't update U2F registrations correctly, expected 0 found %d", n)
+	}
+	if n := len(user.AppSpecificPasswords); n > 0 {
+		t.Errorf("user still has app-specific passwords (%d)", n)
+	}
+	if user.Has2FA {
+		t.Errorf("user still has Has2FA=true")
+	}
+}
+
 func registrationEqual(a, b *ct.U2FRegistration) bool {
 	return (bytes.Equal(a.PublicKey, b.PublicKey) &&
 		bytes.Equal(a.KeyHandle, b.KeyHandle) &&