feat(core): Add SAML post and test endpoints #5595
Conversation
# Conflicts: # packages/cli/src/sso/saml/middleware/samlEnabledMiddleware.ts # packages/cli/src/sso/saml/samlHelpers.ts
flipswitchingmonkey
changed the base branch from
master
to
pay-156-change-user-update-endpoint
…e-internal-api-endpoints-to-set-up
n8n-assistant
bot
added
core
flipswitchingmonkey
changed the title
| */ | ||
| samlControllerProtected.post( | ||
| SamlUrls.config, | ||
| samlLicensedOwnerMiddleware, | ||
| async (req: SamlConfiguration.Update, res: express.Response) => { | ||
| if (isSamlPreferences(req.body)) { | ||
| const validationResult = await validate(req.body); | ||
| if (validationResult.length === 0) { |
There was a problem hiding this comment.
Better to return early?
if (!validationResult) throw new BadRequestError(...)
const result = await SamlService.getInstance().setSamlPreferences(req.body);
return res.send(result);
There was a problem hiding this comment.
No, validationresult is supposed to be empty (validate() returns an array with the number of errors it found, so ideally length=0)
| SamlUrls.configToggleEnabled, | ||
| samlLicensedOwnerMiddleware, | ||
| async (req: SamlConfiguration.Toggle, res: express.Response) => { | ||
| if (req.body.loginEnabled !== undefined) { |
There was a problem hiding this comment.
const { loginEnabled } = req.body;
if (loginEnabled === undefined ) throw new BadRequestError('....')
await SamlService.getInstance().setSamlPreferences({ loginEnabled: req.body.loginEnabled });
res.sendStatus(200);
| if (result?.binding === 'redirect') { | ||
| // forced client side redirect | ||
| return res.send(getInitSSOPostView(result.context)); | ||
| // return res.status(301).send(result.context.context); |
| samlControllerProtected.get( | ||
| SamlUrls.configTest, | ||
| async (req: express.Request, res: express.Response) => { | ||
| const testResult = await SamlService.getInstance().testSamlConnection(); |
There was a problem hiding this comment.
Should we handle the case when the testing connection fails? That way, we can let the front end know that there might be an issue with the settings.
There was a problem hiding this comment.
Yes this is just a very basic implementation for the moment, until I know how the frontend wants to call it. Right now it's a return of true/false for the test, rather than an exception. The entire test process may change, since it may be better to have the owner go through a full sso flow instead to confirm it is working.
| @@ -53,7 +61,7 @@ export class SamlService { | |||
| } | |||
|
|
|||
| constructor() { | |||
| this.loadSamlPreferences() | |||
| this.loadFromDbAndApplySamlPreferences() | |||
There was a problem hiding this comment.
Is it not enough to call loadFromDbAndApplySamlPreferences only in the init method?
|
i'll make the small changes as part of #5600 which already had many of them... |
flipswitchingmonkey
deleted the
pay-153-create-internal-api-endpoints-to-set-up
branch
|
Got released with |
* consolidate SSO settings * update saml settings * fix type error * limit user changes when saml is enabled * add test * add toggle endpoint and fetch metadata * rename enabled param * add handling of POST saml login request * add config test endpoint
Github issue / Community forum post (link here to close automatically):