Skip to content

Commit c22f2d3

Browse files
0xi4oyau-wd
0xi4o
and
yau-wd
authored
fix: read loginmethod (#5805)
* fix: auth for read loginmethod endpoint * fix: review feedback * fix: lint issues * fix: validate auth0 domain * fix: lint issues * fix: wrong loginmethod endpoint in org setup page --------- Co-authored-by: yau-wd <yau.ong@workday.com>
1 parent 5cc657b commit c22f2d3

7 files changed

Lines changed: 136 additions & 24 deletions

File tree

‎packages/server/src/enterprise/controllers/login-method.controller.ts‎

Lines changed: 48 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,26 @@ import GoogleSSO from '../sso/GoogleSSO'
1414
import { decrypt } from '../utils/encryption.util'
1515

1616
export class LoginMethodController {
17+
constructor() {
18+
this.create = this.create.bind(this)
19+
this.read = this.read.bind(this)
20+
this.update = this.update.bind(this)
21+
this.defaultMethods = this.defaultMethods.bind(this)
22+
this.testConfig = this.testConfig.bind(this)
23+
}
24+
1725
private assertEnterprisePlatform(): void {
1826
const platformType = getRunningExpressApp().identityManager.getPlatformType()
1927
if (platformType === Platform.CLOUD || platformType === Platform.OPEN_SOURCE) {
2028
throw new InternalFlowiseError(StatusCodes.FORBIDDEN, GeneralErrorMessage.FORBIDDEN)
2129
}
2230
}
2331

32+
private async getSafeConfig(encryptedConfig: string): Promise<Record<string, unknown>> {
33+
const { clientSecret: _, ...safe } = JSON.parse(await decrypt(encryptedConfig)) as Record<string, unknown>
34+
return safe
35+
}
36+
2437
public async create(req: Request, res: Response, next: NextFunction) {
2538
try {
2639
this.assertEnterprisePlatform()
@@ -73,6 +86,10 @@ export class LoginMethodController {
7386
let queryRunner
7487
try {
7588
this.assertEnterprisePlatform()
89+
const user = (req as any).user
90+
if (!user?.activeOrganizationId) {
91+
throw new InternalFlowiseError(StatusCodes.FORBIDDEN, GeneralErrorMessage.FORBIDDEN)
92+
}
7693
queryRunner = getRunningExpressApp().AppDataSource.createQueryRunner()
7794
await queryRunner.connect()
7895
const query = req.query as Partial<LoginMethod>
@@ -91,12 +108,18 @@ export class LoginMethodController {
91108
if (query.id) {
92109
loginMethod = await loginMethodService.readLoginMethodById(query.id, queryRunner)
93110
if (!loginMethod) throw new InternalFlowiseError(StatusCodes.NOT_FOUND, LoginMethodErrorMessage.LOGIN_METHOD_NOT_FOUND)
94-
loginMethod.config = JSON.parse(await decrypt(loginMethod.config))
111+
if (loginMethod.organizationId !== user.activeOrganizationId) {
112+
throw new InternalFlowiseError(StatusCodes.FORBIDDEN, GeneralErrorMessage.FORBIDDEN)
113+
}
114+
loginMethod.config = await this.getSafeConfig(loginMethod.config)
95115
} else if (query.organizationId) {
116+
if (query.organizationId !== user.activeOrganizationId) {
117+
throw new InternalFlowiseError(StatusCodes.FORBIDDEN, GeneralErrorMessage.FORBIDDEN)
118+
}
96119
loginMethod = await loginMethodService.readLoginMethodByOrganizationId(query.organizationId, queryRunner)
97120

98121
for (let method of loginMethod) {
99-
method.config = JSON.parse(await decrypt(method.config))
122+
method.config = await this.getSafeConfig(method.config)
100123
}
101124
loginMethodConfig.providers = loginMethod
102125
} else {
@@ -131,25 +154,39 @@ export class LoginMethodController {
131154
}
132155
}
133156
public async testConfig(req: Request, res: Response, next: NextFunction) {
157+
let queryRunner
134158
try {
135-
const providers = req.body.providers
136-
if (req.body.providerName === 'azure') {
137-
const response = await AzureSSO.testSetup(providers[0].config)
159+
const providers = req.body.providers as { config: Record<string, unknown> }[]
160+
const providerName = req.body.providerName as string
161+
const organizationId = req.body.organizationId as string | undefined
162+
let config = providers[0]?.config ?? {}
163+
164+
if (organizationId) {
165+
queryRunner = getRunningExpressApp().AppDataSource.createQueryRunner()
166+
await queryRunner.connect()
167+
const loginMethodService = new LoginMethodService()
168+
config = await loginMethodService.getConfigWithSecrets(organizationId, providerName, config, queryRunner)
169+
}
170+
171+
if (providerName === 'azure') {
172+
const response = await AzureSSO.testSetup(config)
138173
return res.json(response)
139-
} else if (req.body.providerName === 'google') {
140-
const response = await GoogleSSO.testSetup(providers[0].config)
174+
} else if (providerName === 'google') {
175+
const response = await GoogleSSO.testSetup(config)
141176
return res.json(response)
142-
} else if (req.body.providerName === 'auth0') {
143-
const response = await Auth0SSO.testSetup(providers[0].config)
177+
} else if (providerName === 'auth0') {
178+
const response = await Auth0SSO.testSetup(config)
144179
return res.json(response)
145-
} else if (req.body.providerName === 'github') {
146-
const response = await GithubSSO.testSetup(providers[0].config)
180+
} else if (providerName === 'github') {
181+
const response = await GithubSSO.testSetup(config)
147182
return res.json(response)
148183
} else {
149184
return res.json({ error: 'Provider not supported' })
150185
}
151186
} catch (error) {
152187
next(error)
188+
} finally {
189+
if (queryRunner) await queryRunner.release()
153190
}
154191
}
155192
}

‎packages/server/src/enterprise/routes/login-method.route.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { checkPermission } from '../rbac/PermissionCheck'
55
const router = express.Router()
66
const loginMethodController = new LoginMethodController()
77

8-
router.get('/', loginMethodController.read)
8+
router.get('/', checkPermission('sso:manage'), loginMethodController.read)
99

1010
router.get('/default', loginMethodController.defaultMethods)
1111

‎packages/server/src/enterprise/services/login-method.service.ts‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,35 @@ export class LoginMethodService {
7171
return await queryRunner.manager.save(LoginMethod, data)
7272
}
7373

74+
private isPlaceholderSecret(value: unknown): boolean {
75+
return !value || (typeof value === 'string' && /^\*+$/.test(value))
76+
}
77+
78+
private mergeWithStoredClientSecret(incoming: Record<string, unknown>, existing: Record<string, unknown>): Record<string, unknown> {
79+
const sent = incoming.clientSecret
80+
if (this.isPlaceholderSecret(sent) && existing.clientSecret) {
81+
return { ...incoming, clientSecret: existing.clientSecret }
82+
}
83+
return { ...incoming }
84+
}
85+
86+
/**
87+
* Returns config with clientSecret filled from stored config when the incoming value is a placeholder (empty or asterisks).
88+
* Used for both testing and saving so logic stays in one place.
89+
*/
90+
public async getConfigWithSecrets(
91+
organizationId: string,
92+
providerName: string,
93+
incomingConfig: Record<string, unknown>,
94+
queryRunner: QueryRunner
95+
): Promise<Record<string, unknown>> {
96+
const methods = await this.readLoginMethodByOrganizationId(organizationId, queryRunner)
97+
const existingProvider = methods?.find((m) => m.name === providerName)
98+
if (!existingProvider?.config) return { ...incomingConfig }
99+
const existing = JSON.parse(await this.decryptLoginMethodConfig(existingProvider.config)) as Record<string, unknown>
100+
return this.mergeWithStoredClientSecret(incomingConfig, existing)
101+
}
102+
74103
public async createLoginMethod(data: Partial<LoginMethod>) {
75104
let queryRunner: QueryRunner | undefined
76105
let newLoginMethod: Partial<LoginMethod>
@@ -121,14 +150,17 @@ export class LoginMethodService {
121150

122151
const name = provider.providerName
123152
const loginMethod = await queryRunner.manager.findOneBy(LoginMethod, { organizationId, name })
153+
let configToSave: Record<string, unknown>
124154
if (loginMethod) {
125-
/* empty */
155+
const existing = JSON.parse(await this.decryptLoginMethodConfig(loginMethod.config)) as Record<string, unknown>
156+
configToSave = this.mergeWithStoredClientSecret(provider.config, existing)
126157
loginMethod.status = provider.status
127-
loginMethod.config = await this.encryptLoginMethodConfig(JSON.stringify(provider.config))
158+
loginMethod.config = await this.encryptLoginMethodConfig(JSON.stringify(configToSave))
128159
loginMethod.updatedBy = userId
129160
await this.saveLoginMethod(loginMethod, queryRunner)
130161
} else {
131-
const encryptedConfig = await this.encryptLoginMethodConfig(JSON.stringify(provider.config))
162+
configToSave = { ...provider.config }
163+
const encryptedConfig = await this.encryptLoginMethodConfig(JSON.stringify(configToSave))
132164
let newLoginMethod = queryRunner.manager.create(LoginMethod, {
133165
organizationId,
134166
name,

‎packages/server/src/enterprise/sso/Auth0SSO.ts‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,32 @@ import axios from 'axios'
1010

1111
const PROVIDER_NAME_AUTH0_SSO = 'Auth0 SSO'
1212

13+
function validateAuth0Domain(domain: string): string | null {
14+
if (!domain || typeof domain !== 'string') {
15+
return null
16+
}
17+
18+
const trimmed = domain.trim()
19+
20+
// Reject characters that could introduce scheme, port, path, or query
21+
if (/[/\\?#:]/.test(trimmed)) {
22+
return null
23+
}
24+
25+
// Basic hostname validation
26+
const hostnameRegex = /^(?=.{1,253}$)([a-zA-Z0-9-]{1,63}\.)+[a-zA-Z]{2,63}$/
27+
if (!hostnameRegex.test(trimmed)) {
28+
return null
29+
}
30+
31+
// Restrict to Auth0 domains
32+
if (!trimmed.toLowerCase().endsWith('.auth0.com')) {
33+
return null
34+
}
35+
36+
return trimmed
37+
}
38+
1339
class Auth0SSO extends SSOBase {
1440
static LOGIN_URI = '/api/v1/auth0/login'
1541
static CALLBACK_URI = '/api/v1/auth0/callback'
@@ -113,13 +139,19 @@ class Auth0SSO extends SSOBase {
113139
static async testSetup(ssoConfig: any) {
114140
const { domain, clientID, clientSecret } = ssoConfig
115141

142+
const validatedDomain = validateAuth0Domain(domain)
143+
if (!validatedDomain) {
144+
const errorMessage = 'Auth0 Configuration test failed. Invalid Auth0 domain.'
145+
return { error: errorMessage }
146+
}
147+
116148
try {
117149
const tokenResponse = await axios.post(
118-
`https://${domain}/oauth/token`,
150+
`https://${validatedDomain}/oauth/token`,
119151
{
120152
client_id: clientID,
121153
client_secret: clientSecret,
122-
audience: `https://${domain}/api/v2/`,
154+
audience: `https://${validatedDomain}/api/v2/`,
123155
grant_type: 'client_credentials'
124156
},
125157
{
@@ -136,9 +168,15 @@ class Auth0SSO extends SSOBase {
136168
async refreshToken(ssoRefreshToken: string) {
137169
const { domain, clientID, clientSecret } = this.ssoConfig
138170

171+
const validatedDomain = validateAuth0Domain(domain)
172+
if (!validatedDomain) {
173+
const errorMessage = 'Auth0 Configuration test failed. Invalid Auth0 domain.'
174+
return { error: errorMessage }
175+
}
176+
139177
try {
140178
const response = await axios.post(
141-
`https://${domain}/oauth/token`,
179+
`https://${validatedDomain}/oauth/token`,
142180
{
143181
client_id: clientID,
144182
client_secret: clientSecret,

‎packages/server/src/utils/constants.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export const WHITELIST_URLS = [
3333
'/api/v1/account/forgot-password',
3434
'/api/v1/account/reset-password',
3535
'/api/v1/account/basic-auth',
36-
'/api/v1/loginmethod',
36+
'/api/v1/loginmethod/default',
3737
'/api/v1/pricing',
3838
'/api/v1/user/test',
3939
'/api/v1/oauth2-credential/callback',

‎packages/ui/src/views/auth/ssoConfig.jsx‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ import GithubSVG from '@/assets/images/github.svg'
3333
// const
3434
import { gridSpacing } from '@/store/constant'
3535

36+
// API never sends clientSecret; show asterisks when a secret is already configured
37+
const PLACEHOLDER_SECRET = '********'
38+
3639
const SSOConfigPage = () => {
3740
useNotifier()
3841
const { error, setError } = useError()
@@ -347,7 +350,8 @@ const SSOConfigPage = () => {
347350
if (azureConfig) {
348351
setAzureTenantID(azureConfig.config.tenantID)
349352
setAzureClientID(azureConfig.config.clientID)
350-
setAzureClientSecret(azureConfig.config.clientSecret)
353+
const hasConfig = azureConfig.config.tenantID || azureConfig.config.clientID
354+
setAzureClientSecret(azureConfig.config.clientSecret ?? (hasConfig ? PLACEHOLDER_SECRET : ''))
351355
setAzureConfigEnabled(azureConfig.status === 'enable')
352356
}
353357
const googleConfig = data.providers.find((provider) => provider.name === 'google')
@@ -357,7 +361,7 @@ const SSOConfigPage = () => {
357361
}
358362
if (googleConfig) {
359363
setGoogleClientID(googleConfig.config.clientID)
360-
setGoogleClientSecret(googleConfig.config.clientSecret)
364+
setGoogleClientSecret(googleConfig.config.clientSecret ?? (googleConfig.config.clientID ? PLACEHOLDER_SECRET : ''))
361365
setGoogleConfigEnabled(googleConfig.status === 'enable')
362366
}
363367
const auth0Config = data.providers.find((provider) => provider.name === 'auth0')
@@ -369,7 +373,8 @@ const SSOConfigPage = () => {
369373
if (auth0Config) {
370374
setAuth0Domain(auth0Config.config.domain)
371375
setAuth0ClientID(auth0Config.config.clientID)
372-
setAuth0ClientSecret(auth0Config.config.clientSecret)
376+
const hasConfig = auth0Config.config.domain || auth0Config.config.clientID
377+
setAuth0ClientSecret(auth0Config.config.clientSecret ?? (hasConfig ? PLACEHOLDER_SECRET : ''))
373378
setAuth0ConfigEnabled(auth0Config.status === 'enable')
374379
}
375380

@@ -380,7 +385,7 @@ const SSOConfigPage = () => {
380385
}
381386
if (githubConfig) {
382387
setGithubClientID(githubConfig.config.clientID)
383-
setGithubClientSecret(githubConfig.config.clientSecret)
388+
setGithubClientSecret(githubConfig.config.clientSecret ?? (githubConfig.config.clientID ? PLACEHOLDER_SECRET : ''))
384389
setGithubConfigEnabled(githubConfig.status === 'enable')
385390
}
386391
setLoading(false)

‎packages/ui/src/views/organization/index.jsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ const OrganizationSetupPage = () => {
104104
const getBasicAuthApi = useApi(accountApi.getBasicAuth)
105105
const navigate = useNavigate()
106106

107-
const getDefaultProvidersApi = useApi(loginMethodApi.getLoginMethods)
107+
const getDefaultProvidersApi = useApi(loginMethodApi.getDefaultLoginMethods)
108108
const [configuredSsoProviders, setConfiguredSsoProviders] = useState([])
109109

110110
const register = async (event) => {

0 commit comments

Comments
 (0)