From d63e66491221dcb0f040a55348dc46f1adf52329 Mon Sep 17 00:00:00 2001 From: Shawn Hsu Date: Fri, 11 Sep 2026 16:22:14 +0800 Subject: [PATCH] feat(keycloak): let ExecuteActionsEmail set a custom action-token lifespan Helper.ExecuteActionsEmail never set gocloak.ExecuteActionsEmail's Lifespan field, so every caller got Keycloak's own 12-hour default for admin-triggered action tokens -- far longer than intended for something like a password-reset link, and inconsistent with the much shorter lifespan Keycloak's self-service action-token flow already defaults to. Add a lifespanSeconds parameter, threaded into gocloak's Lifespan field (already supported server-side, just never set by this wrapper) only when it's genuinely positive. gocloak's Lifespan is *int with `omitempty`, which only checks pointer-nilness, not the pointee's value -- a naive `&lifespanSeconds` would send a literal lifespan=0 for a zero/uninitialized argument, minting an already-expired action token with no error surfaced anywhere. Guarding on lifespanSeconds > 0 instead leaves the field unset (nil) for that case, falling back to Keycloak's own realm default -- the same behavior this method had before Lifespan existed at all. Existing callers must now pass an explicit value -- there's exactly one today (cubecmp's admin-triggered reset-password routes, paired change in that repo). Signed-off-by: Shawn Hsu Co-Authored-By: Claude Sonnet 5 --- pkg/keycloak/keycloak.go | 23 +++++++--- pkg/keycloak/keycloak_test.go | 83 ++++++++++++++++++++++++++--------- 2 files changed, 80 insertions(+), 26 deletions(-) diff --git a/pkg/keycloak/keycloak.go b/pkg/keycloak/keycloak.go index aba7e8f..8f4e924 100644 --- a/pkg/keycloak/keycloak.go +++ b/pkg/keycloak/keycloak.go @@ -317,17 +317,30 @@ func (h *Helper) SetPassword(realm, userID, password string) error { return h.Client.SetPassword(ctx, h.Token, userID, realm, password, false) } -func (h *Helper) ExecuteActionsEmail(realm, userID string, actions []string) error { +func (h *Helper) ExecuteActionsEmail(realm, userID string, actions []string, lifespanSeconds int) error { if actions == nil { actions = []string{} } - ctx, cancel := context.WithTimeout(wait.CtxSeconds(10)) - defer cancel() - return h.Client.ExecuteActionsEmail(ctx, h.Token, realm, gocloak.ExecuteActionsEmail{ + params := gocloak.ExecuteActionsEmail{ UserID: &userID, Actions: &actions, - }) + } + /* + * gocloak's Lifespan is *int with `omitempty` -- that only checks + * pointer-nilness, not the pointee's value, so a non-nil pointer to 0 + * would still serialize as an explicit lifespan=0 query param and mint + * an already-expired action token. Only set it for a genuinely + * positive value; anything else falls back to Keycloak's own realm + * default, matching this method's pre-lifespan-parameter behavior. + */ + if lifespanSeconds > 0 { + params.Lifespan = gocloak.IntP(lifespanSeconds) + } + + ctx, cancel := context.WithTimeout(wait.CtxSeconds(10)) + defer cancel() + return h.Client.ExecuteActionsEmail(ctx, h.Token, realm, params) } func (h *Helper) DeleteUser(realm, userID string) error { diff --git a/pkg/keycloak/keycloak_test.go b/pkg/keycloak/keycloak_test.go index fb14ac7..677fd56 100644 --- a/pkg/keycloak/keycloak_test.go +++ b/pkg/keycloak/keycloak_test.go @@ -939,32 +939,67 @@ func TestHelperExecuteActionsEmail(t *testing.T) { errBoom := errors.New("boom") tests := []struct { - name string - actions []string - expectedActions []string - clientError error - expectedError error + name string + actions []string + expectedActions []string + lifespanSeconds int + expectedLifespan *int + clientError error + expectedError error }{ { - name: "Should send the given actions when actions is non-nil", - actions: []string{"UPDATE_PASSWORD"}, - expectedActions: []string{"UPDATE_PASSWORD"}, - clientError: nil, - expectedError: nil, + name: "Should send the given actions and lifespan when actions is non-nil", + actions: []string{"UPDATE_PASSWORD"}, + expectedActions: []string{"UPDATE_PASSWORD"}, + lifespanSeconds: 300, + expectedLifespan: gocloak.IntP(300), + clientError: nil, + expectedError: nil, }, { - name: "Should send an empty slice instead of nil when actions is nil", - actions: nil, - expectedActions: []string{}, - clientError: nil, - expectedError: nil, + name: "Should send an empty slice instead of nil when actions is nil", + actions: nil, + expectedActions: []string{}, + lifespanSeconds: 300, + expectedLifespan: gocloak.IntP(300), + clientError: nil, + expectedError: nil, }, { - name: "Should propagate an error when the client call fails", - actions: []string{"UPDATE_PASSWORD"}, - expectedActions: []string{"UPDATE_PASSWORD"}, - clientError: errBoom, - expectedError: errBoom, + name: "Should thread through a different lifespan value rather than a hardcoded one", + actions: []string{"UPDATE_PASSWORD"}, + expectedActions: []string{"UPDATE_PASSWORD"}, + lifespanSeconds: 600, + expectedLifespan: gocloak.IntP(600), + clientError: nil, + expectedError: nil, + }, + { + name: "Should leave Lifespan nil when lifespanSeconds is zero, instead of sending an already-expired lifespan=0", + actions: []string{"UPDATE_PASSWORD"}, + expectedActions: []string{"UPDATE_PASSWORD"}, + lifespanSeconds: 0, + expectedLifespan: nil, + clientError: nil, + expectedError: nil, + }, + { + name: "Should leave Lifespan nil when lifespanSeconds is negative", + actions: []string{"UPDATE_PASSWORD"}, + expectedActions: []string{"UPDATE_PASSWORD"}, + lifespanSeconds: -1, + expectedLifespan: nil, + clientError: nil, + expectedError: nil, + }, + { + name: "Should propagate an error when the client call fails", + actions: []string{"UPDATE_PASSWORD"}, + expectedActions: []string{"UPDATE_PASSWORD"}, + lifespanSeconds: 300, + expectedLifespan: gocloak.IntP(300), + clientError: errBoom, + expectedError: errBoom, }, } @@ -978,11 +1013,17 @@ func TestHelperExecuteActionsEmail(t *testing.T) { require.Equal(t, "user-id", *params.UserID) require.NotNil(t, params.Actions) require.Equal(t, tc.expectedActions, *params.Actions) + if tc.expectedLifespan == nil { + require.Nil(t, params.Lifespan) + } else { + require.NotNil(t, params.Lifespan) + require.Equal(t, *tc.expectedLifespan, *params.Lifespan) + } }). Return(tc.clientError) h := &Helper{Client: client} - err := h.ExecuteActionsEmail("master", "user-id", tc.actions) + err := h.ExecuteActionsEmail("master", "user-id", tc.actions, tc.lifespanSeconds) require.ErrorIs(t, err, tc.expectedError) })