ING-2050: Ensure user passwords

[Westwooo]

Summary

Adds the ability for EnsureUser to confirm a changes to existing user settings (password, roles or groups) have propagated to all nodes.

Design notes

• EnsureUser accepts WantSettings that describe the desired groups and roles, as well as PasswordChanged a time that the User's password must have changed at or after for the EnsureUser call to return.
• THe initial design introduced a PasswordChangedMarker an opaque type that would prevent users passing any time into EnsureUser. After consideration the extra complexity was not worth the protection of giving an arbitrary time.

[Copilot]

Pull request overview

This PR extends user-management propagation checks by enabling EnsureUser to wait for a password change to propagate to all nodes, using the server-provided password_change_date as an opaque marker captured via UpsertUser.

Changes:

• Change UpsertUser to return an UpsertUserResult containing a baseline PreviousPasswordChanged marker when a password is updated.
• Add PasswordChangedMarker and plumb an optional SincePasswordChanged marker through EnsureUser/EnsureUserHelper to wait for propagation.
• Add a dino-based integration test covering ensure semantics for create, password change, and delete.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
mgmtcomponent.go	Updates component UpsertUser signature and wires SincePasswordChanged into EnsureUser.
agent_ops.go	Updates Agent.UpsertUser signature to return UpsertUserResult.
cbmgmtx/mgmt.go	Implements pre-write probe for password_change_date, returns UpsertUserResult, and swaps UserJson.PasswordChanged to the marker type.
cbmgmtx/passwordchangedmarker.go	Introduces the opaque PasswordChangedMarker type and comparison logic.
cbmgmtx/ensureuserhelper.go	Adds optional propagation waiting by comparing password_change_date markers during polling.
cbmgmtx/ensureuserhelper_int_test.go	Adds a dino integration test for EnsureUser propagation behavior.
cbmgmtx/mgmt_int_test.go	Updates existing integration tests for new UpsertUser return signature.
cbauthx/cbauth_int_test.go	Updates integration tests for new UpsertUser return signature.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Suppressed comments (2)

cbmgmtx/passwordchangedmarker.go:56

• NewPasswordChangedMarkerForTest is an exported, production-build symbol even though it is described as test-only, and it is currently unused in the repo. This weakens the stated goal of keeping PasswordChangedMarker opaque (production callers can construct arbitrary markers) and adds surface area to the public API.

// NewPasswordChangedMarkerForTest constructs a PasswordChangedMarker from an
// arbitrary time.Time. This exists only to support testing code that needs to
// fabricate markers, and must not be used to construct a marker from a
// caller's local clock in production code paths (use a value obtained from
// GetUser or UpsertUser instead).
func NewPasswordChangedMarkerForTest(t time.Time) PasswordChangedMarker {
	return PasswordChangedMarker{t: t}
}

cbmgmtx/ensureuserhelper_int_test.go:162

• The test creates a user group but never deletes it. Unlike the other dino integration tests (bucket/manifest/index) which clean up resources, this can leave persistent RBAC groups behind on the cluster across runs and slowly pollute the test environment.

	err := mgmt.UpsertUserGroup(ctx, &cbmgmtx.UpsertUserGroupOptions{
		GroupName: testGroupName,
		Roles:     []string{"ro_admin"},
	})
	require.NoError(t, err)

[Westwooo]

@droid review

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

cbmgmtx/mgmt.go:1387

• The comment on PreviousPasswordChanged overstates the guarantee: the marker is captured by a separate GetUser request before the PUT, so it is not necessarily the value from “immediately before this call took effect” under concurrent updates. Rewording avoids implying a stronger ordering guarantee than the API can provide.

	// PreviousPasswordChanged holds the user's PasswordChangedMarker from
	// immediately before this call took effect.
	PreviousPasswordChanged *PasswordChangedMarker

cbmgmtx/ensureuserhelper_int_test.go:163

• This integration test creates a user group (testGroupName) but never deletes it, which can pollute a shared dino cluster and make repeated runs noisier over time. Add cleanup to remove the group after the test completes.

	testGroupName := "testgroup-" + uuid.NewString()[:6]
	err := mgmt.UpsertUserGroup(ctx, &cbmgmtx.UpsertUserGroupOptions{
		GroupName: testGroupName,
		Roles:     []string{"ro_admin"},
	})

[chvck]

It seems odd that we're string matching these? Would we be better translating the RoleJson and RoleWithOriginsJson into a proper type?

[Westwooo]

Good point, if we got a malformed role then the string match would fail and we'd poll forever. I changed it so that we decode the string array into the appropriate JSON and compare that.