Skip to content

Commit

Permalink
fix(core): oauth popup blocked in safari (#8144)
Browse files Browse the repository at this point in the history
fix(core): oauth popup blocked in safari

fix(core): standarize auth routes

fix AF-1352
  • Loading branch information
forehalo committed Sep 6, 2024
1 parent be4df0f commit bffc294
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 50 deletions.
2 changes: 1 addition & 1 deletion packages/frontend/core/public/robots.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Disallow: /oauth/*
Disallow: /workspace/*
Disallow: /public-workspace/*
Disallow: /auth/*
Disallow: /signin/*
Disallow: /sign-in
Disallow: /_next/*
Disallow: /invite/*
Disallow: /*/invite/*
Expand Down
55 changes: 17 additions & 38 deletions packages/frontend/core/src/components/affine/auth/oauth.tsx
Original file line number Diff line number Diff line change
@@ -1,16 +1,11 @@
import { notify, Skeleton } from '@affine/component';
import { Skeleton } from '@affine/component';
import { Button } from '@affine/component/ui/button';
import { useAsyncCallback } from '@affine/core/hooks/affine-async-hooks';
import { track } from '@affine/core/mixpanel';
import { popupWindow } from '@affine/core/utils';
import { apis } from '@affine/electron-api';
import { OAuthProviderType } from '@affine/graphql';
import { GithubIcon, GoogleDuotoneIcon } from '@blocksuite/icons/rc';
import { useLiveData, useService } from '@toeverything/infra';
import type { ReactElement } from 'react';
import { useState } from 'react';

import { AuthService, ServerConfigService } from '../../../modules/cloud';
import { ServerConfigService } from '../../../modules/cloud';

const OAuthProviderMap: Record<
OAuthProviderType,
Expand Down Expand Up @@ -50,39 +45,23 @@ export function OAuth() {

function OAuthProvider({ provider }: { provider: OAuthProviderType }) {
const { icon } = OAuthProviderMap[provider];
const authService = useService(AuthService);
const [isConnecting, setIsConnecting] = useState(false);

const onClick = useAsyncCallback(async () => {
try {
setIsConnecting(true);
const url = await authService.oauthPreflight(provider);
if (environment.isElectron) {
await apis?.ui.openExternal(url);
} else {
popupWindow(url);
}
} catch (err) {
console.error(err);
notify.error({ title: 'Failed to sign in, please try again.' });
} finally {
setIsConnecting(false);
track.$.$.auth.oauth({ provider });
}
}, [authService, provider]);

return (
<Button
key={provider}
variant="primary"
block
size="extraLarge"
style={{ marginTop: 30, width: '100%' }}
prefix={icon}
onClick={onClick}
<a
href={`/oauth/login?provider=${provider}`}
target="_blank"
rel="noreferrer"
>
Continue with {provider}
{isConnecting && '...'}
</Button>
<Button
key={provider}
variant="primary"
block
size="extraLarge"
style={{ marginTop: 30, width: '100%' }}
prefix={icon}
>
Continue with {provider}
</Button>
</a>
);
}
2 changes: 1 addition & 1 deletion packages/frontend/core/src/hooks/use-navigate-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export function useNavigateHelper() {
}

return navigate(
'/signIn' +
'/sign-in' +
(searchParams.toString() ? '?' + searchParams.toString() : ''),
{
replace: logic === RouteLogic.REPLACE,
Expand Down
4 changes: 2 additions & 2 deletions packages/frontend/core/src/pages/auth/magic-link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const loader: LoaderFunction = ({ request }) => {
const redirectUri = params.get('redirect_uri');

if (!email || !token) {
return redirect('/signIn?error=Invalid magic link');
return redirect('/sign-in?error=Invalid magic link');
}

const payload: LoaderData = {
Expand Down Expand Up @@ -61,7 +61,7 @@ export const Component = () => {
nav(data.redirectUri ?? '/');
})
.catch(e => {
nav(`/signIn?error=${encodeURIComponent(e.message)}`);
nav(`/sign-in?error=${encodeURIComponent(e.message)}`);
});
}, [data, auth, nav]);

Expand Down
6 changes: 3 additions & 3 deletions packages/frontend/core/src/pages/auth/oauth-callback.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const loader: LoaderFunction = async ({ request }) => {
let stateStr = queries.get('state') ?? '{}';

if (!code || !stateStr) {
return redirect('/signIn?error=Invalid oauth callback parameters');
return redirect('/sign-in?error=Invalid oauth callback parameters');
}

try {
Expand All @@ -46,7 +46,7 @@ export const loader: LoaderFunction = async ({ request }) => {
`/open-app/url?url=${encodeURIComponent(`${client}://authentication?${authParams.toString()}`)}`
);
} catch {
return redirect('/signIn?error=Invalid oauth callback parameters');
return redirect('/sign-in?error=Invalid oauth callback parameters');
}
};

Expand All @@ -64,7 +64,7 @@ export const Component = () => {
nav(redirectUri ?? '/');
})
.catch(e => {
nav(`/signIn?error=${encodeURIComponent(e.message)}`);
nav(`/sign-in?error=${encodeURIComponent(e.message)}`);
});
}, [data, auth, nav]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ export const loader: LoaderFunction = async ({ request }) => {
}

return redirect(
`/signIn?error=${encodeURIComponent(`Invalid oauth provider ${provider}`)}`
`/sign-in?error=${encodeURIComponent(`Invalid oauth provider ${provider}`)}`
);
};

Expand All @@ -54,7 +54,7 @@ export const Component = () => {
location.href = url;
})
.catch(e => {
nav(`/signIn?error=${encodeURIComponent(e.message)}`);
nav(`/sign-in?error=${encodeURIComponent(e.message)}`);
});
}, [data, auth, nav]);

Expand Down
20 changes: 17 additions & 3 deletions packages/frontend/core/src/router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export const topLevelRoutes = [
path: '/try-cloud',
loader: () => {
return redirect(
`/signIn?redirect_uri=${encodeURIComponent('/?initCloud=true')}`
`/sign-in?redirect_uri=${encodeURIComponent('/?initCloud=true')}`
);
},
},
Expand All @@ -99,7 +99,7 @@ export const topLevelRoutes = [
lazy: () => import(/* webpackChunkName: "auth" */ './pages/auth/auth'),
},
{
path: '/signIn',
path: '/sign-In',
lazy: () =>
import(/* webpackChunkName: "auth" */ './pages/auth/sign-in'),
},
Expand All @@ -108,6 +108,11 @@ export const topLevelRoutes = [
lazy: () =>
import(/* webpackChunkName: "auth" */ './pages/auth/magic-link'),
},
{
path: '/oauth/login',
lazy: () =>
import(/* webpackChunkName: "auth" */ './pages/auth/oauth-login'),
},
{
path: '/oauth/callback',
lazy: () =>
Expand All @@ -117,7 +122,16 @@ export const topLevelRoutes = [
// TODO(@forehalo): remove
{
path: '/desktop-signin',
lazy: () => import('./pages/auth/desktop-signin'),
lazy: () =>
import(/* webpackChunkName: "auth" */ './pages/auth/oauth-login'),
},
// deprecated, keep for old client compatibility
// use '/sign-in'
// TODO(@forehalo): remove
{
path: '/signIn',
lazy: () =>
import(/* webpackChunkName: "auth" */ './pages/auth/sign-in'),
},
{
path: '/open-app/:action',
Expand Down
21 changes: 21 additions & 0 deletions packages/frontend/mobile/src/router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,27 @@ export const topLevelRoutes = [
path: '/sign-in',
lazy: () => import('./pages/sign-in'),
},
{
path: '/magic-link',
lazy: () =>
import(
/* webpackChunkName: "auth" */ '@affine/core/pages/auth/magic-link'
),
},
{
path: '/oauth/login',
lazy: () =>
import(
/* webpackChunkName: "auth" */ '@affine/core/pages/auth/oauth-login'
),
},
{
path: '/oauth/callback',
lazy: () =>
import(
/* webpackChunkName: "auth" */ '@affine/core/pages/auth/oauth-callback'
),
},
{
path: '/redirect-proxy',
lazy: () => import('@affine/core/pages/redirect'),
Expand Down

0 comments on commit bffc294

Please sign in to comment.