feat(workflows): add invite-user step - #49301
Conversation
|
External links check fix - #49291 |
a943f9f to
71867cb
Compare
25434e0 to
46f6a29
Compare
Adds an invite-user workflow step that sends an invitation email containing
an action-token link. The link drives the recipient through a configurable
set of required actions (defaults: UPDATE_PASSWORD and VERIFY_EMAIL) and can
optionally redirect to a client-managed URL afterwards.
Step configuration:
- actions: required action names the user must complete
- client-id: client associated with the action token (defaults to the
realm system client)
- redirect-uri: post-completion redirect URI; requires client-id
Architectural notes:
- A dedicated EmailTemplateProvider.sendInviteUserEmail method (with a
default that falls back to sendExecuteActions for existing
implementations) renders the invitation via its own invite-user.ftl
template, so the wording differs from the admin-initiated
"update your account" email.
- Action-token construction (token + URL + EmailTemplateProvider call) is
encapsulated in org.keycloak.email.ActionTokenEmail.
- DefaultActionToken gains a serialize(KeycloakSession, RealmModel, URI)
overload so callers without an active HTTP request (e.g. workflow
executors) can serialise tokens without fabricating a UriInfo.
- The base URI is resolved via HostnameProvider rather than
KeycloakContext.getUri(), which is request-scoped and not available on
the workflow executor thread. The factory rejects the step configuration
up-front if neither --hostname=<full URL> nor the realm 'frontendUrl'
attribute is configured.
- WorkflowsResource maps ComponentValidationException to HTTP 400 so
invalid step configuration is reported as a client error rather than a
leaking 500.
Closes keycloak#49297
Signed-off-by: bilkoua <git@bil.co.ua>
46f6a29 to
8a60e84
Compare
sguilhen
left a comment
There was a problem hiding this comment.
Nice contribution - the ActionTokenEmail utility and the DefaultActionToken.serialize(URI) refactoring are well done, and the step fills a practical gap for automated onboarding workflows. A few items to address before this can be merged:
1. Rebase needed - WorkflowsResource.create() on main now uses ErrorResponse.error(session, locale, me, ...) with locale support. The PR is based on an older version using ErrorResponse.error(me.getMessage(), ...). The new ComponentValidationException catch should also use the locale-aware variant.
2. Update path - WorkflowResource.update() also needs to catch ComponentValidationException. When there are no scheduled steps, updateWorkflow() deletes and re-creates the workflow via toModel(), which goes through realm.addComponentModel() and triggers validateConfiguration(). Even the scheduled-steps path calls workflow.updateConfig() which can trigger realm.updateComponent() - and that also calls validateConfiguration().
| .type(ProviderConfigProperty.MULTIVALUED_STRING_TYPE) | ||
| .add() | ||
| .property() | ||
| .name(InviteUserStepProvider.CONFIG_CLIENT_ID) |
There was a problem hiding this comment.
This getConfigProperties() override should be removed. No other workflow step factory overrides it - workflows are configured via YAML with: blocks that map directly to MultivaluedHashMap, not via the Component admin API. This override is unused and misleading.
| */ | ||
| static URI resolveBaseUri(KeycloakSession session) { | ||
| try { | ||
| return session.getProvider(HostnameProvider.class).getBaseUri(null, UrlType.FRONTEND); |
There was a problem hiding this comment.
Using NPE as control flow is fragile - it relies on HostnameV2Provider.getFrontUriBuilder(null) calling originalUriInfo.getBaseUriBuilder() on null, and could mask unrelated NPE bugs. Consider checking the hostname configuration more explicitly, or adding a HostnameProvider method that supports null UriInfo cleanly.
| return; | ||
| } | ||
| if (resolved.getParams().isEmpty()) { | ||
| return; |
There was a problem hiding this comment.
When resolveParams() returns an ineligibility reason (no email, disabled user, bad client), the step silently returns without logging which reason caused the skip. Other steps (e.g., NotifyUserStepProvider) log warnings for similar cases. Consider adding something like LOG.debugf("Skipping invite for user %s: %s", user.getUsername(), resolved.getIneligibility().get()).
| assertThat(response.getStatus(), is(Response.Status.BAD_REQUEST.getStatusCode())); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
A few test gaps worth filling:
- Happy path with valid
client-id+redirect-uri(the main configurable feature of the step) - Custom
actionslist (e.g., onlyVERIFY_EMAIL) - verify the config is actually used in the token - Non-existent
client-idvalidation (should return 400) - Disabled user being skipped (
Ineligibility.USER_DISABLEDpath)
| |=== | ||
|
|
||
| The `invite-user` step builds links from the configured hostname (`--hostname` or the realm `frontendUrl`). If neither is set, the step is skipped. | ||
|
|
There was a problem hiding this comment.
"If neither is set, the step is skipped" is inaccurate. The factory's validateConfiguration() rejects the step at workflow creation time with a ComponentValidationException, so the workflow can't be created without a hostname. The step is only skipped at runtime if the hostname config changes after the workflow was created.
| @@ -70,6 +71,8 @@ public Response create(WorkflowRepresentation rep) { | |||
There was a problem hiding this comment.
This needs a rebase - main now uses ErrorResponse.error(session, locale, me, ...) with locale support. The new ComponentValidationException catch should also use the locale-aware variant. Additionally, WorkflowResource.update() needs the same catch - see the overall review comment for details.
Closes #49297
Adds a new
invite-userworkflow step that sends an invitation email to the user the workflow is processing, with a one-time action token link.Usage
Drafted with an AI coding agent; reviewed and tested locally.