From 8e5e5ee46e51fd4f9f5e570f58553c50d523277f Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 22 Jun 2021 20:16:26 +0200 Subject: [PATCH 1/6] auth: show better error if auth provider keys are missing Signed-off-by: Himanshu Mishra --- plugins/auth-backend/src/service/router.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index fa551f6ec2..a79ad66179 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -139,7 +139,16 @@ export async function createRouter({ router.use('/:provider/', req => { const { provider } = req.params; - throw new NotFoundError(`No auth provider registered for '${provider}'`); + if (providers.includes(provider)) { + // If they added the provider under auth.providers but the clientId and clientSecret etc. were not found. + throw new NotFoundError( + `Auth provider registered for '${provider}' is misconfigured. This could mean the configs under ` + + `auth.providers.${provider} are missing or the environment variables used are not defined. ` + + `Check the auth backend plugin logs when the backend starts to see more details.`, + ); + } else { + throw new NotFoundError(`No auth provider registered for '${provider}'`); + } }); return router; From 72574ac4d2dcd370165c008b135ad56aacbdab92 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 22 Jun 2021 20:17:42 +0200 Subject: [PATCH 2/6] auth: add changeset for error improvement Signed-off-by: Himanshu Mishra --- .changeset/thirty-turtles-carry.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/thirty-turtles-carry.md diff --git a/.changeset/thirty-turtles-carry.md b/.changeset/thirty-turtles-carry.md new file mode 100644 index 0000000000..96b0495561 --- /dev/null +++ b/.changeset/thirty-turtles-carry.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-auth-backend': patch +--- + +Show better error message when configs defined under auth.providers. are undefined. From d91ec5b1b0cb2e6e17896cbda48716a093d2b990 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Tue, 22 Jun 2021 20:19:58 +0200 Subject: [PATCH 3/6] auth: update language Signed-off-by: Himanshu Mishra --- plugins/auth-backend/src/service/router.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index a79ad66179..548c551150 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -140,7 +140,7 @@ export async function createRouter({ router.use('/:provider/', req => { const { provider } = req.params; if (providers.includes(provider)) { - // If they added the provider under auth.providers but the clientId and clientSecret etc. were not found. + // If the user added the provider under auth.providers but the clientId and clientSecret etc. were not found. throw new NotFoundError( `Auth provider registered for '${provider}' is misconfigured. This could mean the configs under ` + `auth.providers.${provider} are missing or the environment variables used are not defined. ` + From dda7e862a2bccc48fc42a51bc732cc615809ef90 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Wed, 23 Jun 2021 15:13:02 +0200 Subject: [PATCH 4/6] remove the "catch-all" auth endpoint and register each separately 200: for properly registered auth providers 404 (with proper message): misconfigured 404 (plain): for unknown providers Co-authored-by: Mike Lewis Signed-off-by: Himanshu Mishra --- plugins/auth-backend/src/service/router.ts | 107 +++++++++++---------- 1 file changed, 55 insertions(+), 52 deletions(-) diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 548c551150..1945e6cb2d 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -86,47 +86,64 @@ export async function createRouter({ ...providerFactories, }; const providersConfig = config.getConfig('auth.providers'); - const providers = providersConfig.keys(); + const configuredProviders = providersConfig.keys(); - for (const providerId of providers) { - logger.info(`Configuring provider, ${providerId}`); - try { - const providerFactory = allProviderFactories[providerId]; - if (!providerFactory) { - throw Error(`No auth provider available for '${providerId}'`); + for (const providerId of Object.keys(allProviderFactories)) { + if (configuredProviders.includes(providerId)) { + logger.info(`Configuring provider, ${providerId}`); + try { + const providerFactory = allProviderFactories[providerId]; + if (!providerFactory) { + throw Error(`No auth provider available for '${providerId}'`); + } + + const provider = providerFactory({ + providerId, + globalConfig: { baseUrl: authUrl, appUrl }, + config: providersConfig.getConfig(providerId), + logger, + tokenIssuer, + discovery, + catalogApi, + }); + + const r = Router(); + + r.get('/start', provider.start.bind(provider)); + r.get('/handler/frame', provider.frameHandler.bind(provider)); + r.post('/handler/frame', provider.frameHandler.bind(provider)); + if (provider.logout) { + r.post('/logout', provider.logout.bind(provider)); + } + if (provider.refresh) { + r.get('/refresh', provider.refresh.bind(provider)); + } + + router.use(`/${providerId}`, r); + } catch (e) { + if (process.env.NODE_ENV !== 'development') { + throw new Error( + `Failed to initialize ${providerId} auth provider, ${e.message}`, + ); + } + + logger.warn(`Skipping ${providerId} auth provider, ${e.message}`); + + router.use(`/${providerId}`, () => { + // If the user added the provider under auth.providers but the clientId and clientSecret etc. were not found. + throw new NotFoundError( + `Auth provider registered for '${providerId}' is misconfigured. This could mean the configs under ` + + `auth.providers.${providerId} are missing or the environment variables used are not defined. ` + + `Check the auth backend plugin logs when the backend starts to see more details.`, + ); + }); } - - const provider = providerFactory({ - providerId, - globalConfig: { baseUrl: authUrl, appUrl }, - config: providersConfig.getConfig(providerId), - logger, - tokenIssuer, - discovery, - catalogApi, - }); - - const r = Router(); - - r.get('/start', provider.start.bind(provider)); - r.get('/handler/frame', provider.frameHandler.bind(provider)); - r.post('/handler/frame', provider.frameHandler.bind(provider)); - if (provider.logout) { - r.post('/logout', provider.logout.bind(provider)); - } - if (provider.refresh) { - r.get('/refresh', provider.refresh.bind(provider)); - } - - router.use(`/${providerId}`, r); - } catch (e) { - if (process.env.NODE_ENV !== 'development') { - throw new Error( - `Failed to initialize ${providerId} auth provider, ${e.message}`, + } else { + router.use(`/${providerId}`, () => { + throw new NotFoundError( + `No auth provider registered for '${providerId}'`, ); - } - - logger.warn(`Skipping ${providerId} auth provider, ${e.message}`); + }); } } @@ -137,19 +154,5 @@ export async function createRouter({ }), ); - router.use('/:provider/', req => { - const { provider } = req.params; - if (providers.includes(provider)) { - // If the user added the provider under auth.providers but the clientId and clientSecret etc. were not found. - throw new NotFoundError( - `Auth provider registered for '${provider}' is misconfigured. This could mean the configs under ` + - `auth.providers.${provider} are missing or the environment variables used are not defined. ` + - `Check the auth backend plugin logs when the backend starts to see more details.`, - ); - } else { - throw new NotFoundError(`No auth provider registered for '${provider}'`); - } - }); - return router; } From e7b5cf888f1b28a44d696ae0ceb0a0906bc53f2f Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 24 Jun 2021 11:32:57 +0200 Subject: [PATCH 5/6] remove unnecessary check for missing provider factory Signed-off-by: Himanshu Mishra --- plugins/auth-backend/src/service/router.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 1945e6cb2d..9841df399a 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -88,15 +88,12 @@ export async function createRouter({ const providersConfig = config.getConfig('auth.providers'); const configuredProviders = providersConfig.keys(); - for (const providerId of Object.keys(allProviderFactories)) { + for (const [providerId, providerFactory] of Object.entries( + allProviderFactories, + )) { if (configuredProviders.includes(providerId)) { logger.info(`Configuring provider, ${providerId}`); try { - const providerFactory = allProviderFactories[providerId]; - if (!providerFactory) { - throw Error(`No auth provider available for '${providerId}'`); - } - const provider = providerFactory({ providerId, globalConfig: { baseUrl: authUrl, appUrl }, From 686a740eb935ef0ee5e0f6ad0364fc49619d01e0 Mon Sep 17 00:00:00 2001 From: Himanshu Mishra Date: Thu, 24 Jun 2021 11:37:49 +0200 Subject: [PATCH 6/6] show better 404 message for unknown provider endpoints Signed-off-by: Himanshu Mishra --- plugins/auth-backend/src/service/router.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/auth-backend/src/service/router.ts b/plugins/auth-backend/src/service/router.ts index 9841df399a..e988fde97c 100644 --- a/plugins/auth-backend/src/service/router.ts +++ b/plugins/auth-backend/src/service/router.ts @@ -144,6 +144,11 @@ export async function createRouter({ } } + router.use('/:provider/', req => { + const { provider } = req.params; + throw new NotFoundError(`Unknown auth provider '${provider}'`); + }); + router.use( createOidcRouter({ tokenIssuer,