Skip to content

Commit 93f13f5

Browse files
waleedlatif1claude
waleedlatif1
and
claude
committed
fix(okta): address PR review — SSRF prevention, safe response parsing, consistent sendEmail
- Add validateOktaDomain() to prevent SSRF via user-supplied domain param - Fix 9 tools to check response.ok before calling response.json() - Make sendEmail query param explicit in deactivate_user and delete_user Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 1a4e90b commit 93f13f5

19 files changed

+208
-142
lines changed

‎apps/sim/tools/okta/activate_user.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { createLogger } from '@sim/logger'
2-
import type {
3-
OktaActivateUserParams,
4-
OktaActivateUserResponse,
5-
OktaApiError,
2+
import {
3+
type OktaActivateUserParams,
4+
type OktaActivateUserResponse,
5+
type OktaApiError,
6+
validateOktaDomain,
67
} from '@/tools/okta/types'
78
import type { ToolConfig } from '@/tools/types'
89

@@ -44,7 +45,7 @@ export const oktaActivateUserTool: ToolConfig<OktaActivateUserParams, OktaActiva
4445

4546
request: {
4647
url: (params) => {
47-
const domain = params.domain.replace(/^https?:\/\//, '').replace(/\/$/, '')
48+
const domain = validateOktaDomain(params.domain)
4849
const sendEmail = params.sendEmail !== false
4950
return `https://${domain}/api/v1/users/${encodeURIComponent(params.userId)}/lifecycle/activate?sendEmail=${sendEmail}`
5051
},

‎apps/sim/tools/okta/add_user_to_group.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { createLogger } from '@sim/logger'
2-
import type {
3-
OktaAddUserToGroupParams,
4-
OktaAddUserToGroupResponse,
5-
OktaApiError,
2+
import {
3+
type OktaAddUserToGroupParams,
4+
type OktaAddUserToGroupResponse,
5+
type OktaApiError,
6+
validateOktaDomain,
67
} from '@/tools/okta/types'
78
import type { ToolConfig } from '@/tools/types'
89

@@ -46,7 +47,7 @@ export const oktaAddUserToGroupTool: ToolConfig<
4647

4748
request: {
4849
url: (params) => {
49-
const domain = params.domain.replace(/^https?:\/\//, '').replace(/\/$/, '')
50+
const domain = validateOktaDomain(params.domain)
5051
return `https://${domain}/api/v1/groups/${encodeURIComponent(params.groupId)}/users/${encodeURIComponent(params.userId)}`
5152
},
5253
method: 'PUT',

‎apps/sim/tools/okta/create_group.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { createLogger } from '@sim/logger'
2-
import type {
3-
OktaApiError,
4-
OktaCreateGroupParams,
5-
OktaCreateGroupResponse,
6-
OktaGroup,
2+
import {
3+
type OktaApiError,
4+
type OktaCreateGroupParams,
5+
type OktaCreateGroupResponse,
6+
type OktaGroup,
7+
validateOktaDomain,
78
} from '@/tools/okta/types'
89
import type { ToolConfig } from '@/tools/types'
910

@@ -44,7 +45,7 @@ export const oktaCreateGroupTool: ToolConfig<OktaCreateGroupParams, OktaCreateGr
4445

4546
request: {
4647
url: (params) => {
47-
const domain = params.domain.replace(/^https?:\/\//, '').replace(/\/$/, '')
48+
const domain = validateOktaDomain(params.domain)
4849
return `https://${domain}/api/v1/groups`
4950
},
5051
method: 'POST',
@@ -61,15 +62,18 @@ export const oktaCreateGroupTool: ToolConfig<OktaCreateGroupParams, OktaCreateGr
6162
},
6263

6364
transformResponse: async (response: Response) => {
64-
const data: OktaGroup | OktaApiError = await response.json()
65-
6665
if (!response.ok) {
67-
const error = data as OktaApiError
66+
let error: OktaApiError = {}
67+
try {
68+
error = await response.json()
69+
} catch {
70+
// non-JSON error body
71+
}
6872
logger.error('Okta API request failed', { data: error, status: response.status })
6973
throw new Error(error.errorSummary || 'Failed to create group in Okta')
7074
}
7175

72-
const group = data as OktaGroup
76+
const group: OktaGroup = await response.json()
7377
return {
7478
success: true,
7579
output: {

‎apps/sim/tools/okta/create_user.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { createLogger } from '@sim/logger'
2-
import type {
3-
OktaApiError,
4-
OktaCreateUserParams,
5-
OktaCreateUserResponse,
6-
OktaUser,
2+
import {
3+
type OktaApiError,
4+
type OktaCreateUserParams,
5+
type OktaCreateUserResponse,
6+
type OktaUser,
7+
validateOktaDomain,
78
} from '@/tools/okta/types'
89
import type { ToolConfig } from '@/tools/types'
910

@@ -86,7 +87,7 @@ export const oktaCreateUserTool: ToolConfig<OktaCreateUserParams, OktaCreateUser
8687

8788
request: {
8889
url: (params) => {
89-
const domain = params.domain.replace(/^https?:\/\//, '').replace(/\/$/, '')
90+
const domain = validateOktaDomain(params.domain)
9091
const activate = params.activate !== false
9192
return `https://${domain}/api/v1/users?activate=${activate}`
9293
},
@@ -121,15 +122,18 @@ export const oktaCreateUserTool: ToolConfig<OktaCreateUserParams, OktaCreateUser
121122
},
122123

123124
transformResponse: async (response: Response) => {
124-
const data: OktaUser | OktaApiError = await response.json()
125-
126125
if (!response.ok) {
127-
const error = data as OktaApiError
126+
let error: OktaApiError = {}
127+
try {
128+
error = await response.json()
129+
} catch {
130+
// non-JSON error body
131+
}
128132
logger.error('Okta API request failed', { data: error, status: response.status })
129133
throw new Error(error.errorSummary || 'Failed to create user in Okta')
130134
}
131135

132-
const user = data as OktaUser
136+
const user: OktaUser = await response.json()
133137
return {
134138
success: true,
135139
output: {

‎apps/sim/tools/okta/deactivate_user.ts‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { createLogger } from '@sim/logger'
2-
import type {
3-
OktaApiError,
4-
OktaDeactivateUserParams,
5-
OktaDeactivateUserResponse,
2+
import {
3+
type OktaApiError,
4+
type OktaDeactivateUserParams,
5+
type OktaDeactivateUserResponse,
6+
validateOktaDomain,
67
} from '@/tools/okta/types'
78
import type { ToolConfig } from '@/tools/types'
89

@@ -47,12 +48,9 @@ export const oktaDeactivateUserTool: ToolConfig<
4748

4849
request: {
4950
url: (params) => {
50-
const domain = params.domain.replace(/^https?:\/\//, '').replace(/\/$/, '')
51-
const queryParams = new URLSearchParams()
52-
if (params.sendEmail) queryParams.append('sendEmail', 'true')
53-
const queryString = queryParams.toString()
54-
const base = `https://${domain}/api/v1/users/${encodeURIComponent(params.userId)}/lifecycle/deactivate`
55-
return queryString ? `${base}?${queryString}` : base
51+
const domain = validateOktaDomain(params.domain)
52+
const sendEmail = params.sendEmail === true
53+
return `https://${domain}/api/v1/users/${encodeURIComponent(params.userId)}/lifecycle/deactivate?sendEmail=${sendEmail}`
5654
},
5755
method: 'POST',
5856
headers: (params) => ({

‎apps/sim/tools/okta/delete_group.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { createLogger } from '@sim/logger'
2-
import type {
3-
OktaApiError,
4-
OktaDeleteGroupParams,
5-
OktaDeleteGroupResponse,
2+
import {
3+
type OktaApiError,
4+
type OktaDeleteGroupParams,
5+
type OktaDeleteGroupResponse,
6+
validateOktaDomain,
67
} from '@/tools/okta/types'
78
import type { ToolConfig } from '@/tools/types'
89

@@ -38,7 +39,7 @@ export const oktaDeleteGroupTool: ToolConfig<OktaDeleteGroupParams, OktaDeleteGr
3839

3940
request: {
4041
url: (params) => {
41-
const domain = params.domain.replace(/^https?:\/\//, '').replace(/\/$/, '')
42+
const domain = validateOktaDomain(params.domain)
4243
return `https://${domain}/api/v1/groups/${encodeURIComponent(params.groupId)}`
4344
},
4445
method: 'DELETE',

‎apps/sim/tools/okta/delete_user.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { createLogger } from '@sim/logger'
2-
import type { OktaApiError, OktaDeleteUserParams, OktaDeleteUserResponse } from '@/tools/okta/types'
2+
import {
3+
type OktaApiError,
4+
type OktaDeleteUserParams,
5+
type OktaDeleteUserResponse,
6+
validateOktaDomain,
7+
} from '@/tools/okta/types'
38
import type { ToolConfig } from '@/tools/types'
49

510
const logger = createLogger('OktaDeleteUser')
@@ -40,12 +45,9 @@ export const oktaDeleteUserTool: ToolConfig<OktaDeleteUserParams, OktaDeleteUser
4045

4146
request: {
4247
url: (params) => {
43-
const domain = params.domain.replace(/^https?:\/\//, '').replace(/\/$/, '')
44-
const queryParams = new URLSearchParams()
45-
if (params.sendEmail) queryParams.append('sendEmail', 'true')
46-
const queryString = queryParams.toString()
47-
const base = `https://${domain}/api/v1/users/${encodeURIComponent(params.userId)}`
48-
return queryString ? `${base}?${queryString}` : base
48+
const domain = validateOktaDomain(params.domain)
49+
const sendEmail = params.sendEmail === true
50+
return `https://${domain}/api/v1/users/${encodeURIComponent(params.userId)}?sendEmail=${sendEmail}`
4951
},
5052
method: 'DELETE',
5153
headers: (params) => ({

‎apps/sim/tools/okta/get_group.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { createLogger } from '@sim/logger'
2-
import type {
3-
OktaApiError,
4-
OktaGetGroupParams,
5-
OktaGetGroupResponse,
6-
OktaGroup,
2+
import {
3+
type OktaApiError,
4+
type OktaGetGroupParams,
5+
type OktaGetGroupResponse,
6+
type OktaGroup,
7+
validateOktaDomain,
78
} from '@/tools/okta/types'
89
import type { ToolConfig } from '@/tools/types'
910

@@ -38,7 +39,7 @@ export const oktaGetGroupTool: ToolConfig<OktaGetGroupParams, OktaGetGroupRespon
3839

3940
request: {
4041
url: (params) => {
41-
const domain = params.domain.replace(/^https?:\/\//, '').replace(/\/$/, '')
42+
const domain = validateOktaDomain(params.domain)
4243
return `https://${domain}/api/v1/groups/${encodeURIComponent(params.groupId)}`
4344
},
4445
method: 'GET',
@@ -50,15 +51,18 @@ export const oktaGetGroupTool: ToolConfig<OktaGetGroupParams, OktaGetGroupRespon
5051
},
5152

5253
transformResponse: async (response: Response) => {
53-
const data: OktaGroup | OktaApiError = await response.json()
54-
5554
if (!response.ok) {
56-
const error = data as OktaApiError
55+
let error: OktaApiError = {}
56+
try {
57+
error = await response.json()
58+
} catch {
59+
// non-JSON error body
60+
}
5761
logger.error('Okta API request failed', { data: error, status: response.status })
5862
throw new Error(error.errorSummary || 'Failed to get group from Okta')
5963
}
6064

61-
const group = data as OktaGroup
65+
const group: OktaGroup = await response.json()
6266
return {
6367
success: true,
6468
output: {

‎apps/sim/tools/okta/get_user.ts‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { createLogger } from '@sim/logger'
2-
import type {
3-
OktaApiError,
4-
OktaGetUserParams,
5-
OktaGetUserResponse,
6-
OktaUser,
2+
import {
3+
type OktaApiError,
4+
type OktaGetUserParams,
5+
type OktaGetUserResponse,
6+
type OktaUser,
7+
validateOktaDomain,
78
} from '@/tools/okta/types'
89
import type { ToolConfig } from '@/tools/types'
910

@@ -38,7 +39,7 @@ export const oktaGetUserTool: ToolConfig<OktaGetUserParams, OktaGetUserResponse>
3839

3940
request: {
4041
url: (params) => {
41-
const domain = params.domain.replace(/^https?:\/\//, '').replace(/\/$/, '')
42+
const domain = validateOktaDomain(params.domain)
4243
return `https://${domain}/api/v1/users/${encodeURIComponent(params.userId)}`
4344
},
4445
method: 'GET',
@@ -50,15 +51,18 @@ export const oktaGetUserTool: ToolConfig<OktaGetUserParams, OktaGetUserResponse>
5051
},
5152

5253
transformResponse: async (response: Response) => {
53-
const data: OktaUser | OktaApiError = await response.json()
54-
5554
if (!response.ok) {
56-
const error = data as OktaApiError
55+
let error: OktaApiError = {}
56+
try {
57+
error = await response.json()
58+
} catch {
59+
// non-JSON error body
60+
}
5761
logger.error('Okta API request failed', { data: error, status: response.status })
5862
throw new Error(error.errorSummary || 'Failed to get user from Okta')
5963
}
6064

61-
const user = data as OktaUser
65+
const user: OktaUser = await response.json()
6266
return {
6367
success: true,
6468
output: {

‎apps/sim/tools/okta/list_group_members.ts‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { createLogger } from '@sim/logger'
2-
import type {
3-
OktaApiError,
4-
OktaListGroupMembersParams,
5-
OktaListGroupMembersResponse,
6-
OktaUser,
2+
import {
3+
type OktaApiError,
4+
type OktaListGroupMembersParams,
5+
type OktaListGroupMembersResponse,
6+
type OktaUser,
7+
validateOktaDomain,
78
} from '@/tools/okta/types'
89
import type { ToolConfig } from '@/tools/types'
910

@@ -47,7 +48,7 @@ export const oktaListGroupMembersTool: ToolConfig<
4748

4849
request: {
4950
url: (params) => {
50-
const domain = params.domain.replace(/^https?:\/\//, '').replace(/\/$/, '')
51+
const domain = validateOktaDomain(params.domain)
5152
const queryParams = new URLSearchParams()
5253

5354
if (params.limit) queryParams.append('limit', params.limit.toString())
@@ -65,15 +66,20 @@ export const oktaListGroupMembersTool: ToolConfig<
6566
},
6667

6768
transformResponse: async (response: Response) => {
68-
const data: OktaUser[] | OktaApiError = await response.json()
69-
7069
if (!response.ok) {
71-
const error = data as OktaApiError
70+
let error: OktaApiError = {}
71+
try {
72+
error = await response.json()
73+
} catch {
74+
// non-JSON error body
75+
}
7276
logger.error('Okta API request failed', { data: error, status: response.status })
7377
throw new Error(error.errorSummary || 'Failed to list group members from Okta')
7478
}
7579

76-
const members = (data as OktaUser[]).map((user) => ({
80+
const data: OktaUser[] = await response.json()
81+
82+
const members = data.map((user) => ({
7783
id: user.id,
7884
status: user.status,
7985
firstName: user.profile?.firstName ?? null,

0 commit comments

Comments
 (0)