workday: fix infinite 401 retry loop in activity CEL program#20072
Conversation
The token validity check only tested presence, not expiry. When the Workday API returned 401, the handler set want_more:true without clearing the cached access_token, causing an infinite retry loop against the stale token with no backoff. Add TTL-based token expiry tracking using the expires_in value from the token endpoint response. When expires_in is absent or negative (Workday's non-enforced expiry mode), default to 60 minutes, this is conservative but within the documented "several hours" actual lifetime. The 401 handler now sets want_more:false so the next poll interval triggers a fresh token exchange.
✅ Elastic Docs Style Checker (Vale)No issues found on modified lines! The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
|
✅ All changelog entries have the correct PR link. |
💚 Build Succeeded
cc @efd6 |
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
| }, | ||
| "want_more": true, | ||
| "offset": state.offset, | ||
| "want_more": false, |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/workday/data_stream/activity/agent/stream/cel.yml.hbs:173
The 401 handler halts the loop but never invalidates the cached token, so a token the local TTL still considers valid is reused every interval until it lapses; expire the token in the cursor on 401 to force an immediate re-auth.
Details
The 401 branch returns {"events": {"error": ...}, "want_more": false, "offset": 0} to state.with(...). state.with merges into state, so because the returned object has no cursor key, the previous cursor (holding access_token and a still-future token_expiry) is preserved. On the next execution the refresh gate at the top of the program (!has(state.?cursor.access_token) || !has(state.?cursor.token_expiry) || timestamp(state.cursor.token_expiry) < now - duration("30s")) all evaluate false, so no refresh occurs and the same server-rejected token is sent again — producing another 401. This is precisely the case this branch exists to handle ("token expired"), yet nothing here forces a re-auth: recovery depends entirely on the tracked TTL elapsing rather than on the 401. Setting want_more: false correctly stops the tight in-run loop the PR targets, but a token that the server rejects before its tracked expiry will keep 401ing on each poll interval until token_expiry passes. The conservative TTL (60s margin subtracted; 3540s default) makes this unlikely in the normal Workday case, but the 401 path itself provides no recovery.
Recommendation:
On the 401 path, expire the cached token in the cursor so the next execution's refresh gate fires immediately, while preserving pagination progress (last_timestamp / max_ingested_time):
: (resp.StatusCode == 401) ?
{
"events": {
"error": {
"code": string(resp.StatusCode),
"id": resp.Status,
"message": "GET " + base_url + "/activityLogging: token expired",
},
},
"cursor": state.?cursor.orValue({}).with({
"token_expiry": (now - duration("1h")).format(time_layout.RFC3339),
}),
"want_more": false,
"offset": 0,
}This leaves access_token and the paging fields intact but backdates token_expiry, so the next run re-authenticates rather than replaying the rejected token.
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
There was a problem hiding this comment.
This is an intentional trade-off and the proposed solution is incorrect; it would not update the cursor since it an error object return. The trade-off is whether to attempt to resolve immediately silently (what the correct version of Vera's proposal would be), or to noisily fail a limited number of times (what the current code does). With the current code and default interval of 5m, there would be 12 error polls until the cursor's token expires; it is not a tight loop.
Review summaryIssues found across the latest commits e6b08db — 1 medium
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
|
|
@muskan-agarwal26 FYI on this issue, if you can ensure other Workday data streams take this into account as you're working on them. @efd6 thanks for jumping on this. @narph we have a few customers tracking and waiting on this to merge if we can prioritise a review from the team please. |
|
Tick the box to add this pull request to the merge queue (same as
|
|
Package workday - 0.2.0 containing this change is available at https://epr.elastic.co/package/workday/0.2.0/ |
Proposed commit message
Checklist
changelog.ymlfile.Author's Checklist
How to test this PR locally
Related issues
Screenshots