feat(cognito-idp): adding create/list/delete user pool client secret cognito-idp actions

[danielabeledo]

Summary

Closes #344

Introduces 3 new actions for the Cognito IDP service, the 3 of them related to a new capability in Cognito that allows to have more than one Client Secret to allow no-downtime rotations.

Type of change

• Bug fix (fix:)
• New feature (feat:)
• Breaking change (feat!: or fix!:)
• Docs / chore

AWS Compatibility

• aws-cli/2.34.28

Checklist

• ./mvnw test passes locally (1723/1723)
• New or updated integration test added
• Commit messages follow Conventional Commits

[hampsterx]

Nice addition. A few things to address before merge.

Blockers

1. Trailing space in error code — CognitoService.java:151

throw new AwsException("LimitExceededException ", ...);

AWS SDKs match error codes by exact string equality. The trailing space means boto3/aws-cli/the Java SDK won't recognise this as LimitExceededException and won't retry or handle it correctly.

2. deleteUserPoolClientSecret leaves client.clientSecret out of sync — CognitoService.java:189-193

if (userPoolClientSecret.getClientSecretValue().equals(client.getClientSecret())) {
    client.setClientSecret(null);
}
client.getUserPoolClientSecrets().remove(userPoolClientSecret);

If a client has two secrets [A, B] and A matches client.clientSecret, deleting A nulls the legacy field even though B is still valid. Downstream clientToNode() / DescribeUserPoolClient / any token flow reading client.getClientSecret() will then see null despite a valid secret existing. Reassign client.clientSecret to a remaining entry, or drop the singular field and derive it from the list.

3. AddUserPoolClientSecret request/response shape doesn't match AWS — CognitoJsonHandler.java:63-75

Per the AWS API reference:

• Request: field is ClientSecret (PascalCase), not clientSecret. As written, aws cognito-idp add-user-pool-client-secret --client-secret xxx serialises as "ClientSecret": "xxx", which this handler ignores and falls back to auto-generating.
• Response: wrapped under a top-level ClientSecretDescriptor key:

  {"ClientSecretDescriptor": {"ClientSecretId": "...", "ClientSecretValue": "...", "ClientSecretCreateDate": 0}}

  The mock currently returns those fields unwrapped, which won't unmarshal into the SDK's response type.

The conditional ClientSecretValue exclusion when a custom secret is supplied is correct and matches AWS — keep that.

4. No validation of explicit ClientSecret value — CognitoService.java addUserPoolClientSecret

AWS enforces Min: 24, Max: 64, Pattern: [\w+]+ on the ClientSecret input. The test happily passes "my-explicit-secret" (17 chars). Real AWS would reject it with InvalidParameterException. Either validate to match, or document the deliberate divergence.

Out of scope

5. ListUserPoolClients response shape changed — CognitoJsonHandler.java:28

Swapping clientToNode → userPoolClientToNode strips fields from the existing ListUserPoolClients response. This is a correctness fix (real AWS UserPoolClientDescription only returns ClientId/UserPoolId/ClientName) but it's unrelated to the rotation feature and silently changes an existing endpoint's contract. Split into its own PR, or call it out explicitly in the PR description.

Behavioural — please confirm intent

6. validateClientSecret dropped !isGenerateSecret() — CognitoService.java:1033-1051

The new check is client.getUserPoolClientSecrets().isEmpty(). Two questions:

• For any client that has a legacy clientSecret set but an empty userPoolClientSecrets list, auth now fails with "Client must have a secret". If floci state is always ephemeral this is moot — confirm.
• The guard no longer distinguishes confidential vs public clients. Consider an explicit check so a public client with list entries can't pass.

7. createUserPoolClient seeds the list when generateSecret=true — CognitoService.java:178-194

Correct. Worth a comment on listUserPoolClientSecretsInitiallyEmpty explaining the precondition, so a future refactor of the shared clientId setup doesn't silently break it.

Test coverage

• No positive rotation test. This is the whole point of the feature. Add two secrets, authenticate client_credentials with secret A (succeeds), with secret B (succeeds), delete A, re-authenticate with A (fails), with B (succeeds).
• No test pins the delete state bug. A test that adds two secrets, deletes the one matching client.clientSecret, and asserts client.getClientSecret() != null would red-flag blocker #2.
• No cross-pool isolation test (wrong UserPoolId → 404).
• addUserPoolClientSecretExceedsLimit only asserts status 400 — assert the response error code too (ties back to blocker #1).

Nits

• CognitoService.java:182 — extra space: <= 1 ).
• addUserPoolClientSecret(clientId, clientSecret, userPoolId) param order doesn't match the sibling methods. Pick one convention.

---

AI-assisted review (Opus 4.6, direct, diff + repo verified + AWS docs cross-referenced + Gemini 2.5 Pro + Codex). Please verify.

[danielabeledo]

Hey @hampsterx thanks for the detailed review

I will address your comments, just one thing

On your point number 2, it seems the behaviour in the PR is the behaviour in the real AWS Cognito

aws cognito-idp create-user-pool-client  --user-pool-id <pool-id> --generate-secret --client-name <name>

Will create and populate a random clientSecret record. When creating a new secret via

aws cognito-idp add-user-pool-client-secret --user-pool-id <user-pool> --client-id <client-id>

it will create a second secret, valid for authentication, but the clientSecret record stays the same, it doesn't change.
If we delete this second secret, nothing happens to the ClientSecret property
If we delete the original secret, that will be listed in the UserPoolClientSecret list, the Client Secret Property will be deleted as well as the UserPoolClientSecret
As there is no way of setting the ClientSecret property after that (only created at UserPoolClient creation) the ClientSecret value stays null.

[danielabeledo]

Hi @hampsterx @hectorvent - I believe I have addressed everything raised in the previous comment, apart from number 2.

[hectorvent]

@hampsterx could you please do the review? All test are green.

Thanks @danielabeledo

[hampsterx]

Follow-up review (most items addressed, three remaining

#2 from the original review (clientSecret sync on delete)) accepted. Your explanation of real AWS behaviour is clear and makes sense. Thanks for verifying.

Trailing space, request field casing, ListUserPoolClients scope creep, all resolved. The squashed commit is clean.

Remaining

1. AddUserPoolClientSecret response still needs ClientSecretDescriptor wrapper

Per the AWS API reference, the response is:

{"ClientSecretDescriptor": {"ClientSecretId": "...", "ClientSecretValue": "...", "ClientSecretCreateDate": 0}}

Current code in handleAddUserPoolClientSecret returns the fields unwrapped. SDKs deserialize into a typed response class keyed on ClientSecretDescriptor, so this will silently produce nulls.

Fix: wrap the clientSecretToNode result:

ObjectNode wrapper = objectMapper.createObjectNode();
wrapper.set("ClientSecretDescriptor", clientSecretToNode(cs, includeClientSecretValue));
return Response.ok(wrapper).build();

ListUserPoolClientSecrets shape is fine as-is (ClientSecrets array at top level matches AWS).

2. UserPoolClientSecret missing no-arg constructor, Jackson deserialization fails on RocksDB

The class only has the 3-arg constructor. Jackson can't deserialize without either a no-arg constructor or @JsonCreator. Tests pass because they use in-memory storage (no serialization round-trip), but the RocksDB backend will throw on restart.

Fix: add public UserPoolClientSecret() {}.

3. fullRotateScenario, missing the final assertion

After deleting secret1, the test asserts secret1 → 400 but never asserts secret2 → 200. That's the whole point of rotation: the surviving secret still works. One line:

// authentication with client credentials 1 fails
oauthToken(rotClientId, secret1Value).then().statusCode(400);

// secret 2 still works after rotation
oauthToken(rotClientId, secret2Value).then().statusCode(200);

---

AI-assisted review (Opus 4.6, diff-only follow-up).

[danielabeledo]

Hey! sorry for all the back-n-forth - Comments addressed

[hampsterx]

Warning - stale clientSecret after rotation
CognitoService.deleteUserPoolClientSecret (around line 259) nulls client.clientSecret when the deleted secret matches it, but never promotes a remaining secret from userPoolClientSecrets. Since CognitoJsonHandler.clientToNode (around line 511) only serializes the legacy clientSecret field, DescribeUserPoolClient/ListUserPoolClients will report no secret after rotation even though the remaining one still authenticates successfully. Observable protocol inconsistency, likely to break callers that read the describe output to pick up the active secret.

Fix options: repoint client.clientSecret to a remaining secret on delete, or make clientToNode fall back to the first entry in userPoolClientSecrets when the legacy field is null.

Warning - AddUserPoolClientSecret accepts blank secrets
CognitoJsonHandler pulls ClientSecret with asText(null) and the service stores it as-is. validateClientSecret at line ~1029 then rejects the same blank value at auth time, so the API can persist a secret that is unusable by design. Reject empty/whitespace in addUserPoolClientSecret with InvalidParameterException before persisting.

Suggestion - non-standard ClientSecret input param
Real AWS AddUserPoolClientSecret always generates the secret, never accepts a client-supplied value. Either drop the param and always generate, or call out as a Floci-specific extension in a comment.

Suggestion - test gaps

• No DescribeUserPoolClient call after deleting the first secret in the rotation flow (would have caught the warning above).
• No coverage for blank/missing ClientSecret input, or explicit-secret usability via client_secret_post.
• No new CognitoServiceTest cases for the three new service methods.

Suggestion - ID collision risk
UserPoolClientSecret.generateId uses clientId + "--" + currentTimeMillis(). Rapid successive adds can collide. Prefer nanoTime() or a UUID suffix.

---

AI-assisted review (Claude Opus 4.6 + Codex + Gemini). Findings spot-checked against the diff but please verify before applying.

[danielabeledo]

Warning - stale clientSecret after rotation

This maps the current behaviour of the real AWS cognito - no "secrets" get promoted and no secrets are returned with DescribeUserPoolClient in this scenario.

Suggestion - non-standard ClientSecret input param

AddUserPoolClientSecret does in fact allow to pass the clientSecret as an optional parameter https://docs.aws.amazon.com/cognito-user-identity-pools/latest/APIReference/API_AddUserPoolClientSecret.html

Suggestion - ID collision risk

This is the current behaviour of AWS Cognito https://docs.aws.amazon.com/cognito-user-identity-pools/latest/APIReference/API_ClientSecretDescriptorType.html

Warning - AddUserPoolClientSecret accepts blank secrets

I've added a client secret validation step

[hectorvent]

Thanks @danielabeledo for you work.