From 5f94c7295ab8594ed2ed615628214e869a02da2d Mon Sep 17 00:00:00 2001 From: Qing Fu <16662647+zz5840@users.noreply.github.com> Date: Fri, 1 Dec 2023 05:41:06 +0800 Subject: [PATCH] feat(oauth): limited discord server sign-in (#346) * feat(oauth): limited discord server sign-in * fix: typo * style: change undefined to optional * style: remove conditional operator --- backend/prisma/seed/config.seed.ts | 8 ++- .../oauth/exceptions/errorPage.exception.ts | 2 +- .../oauth/filter/errorPageException.filter.ts | 17 +++++- .../src/oauth/provider/discord.provider.ts | 54 ++++++++++++++++--- .../oauth/provider/genericOidc.provider.ts | 11 +++- backend/src/oauth/provider/github.provider.ts | 13 ++--- frontend/src/i18n/translations/en-US.ts | 5 ++ 7 files changed, 89 insertions(+), 21 deletions(-) diff --git a/backend/prisma/seed/config.seed.ts b/backend/prisma/seed/config.seed.ts index bc74a5b6..18730637 100644 --- a/backend/prisma/seed/config.seed.ts +++ b/backend/prisma/seed/config.seed.ts @@ -180,6 +180,10 @@ const configVariables: ConfigVariables = { type: "boolean", defaultValue: "false", }, + "discord-limitedGuild": { + type: "string", + defaultValue: "", + }, "discord-clientId": { type: "string", defaultValue: "", @@ -262,8 +266,8 @@ async function migrateConfigVariables() { for (const existingConfigVariable of existingConfigVariables) { const configVariable = configVariables[existingConfigVariable.category]?.[ - existingConfigVariable.name - ]; + existingConfigVariable.name + ]; if (!configVariable) { await prisma.config.delete({ where: { diff --git a/backend/src/oauth/exceptions/errorPage.exception.ts b/backend/src/oauth/exceptions/errorPage.exception.ts index 6ebb10a6..0bfd62a2 100644 --- a/backend/src/oauth/exceptions/errorPage.exception.ts +++ b/backend/src/oauth/exceptions/errorPage.exception.ts @@ -7,7 +7,7 @@ export class ErrorPageException extends Error { */ constructor( public readonly key: string = "default", - public readonly redirect: string = "/", + public readonly redirect?: string, public readonly params?: string[], ) { super("error"); diff --git a/backend/src/oauth/filter/errorPageException.filter.ts b/backend/src/oauth/filter/errorPageException.filter.ts index bc4b6858..055efa3d 100644 --- a/backend/src/oauth/filter/errorPageException.filter.ts +++ b/backend/src/oauth/filter/errorPageException.filter.ts @@ -9,14 +9,27 @@ export class ErrorPageExceptionFilter implements ExceptionFilter { constructor(private config: ConfigService) {} catch(exception: ErrorPageException, host: ArgumentsHost) { - this.logger.error(exception); + this.logger.error( + JSON.stringify({ + error: exception.key, + params: exception.params, + redirect: exception.redirect, + }), + ); const ctx = host.switchToHttp(); const response = ctx.getResponse(); const url = new URL(`${this.config.get("general.appUrl")}/error`); - url.searchParams.set("redirect", exception.redirect); url.searchParams.set("error", exception.key); + if (exception.redirect) { + url.searchParams.set("redirect", exception.redirect); + } else { + const redirect = ctx.getRequest().cookies.access_token + ? "/account" + : "/auth/signIn"; + url.searchParams.set("redirect", redirect); + } if (exception.params) { url.searchParams.set("params", exception.params.join(",")); } diff --git a/backend/src/oauth/provider/discord.provider.ts b/backend/src/oauth/provider/discord.provider.ts index 97a8b5d1..30bd2398 100644 --- a/backend/src/oauth/provider/discord.provider.ts +++ b/backend/src/oauth/provider/discord.provider.ts @@ -1,15 +1,19 @@ -import { OAuthProvider, OAuthToken } from "./oauthProvider.interface"; +import { Injectable } from "@nestjs/common"; +import fetch from "node-fetch"; +import { ConfigService } from "../../config/config.service"; import { OAuthCallbackDto } from "../dto/oauthCallback.dto"; import { OAuthSignInDto } from "../dto/oauthSignIn.dto"; -import { ConfigService } from "../../config/config.service"; -import { BadRequestException, Injectable } from "@nestjs/common"; -import fetch from "node-fetch"; - +import { ErrorPageException } from "../exceptions/errorPage.exception"; +import { OAuthProvider, OAuthToken } from "./oauthProvider.interface"; @Injectable() export class DiscordProvider implements OAuthProvider { constructor(private config: ConfigService) {} getAuthEndpoint(state: string): Promise { + let scope = "identify email"; + if (this.config.get("oauth.discord-limitedGuild")) { + scope += " guilds"; + } return Promise.resolve( "https://discord.com/api/oauth2/authorize?" + new URLSearchParams({ @@ -17,8 +21,8 @@ export class DiscordProvider implements OAuthProvider { redirect_uri: this.config.get("general.appUrl") + "/api/oauth/callback/discord", response_type: "code", - state: state, - scope: "identify email", + state, + scope, }).toString(), ); } @@ -69,7 +73,14 @@ export class DiscordProvider implements OAuthProvider { }); const user = (await res.json()) as DiscordUser; if (user.verified === false) { - throw new BadRequestException("Unverified account."); + throw new ErrorPageException("unverified_account", undefined, [ + "provider_discord", + ]); + } + + const guild = this.config.get("oauth.discord-limitedGuild"); + if (guild) { + await this.checkLimitedGuild(token, guild); } return { @@ -79,6 +90,24 @@ export class DiscordProvider implements OAuthProvider { email: user.email, }; } + + async checkLimitedGuild(token: OAuthToken, guildId: string) { + try { + const res = await fetch("https://discord.com/api/v10/users/@me/guilds", { + method: "get", + headers: { + Accept: "application/json", + Authorization: `${token.tokenType || "Bearer"} ${token.accessToken}`, + }, + }); + const guilds = (await res.json()) as DiscordPartialGuild[]; + if (!guilds.some((guild) => guild.id === guildId)) { + throw new ErrorPageException("discord_guild_permission_denied"); + } + } catch { + throw new ErrorPageException("discord_guild_permission_denied"); + } + } } export interface DiscordToken { @@ -96,3 +125,12 @@ export interface DiscordUser { email: string; verified: boolean; } + +export interface DiscordPartialGuild { + id: string; + name: string; + icon: string; + owner: boolean; + permissions: string; + features: string[]; +} diff --git a/backend/src/oauth/provider/genericOidc.provider.ts b/backend/src/oauth/provider/genericOidc.provider.ts index bec617d4..8bcd4c3e 100644 --- a/backend/src/oauth/provider/genericOidc.provider.ts +++ b/backend/src/oauth/provider/genericOidc.provider.ts @@ -1,4 +1,4 @@ -import { BadRequestException } from "@nestjs/common"; +import { Logger } from "@nestjs/common"; import fetch from "node-fetch"; import { ConfigService } from "../../config/config.service"; import { JwtService } from "@nestjs/jwt"; @@ -7,11 +7,15 @@ import { nanoid } from "nanoid"; import { OAuthCallbackDto } from "../dto/oauthCallback.dto"; import { OAuthProvider, OAuthToken } from "./oauthProvider.interface"; import { OAuthSignInDto } from "../dto/oauthSignIn.dto"; +import { ErrorPageException } from "../exceptions/errorPage.exception"; export abstract class GenericOidcProvider implements OAuthProvider { protected discoveryUri: string; private configuration: OidcConfigurationCache; private jwk: OidcJwkCache; + private logger: Logger = new Logger( + Object.getPrototypeOf(this).constructor.name, + ); protected constructor( protected name: string, @@ -112,7 +116,10 @@ export abstract class GenericOidcProvider implements OAuthProvider { const nonce = await this.cache.get(key); await this.cache.del(key); if (nonce !== idTokenData.nonce) { - throw new BadRequestException("Invalid token"); + this.logger.error( + `Invalid nonce. Expected ${nonce}, but got ${idTokenData.nonce}`, + ); + throw new ErrorPageException("invalid_token"); } return { diff --git a/backend/src/oauth/provider/github.provider.ts b/backend/src/oauth/provider/github.provider.ts index 45258f9d..9c3c131c 100644 --- a/backend/src/oauth/provider/github.provider.ts +++ b/backend/src/oauth/provider/github.provider.ts @@ -1,9 +1,10 @@ -import { OAuthProvider, OAuthToken } from "./oauthProvider.interface"; +import { Injectable } from "@nestjs/common"; +import fetch from "node-fetch"; +import { ConfigService } from "../../config/config.service"; import { OAuthCallbackDto } from "../dto/oauthCallback.dto"; import { OAuthSignInDto } from "../dto/oauthSignIn.dto"; -import { ConfigService } from "../../config/config.service"; -import fetch from "node-fetch"; -import { BadRequestException, Injectable } from "@nestjs/common"; +import { ErrorPageException } from "../exceptions/errorPage.exception"; +import { OAuthProvider, OAuthToken } from "./oauthProvider.interface"; @Injectable() export class GitHubProvider implements OAuthProvider { @@ -48,12 +49,12 @@ export class GitHubProvider implements OAuthProvider { async getUserInfo(token: OAuthToken): Promise { if (!token.scope.includes("user:email")) { - throw new BadRequestException("No email permission granted"); + throw new ErrorPageException("no_email", undefined, ["provider_github"]); } const user = await this.getGitHubUser(token); const email = await this.getGitHubEmail(token); if (!email) { - throw new BadRequestException("No email found"); + throw new ErrorPageException("no_email", undefined, ["provider_github"]); } return { diff --git a/frontend/src/i18n/translations/en-US.ts b/frontend/src/i18n/translations/en-US.ts index 36eefcc4..c56cd2ba 100644 --- a/frontend/src/i18n/translations/en-US.ts +++ b/frontend/src/i18n/translations/en-US.ts @@ -472,6 +472,8 @@ export default { "admin.config.oauth.microsoft-client-secret.description": "Client secret of the Microsoft OAuth app", "admin.config.oauth.discord-enabled": "Discord", "admin.config.oauth.discord-enabled.description": "Whether Discord login is enabled", + "admin.config.oauth.discord-limited-guild": "Discord limited server ID", + "admin.config.oauth.discord-limited-guild.description": "Limit signing in to users in a specific server. Leave it blank to disable.", "admin.config.oauth.discord-client-id": "Discord Client ID", "admin.config.oauth.discord-client-id.description": "Client ID of the Discord OAuth app", "admin.config.oauth.discord-client-secret": "Discord Client secret", @@ -496,10 +498,13 @@ export default { "error.msg.default": "Something went wrong.", "error.msg.access_denied": "You canceled the authentication process, please try again.", "error.msg.expired_token": "The authentication process took too long, please try again.", + "error.msg.invalid_token": "Internal Error", "error.msg.no_user": "User linked to this {0} account doesn't exist.", "error.msg.no_email": "Can't get email address from this {0} account.", "error.msg.already_linked": "This {0} account is already linked to another account.", "error.msg.not_linked": "This {0} account haven't linked to any account yet.", + "error.msg.unverified_account": "This {0} account is unverified, please try again after verification.", + "error.msg.discord_guild_permission_denied": "You are not allowed to sign in.", "error.param.provider_github": "GitHub", "error.param.provider_google": "Google", "error.param.provider_microsoft": "Microsoft",