Skip to content

Commit

Permalink
Auth Refactor (#3779)
Browse files Browse the repository at this point in the history
### 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.

---

<!-- start pr-codex -->

---

## 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}`

<!-- end pr-codex -->
  • Loading branch information
gregfromstl committed Jul 22, 2024
1 parent a9a3f0f commit d3c8302
Show file tree
Hide file tree
Showing 11 changed files with 95 additions and 135 deletions.
5 changes: 5 additions & 0 deletions .changeset/famous-bats-talk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"thirdweb": patch
---

Improved in-app wallet sign-in speed
12 changes: 10 additions & 2 deletions apps/dashboard/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
]
}
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -30,8 +30,11 @@ function getOauthLoginPath(
ecosystem?: Ecosystem,
) {
switch (authOption) {
case "apple":
case "facebook":
case "google":

Check warning on line 35 in packages/thirdweb/src/react/web/wallets/shared/openOauthSignInWindow.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/react/web/wallets/shared/openOauthSignInWindow.ts#L33-L35

Added lines #L33 - L35 were not covered by tests
case "discord":
return getDiscordLoginPath(client, ecosystem);
return getSocialAuthLoginPath(authOption, client, ecosystem);

Check warning on line 37 in packages/thirdweb/src/react/web/wallets/shared/openOauthSignInWindow.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/react/web/wallets/shared/openOauthSignInWindow.ts#L37

Added line #L37 was not covered by tests
default:
return "";
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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,

Check warning on line 7 in packages/thirdweb/src/wallets/in-app/core/authentication/getLoginPath.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/in-app/core/authentication/getLoginPath.ts#L7

Added line #L7 was not covered by tests
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}`;

Check warning on line 11 in packages/thirdweb/src/wallets/in-app/core/authentication/getLoginPath.ts

View check run for this annotation

Codecov / codecov/patch

packages/thirdweb/src/wallets/in-app/core/authentication/getLoginPath.ts#L11

Added line #L11 was not covered by tests
if (ecosystem?.partnerId) {
return `${baseUrl}&ecosystemId=${ecosystem.id}&ecosystemPartnerId=${ecosystem.partnerId}`;
}
Expand Down
12 changes: 1 addition & 11 deletions packages/thirdweb/src/wallets/in-app/core/authentication/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export enum AuthProvider {
}

export type OauthOption = {
provider: AuthProvider;
strategy: SocialAuthOption;
redirectUrl: string;
};

Expand Down Expand Up @@ -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,
};
57 changes: 10 additions & 47 deletions packages/thirdweb/src/wallets/in-app/native/auth/native-auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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";
Expand Down Expand Up @@ -207,48 +205,14 @@ export async function validateEmailOTP(options: {
}

export async function socialLogin(
oauthOptions: OauthOption,
auth: OauthOption,
client: ThirdwebClient,
): Promise<AuthStoredTokenWithCookieReturnType> {
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,
Expand All @@ -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");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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");
}
Expand Down
25 changes: 6 additions & 19 deletions packages/thirdweb/src/wallets/in-app/native/native-connector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
});
}
Expand Down Expand Up @@ -181,14 +179,9 @@ export class InAppNativeConnector implements InAppConnector {
return deleteActiveAccount({ client: this.options.client });
}

private async socialLogin(
oauthOption: OauthOption,
): Promise<AuthLoginReturnType> {
private async socialLogin(auth: OauthOption): Promise<AuthLoginReturnType> {
try {
const { storedToken } = await socialLogin(
oauthOption,
this.options.client,
);
const { storedToken } = await socialLogin(auth, this.options.client);
const account = await this.getAccount();
return {
user: {
Expand All @@ -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}`);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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<AuthStoredTokenWithCookieReturnType> {
}): Promise<AuthStoredTokenWithCookieReturnType> => {
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;
Expand All @@ -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);
Expand Down Expand Up @@ -89,4 +125,4 @@ export async function loginWithDiscord(options: {
},
);
return result;
}
};
19 changes: 0 additions & 19 deletions packages/thirdweb/src/wallets/in-app/web/lib/auth/utils.ts

This file was deleted.

Loading

0 comments on commit d3c8302

Please sign in to comment.