Improve test coverage for ChangeUserPassword

Verify that we can set and update storage encryption keys correctly.

actions.go

@@ -140,7 +140,7 @@ type ChangeUserPasswordRequest struct {
 	Password string `json:"password"`
 }
 
-// Vaildate the request.
+// Validate the request.
 func (r *ChangeUserPasswordRequest) Validate(ctx context.Context, s *AccountService) error {
 	return s.passwordValidator(ctx, r.Password)
 }
@@ -156,7 +156,7 @@ func (s *AccountService) ChangeUserPassword(ctx context.Context, tx TX, req *Cha
 	return s.withRequest(ctx, req, func(ctx context.Context) error {
 		// If the user does not yet have an encryption key, generate one now.
 		if !user.HasEncryptionKeys {
-			err = s.initializeUserEncryptionKeys(ctx, tx, user, req.CurPassword)
+			err = s.initializeUserEncryptionKeys(ctx, tx, user, req.Password)
 		} else {
 			err = s.updateUserEncryptionKeys(ctx, tx, user, req.CurPassword, req.Password, UserEncryptionKeyMainID)
 		}

actions_test.go

@@ -50,6 +50,7 @@ func (b *fakeBackend) GetUserEncryptionKeys(_ context.Context, user *User) ([]*U
 
 func (b *fakeBackend) SetUserEncryptionKeys(_ context.Context, user *User, keys []*UserEncryptionKey) error {
 	b.encryptionKeys[user.Name] = keys
+	b.users[user.Name].HasEncryptionKeys = true
 	return nil
 }
 
@@ -189,19 +190,35 @@ func TestService_ChangePassword(t *testing.T) {
 	tx, _ := fb.NewTransaction()
 	svc := newAccountServiceWithSSO(fb, testConfig(), &fakeValidator{})
 
-	req := &ChangeUserPasswordRequest{
-		PrivilegedRequestBase: PrivilegedRequestBase{
-			RequestBase: RequestBase{
-				Username: "testuser",
-				SSO:      "testuser",
-			},
-			CurPassword: "cur",
-		},
-		Password: "a very good secret password",
+	testdata := []struct {
+		password    string
+		newPassword string
+		expectedOk  bool
+	}{
+		// Ordering is important as it is meant to emulate
+		// setting the password, failing to reset it, then
+		// succeeding.
+		{"password", "new_password", true},
+		{"BADPASS", "new_password_2", false},
+		{"new_password", "new_password_2", true},
 	}
-	err := svc.ChangeUserPassword(context.TODO(), tx, req)
-	if err != nil {
-		t.Fatal(err)
+	for _, td := range testdata {
+		req := &ChangeUserPasswordRequest{
+			PrivilegedRequestBase: PrivilegedRequestBase{
+				RequestBase: RequestBase{
+					Username: "testuser",
+					SSO:      "testuser",
+				},
+				CurPassword: td.password,
+			},
+			Password: td.newPassword,
+		}
+		err := svc.ChangeUserPassword(context.TODO(), tx, req)
+		if err == nil && !td.expectedOk {
+			t.Fatalf("ChangeUserPassword(old=%s new=%s) should have failed but didn't", td.password, td.newPassword)
+		} else if err != nil && td.expectedOk {
+			t.Fatalf("ChangeUserPassword(old=%s new=%s) failed: %v", td.password, td.newPassword, err)
+		}
 	}
 
 	if _, ok := fb.passwords["testuser"]; !ok {
@@ -211,3 +228,30 @@ func TestService_ChangePassword(t *testing.T) {
 		t.Errorf("no encryption keys were set")
 	}
 }
+
+// Lower level test that basically corresponds to the same operations
+// as TestService_ChangePassword above, but exercises the
+// initializeUserEncryptionKeys / updateUserEncryptionKeys code path
+// directly.
+func TestService_EncryptionKeys(t *testing.T) {
+	fb := createFakeBackend()
+	svc := newAccountServiceWithSSO(fb, testConfig(), &fakeValidator{})
+	tx, _ := fb.NewTransaction()
+	ctx := context.Background()
+
+	user, _ := svc.getUser(ctx, tx, "testuser")
+
+	if err := svc.initializeUserEncryptionKeys(ctx, tx, user, "password"); err != nil {
+		t.Fatal("init", err)
+	}
+	if err := svc.updateUserEncryptionKeys(ctx, tx, user, "BADPASS", "new_password", UserEncryptionKeyMainID); err == nil {
+		t.Fatal("update with bad password did not fail")
+	}
+	if err := svc.updateUserEncryptionKeys(ctx, tx, user, "password", "new_password", UserEncryptionKeyMainID); err != nil {
+		t.Fatal("update", err)
+	}
+
+	if n := len(fb.encryptionKeys["testuser"]); n != 1 {
+		t.Fatalf("found %d encryption keys, expected 1", n)
+	}
+}

integrationtest/integration_test.go

@@ -170,9 +170,7 @@ func TestIntegration_GetUser_Auth(t *testing.T) {
 	}
 }
 
-// Verify that authentication on GetResource works as expected:
-// - users can fetch their own data but not other users'
-// - admins can read everything.
+// Verify that a user can change their password.
 func TestIntegration_ChangeUserPassword(t *testing.T) {
 	stop, c := startService(t)
 	defer stop()
@@ -183,7 +181,11 @@ func TestIntegration_ChangeUserPassword(t *testing.T) {
 				Username: "uno@investici.org",
 				SSO:      c.ssoTicket("uno@investici.org"),
 			},
-			CurPassword: "password",
+			// Since the user in the test fixture does
+			// *not* have encryption key, and
+			// authenticateWithPassword is currently a
+			// no-op, this test will pass.
+			CurPassword: "ha ha, really not the current password",
 		},
 		Password: "new_password",
 	}, nil)
@@ -192,3 +194,39 @@ func TestIntegration_ChangeUserPassword(t *testing.T) {
 		t.Fatal("ChangePassword", err)
 	}
 }
+
+// Verify that changing the password sets user encryption keys.
+func TestIntegration_ChangeUserPassword_SetsEncryptionKeys(t *testing.T) {
+	stop, c := startService(t)
+	defer stop()
+
+	testdata := []struct {
+		password    string
+		newPassword string
+		expectedOk  bool
+	}{
+		// Ordering is important as it is meant to emulate
+		// setting the password, failing to reset it, then
+		// succeeding.
+		{"password", "new_password", true},
+		{"BADPASS", "new_password_2", false},
+		{"new_password", "new_password_2", true},
+	}
+	for _, td := range testdata {
+		err := c.request("/api/user/change_password", &accountserver.ChangeUserPasswordRequest{
+			PrivilegedRequestBase: accountserver.PrivilegedRequestBase{
+				RequestBase: accountserver.RequestBase{
+					Username: "uno@investici.org",
+					SSO:      c.ssoTicket("uno@investici.org"),
+				},
+				CurPassword: td.password,
+			},
+			Password: td.newPassword,
+		}, nil)
+		if err == nil && !td.expectedOk {
+			t.Fatalf("ChangeUserPassword(old=%s new=%s) should have failed but didn't", td.password, td.newPassword)
+		} else if err != nil && td.expectedOk {
+			t.Fatalf("ChangeUserPassword(old=%s new=%s) failed: %v", td.password, td.newPassword, err)
+		}
+	}
+}