Add client attribute condition and client activity event workflow providers - #51629
Draft
antoniocasagrande-airia wants to merge 3 commits into
Conversation
… the same clock JpaWorkflowStateProvider.scheduleStep stamped scheduledStepTimestamp with Instant.now() while getDueScheduledSteps compares it against Time.currentTimeMillis(). Production behavior is unchanged (Time delegates to the system clock), but under a test time offset steps were scheduled against the wrong clock, making restart scenarios untestable with TimeOffSet. Steps scheduled during an offset run are now scheduled relative to the simulated clock, so the cumulative offsets in BrokeredUserLifecycleWorkflowTest are adjusted accordingly. Signed-off-by: Antonio Casagrande <antoniocasagrande@airia.com>
…viders Adds two client-scoped workflow providers for managing the lifecycle of dynamically registered clients: - has-client-attribute: mirrors has-user-attribute for CLIENTS resources, supporting presence-only (key) and exact-value (key:value) matching, with a JPA predicate so scheduled activation picks up eligible clients. - client-activity: complements client-authenticated (which only sees CLIENT_LOGIN) by treating LOGIN, CODE_TO_TOKEN and REFRESH_TOKEN user events as activity on the event's client, so restart-in-progress keeps actively used public clients out of disable/delete steps. Closes keycloak#51595 Signed-off-by: Antonio Casagrande <antoniocasagrande@airia.com>
LOGIN_ERROR, CODE_TO_TOKEN_ERROR and REFRESH_TOKEN_ERROR carry the client id but must not count as activity: a bad actor spamming credentials against a dormant client would otherwise keep resetting its idle clock. Signed-off-by: Antonio Casagrande <antoniocasagrande@airia.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #51595
Adds the two client-scoped Workflows providers proposed in #51595, so client lifecycle workflows can select clients by attribute and track user-driven activity on public clients:
has-client-attributecondition — mirrorshas-user-attributeforCLIENTSresources, takingkey(presence-only) orkey:valueconfig.evaluatechecks the client attribute map;toPredicatebuilds an EXISTS subquery overClientAttributeEntityso scheduled activation picks up eligible clients directly in the JPA query. Since client attributes are single-valued, the expected value is compared literally (no multi-value set semantics). The value comparison reuses the per-database handling fromJpaRealmProvider.searchClientsByAttributes(OracleDBMS_LOB.COMPARE, PostgreSQLsubstr+ full comparison).client-activityevent — complementsclient-authenticated(which only seesCLIENT_LOGIN, an event public clients can never produce) by treatingLOGIN,CODE_TO_TOKENandREFRESH_TOKENuser events as activity on the event's client. Combined withrestart-in-progress, this lets a disable/delete workflow keep actively used clients out of the steps while dormant ones progress.The motivating use case (detailed in #51595) is lifecycle management of dynamically registered OAuth clients (MCP clients registering via RFC 7591): flag them with an attribute at registration, disable after N days of inactivity, delete after M more.
Also includes a small consistency fix this work surfaced:
JpaWorkflowStateProvider.scheduleStepstampedscheduledStepTimestampwithInstant.now()whilegetDueScheduledStepscompares it againstTime.currentTimeMillis()in the same class. Production behavior is identical (Timedelegates to the system clock), but under a test time offset a workflow restart schedules against the wrong clock, which made the activity-reset scenario untestable —ClientActivityWorkflowTestacts as the regression test. With the fix, steps scheduled during an offset run are scheduled relative to the simulated clock (cumulative), soBrokeredUserLifecycleWorkflowTest's tail offsets were adjusted from5/10/10to cumulative5/15/25days — arguably what the simulated timeline meant all along.ScheduleWorkflowTask/RunWorkflowTaskhave the sameSystem.currentTimeMillis()pattern for the informationalscheduledTimeevent payload; left untouched to keep the diff minimal. Fullorg.keycloak.tests.workflowpackage passes locally (103 tests).Notes for reviewers
on: client-created, because the client creation event is currently published before attributes are persisted and can never match an attribute condition — that is Workflows:on: client-createdconditions are evaluated before the client is populated, so they can never match #51594 (fix in flight at fix: defer ClientCreationEvent to after transaction commit [fj4WqyCCw3C5ShR1RfB7MoBPTpkRrBFYP1uT35g3MvT] #51598). Once that lands, event-based activation works with this condition unchanged.ClientAttributeWorkflowConditionProviderreusesUserAttributeWorkflowConditionProvider.parseKeyValuePairso the two conditions keep one config grammar. Worth knowing that this inherits twoProperties-syntax edge cases (affectinghas-user-attributeequally today): a#-prefixed key parses as a comment and is silently dropped, and multi-line input produces multiple entries withstringPropertyNames().iterator().next()picking one non-deterministically. Kept as-is here for grammar consistency between the two conditions; happy to harden the shared parser (single-entry check, or a first-colon split) in this PR or a follow-up, whichever you prefer.LOGIN_ERROR,CODE_TO_TOKEN_ERROR,REFRESH_TOKEN_ERROR) also carry the client id but are deliberately excluded from the activity set — otherwise spamming bad credentials against a dormant client would keep resetting its idle clock.ClientActivityWorkflowTest#testFailedLoginDoesNotActivateWorkflowpins this.client-activityvs. extendingclient-authenticatedto cover the user-driven events. A separate provider avoids changing the semantics of existingclient-authenticatedworkflows.This implementation is ported from providers we run out-of-tree in production against 26.6.4, adapted to the current SPI (
getSupportedResourceType/validate/Root-basedtoPredicate).AI disclosure
Per the contributing guidelines: AI agents were used to generate this port and its tests (adapted from our out-of-tree implementation). The changes have been reviewed and are understood and owned by the submitter.