Skip to content

Commit 4551cc9

Browse files
committed
fix(okta,servicenow): stop sending requests the APIs reject
Okta documents `since` and `after` on the System Log as mutually exclusive, so `get_logs` lets the cursor win rather than sending both — the shape a scheduled poll that persists the cursor would otherwise send. Seven boolean query params reached Okta interpolated raw, so an agent tool call supplying `yes` was rejected. Each now routes through `isOktaFlagEnabled`, keeping its existing send-or-omit behavior. A cleared ServiceNow limit/offset/quantity stayed `''` through the block mapper and was appended as a valueless `sysparm_limit=`. The mapper now resolves a blank to undefined, and the tools skip a blank as well.
1 parent 31f98e3 commit 4551cc9

13 files changed

Lines changed: 235 additions & 25 deletions

apps/sim/blocks/blocks/servicenow.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1683,11 +1683,19 @@ Output: {"state": "2", "assigned_to": "john.doe", "work_notes": "Assigned and st
16831683
if (operation === 'servicenow_list_attachments') {
16841684
rest.limit =
16851685
attachmentLimit != null && attachmentLimit !== '' ? Number(attachmentLimit) : undefined
1686-
} else if (rest.limit != null && rest.limit !== '') {
1687-
rest.limit = Number(rest.limit)
1686+
} else if (rest.limit != null) {
1687+
rest.limit = rest.limit === '' ? undefined : Number(rest.limit)
1688+
}
1689+
/**
1690+
* A short-input stores `''` once a user types a value and clears it
1691+
* again, so a blank must resolve to `undefined` rather than stay in
1692+
* place — the tools only skip a param that is absent, and a retained
1693+
* `''` reaches ServiceNow as `sysparm_limit=`.
1694+
*/
1695+
if (rest.offset != null) rest.offset = rest.offset === '' ? undefined : Number(rest.offset)
1696+
if (rest.quantity != null) {
1697+
rest.quantity = rest.quantity === '' ? undefined : Number(rest.quantity)
16881698
}
1689-
if (rest.offset != null && rest.offset !== '') rest.offset = Number(rest.offset)
1690-
if (rest.quantity != null && rest.quantity !== '') rest.quantity = Number(rest.quantity)
16911699

16921700
if (rest.inputDisplayValue != null) {
16931701
rest.inputDisplayValue =

apps/sim/tools/okta/clear_user_sessions.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { validateOktaDomain } from '@/lib/core/security/input-validation'
33
import type { OktaClearUserSessionsParams, OktaClearUserSessionsResponse } from '@/tools/okta/types'
4-
import { oktaHeaders, throwOktaError } from '@/tools/okta/utils'
4+
import { isOktaFlagEnabled, oktaHeaders, throwOktaError } from '@/tools/okta/utils'
55
import type { ToolConfig } from '@/tools/types'
66

77
const logger = createLogger('OktaClearUserSessions')
@@ -56,10 +56,10 @@ export const oktaClearUserSessionsTool: ToolConfig<
5656
const queryParams = new URLSearchParams()
5757

5858
if (params.oauthTokens !== undefined) {
59-
queryParams.append('oauthTokens', String(params.oauthTokens))
59+
queryParams.append('oauthTokens', String(isOktaFlagEnabled(params.oauthTokens)))
6060
}
6161
if (params.forgetDevices !== undefined) {
62-
queryParams.append('forgetDevices', String(params.forgetDevices))
62+
queryParams.append('forgetDevices', String(isOktaFlagEnabled(params.forgetDevices)))
6363
}
6464

6565
const queryString = queryParams.toString()

apps/sim/tools/okta/create_user.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { validateOktaDomain } from '@/lib/core/security/input-validation'
33
import type { OktaCreateUserParams, OktaCreateUserResponse, OktaUser } from '@/tools/okta/types'
4-
import { oktaHeaders, throwOktaError } from '@/tools/okta/utils'
4+
import { isOktaFlagEnabled, oktaHeaders, throwOktaError } from '@/tools/okta/utils'
55
import type { ToolConfig } from '@/tools/types'
66

77
const logger = createLogger('OktaCreateUser')
@@ -84,7 +84,10 @@ export const oktaCreateUserTool: ToolConfig<OktaCreateUserParams, OktaCreateUser
8484
request: {
8585
url: (params) => {
8686
const domain = validateOktaDomain(params.domain)
87-
const activate = params.activate ?? true
87+
const activate =
88+
params.activate === undefined || params.activate === null
89+
? true
90+
: isOktaFlagEnabled(params.activate)
8891
return `https://${domain}/api/v1/users?activate=${activate}`
8992
},
9093
method: 'POST',

apps/sim/tools/okta/delete_group_rule.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { validateOktaDomain } from '@/lib/core/security/input-validation'
33
import type { OktaDeleteGroupRuleParams, OktaDeleteGroupRuleResponse } from '@/tools/okta/types'
4-
import { oktaHeaders, throwOktaError } from '@/tools/okta/utils'
4+
import { isOktaFlagEnabled, oktaHeaders, throwOktaError } from '@/tools/okta/utils'
55
import type { ToolConfig } from '@/tools/types'
66

77
const logger = createLogger('OktaDeleteGroupRule')
@@ -48,7 +48,9 @@ export const oktaDeleteGroupRuleTool: ToolConfig<
4848
url: (params) => {
4949
const domain = validateOktaDomain(params.domain)
5050
const base = `https://${domain}/api/v1/groups/rules/${encodeURIComponent(params.groupRuleId.trim())}`
51-
return params.removeUsers === undefined ? base : `${base}?removeUsers=${params.removeUsers}`
51+
return params.removeUsers === undefined
52+
? base
53+
: `${base}?removeUsers=${isOktaFlagEnabled(params.removeUsers)}`
5254
},
5355
method: 'DELETE',
5456
headers: (params) => oktaHeaders(params.apiKey),

apps/sim/tools/okta/enroll_factor.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type {
55
OktaEnrollFactorResponse,
66
OktaFactor,
77
} from '@/tools/okta/types'
8-
import { oktaHeaders, throwOktaError } from '@/tools/okta/utils'
8+
import { isOktaFlagEnabled, oktaHeaders, throwOktaError } from '@/tools/okta/utils'
99
import type { ToolConfig } from '@/tools/types'
1010

1111
const logger = createLogger('OktaEnrollFactor')
@@ -89,7 +89,9 @@ export const oktaEnrollFactorTool: ToolConfig<OktaEnrollFactorParams, OktaEnroll
8989
url: (params) => {
9090
const domain = validateOktaDomain(params.domain)
9191
const base = `https://${domain}/api/v1/users/${encodeURIComponent(params.userId.trim())}/factors`
92-
return params.activate === undefined ? base : `${base}?activate=${params.activate}`
92+
return params.activate === undefined
93+
? base
94+
: `${base}?activate=${isOktaFlagEnabled(params.activate)}`
9395
},
9496
method: 'POST',
9597
headers: (params) => oktaHeaders(params.apiKey),

apps/sim/tools/okta/get_logs.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export const oktaGetLogsTool: ToolConfig<OktaGetLogsParams, OktaGetLogsResponse>
3131
required: false,
3232
visibility: 'user-or-llm',
3333
description:
34-
'Start of the query time window as an ISO 8601 timestamp (default: 7 days before "until")',
34+
'Start of the query time window as an ISO 8601 timestamp (default: 7 days before "until"). Ignored when a cursor is supplied in "after", which already encodes the resume position',
3535
},
3636
until: {
3737
type: 'string',
@@ -78,12 +78,18 @@ export const oktaGetLogsTool: ToolConfig<OktaGetLogsParams, OktaGetLogsResponse>
7878
const domain = validateOktaDomain(params.domain)
7979
const queryParams = new URLSearchParams()
8080

81-
if (params.since) queryParams.append('since', params.since)
81+
/**
82+
* Okta documents `since` and `after` as mutually exclusive. A cursor
83+
* already encodes the position it resumes from, so it wins over the
84+
* window start whenever both are supplied — otherwise a scheduled poll
85+
* that has both configured would send a request Okta rejects.
86+
*/
87+
if (params.after) queryParams.append('after', params.after)
88+
else if (params.since) queryParams.append('since', params.since)
8289
if (params.until) queryParams.append('until', params.until)
8390
if (params.filter) queryParams.append('filter', params.filter)
8491
if (params.q) queryParams.append('q', params.q)
8592
if (params.sortOrder) queryParams.append('sortOrder', params.sortOrder)
86-
if (params.after) queryParams.append('after', params.after)
8793
/** `0` is a documented limit on this endpoint, so it must not read as absent. */
8894
if (params.limit !== undefined && params.limit !== null) {
8995
queryParams.append('limit', params.limit.toString())

apps/sim/tools/okta/list_apps.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
import { createLogger } from '@sim/logger'
22
import { validateOktaDomain } from '@/lib/core/security/input-validation'
33
import type { OktaApplication, OktaListAppsParams, OktaListAppsResponse } from '@/tools/okta/types'
4-
import { oktaHeaders, parseOktaPagination, throwOktaError } from '@/tools/okta/utils'
4+
import {
5+
isOktaFlagEnabled,
6+
oktaHeaders,
7+
parseOktaPagination,
8+
throwOktaError,
9+
} from '@/tools/okta/utils'
510
import type { ToolConfig } from '@/tools/types'
611

712
const logger = createLogger('OktaListApps')
@@ -67,7 +72,7 @@ export const oktaListAppsTool: ToolConfig<OktaListAppsParams, OktaListAppsRespon
6772
if (params.q) queryParams.append('q', params.q)
6873
if (params.filter) queryParams.append('filter', params.filter)
6974
if (params.includeNonDeleted !== undefined) {
70-
queryParams.append('includeNonDeleted', String(params.includeNonDeleted))
75+
queryParams.append('includeNonDeleted', String(isOktaFlagEnabled(params.includeNonDeleted)))
7176
}
7277
if (params.after) queryParams.append('after', params.after)
7378
if (params.limit) queryParams.append('limit', params.limit.toString())

apps/sim/tools/okta/remove_user_from_app.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { validateOktaDomain } from '@/lib/core/security/input-validation'
33
import type { OktaRemoveUserFromAppParams, OktaRemoveUserFromAppResponse } from '@/tools/okta/types'
4-
import { oktaHeaders, throwOktaError } from '@/tools/okta/utils'
4+
import { isOktaFlagEnabled, oktaHeaders, throwOktaError } from '@/tools/okta/utils'
55
import type { ToolConfig } from '@/tools/types'
66

77
const logger = createLogger('OktaRemoveUserFromApp')
@@ -53,7 +53,9 @@ export const oktaRemoveUserFromAppTool: ToolConfig<
5353
url: (params) => {
5454
const domain = validateOktaDomain(params.domain)
5555
const base = `https://${domain}/api/v1/apps/${encodeURIComponent(params.appId.trim())}/users/${encodeURIComponent(params.userId.trim())}`
56-
return params.sendEmail === undefined ? base : `${base}?sendEmail=${params.sendEmail}`
56+
return params.sendEmail === undefined
57+
? base
58+
: `${base}?sendEmail=${isOktaFlagEnabled(params.sendEmail)}`
5759
},
5860
method: 'DELETE',
5961
headers: (params) => oktaHeaders(params.apiKey),

apps/sim/tools/okta/reset_factor.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { validateOktaDomain } from '@/lib/core/security/input-validation'
33
import type { OktaResetFactorParams, OktaResetFactorResponse } from '@/tools/okta/types'
4-
import { oktaHeaders, throwOktaError } from '@/tools/okta/utils'
4+
import { isOktaFlagEnabled, oktaHeaders, throwOktaError } from '@/tools/okta/utils'
55
import type { ToolConfig } from '@/tools/types'
66

77
const logger = createLogger('OktaResetFactor')
@@ -53,7 +53,7 @@ export const oktaResetFactorTool: ToolConfig<OktaResetFactorParams, OktaResetFac
5353
const base = `https://${domain}/api/v1/users/${encodeURIComponent(params.userId.trim())}/factors/${encodeURIComponent(params.factorId.trim())}`
5454
return params.removeRecoveryEnrollment === undefined
5555
? base
56-
: `${base}?removeRecoveryEnrollment=${params.removeRecoveryEnrollment}`
56+
: `${base}?removeRecoveryEnrollment=${isOktaFlagEnabled(params.removeRecoveryEnrollment)}`
5757
},
5858
method: 'DELETE',
5959
headers: (params) => oktaHeaders(params.apiKey),

apps/sim/tools/okta/tools.test.ts

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,17 @@
44
import { afterEach, describe, expect, it, vi } from 'vitest'
55
import { OktaBlock } from '@/blocks/blocks/okta'
66
import { oktaActivateUserTool } from '@/tools/okta/activate_user'
7+
import { oktaClearUserSessionsTool } from '@/tools/okta/clear_user_sessions'
8+
import { oktaCreateUserTool } from '@/tools/okta/create_user'
79
import { oktaDeactivateUserTool } from '@/tools/okta/deactivate_user'
10+
import { oktaDeleteGroupRuleTool } from '@/tools/okta/delete_group_rule'
811
import { oktaDeleteUserTool } from '@/tools/okta/delete_user'
12+
import { oktaEnrollFactorTool } from '@/tools/okta/enroll_factor'
913
import { oktaGetLogsTool } from '@/tools/okta/get_logs'
1014
import { oktaGetUserTool } from '@/tools/okta/get_user'
15+
import { oktaListAppsTool } from '@/tools/okta/list_apps'
16+
import { oktaRemoveUserFromAppTool } from '@/tools/okta/remove_user_from_app'
17+
import { oktaResetFactorTool } from '@/tools/okta/reset_factor'
1118
import { oktaResetPasswordTool } from '@/tools/okta/reset_password'
1219
import { oktaUpdateGroupTool } from '@/tools/okta/update_group'
1320
import { oktaUpdateUserTool } from '@/tools/okta/update_user'
@@ -201,6 +208,32 @@ describe('okta get_logs query building', () => {
201208
const url = oktaGetLogsTool.request.url({ ...AUTH })
202209
expect(url).not.toContain('limit=')
203210
})
211+
212+
/**
213+
* Okta documents `since` and `after` as mutually exclusive, and a scheduled
214+
* poll that persists the cursor normally also has a start time configured —
215+
* so the resume request would otherwise be one Okta rejects.
216+
*/
217+
it('drops since when a cursor is supplied', () => {
218+
const url = builtUrl(oktaGetLogsTool.request.url, {
219+
...AUTH,
220+
since: '2026-08-01T00:00:00.000Z',
221+
after: 'CURSOR123',
222+
})
223+
224+
expect(url).toContain('after=CURSOR123')
225+
expect(url).not.toContain('since=')
226+
})
227+
228+
it('still sends since when no cursor is supplied', () => {
229+
const url = builtUrl(oktaGetLogsTool.request.url, {
230+
...AUTH,
231+
since: '2026-08-01T00:00:00.000Z',
232+
})
233+
234+
expect(url).toContain('since=2026-08-01T00%3A00%3A00.000Z')
235+
expect(url).not.toContain('after=')
236+
})
204237
})
205238

206239
describe('okta get_logs pagination termination', () => {
@@ -303,6 +336,92 @@ describe('okta lifecycle flags are coerced rather than interpolated raw', () =>
303336
})
304337
})
305338

339+
describe('okta query-string flags are coerced rather than interpolated raw', () => {
340+
/**
341+
* Every one of these is `visibility: 'user-or-llm'` and typed `boolean` in
342+
* Okta's spec, so a direct or agent tool call can deliver `"yes"` for any of
343+
* them. Omission must still leave the parameter off entirely so Okta applies
344+
* its own documented default.
345+
*/
346+
const CASES: Array<{
347+
name: string
348+
build: (params: Record<string, unknown>) => string
349+
param: string
350+
base: Record<string, unknown>
351+
}> = [
352+
{
353+
name: 'remove_user_from_app.sendEmail',
354+
build: (params) => builtUrl(oktaRemoveUserFromAppTool.request.url, params),
355+
param: 'sendEmail',
356+
base: { ...AUTH, appId: '0oa1', userId: '00u1' },
357+
},
358+
{
359+
name: 'enroll_factor.activate',
360+
build: (params) => builtUrl(oktaEnrollFactorTool.request.url, params),
361+
param: 'activate',
362+
base: { ...AUTH, userId: '00u1', factorType: 'sms', provider: 'OKTA' },
363+
},
364+
{
365+
name: 'reset_factor.removeRecoveryEnrollment',
366+
build: (params) => builtUrl(oktaResetFactorTool.request.url, params),
367+
param: 'removeRecoveryEnrollment',
368+
base: { ...AUTH, userId: '00u1', factorId: 'fac1' },
369+
},
370+
{
371+
name: 'delete_group_rule.removeUsers',
372+
build: (params) => builtUrl(oktaDeleteGroupRuleTool.request.url, params),
373+
param: 'removeUsers',
374+
base: { ...AUTH, groupRuleId: '0pr1' },
375+
},
376+
{
377+
name: 'clear_user_sessions.oauthTokens',
378+
build: (params) => builtUrl(oktaClearUserSessionsTool.request.url, params),
379+
param: 'oauthTokens',
380+
base: { ...AUTH, userId: '00u1' },
381+
},
382+
{
383+
name: 'clear_user_sessions.forgetDevices',
384+
build: (params) => builtUrl(oktaClearUserSessionsTool.request.url, params),
385+
param: 'forgetDevices',
386+
base: { ...AUTH, userId: '00u1' },
387+
},
388+
{
389+
name: 'list_apps.includeNonDeleted',
390+
build: (params) => builtUrl(oktaListAppsTool.request.url, params),
391+
param: 'includeNonDeleted',
392+
base: { ...AUTH },
393+
},
394+
]
395+
396+
it.each(CASES)('$name coerces a stringy truthy to true', ({ build, param, base }) => {
397+
expect(build({ ...base, [param]: 'yes' })).toContain(`${param}=true`)
398+
})
399+
400+
it.each(CASES)('$name coerces a stringy falsy to false', ({ build, param, base }) => {
401+
expect(build({ ...base, [param]: 'false' })).toContain(`${param}=false`)
402+
})
403+
404+
it.each(CASES)('$name omits the param when undefined', ({ build, param, base }) => {
405+
expect(build(base)).not.toContain(`${param}=`)
406+
})
407+
408+
/**
409+
* `create_user.activate` is the one flag Okta itself defaults to `true`, so
410+
* omission must keep sending `true` rather than fall through the coercion.
411+
*/
412+
it('create_user.activate coerces a stringy value and still defaults to true', () => {
413+
const base = { ...AUTH, firstName: 'A', lastName: 'B', email: 'a@b.com' }
414+
415+
expect(builtUrl(oktaCreateUserTool.request.url, base)).toContain('activate=true')
416+
expect(builtUrl(oktaCreateUserTool.request.url, { ...base, activate: 'yes' })).toContain(
417+
'activate=true'
418+
)
419+
expect(builtUrl(oktaCreateUserTool.request.url, { ...base, activate: 'false' })).toContain(
420+
'activate=false'
421+
)
422+
})
423+
})
424+
306425
describe('okta update_group declarative fallback', () => {
307426
/**
308427
* `PUT /api/v1/groups/{groupId}` replaces an extensible profile wholesale, so

0 commit comments

Comments
 (0)