From d3c83023f8d802d0cae54a27239d5c98668bf4d5 Mon Sep 17 00:00:00 2001 From: gregfromstl <17715009+gregfromstl@users.noreply.github.com> Date: Mon, 22 Jul 2024 20:58:26 +0000 Subject: [PATCH] Auth Refactor (#3779) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### TL;DR Improved the in-app wallet sign-in speed by refactoring OAuth path handling to make it more generic and efficient. ### What changed? - Standardized the OAuth path handling to make it more generic and reusable across multiple providers. - Updated `getLoginPath` to `getSocialAuthLoginPath` for OAuth providers like Google, Facebook, Apple, and Discord. - Improved login speed and reduced redundancy by streamlining the OAuth path logic. - Removed outdated and redundant code related to individual OAuth provider handling. ### How to test? 1. Test the sign-in process using various OAuth providers such as Google, Facebook, Apple, and Discord. 2. Ensure the redirection and sign-in speed are improved. 3. Verify that the in-app wallet sign-in works seamlessly without any errors. ### Why make this change? The changes were necessary to optimize and standardize the OAuth sign-in process, improving the overall user experience. The previous implementation had redundant code for individual providers, which was refactored to be more efficient and easier to maintain. --- --- ## PR-Codex overview This PR improves in-app wallet sign-in speed by updating authentication strategies and paths. ### Detailed summary - Updated OAuth authentication strategy to `SocialAuthOption` - Improved login path retrieval for social authentication providers - Refactored Discord-specific authentication to generic OAuth approach > The following files were skipped due to too many changes: `packages/thirdweb/src/wallets/in-app/native/auth/native-auth.ts` > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` --- .changeset/famous-bats-talk.md | 5 ++ apps/dashboard/package.json | 12 +++- .../wallets/shared/openOauthSignInWindow.ts | 7 ++- .../core/authentication/getLoginPath.ts | 7 ++- .../in-app/core/authentication/type.ts | 12 +--- .../wallets/in-app/native/auth/native-auth.ts | 57 ++++--------------- .../in-app/native/helpers/auth/middleware.ts | 11 +--- .../wallets/in-app/native/native-connector.ts | 25 ++------ .../web/lib/auth/{discord.ts => oauth.ts} | 56 ++++++++++++++---- .../src/wallets/in-app/web/lib/auth/utils.ts | 19 ------- .../wallets/in-app/web/lib/web-connector.ts | 19 ++----- 11 files changed, 95 insertions(+), 135 deletions(-) create mode 100644 .changeset/famous-bats-talk.md rename packages/thirdweb/src/wallets/in-app/web/lib/auth/{discord.ts => oauth.ts} (69%) delete mode 100644 packages/thirdweb/src/wallets/in-app/web/lib/auth/utils.ts diff --git a/.changeset/famous-bats-talk.md b/.changeset/famous-bats-talk.md new file mode 100644 index 00000000000..e2b97b4f688 --- /dev/null +++ b/.changeset/famous-bats-talk.md @@ -0,0 +1,5 @@ +--- +"thirdweb": patch +--- + +Improved in-app wallet sign-in speed diff --git a/apps/dashboard/package.json b/apps/dashboard/package.json index c96deddae18..50ee5b46773 100644 --- a/apps/dashboard/package.json +++ b/apps/dashboard/package.json @@ -166,8 +166,16 @@ "tw-components", "contract-ui" ], - "exclude": ["node_modules", "types", "tw-components"], + "exclude": [ + "node_modules", + "types", + "tw-components" + ], "entrypoints": [] }, - "browserslist": ["defaults", "unreleased versions", "not UCAndroid > 0"] + "browserslist": [ + "defaults", + "unreleased versions", + "not UCAndroid > 0" + ] } diff --git a/packages/thirdweb/src/react/web/wallets/shared/openOauthSignInWindow.ts b/packages/thirdweb/src/react/web/wallets/shared/openOauthSignInWindow.ts index fc298cf538c..4f38f55ad4d 100644 --- a/packages/thirdweb/src/react/web/wallets/shared/openOauthSignInWindow.ts +++ b/packages/thirdweb/src/react/web/wallets/shared/openOauthSignInWindow.ts @@ -1,5 +1,5 @@ import type { ThirdwebClient } from "../../../../client/client.js"; -import { getDiscordLoginPath } from "../../../../wallets/in-app/core/authentication/getLoginPath.js"; +import { getSocialAuthLoginPath } from "../../../../wallets/in-app/core/authentication/getLoginPath.js"; import type { InAppWalletSocialAuth } from "../../../../wallets/in-app/core/wallet/types.js"; import type { Ecosystem } from "../../../../wallets/in-app/web/types.js"; import type { Theme } from "../../../core/design-system/index.js"; @@ -30,8 +30,11 @@ function getOauthLoginPath( ecosystem?: Ecosystem, ) { switch (authOption) { + case "apple": + case "facebook": + case "google": case "discord": - return getDiscordLoginPath(client, ecosystem); + return getSocialAuthLoginPath(authOption, client, ecosystem); default: return ""; } diff --git a/packages/thirdweb/src/wallets/in-app/core/authentication/getLoginPath.ts b/packages/thirdweb/src/wallets/in-app/core/authentication/getLoginPath.ts index f1bf87b7e55..21a3b34468d 100644 --- a/packages/thirdweb/src/wallets/in-app/core/authentication/getLoginPath.ts +++ b/packages/thirdweb/src/wallets/in-app/core/authentication/getLoginPath.ts @@ -1,13 +1,14 @@ import type { ThirdwebClient } from "../../../../client/client.js"; import { getThirdwebBaseUrl } from "../../../../utils/domains.js"; +import type { SocialAuthOption } from "../../../../wallets/types.js"; import type { Ecosystem } from "../../web/types.js"; -// TODO: make this generic for all auth providers -export const getDiscordLoginPath = ( +export const getSocialAuthLoginPath = ( + authOption: SocialAuthOption, client: ThirdwebClient, ecosystem?: Ecosystem, ) => { - const baseUrl = `${getThirdwebBaseUrl("inAppWallet")}/api/2024-05-05/login/discord?clientId=${client.clientId}`; + const baseUrl = `${getThirdwebBaseUrl("inAppWallet")}/api/2024-05-05/login/${authOption}?clientId=${client.clientId}`; if (ecosystem?.partnerId) { return `${baseUrl}&ecosystemId=${ecosystem.id}&ecosystemPartnerId=${ecosystem.partnerId}`; } diff --git a/packages/thirdweb/src/wallets/in-app/core/authentication/type.ts b/packages/thirdweb/src/wallets/in-app/core/authentication/type.ts index 3268177f4b1..916f900ac56 100644 --- a/packages/thirdweb/src/wallets/in-app/core/authentication/type.ts +++ b/packages/thirdweb/src/wallets/in-app/core/authentication/type.ts @@ -65,7 +65,7 @@ export enum AuthProvider { } export type OauthOption = { - provider: AuthProvider; + strategy: SocialAuthOption; redirectUrl: string; }; @@ -207,13 +207,3 @@ export type GetUser = export type GetAuthenticatedUserParams = { client: ThirdwebClient; }; - -export const oauthStrategyToAuthProvider: Record< - SocialAuthOption, - AuthProvider -> = { - google: AuthProvider.GOOGLE, - facebook: AuthProvider.FACEBOOK, - apple: AuthProvider.APPLE, - discord: AuthProvider.DISCORD, -}; diff --git a/packages/thirdweb/src/wallets/in-app/native/auth/native-auth.ts b/packages/thirdweb/src/wallets/in-app/native/auth/native-auth.ts index 663ee2fb8eb..cf95b4d65b2 100644 --- a/packages/thirdweb/src/wallets/in-app/native/auth/native-auth.ts +++ b/packages/thirdweb/src/wallets/in-app/native/auth/native-auth.ts @@ -3,7 +3,7 @@ import type { CognitoUser } from "amazon-cognito-identity-js"; import { Auth } from "aws-amplify"; import * as WebBrowser from "expo-web-browser"; import type { ThirdwebClient } from "../../../../client/client.js"; -import { getDiscordLoginPath } from "../../core/authentication/getLoginPath.js"; +import { getSocialAuthLoginPath } from "../../core/authentication/getLoginPath.js"; import { AuthProvider, type AuthStoredTokenWithCookieReturnType, @@ -30,10 +30,8 @@ import { preAuth, } from "../helpers/auth/middleware.js"; import { - DOMAIN_URL_2023, ROUTE_AUTH_ENDPOINT_CALLBACK, ROUTE_AUTH_JWT_CALLBACK, - ROUTE_HEADLESS_OAUTH_LOGIN, } from "../helpers/constants.js"; import { createErrorMessage } from "../helpers/errors.js"; import { isDeviceSharePresentForUser } from "../helpers/storage/local.js"; @@ -207,48 +205,14 @@ export async function validateEmailOTP(options: { } export async function socialLogin( - oauthOptions: OauthOption, + auth: OauthOption, client: ThirdwebClient, ): Promise { - const encodedProvider = encodeURIComponent(oauthOptions.provider); - const headlessLoginLinkWithParams = `${ROUTE_HEADLESS_OAUTH_LOGIN}?authProvider=${encodedProvider}&baseUrl=${encodeURIComponent( - DOMAIN_URL_2023, - )}&platform=${encodeURIComponent("mobile")}`; + const loginUrl = `${getSocialAuthLoginPath(auth.strategy, client)}&redirectUrl=${encodeURIComponent(auth.redirectUrl)}`; - const resp = await fetch(headlessLoginLinkWithParams, { - headers: { - ...getSessionHeaders(), - }, - }); - - if (!resp.ok) { - const error = await resp.json(); - throw new Error(`Error getting headless sign in link: ${error.message}`); - } - - const json = await resp.json(); - - const { platformLoginLink } = json; - - // Temporary fork for discord until we migrate all methods to the new auth flow - const loginUrl = (() => { - if (oauthOptions.provider === AuthProvider.DISCORD) { - return `${getDiscordLoginPath(client)}&redirectUrl=${encodeURIComponent( - oauthOptions.redirectUrl, - )}`; - } else { - return `${platformLoginLink}?developerClientId=${encodeURIComponent( - client.clientId, - )}&platform=${encodeURIComponent("mobile")}&redirectUrl=${encodeURIComponent( - oauthOptions.redirectUrl, - )}&authOption=${encodedProvider}`; - } - })(); - - // TODO platform specific code should be extracted out const result = await WebBrowser.openAuthSessionAsync( loginUrl, - oauthOptions.redirectUrl, + auth.redirectUrl, { preferEphemeralSession: false, showTitle: false, @@ -262,19 +226,18 @@ export async function socialLogin( } if (result.type !== "success") { - throw new Error(`Can't sign in with ${oauthOptions.provider}: ${result}`); + throw new Error(`Can't sign in with ${auth.strategy}: ${result}`); } - const decodedUrl = decodeURIComponent(result.url); + const resultURL = new URL(result.url); + const authResult = resultURL.searchParams.get("authResult"); + const error = resultURL.searchParams.get("error"); - const parts = decodedUrl.split("?authResult="); - if (parts.length < 2) { - // assume error - const error = decodedUrl.split("?error=")?.[1]; + // assume error + if (error) { throw new Error(`Something went wrong: ${error}`); } - const authResult = parts[1]; if (!authResult) { throw new Error("No auth result found"); } diff --git a/packages/thirdweb/src/wallets/in-app/native/helpers/auth/middleware.ts b/packages/thirdweb/src/wallets/in-app/native/helpers/auth/middleware.ts index 7eb6b73d0ec..edf37ffdf6f 100644 --- a/packages/thirdweb/src/wallets/in-app/native/helpers/auth/middleware.ts +++ b/packages/thirdweb/src/wallets/in-app/native/helpers/auth/middleware.ts @@ -11,10 +11,7 @@ import { setWallerUserDetails, } from "../storage/local.js"; import { setUpNewUserWallet } from "../wallet/creation.js"; -import { - getCognitoRecoveryPasswordV1, - getCognitoRecoveryPasswordV2, -} from "../wallet/recoveryCode.js"; +import { getCognitoRecoveryPasswordV2 } from "../wallet/recoveryCode.js"; import { setUpShareForNewDevice } from "../wallet/retrieval.js"; export async function preAuth(args: { @@ -154,11 +151,7 @@ async function getRecoveryCode( return recoveryCode; } else { try { - // temporary fork for discord until we migrate all methods to the new auth flow - const code = await (storedToken.authProvider === AuthProvider.DISCORD - ? getCognitoRecoveryPasswordV2(client) - : getCognitoRecoveryPasswordV1(client)); - return code; + return await getCognitoRecoveryPasswordV2(client); } catch (e) { throw new Error("Something went wrong getting cognito recovery code"); } diff --git a/packages/thirdweb/src/wallets/in-app/native/native-connector.ts b/packages/thirdweb/src/wallets/in-app/native/native-connector.ts index b5fd22bc329..90d22afb1ee 100644 --- a/packages/thirdweb/src/wallets/in-app/native/native-connector.ts +++ b/packages/thirdweb/src/wallets/in-app/native/native-connector.ts @@ -9,7 +9,6 @@ import { type PreAuthArgsType, type SendEmailOtpReturnType, UserWalletStatus, - oauthStrategyToAuthProvider, } from "../core/authentication/type.js"; import type { InAppConnector } from "../core/interfaces/connector.js"; import { @@ -114,9 +113,8 @@ export class InAppNativeConnector implements InAppConnector { const ExpoLinking = require("expo-linking"); const redirectUrl = params.redirectUrl || (ExpoLinking.createURL("") as string); - const oauthProvider = oauthStrategyToAuthProvider[strategy]; return this.socialLogin({ - provider: oauthProvider, + strategy, redirectUrl, }); } @@ -181,14 +179,9 @@ export class InAppNativeConnector implements InAppConnector { return deleteActiveAccount({ client: this.options.client }); } - private async socialLogin( - oauthOption: OauthOption, - ): Promise { + private async socialLogin(auth: OauthOption): Promise { try { - const { storedToken } = await socialLogin( - oauthOption, - this.options.client, - ); + const { storedToken } = await socialLogin(auth, this.options.client); const account = await this.getAccount(); return { user: { @@ -199,17 +192,11 @@ export class InAppNativeConnector implements InAppConnector { }, }; } catch (error) { - console.error( - `Error while signing in with: ${oauthOption.provider}. ${error}`, - ); + console.error(`Error while signing in with: ${auth}. ${error}`); if (error instanceof Error) { - throw new Error( - `Error signing in with ${oauthOption.provider}: ${error.message}`, - ); + throw new Error(`Error signing in with ${auth}: ${error.message}`); } - throw new Error( - `An unknown error occurred signing in with ${oauthOption.provider}`, - ); + throw new Error(`An unknown error occurred signing in with ${auth}`); } } diff --git a/packages/thirdweb/src/wallets/in-app/web/lib/auth/discord.ts b/packages/thirdweb/src/wallets/in-app/web/lib/auth/oauth.ts similarity index 69% rename from packages/thirdweb/src/wallets/in-app/web/lib/auth/discord.ts rename to packages/thirdweb/src/wallets/in-app/web/lib/auth/oauth.ts index ff08173207a..050113d88cb 100644 --- a/packages/thirdweb/src/wallets/in-app/web/lib/auth/discord.ts +++ b/packages/thirdweb/src/wallets/in-app/web/lib/auth/oauth.ts @@ -1,23 +1,62 @@ import type { ThirdwebClient } from "../../../../../client/client.js"; import { getThirdwebBaseUrl } from "../../../../../utils/domains.js"; import type { AuthStoredTokenWithCookieReturnType } from "../../../../../wallets/in-app/core/authentication/type.js"; -import { getDiscordLoginPath } from "../../../core/authentication/getLoginPath.js"; +import type { SocialAuthOption } from "../../../../../wallets/types.js"; import type { Ecosystem } from "../../types.js"; import { DEFAULT_POP_UP_SIZE } from "./constants.js"; -import { closeWindow } from "./utils.js"; -export async function loginWithDiscord(options: { +const closeWindow = ({ + isWindowOpenedByFn, + win, + closeOpenedWindow, +}: { + win?: Window | null; + isWindowOpenedByFn: boolean; + closeOpenedWindow?: (openedWindow: Window) => void; +}) => { + if (isWindowOpenedByFn) { + win?.close(); + } else { + if (win && closeOpenedWindow) { + closeOpenedWindow(win); + } else if (win) { + win.close(); + } + } +}; + +export const getSocialAuthLoginPath = ( + authOption: SocialAuthOption, + client: ThirdwebClient, + ecosystem?: Ecosystem, +) => { + const baseUrl = `${getThirdwebBaseUrl("inAppWallet")}/api/2024-05-05/login/${authOption}?clientId=${client.clientId}`; + if (ecosystem?.partnerId) { + return `${baseUrl}&ecosystemId=${ecosystem.id}&ecosystemPartnerId=${ecosystem.partnerId}`; + } + if (ecosystem) { + return `${baseUrl}&ecosystemId=${ecosystem.id}`; + } + return baseUrl; +}; + +export const loginWithOauth = async (options: { + authOption: SocialAuthOption; client: ThirdwebClient; ecosystem?: Ecosystem; openedWindow?: Window | null | undefined; closeOpenedWindow?: ((openedWindow: Window) => void) | undefined; -}): Promise { +}): Promise => { let win = options.openedWindow; let isWindowOpenedByFn = false; if (!win) { win = window.open( - getDiscordLoginPath(options.client, options.ecosystem), - "Login to discord", + getSocialAuthLoginPath( + options.authOption, + options.client, + options.ecosystem, + ), + `Login to ${options.authOption}`, DEFAULT_POP_UP_SIZE, ); isWindowOpenedByFn = true; @@ -30,9 +69,6 @@ export async function loginWithDiscord(options: { (resolve, reject) => { // detect when the user closes the login window const pollTimer = window.setInterval(async () => { - if (!win) { - return; - } if (win.closed) { clearInterval(pollTimer); window.removeEventListener("message", messageListener); @@ -89,4 +125,4 @@ export async function loginWithDiscord(options: { }, ); return result; -} +}; diff --git a/packages/thirdweb/src/wallets/in-app/web/lib/auth/utils.ts b/packages/thirdweb/src/wallets/in-app/web/lib/auth/utils.ts deleted file mode 100644 index 4b02f5f3caa..00000000000 --- a/packages/thirdweb/src/wallets/in-app/web/lib/auth/utils.ts +++ /dev/null @@ -1,19 +0,0 @@ -export const closeWindow = ({ - isWindowOpenedByFn, - win, - closeOpenedWindow, -}: { - win?: Window | null; - isWindowOpenedByFn: boolean; - closeOpenedWindow?: (openedWindow: Window) => void; -}) => { - if (isWindowOpenedByFn) { - win?.close(); - } else { - if (win && closeOpenedWindow) { - closeOpenedWindow(win); - } else if (win) { - win.close(); - } - } -}; diff --git a/packages/thirdweb/src/wallets/in-app/web/lib/web-connector.ts b/packages/thirdweb/src/wallets/in-app/web/lib/web-connector.ts index dbc25cc8ae7..d5ef6d79e0d 100644 --- a/packages/thirdweb/src/wallets/in-app/web/lib/web-connector.ts +++ b/packages/thirdweb/src/wallets/in-app/web/lib/web-connector.ts @@ -10,13 +10,12 @@ import { type SendEmailOtpReturnType, type SingleStepAuthArgsType, UserWalletStatus, - oauthStrategyToAuthProvider, } from "../../core/authentication/type.js"; import type { InAppConnector } from "../../core/interfaces/connector.js"; import type { InAppWalletConstructorType } from "../types.js"; import { InAppWalletIframeCommunicator } from "../utils/iFrameCommunication/InAppWalletIframeCommunicator.js"; -import { loginWithDiscord } from "./auth/discord.js"; import { Auth, type AuthQuerierTypes } from "./auth/iframe-auth.js"; +import { loginWithOauth } from "./auth/oauth.js"; import { loginWithPasskey, registerPasskey } from "./auth/passkeys.js"; import { IFrameWallet } from "./in-app-account.js"; @@ -169,16 +168,6 @@ export class InAppWebConnector implements InAppConnector { phoneNumber: args.phoneNumber, }); } - case "apple": - case "facebook": - case "google": { - const oauthProvider = oauthStrategyToAuthProvider[strategy]; - return this.auth.loginWithOauth({ - oauthProvider, - closeOpenedWindow: args.closeOpenedWindow, - openedWindow: args.openedWindow, - }); - } case "jwt": { return this.auth.loginWithCustomJwt({ jwt: args.jwt, @@ -216,8 +205,12 @@ export class InAppWebConnector implements InAppConnector { }); return this.auth.loginWithAuthToken(authToken); } + case "apple": + case "facebook": + case "google": case "discord": { - const authToken = await loginWithDiscord({ + const authToken = await loginWithOauth({ + authOption: strategy, client: this.wallet.client, ecosystem: this.wallet.ecosystem, closeOpenedWindow: args.closeOpenedWindow,