From 41f97b05632b2983cc8ab2d6627da630d349a910 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sat, 18 Mar 2023 09:17:54 -0400 Subject: [PATCH] fix: Users should not be redirected to disabled authentication providers (#5055 * fix: Users should not be redirected to disabled authentication providers Re-enabled tests in plugin directory * Fix plugin http tests --- .jestconfig.json | 2 +- plugins/email/server/auth/email.test.ts | 37 +++++++++++++++++++ plugins/email/server/auth/email.ts | 18 ++++----- plugins/slack/server/api/hooks.test.ts | 2 +- .../processors/WebhookProcessor.test.ts | 2 +- server/models/User.ts | 11 ++++++ server/routes/api/index.ts | 3 +- 7 files changed, 60 insertions(+), 15 deletions(-) diff --git a/.jestconfig.json b/.jestconfig.json index ab0f48426..3c471750e 100644 --- a/.jestconfig.json +++ b/.jestconfig.json @@ -3,7 +3,7 @@ "projects": [ { "displayName": "server", - "roots": ["/server"], + "roots": ["/server", "/plugins"], "moduleNameMapper": { "^@server/(.*)$": "/server/$1", "^@shared/(.*)$": "/shared/$1" diff --git a/plugins/email/server/auth/email.test.ts b/plugins/email/server/auth/email.test.ts index 63aa332e5..02e47bf9f 100644 --- a/plugins/email/server/auth/email.test.ts +++ b/plugins/email/server/auth/email.test.ts @@ -2,6 +2,7 @@ import sharedEnv from "@shared/env"; import SigninEmail from "@server/emails/templates/SigninEmail"; import WelcomeEmail from "@server/emails/templates/WelcomeEmail"; import env from "@server/env"; +import { AuthenticationProvider } from "@server/models"; import { buildUser, buildGuestUser, buildTeam } from "@server/test/factories"; import { getTestServer } from "@server/test/support"; @@ -33,6 +34,42 @@ describe("email", () => { spy.mockRestore(); }); + it("should respond with success and email to be sent when user has SSO but disabled", async () => { + const spy = jest.spyOn(SigninEmail, "schedule"); + const team = await buildTeam({ + subdomain: "example", + }); + const user = await buildUser({ + teamId: team.id, + }); + + // Disable all the auth providers + await AuthenticationProvider.update( + { + enabled: false, + }, + { + where: { + enabled: true, + }, + } + ); + + const res = await server.post("/auth/email", { + body: { + email: user.email, + }, + headers: { + host: "example.localoutline.com", + }, + }); + const body = await res.json(); + expect(res.status).toEqual(200); + expect(body.success).toEqual(true); + expect(spy).toHaveBeenCalled(); + spy.mockRestore(); + }); + it("should not send email when user is on another subdomain but respond with success", async () => { env.URL = sharedEnv.URL = "http://localoutline.com"; env.SUBDOMAINS_ENABLED = sharedEnv.SUBDOMAINS_ENABLED = true; diff --git a/plugins/email/server/auth/email.ts b/plugins/email/server/auth/email.ts index 215bc417e..5d5735082 100644 --- a/plugins/email/server/auth/email.ts +++ b/plugins/email/server/auth/email.ts @@ -1,5 +1,4 @@ import Router from "koa-router"; -import { find } from "lodash"; import { Client } from "@shared/types"; import { parseDomain } from "@shared/utils/domains"; import InviteAcceptedEmail from "@server/emails/templates/InviteAcceptedEmail"; @@ -59,18 +58,15 @@ router.post( // If the user matches an email address associated with an SSO // provider then just forward them directly to that sign-in page if (user.authentications.length) { - const authProvider = find(team.authenticationProviders, { - id: user.authentications[0].authenticationProviderId, - }); - if (authProvider?.enabled) { - ctx.body = { - redirect: `${team.url}/auth/${authProvider?.name}`, - }; - return; - } + const authenticationProvider = + user.authentications[0].authenticationProvider; + ctx.body = { + redirect: `${team.url}/auth/${authenticationProvider?.name}`, + }; + return; } - // send email to users registered address with a short-lived token + // send email to users email address with a short-lived token await SigninEmail.schedule({ to: user.email, token: user.getEmailSigninToken(), diff --git a/plugins/slack/server/api/hooks.test.ts b/plugins/slack/server/api/hooks.test.ts index 3b334df83..851ffac3c 100644 --- a/plugins/slack/server/api/hooks.test.ts +++ b/plugins/slack/server/api/hooks.test.ts @@ -5,7 +5,7 @@ import { buildDocument, buildIntegration } from "@server/test/factories"; import { seed, getTestServer } from "@server/test/support"; import * as Slack from "../slack"; -jest.mock("@server/utils/slack", () => ({ +jest.mock("../slack", () => ({ post: jest.fn(), })); diff --git a/plugins/webhooks/server/processors/WebhookProcessor.test.ts b/plugins/webhooks/server/processors/WebhookProcessor.test.ts index 9c9fd8510..73bafa1be 100644 --- a/plugins/webhooks/server/processors/WebhookProcessor.test.ts +++ b/plugins/webhooks/server/processors/WebhookProcessor.test.ts @@ -4,7 +4,7 @@ import { UserEvent } from "@server/types"; import DeliverWebhookTask from "../tasks/DeliverWebhookTask"; import WebhookProcessor from "./WebhookProcessor"; -jest.mock("@server/queues/tasks/DeliverWebhookTask"); +jest.mock("../tasks/DeliverWebhookTask"); const ip = "127.0.0.1"; setupTestDatabase(); diff --git a/server/models/User.ts b/server/models/User.ts index 19ca60f2f..43008555a 100644 --- a/server/models/User.ts +++ b/server/models/User.ts @@ -36,6 +36,7 @@ import parseAttachmentIds from "@server/utils/parseAttachmentIds"; import { ValidationError } from "../errors"; import ApiKey from "./ApiKey"; import Attachment from "./Attachment"; +import AuthenticationProvider from "./AuthenticationProvider"; import Collection from "./Collection"; import CollectionUser from "./CollectionUser"; import NotificationSetting from "./NotificationSetting"; @@ -71,8 +72,18 @@ export enum UserRole { withAuthentications: { include: [ { + separate: true, model: UserAuthentication, as: "authentications", + include: [ + { + model: AuthenticationProvider, + as: "authenticationProvider", + where: { + enabled: true, + }, + }, + ], }, ], }, diff --git a/server/routes/api/index.ts b/server/routes/api/index.ts index d94756ab7..6dbce3a0c 100644 --- a/server/routes/api/index.ts +++ b/server/routes/api/index.ts @@ -53,8 +53,9 @@ api.use(apiWrapper()); api.use(editor()); // register package API routes before others to allow for overrides +const rootDir = env.ENVIRONMENT === "test" ? "" : "build"; glob - .sync("build/plugins/*/server/api/!(*.test).js") + .sync(path.join(rootDir, "plugins/*/server/api/!(*.test).[jt]s")) .forEach((filePath: string) => { // eslint-disable-next-line @typescript-eslint/no-var-requires const pkg: Router = require(path.join(process.cwd(), filePath)).default;