Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Arctic for OAuth providers #1851

Merged
merged 1 commit into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
193 changes: 106 additions & 87 deletions waspc/ChangeLog.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
{{={= =}=}}
import { useEffect, useRef, useState } from "react";
import { type AxiosResponse } from "axios";
import { Redirect, useLocation } from 'react-router-dom'
import { useAuth } from 'wasp/client/auth'
import { api } from 'wasp/client/api'
import { initSession } from 'wasp/auth/helpers/user'

const wrapperStyles = {
display: "flex",
alignItems: "center",
justifyContent: "center",
padding: "4rem",
};

const commonMessageStyles = {
display: 'flex',
alignItems: 'center',
gap: '.5rem',
borderRadius: '.5rem',
padding: '1rem',
};

const errorMessageStyles = {
...commonMessageStyles,
borderColor: 'rgb(240 82 82)',
backgroundColor: 'rgb(253 232 232)',
color: 'rgb(200 30 30)',
};

const loadingMessageStyles = {
...commonMessageStyles,
borderColor: 'rgb(107 114 128)',
backgroundColor: 'rgb(243 244 246)',
color: 'rgb(55 65 81)',
};

export function OAuthCallbackPage() {
const { isLoading, error, user } = useOAuthCallbackHandler();

if (user !== undefined && user !== null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come it can be both of these? I'm guessing because undefined happens on the first render, while null happens when the user isn't logged in?

If so, there's a trick that many linters recommend (although I'm not sure I like it). It sits halfway between being fully explicit and using coercion:

if (user != null) {
  // ...
}

This covers both undefined and null.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to skip the non strict check if possible. I know this looks ugly e.g. checking both undefined and null but it's at least not relying on implicit JS behaviour.

return <Redirect to="{= onAuthSucceededRedirectTo =}" />;
}

return (
<div style={wrapperStyles}>
{error && <div style={errorMessageStyles}><MessageIcon /> {error}</div>}
{isLoading && <div style={loadingMessageStyles}><MessageIcon /> Please wait a moment while we log you in.</div>}
</div>
);
}

function useOAuthCallbackHandler() {
const { data: user } = useAuth();
const [isLoading, setIsLoading] = useState(true);
const [error, setError] = useState<string | null>(null);
const location = useLocation();

async function handleCallback() {
try {
setIsLoading(true);
const query = new URLSearchParams(location.search);

// If we got redirect with an error, display it to the user
// and don't continue with the login process.
const errorFromRedirect = query.get('error');
if (errorFromRedirect !== null) {
setError(errorFromRedirect);
return;
}

const code = location.hash.slice(1);
const response = await exchangeOAuthCodeForToken({ code });
if (!isResponseWithSessionId(response)) {
setError("Unable to login with the OAuth provider.");
return;
}
await initSession(response.data.sessionId);
} catch (e: unknown) {
console.error(e);
setError("Unable to login with the OAuth provider.");
} finally {
setIsLoading(false);
}
}

const isFirstRender = useRef(true);
useEffect(() => {
if (isFirstRender.current) {
isFirstRender.current = false;
handleCallback();
}
}, []);
sodic marked this conversation as resolved.
Show resolved Hide resolved

return {
user,
error,
isLoading,
};
}

const MessageIcon = () => (
<svg
xmlns="http://www.w3.org/2000/svg"
width="1.25rem"
height="1.25rem"
fill="currentColor"
stroke="currentColor"
strokeWidth={0}
aria-hidden="true"
viewBox="0 0 20 20"
>
<path
fillRule="evenodd"
stroke="none"
d="M18 10a8 8 0 1 1-16 0 8 8 0 0 1 16 0zm-7-4a1 1 0 1 1-2 0 1 1 0 0 1 2 0zM9 9a1 1 0 0 0 0 2v3a1 1 0 0 0 1 1h1a1 1 0 1 0 0-2v-3a1 1 0 0 0-1-1H9z"
clipRule="evenodd"
/>
</svg>
)

async function exchangeOAuthCodeForToken(data: {
code: string
}): Promise<AxiosResponse<unknown>> {
return api.post('/auth/exchange-code', data)
}

function isResponseWithSessionId(
response: AxiosResponse<unknown>
): response is AxiosResponse<{ sessionId: string }> {
return response.data && typeof (response.data as any).sessionId === 'string'
}

This file was deleted.

10 changes: 3 additions & 7 deletions waspc/data/Generator/templates/react-app/src/router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import createAuthRequiredPage from "./auth/pages/createAuthRequiredPage"
{=/ pagesToImport =}

{=# isExternalAuthEnabled =}
import OAuthCodeExchange from "./auth/pages/OAuthCodeExchange"
import { OAuthCallbackPage } from "./auth/pages/OAuthCallback"
{=/ isExternalAuthEnabled =}

import { routes } from 'wasp/client/router'
Expand All @@ -40,13 +40,9 @@ const router = (
/>
))}
{=# isExternalAuthEnabled =}
{=# externalAuthProviders =}
{=# authProviderEnabled =}
<Route exact path="{= authFrontendUrl =}">
<OAuthCodeExchange pathToApiServerRouteHandlingOauthRedirect="{= authServerOauthRedirectUrl =}" />
<Route exact path="{= oAuthCallbackPath =}">
<OAuthCallbackPage />
sodic marked this conversation as resolved.
Show resolved Hide resolved
</Route>
{=/ authProviderEnabled =}
{=/ externalAuthProviders =}
{=/ isExternalAuthEnabled =}
</Switch>
{=# rootComponent.isDefined =}
Expand Down
10 changes: 1 addition & 9 deletions waspc/data/Generator/templates/sdk/wasp/auth/providers/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,11 @@ export type ProviderConfig = {
// Unique provider identifier, used as part of URL paths
id: ProviderName;
displayName: string;
// Each provider config can have an init method which is ran on setup time
// e.g. for oAuth providers this is the time when the Passport strategy is registered.
init?(provider: ProviderConfig): Promise<InitData>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed anymore

// Every provider must have a setupRouter method which returns the Express router.
// In this function we are flexibile to do what ever is necessary to make the provider work.
createRouter(provider: ProviderConfig, initData: InitData): Router;
createRouter(provider: ProviderConfig): Router;
};

// PRIVATE API
export type InitData = {
[key: string]: any;
}

// PRIVATE API
export type RequestWithWasp = Request & { wasp?: { [key: string]: any } }

Expand Down
12 changes: 7 additions & 5 deletions waspc/data/Generator/templates/sdk/wasp/server/utils.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,6 @@
{{={= =}=}}
import crypto from 'crypto'
import { Request, Response, NextFunction } from 'express'

import { readdir } from 'fs'
import { dirname } from 'path'
import { fileURLToPath } from 'url'

{=# isAuthEnabled =}
import { type AuthUser } from 'wasp/auth'
{=/ isAuthEnabled =}
Expand Down Expand Up @@ -40,3 +35,10 @@ async (req: RequestWithExtraFields, res: Response, next: NextFunction) => {
}

export const sleep = (ms: number): Promise<unknown> => new Promise((r) => setTimeout(r, ms))

export function redirect(res: Response, redirectUri: string) {
return res
.status(302)
.setHeader("Location", redirectUri)
.end();
}

This file was deleted.

Loading
Loading