[CORE-15278] Add `password_set_at` field to the `ScramCredential` proto message (Security Admin API V2) by nguyen-andrew · Pull Request #29328 · redpanda-data/redpanda

nguyen-andrew · 2026-01-20T10:44:50Z

This change extends the internal scram_credential structure and admin API v2 to track when SCRAM passwords were last set. Existing credentials without timestamps are supported and will return the Unix epoch as the password_set_at when queried.

The admin API v2 /v1/security/scram_credentials endpoints now include a read-only password_set_at timestamp field in ScramCredential responses:

CreateScramCredential - returns credential with password_set_at set to creation time
UpdateScramCredential - returns credential with password_set_at updated to modification time
GetScramCredential - returns credential with password_set_at (or the Unix epoch for old credentials)
ListScramCredentials - returns credentials with password_set_at for each entry

The v1 admin API is unchanged and does not expose this field.

Resolves CORE-15278

Backports Required

Release Notes

Improvements

SCRAM credentials now track password_set_at timestamps, exposed via admin API v2. This enables clients like the Kubernetes operator to verify credential state changes have propagated and perform accurate reconciliation.

Copilot

Pull request overview

This PR adds a password_set_at timestamp field to track when SCRAM credentials were created or last modified. The field is exposed through the admin API v2 as a read-only property, enabling clients to verify credential state changes and perform accurate reconciliation.

Changes:

Extended the internal scram_credential structure to include an optional password_set_at timestamp field
Updated admin API v2 proto definitions and generated Python bindings to expose the timestamp field
Modified credential creation and conversion logic to populate timestamps (current time for new credentials, InfinitePast for legacy credentials without timestamps)
Added comprehensive test coverage for timestamp behavior across creation, updates, propagation, and persistence

Reviewed changes

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

Show a summary per file

File	Description
proto/redpanda/core/admin/v2/security.proto	Added `password_set_at` timestamp field to ScramCredential message
tests/rptest/clients/admin/proto/redpanda/core/admin/v2/security_pb2.py	Updated generated Python protobuf code with new timestamp field
tests/rptest/clients/admin/proto/redpanda/core/admin/v2/security_pb2.pyi	Updated Python type stubs for new timestamp field
src/v/security/scram_credential.h	Extended scram_credential class with optional password_set_at field and bumped serde version
src/v/security/scram_algorithm.h	Modified make_credentials to populate password_set_at with current timestamp
src/v/redpanda/admin/services/security.cc	Updated conversion function to set password_set_at from credential or InfinitePast for legacy credentials
tests/rptest/tests/scram_test.py	Added comprehensive test for password_set_at behavior including creation, updates, propagation, and persistence
src/v/security/tests/scram_algorithm_test.cc	Added test verifying make_credentials populates timestamp
src/v/security/tests/credential_store_test.cc	Added test for credential store handling of timestamps
src/v/redpanda/admin/services/tests/security_test.cc	Added tests verifying protobuf conversion handles timestamps correctly
src/v/security/tests/BUILD	Added model dependency for timestamp support in tests

Copilot · 2026-01-20T10:46:02Z

+        # Sleep to ensure timestamp difference (at least 1 second)
+        time.sleep(1)


Hardcoded 1-second sleep can make tests unnecessarily slow and brittle. Consider using a minimal sleep (e.g., 0.01 seconds) combined with explicit timestamp capture before/after the update operation, or mock the timestamp for deterministic testing.

Suggested change

# Sleep to ensure timestamp difference (at least 1 second)

time.sleep(1)

# Minimal sleep to allow timestamp to advance on coarse-grained clocks

time.sleep(0.01)

Copilot · 2026-01-20T10:46:02Z

+      password, security::scram_sha256::min_iterations);
+
+    // Clear the password_set_at to simulate a credential without a timestamp
+    auto& password_set_at = std::get<4>(security_cred.serde_fields());


Using a magic number index (4) to access tuple elements is fragile and will break if the order of fields in serde_fields() changes. Consider using structured bindings with descriptive names or adding a named accessor method to scram_credential for setting password_set_at.

Suggested change

auto& password_set_at = std::get<4>(security_cred.serde_fields());

auto& [salt, server_key, stored_key, iterations, password_set_at]

= security_cred.serde_fields();

agree with bot here

nguyen-andrew · 2026-01-20T17:20:33Z

Force push to fix unit test affected by the addition of the password_set_at.

rockwotj · 2026-01-20T12:45:45Z

+    pb_cred.set_password_set_at(
+      cred.password_set_at().has_value()
+        ? absl::FromChrono(model::to_time_point(cred.password_set_at().value()))
+        : absl::InfinitePast());


I don't think you want infinite past, and this likely causes an error during serialization. You want absl::UnixEpoch

rockwotj · 2026-01-20T17:41:35Z

+        assert updated_at >= before_update, (
+            f"password_set_at {updated_at} should be >= {before_update}"
+        )
+
+        assert updated_at > created_at, (
+            f"password_set_at should be updated: {updated_at} < {created_at}"
+        )


need to account for clock skew in CDT. Maybe let's just ensure it's within an hour or something of now?

michael-redpanda · 2026-01-20T17:22:52Z

+    // Records when the password was last set
+    std::optional<model::timestamp> _password_set_at;


I'm not sure we need a std::optional here, instead I think we can depend on the value being timestamp::missing() if it's not provided.

michael-redpanda · 2026-01-20T17:23:59Z

+    BOOST_REQUIRE_EQUAL(r1->password_set_at().has_value(), true);
+    BOOST_REQUIRE_EQUAL(r1->password_set_at().value(), now);


nit: YOu can probably just do the value comparison without having to check that it has a value.

michael-redpanda · 2026-01-20T17:26:00Z

+    // Copy the server-generated password_set_at timestamp to the expected
+    // credential, since this field is set to the current time at creation and
+    // cannot be predetermined in the test
+    std::get<4>(creds_256.serde_fields()) = cred->password_set_at();


The std::get<4> seems unstable to me... maybe expose a setter method?

Also I wonder if timestamp needs to be part of the comparison at all... I think we do do other cred comparison and I feel comparing the timestamp is unnecessary (especially say when we update/create a user).

michael-redpanda · 2026-01-20T17:30:42Z

+      password, security::scram_sha256::min_iterations);
+
+    // Clear the password_set_at to simulate a credential without a timestamp
+    auto& password_set_at = std::get<4>(security_cred.serde_fields());


agree with bot here

michael-redpanda · 2026-01-20T17:31:24Z

+    auto security_cred = security::scram_sha256::make_credentials(
+      password, security::scram_sha256::min_iterations);


I see what you're testing here - maybe add an optional parameter to make_credentials to take in a timestamp and if provided, use it

Oh good point, that'd would make things easier

vbotbuildovich · 2026-01-20T20:06:59Z

CI test results

test results on build#79335

test_class	test_method	test_arguments	test_kind	job_url	test_status	passed	reason	test_history
DataMigrationsApiTest	test_creating_and_listing_migrations	null	integration	https://buildkite.com/redpanda/redpanda/builds/79335#019bdcb0-90dd-4aed-b861-dad5ad2a1411	FLAKY	10/11	Test PASSES after retries.No significant increase in flaky rate(baseline=0.0000, p0=1.0000, reject_threshold=0.0100. adj_baseline=0.1000, p1=0.3487, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=DataMigrationsApiTest&test_method=test_creating_and_listing_migrations
SimpleEndToEndTest	test_relaxed_acks	{"write_caching": false}	integration	https://buildkite.com/redpanda/redpanda/builds/79335#019bdcb0-90e6-42d9-819d-61c8695d0a52	FLAKY	10/11	Test PASSES after retries.No significant increase in flaky rate(baseline=0.0131, p0=1.0000, reject_threshold=0.0100. adj_baseline=0.1000, p1=0.3487, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=SimpleEndToEndTest&test_method=test_relaxed_acks

test results on build#79419

test_class	test_method	test_arguments	test_kind	job_url	test_status	passed	reason	test_history
ScalingUpTest	test_fast_node_addition	null	integration	https://buildkite.com/redpanda/redpanda/builds/79419#019be198-7298-4881-8868-14aea3cca4e7	FLAKY	10/11	Test PASSES after retries.No significant increase in flaky rate(baseline=0.0279, p0=1.0000, reject_threshold=0.0100. adj_baseline=0.1000, p1=0.3487, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=ScalingUpTest&test_method=test_fast_node_addition

Add a helper method to check if a timestamp has the missing sentinel value (-1). This provides a cleaner API than comparing against timestamp::missing().value() directly. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Add optional password_set_at timestamp field to track when SCRAM credentials were last updated. This bumps the serde version from 0 to 1 while maintaining backward compatibility with compat_version 0.

Add unit tests to verify password_set_at field behavior in credential store, including credentials without timestamp (backward compatibility), credentials with timestamp, and timestamp updates. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Set password_set_at to the current timestamp when generating new SCRAM credentials to track when passwords are created.

Add assertions to verify that password_set_at is set when SCRAM credentials are created via the Kafka API.

Add test to verify that make_credentials properly sets the password_set_at timestamp field when creating SCRAM credentials. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Expose password_set_at timestamp in the admin API v2 ScramCredential message as an output-only field to allow clients to query when passwords were last updated.

Convert and set the password_set_at timestamp when converting internal scram_credential to protobuf ScramCredential for admin API v2 responses.

Add test coverage for the password_set_at field in convert_to_pb_scram_credential: - Verify that credentials created via make_credentials have password_set_at populated - Verify that credentials without timestamps (for backwards compatibility) correctly have no password_set_at field when converted to protobuf Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Regenerate Python protobuf bindings to include the new password_set_at field in ScramCredential message.

Add comprehensive integration test for password_set_at timestamp field in the v2 SCRAM credential API. The test verifies: - password_set_at is set when creating credentials - password_set_at is updated when passwords change - Updated timestamps are strictly greater than original timestamps - Credentials propagate with timestamps correctly across all nodes - Timestamps persist correctly across cluster restarts Also adds `check_credential_on_all_nodes()` helper method to verify credential propagation with optional `password_set_at` validation.

nguyen-andrew · 2026-01-21T16:45:14Z

Force push to address PR comments:

Adding timestamp::is_missing()
Update scram_credential to remove std::optional from _password_set_at and rely on timestamp::missing()
Update scram_credential equality operator to exclude password_set_at.
Update security::scram_algorithm::make_credentials overloads to take a password_set_at parameter with a default value of model::timestamp::now(). This'll allow existing code using make_credentials to create credentials with timestamps automatically, but it also provides flexibility for callers (namely unit test code) to provide a specific timestamp as well.
Update security Admin API v2 endpoint to use absl::UnixEpoch() if a scram credential's password_set_at is missing (instead of InfinitePast). Update proto comments accordingly.
Updating tests to reflect the above changes. Updated test_scram_password_set_at_v2 ducktape test to remove potential flakiness due to possible clock skew.

Copilot

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Copilot · 2026-01-21T18:15:08Z

+    bool operator==(const scram_credential& other) const {
+        return _salt == other._salt && _server_key == other._server_key
+               && _stored_key == other._stored_key
+               && _iterations == other._iterations
+               && _principal == other._principal;
+    }


The comment states that equality excludes password_set_at because it's "metadata rather than part of the credential's identity." However, this design choice may not be obvious to future maintainers and could lead to subtle bugs. Consider expanding the documentation to explain the specific use cases where this behavior is desired (e.g., comparing credentials for authentication purposes) and explicitly warning about scenarios where timestamp comparison might be needed separately.

Copilot · 2026-01-21T18:15:08Z

+        return std::tie(
+          _salt, _server_key, _stored_key, _iterations, _password_set_at);


The serde_fields() method includes _password_set_at for serialization, but _principal is explicitly excluded (as noted in the comment on line 78). This asymmetry between serialized fields and equality comparison could be confusing. Consider adding a comment explaining why _principal is excluded from serialization but included in equality, while _password_set_at is included in serialization but excluded from equality.

Copilot · 2026-01-21T18:15:08Z

+    pb_cred.set_password_set_at(
+      cred.password_set_at().is_missing()
+        ? absl::UnixEpoch()
+        : absl::FromChrono(model::to_time_point(cred.password_set_at())));


The conversion logic maps missing timestamps to Unix epoch, which is consistent with the proto documentation. However, the choice of Unix epoch as the sentinel value for "unknown" is not documented here in the C++ code. Consider adding a brief comment explaining why Unix epoch was chosen over other options (e.g., setting the field to unset/null in protobuf) to help future maintainers understand this design decision.

rockwotj

LGTM, will let Mike do the final signoff

nguyen-andrew requested a review from a team January 20, 2026 10:44

nguyen-andrew self-assigned this Jan 20, 2026

nguyen-andrew requested review from a team and michael-redpanda and removed request for a team January 20, 2026 10:44

nguyen-andrew requested review from a team and rockwotj as code owners January 20, 2026 10:44

Copilot AI review requested due to automatic review settings January 20, 2026 10:44

github-actions Bot added area/build area/redpanda labels Jan 20, 2026

Copilot AI reviewed Jan 20, 2026

View reviewed changes

nguyen-andrew force-pushed the wip/gbac/scram-password-set-at branch from 290b049 to fe4cb01 Compare January 20, 2026 17:18

rockwotj reviewed Jan 20, 2026

View reviewed changes

michael-redpanda reviewed Jan 20, 2026

View reviewed changes

nguyen-andrew force-pushed the wip/gbac/scram-password-set-at branch 2 times, most recently from a7c5553 to ed7196d Compare January 20, 2026 18:24

nguyen-andrew marked this pull request as draft January 21, 2026 14:16

nguyen-andrew and others added 11 commits January 21, 2026 14:33

model: add is_missing() helper to timestamp

bb824b3

Add a helper method to check if a timestamp has the missing sentinel value (-1). This provides a cleaner API than comparing against timestamp::missing().value() directly. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

security: add password_set_at field to scram_credential

61929d1

Add optional password_set_at timestamp field to track when SCRAM credentials were last updated. This bumps the serde version from 0 to 1 while maintaining backward compatibility with compat_version 0.

security: populate password_set_at when generating SCRAM credentials

4ffeab5

Set password_set_at to the current timestamp when generating new SCRAM credentials to track when passwords are created.

kafka/tests: verify password_set_at is populated in SCRAM credentials

ab8be1c

Add assertions to verify that password_set_at is set when SCRAM credentials are created via the Kafka API.

security/tests: add test for make_credentials timestamp population

7ded1a6

Add test to verify that make_credentials properly sets the password_set_at timestamp field when creating SCRAM credentials. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

admin/v2: add password_set_at field to ScramCredential proto

dd972ff

Expose password_set_at timestamp in the admin API v2 ScramCredential message as an output-only field to allow clients to query when passwords were last updated.

admin/v2: populate password_set_at in ScramCredential conversion

33d9c07

Convert and set the password_set_at timestamp when converting internal scram_credential to protobuf ScramCredential for admin API v2 responses.

dt/pb: regenerate Python bindings for ScramCredential

08d8f30

Regenerate Python protobuf bindings to include the new password_set_at field in ScramCredential message.

nguyen-andrew force-pushed the wip/gbac/scram-password-set-at branch from ed7196d to eb6a474 Compare January 21, 2026 16:26

nguyen-andrew marked this pull request as ready for review January 21, 2026 16:48

nguyen-andrew requested a review from Copilot January 21, 2026 18:14

Copilot AI reviewed Jan 21, 2026

View reviewed changes

rockwotj reviewed Jan 21, 2026

View reviewed changes

nguyen-andrew requested a review from michael-redpanda January 22, 2026 03:18

michael-redpanda approved these changes Jan 23, 2026

View reviewed changes

michael-redpanda merged commit eb74e4a into redpanda-data:dev Jan 23, 2026
31 checks passed

nguyen-andrew deleted the wip/gbac/scram-password-set-at branch January 23, 2026 17:35

		# Sleep to ensure timestamp difference (at least 1 second)
		time.sleep(1)

	auto& password_set_at = std::get<4>(security_cred.serde_fields());
	auto& [salt, server_key, stored_key, iterations, password_set_at]
	= security_cred.serde_fields();

		// Records when the password was last set
		std::optional<model::timestamp> _password_set_at;

		BOOST_REQUIRE_EQUAL(r1->password_set_at().has_value(), true);
		BOOST_REQUIRE_EQUAL(r1->password_set_at().value(), now);

		auto security_cred = security::scram_sha256::make_credentials(
		password, security::scram_sha256::min_iterations);

		return std::tie(
		_salt, _server_key, _stored_key, _iterations, _password_set_at);

Conversation

nguyen-andrew commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Backports Required

Release Notes

Improvements

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nguyen-andrew commented Jan 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbotbuildovich commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI test results

Uh oh!

nguyen-andrew commented Jan 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

rockwotj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nguyen-andrew commented Jan 20, 2026 •

edited

Loading

vbotbuildovich commented Jan 20, 2026 •

edited

Loading