From fd44f42f28c0fa2091876b138f170202d9fde04e Mon Sep 17 00:00:00 2001 From: Qing Fu <16662647+zz5840@users.noreply.github.com> Date: Sun, 12 Nov 2023 03:25:05 +0800 Subject: [PATCH] fix(oauth): github and discord login error (#323) * fix(oauth): github and discord login error fixed #322, fixed #302 * feat(oauth): print log when ErrorPageException occurs * refactor(oauth): migrate to Logger * feat(oauth): add logger for OAuthExceptionFilter * docs(oauth): update oauth login docs --- .../oauth/filter/errorPageException.filter.ts | 6 ++- .../src/oauth/filter/oauthException.filter.ts | 9 +++- .../src/oauth/provider/discord.provider.ts | 4 +- backend/src/oauth/provider/github.provider.ts | 3 +- docs/oauth2-guide.md | 45 +++++++------------ 5 files changed, 34 insertions(+), 33 deletions(-) diff --git a/backend/src/oauth/filter/errorPageException.filter.ts b/backend/src/oauth/filter/errorPageException.filter.ts index bdbbcf5..bc4b685 100644 --- a/backend/src/oauth/filter/errorPageException.filter.ts +++ b/backend/src/oauth/filter/errorPageException.filter.ts @@ -1,12 +1,16 @@ -import { ArgumentsHost, Catch, ExceptionFilter } from "@nestjs/common"; +import { ArgumentsHost, Catch, ExceptionFilter, Logger } from "@nestjs/common"; import { ConfigService } from "../../config/config.service"; import { ErrorPageException } from "../exceptions/errorPage.exception"; @Catch(ErrorPageException) export class ErrorPageExceptionFilter implements ExceptionFilter { + private readonly logger = new Logger(ErrorPageExceptionFilter.name); + constructor(private config: ConfigService) {} catch(exception: ErrorPageException, host: ArgumentsHost) { + this.logger.error(exception); + const ctx = host.switchToHttp(); const response = ctx.getResponse(); diff --git a/backend/src/oauth/filter/oauthException.filter.ts b/backend/src/oauth/filter/oauthException.filter.ts index 42f78d7..c5b8277 100644 --- a/backend/src/oauth/filter/oauthException.filter.ts +++ b/backend/src/oauth/filter/oauthException.filter.ts @@ -3,6 +3,7 @@ import { Catch, ExceptionFilter, HttpException, + Logger, } from "@nestjs/common"; import { ConfigService } from "../../config/config.service"; @@ -12,14 +13,20 @@ export class OAuthExceptionFilter implements ExceptionFilter { access_denied: "access_denied", expired_token: "expired_token", }; + private readonly logger = new Logger(OAuthExceptionFilter.name); constructor(private config: ConfigService) {} - catch(_exception: HttpException, host: ArgumentsHost) { + catch(exception: HttpException, host: ArgumentsHost) { const ctx = host.switchToHttp(); const response = ctx.getResponse(); const request = ctx.getRequest(); + this.logger.error(exception.message); + this.logger.error( + "Request query: " + JSON.stringify(request.query, null, 2), + ); + const key = this.errorKeys[request.query.error] || "default"; const url = new URL(`${this.config.get("general.appUrl")}/error`); diff --git a/backend/src/oauth/provider/discord.provider.ts b/backend/src/oauth/provider/discord.provider.ts index b14e5d9..97a8b5d 100644 --- a/backend/src/oauth/provider/discord.provider.ts +++ b/backend/src/oauth/provider/discord.provider.ts @@ -60,8 +60,8 @@ export class DiscordProvider implements OAuthProvider { } async getUserInfo(token: OAuthToken): Promise { - const res = await fetch("https://discord.com/api/v10/user/@me", { - method: "post", + const res = await fetch("https://discord.com/api/v10/users/@me", { + method: "get", headers: { Accept: "application/json", Authorization: `${token.tokenType || "Bearer"} ${token.accessToken}`, diff --git a/backend/src/oauth/provider/github.provider.ts b/backend/src/oauth/provider/github.provider.ts index bf1ae62..45258f9 100644 --- a/backend/src/oauth/provider/github.provider.ts +++ b/backend/src/oauth/provider/github.provider.ts @@ -41,15 +41,16 @@ export class GitHubProvider implements OAuthProvider { return { accessToken: token.access_token, tokenType: token.token_type, + scope: token.scope, rawToken: token, }; } async getUserInfo(token: OAuthToken): Promise { - const user = await this.getGitHubUser(token); if (!token.scope.includes("user:email")) { throw new BadRequestException("No email permission granted"); } + const user = await this.getGitHubUser(token); const email = await this.getGitHubEmail(token); if (!email) { throw new BadRequestException("No email found"); diff --git a/docs/oauth2-guide.md b/docs/oauth2-guide.md index bcc99ea..814c5e6 100644 --- a/docs/oauth2-guide.md +++ b/docs/oauth2-guide.md @@ -10,23 +10,22 @@ ### GitHub -Please follow the [official guide](https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/creating-an-oauth-app) -to create an OAuth app. +Please follow the [official guide](https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/creating-an-oauth-app) to create an OAuth app. Redirect URL: `https:///api/oauth/callback/github` ### Google -Please follow the [official guide](https://developers.google.com/identity/protocols/oauth2/web-server#prerequisites) to -create an OAuth 2.0 App. +Please follow the [official guide](https://developers.google.com/identity/protocols/oauth2/web-server#prerequisites) to create an OAuth 2.0 App. Redirect URL: `https:///api/oauth/callback/google` ### Microsoft -Please follow -the [official guide](https://docs.microsoft.com/en-us/azure/active-directory/develop/quickstart-register-app) -to register an application. +Please follow the [official guide](https://docs.microsoft.com/en-us/azure/active-directory/develop/quickstart-register-app) to register an application. + +> [!IMPORTANT] +> **Microsoft Tenant** you set in the admin panel must match the **supported account types** you set in the Microsoft Entra admin center, otherwise the OAuth login will not work. Refer to the [official documentation](https://learn.microsoft.com/en-us/entra/identity-platform/v2-protocols-oidc#find-your-apps-openid-configuration-document-uri) for more details. Redirect URL: `https:///api/oauth/callback/microsoft` @@ -38,7 +37,7 @@ Redirect URL: `https:///api/oauth/callback/discord` ### OpenID Connect -Generic OpenID Connect provider is also supported, we have tested it on Keycloak and Authentik. +Generic OpenID Connect provider is also supported, we have tested it on Keycloak, Authentik and Casdoor. Redirect URL: `https:///api/oauth/callback/oidc` @@ -74,14 +73,11 @@ const configVariables: ConfigVariables = { ### 2. Create provider class -#### OpenID Connect +#### Generic OpenID Connect -If your provider supports OpenID connect, it's extremely easy to -extend [`GenericOidcProvider`](../backend/src/oauth/provider/genericOidc.provider.ts) to add a new OpenID Connect -provider. +If your provider supports OpenID connect, it's extremely easy to extend [`GenericOidcProvider`](../backend/src/oauth/provider/genericOidc.provider.ts) to add a new OpenID Connect provider. -The [Google provider](../backend/src/oauth/provider/google.provider.ts) -and [Microsoft provider](../backend/src/oauth/provider/microsoft.provider.ts) are good examples. +The [Google provider](../backend/src/oauth/provider/google.provider.ts) and [Microsoft provider](../backend/src/oauth/provider/microsoft.provider.ts) are good examples. Here are some discovery URIs for popular providers: @@ -95,17 +91,13 @@ Here are some discovery URIs for popular providers: #### OAuth 2 -If your provider only supports OAuth 2, you can -implement [`OAuthProvider`](../backend/src/oauth/provider/oauthProvider.interface.ts) interface to add a new OAuth 2 -provider. +If your provider only supports OAuth 2, you can implement [`OAuthProvider`](../backend/src/oauth/provider/oauthProvider.interface.ts) interface to add a new OAuth 2 provider. -The [GitHub provider](../backend/src/oauth/provider/github.provider.ts) -and [Discord provider](../backend/src/oauth/provider/discord.provider.ts) are good examples. +The [GitHub provider](../backend/src/oauth/provider/github.provider.ts) and [Discord provider](../backend/src/oauth/provider/discord.provider.ts) are good examples. ### 3. Register provider -Register your provider in [`OAuthModule`](../backend/src/oauth/oauth.module.ts) -and [`OAuthSignInDto`](../backend/src/oauth/dto/oauthSignIn.dto.ts): +Register your provider in [`OAuthModule`](../backend/src/oauth/oauth.module.ts) and [`OAuthSignInDto`](../backend/src/oauth/dto/oauthSignIn.dto.ts): ```ts @Module({ @@ -117,8 +109,7 @@ and [`OAuthSignInDto`](../backend/src/oauth/dto/oauthSignIn.dto.ts): useFactory(github: GitHubProvider, /* your provider */): Record> { return { github, - google, - oidc, + /* your provider */ }; }, inject: [GitHubProvider, /* your provider */], @@ -131,8 +122,7 @@ export class OAuthModule { ```ts export interface OAuthSignInDto { - provider: 'github' | 'google' | 'microsoft' | 'discord' | 'oidc' /* your provider*/ - ; + provider: 'github' | 'google' | 'microsoft' | 'discord' | 'oidc' /* your provider*/; providerId: string; providerUsername: string; email: string; @@ -161,8 +151,7 @@ Add keys below to your i18n text in [locale file](../frontend/src/i18n/translati - `admin.config.oauth.YOUR_PROVIDER_NAME-enabled` - `admin.config.oauth.YOUR_PROVIDER_NAME-client-id` - `admin.config.oauth.YOUR_PROVIDER_NAME-client-secret` +- `error.param.provider_YOUR_PROVIDER_NAME` - Other config keys you defined in step 1 -Congratulations! 🎉 You have successfully added a new OAuth 2 provider! Pull requests are welcome if you want to share -your provider with others. - +Congratulations! 🎉 You have successfully added a new OAuth 2 provider! Pull requests are welcome if you want to share your provider with others.