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

fix logout #92

Merged
merged 8 commits into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from 7 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
21 changes: 15 additions & 6 deletions middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,15 @@ const checkOnboardingStatus = async (
}
return res;
}
// special case where we require a signed in user to view their profile, but we don't want to redirect them to onboarding cause this happens before they are onboarded
if (req.nextUrl.pathname === '/api/profiles') {
// print req queries
const id = new URL(req.url).searchParams.get('id');
if (!id || id !== session.user.id) {
return NextResponse.json({ error: 'user is unauthorized for this action' });
}
return res;
}
const { subscriptionStatus, subscriptionEndDate } = await getCompanySubscriptionStatus(
supabase,
session.user.id,
Expand Down Expand Up @@ -150,7 +159,10 @@ const checkIsRelayEmployee = async (res: NextResponse, email: string) => {
return res;
};

/** https://supabase.com/docs/guides/auth/auth-helpers/nextjs#auth-with-nextjs-middleware */
/** https://supabase.com/docs/guides/auth/auth-helpers/nextjs#auth-with-nextjs-middleware
* Note: We are applying the middleware to all routes. So almost all routes require authentication. Exceptions are in the `config` object at the bottom of this file.
*
*/
export async function middleware(req: NextRequest) {
// We need to create a response and hand it to the supabase client to be able to modify the response headers.
const res = NextResponse.next();
Expand All @@ -175,10 +187,6 @@ export async function middleware(req: NextRequest) {
if (req.nextUrl.pathname.includes('api'))
return NextResponse.json({ error: 'unauthorized to use endpoint' });

// if already on signup or login page, just return the page
if (req.nextUrl.pathname.includes('signup') || req.nextUrl.pathname.includes('login'))
return res;

const redirectUrl = req.nextUrl.clone();

// unauthenticated pages requests, send to signup
Expand All @@ -197,7 +205,8 @@ export const config = {
* - assets/* (assets files) (public/assets/*)
* - accept invite (accept invite api). User hasn't logged in yet
* - create-employee endpoint (api/company/create-employee)
* - login, signup, logout (login, signup, logout pages)
*/
'/((?!_next/static|_next/image|favicon.ico|assets/*|api/company/accept-invite*|api/company/create-employee*).*)',
'/((?!_next/static|_next/image|favicon.ico|assets/*|api/company/accept-invite*|api/company/create-employee*|login|signup|logout|api/logout).*)',
],
};
33 changes: 33 additions & 0 deletions pages/api/logout.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { createServerSupabaseClient } from '@supabase/auth-helpers-nextjs';
import { NextApiHandler } from 'next';
import { serverLogger } from 'src/utils/logger';

const supabaseUrl = process.env.NEXT_PUBLIC_SUPABASE_URL || '';
const supabaseAnonKey = process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY || '';

if (!supabaseUrl || !supabaseAnonKey) {
serverLogger('NEXT_PUBLIC_SUPABASE_ANON_KEY or NEXT_PUBLIC_SUPABASE_URL not set', 'error');
}

const handler: NextApiHandler = async (req, res) => {
const supabaseClient = createServerSupabaseClient(
{ req, res },
{
supabaseUrl,
supabaseKey: supabaseAnonKey,
},
);
// Note: this doesn't really do much, because the jwt in the cookie will still be valid. All this `signOut` function does is invalidate the refresh token, so that the jwt can't be refreshed. The jwt will still be valid until it expires.
await supabaseClient.auth.signOut();

const cookieExpiry = new Date();

// The cookie should already be deleted by the browser, but just in case, we'll delete it here as well.
res.setHeader(
'Set-Cookie',
`supabase-auth-token=deleted; Max-Age=0; Expires=${cookieExpiry.toUTCString()};`,
);
return res.status(200).json({ success: true });
};

export default handler;
38 changes: 35 additions & 3 deletions pages/api/profiles/index.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,45 @@
import { NextApiHandler } from 'next';
import httpCodes from 'src/constants/httpCodes';
import { ProfileDB, ProfileInsertDB, updateProfile } from 'src/utils/api/db';
import { getProfileById, ProfileDB, ProfileInsertDB, updateProfile } from 'src/utils/api/db';
import { checkSessionIdMatchesID } from 'src/utils/auth';
import { serverLogger } from 'src/utils/logger';

export type ProfileGetQuery = {
/** user id from session */
id: string;
};
export type ProfileGetResponse = ProfileDB;

export type ProfilePutBody = ProfileInsertDB;
export type ProfilePutResponse = ProfileDB;

const Handler: NextApiHandler = async (req, res) => {
// Create authenticated Supabase Client
if (req.method === 'GET') {
const { id } = req.query as ProfileGetQuery;

if (!id) {
Copy link

Choose a reason for hiding this comment

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

It's good to see this pattern of validation after using a type assertion continuing to be applied.

◽ Compliment

Image of Ryan B Ryan B

return res.status(httpCodes.BAD_REQUEST).json({ message: 'missing id' });
}
try {
Copy link

Choose a reason for hiding this comment

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

There are some opportunities for code reuse in this handler.

🔹 Code Reuse (Nice to have)

Image of Ryan B Ryan B

const matchesSession = await checkSessionIdMatchesID(id, req, res);
if (!matchesSession) {
return res.status(httpCodes.UNAUTHORIZED).json({
error: 'user is unauthorized for this action',
});
}
const { data, error: profileGetError } = await getProfileById(id);
if (profileGetError) {
serverLogger(profileGetError, 'error');
return res.status(httpCodes.INTERNAL_SERVER_ERROR).json({});
}
const result: ProfileGetResponse = data;
return res.status(httpCodes.OK).json(result);
} catch (error) {
serverLogger(error, 'error');
return res.status(httpCodes.INTERNAL_SERVER_ERROR).json({});
}
}

if (req.method === 'PUT') {
const profile = JSON.parse(req.body) as ProfilePutBody;

Expand All @@ -24,7 +55,8 @@ const Handler: NextApiHandler = async (req, res) => {
serverLogger(profileUpdateError, 'error');
return res.status(httpCodes.INTERNAL_SERVER_ERROR).json({});
}
return res.status(httpCodes.OK).json(data);
const result: ProfilePutResponse = data;
return res.status(httpCodes.OK).json(result);
} catch (error) {
serverLogger(error, 'error');
return res.status(httpCodes.INTERNAL_SERVER_ERROR).json({});
Expand Down
35 changes: 35 additions & 0 deletions pages/logout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { useRouter } from 'next/router';
import { useCallback, useEffect } from 'react';
import { useUser } from 'src/hooks/use-user';

export default function Logout() {
const router = useRouter();
const { supabaseClient, refreshProfile, profile, user, getProfileController } = useUser();
const { email } = router.query;
const signOut = useCallback(async () => {
getProfileController.current?.abort();
if (!supabaseClient) return;
await supabaseClient.auth.signOut();

const res = await fetch('/api/logout');
const data = await res.json();
if (data.success) {
if (profile?.id || user?.id) {
await refreshProfile(undefined, {
revalidate: false,
optimisticData: undefined,
rollbackOnError: false,
throwOnError: false,
});
return;
}
window.location.href = email ? `/login?email=${email}` : '/login';
}
}, [email, getProfileController, profile?.id, refreshProfile, supabaseClient, user?.id]);

useEffect(() => {
signOut();
}, [signOut]);

return <></>;
}
15 changes: 2 additions & 13 deletions pages/signup/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,9 @@ export default function Register() {
password: '',
confirmPassword: '',
});
const { signup, logout, profile, createEmployee } = useUser();
const { signup, profile, createEmployee } = useUser();
const [loading, setLoading] = useState(false);
const [signupSuccess, setSignupSuccess] = useState(false);
const [initialLoad, setInitialLoad] = useState(true);
Copy link

Choose a reason for hiding this comment

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

It's great to see this being removed.

◽ Compliment

Image of Ryan B Ryan B

useEffect(() => {
const logOutOnLoad = async () => {
// sometimes the cookies and signed in status still persist to this page, so call logout again
// TODO: move logout logic serverside https://toil.kitemaker.co/0JhYl8-relayclub/8sxeDu-v2_project/items/72

await logout();
setInitialLoad(false);
};
if (initialLoad) logOutOnLoad();
}, [logout, initialLoad]);

useEffect(() => {
if (signupSuccess && profile?.id) {
Expand All @@ -49,7 +38,7 @@ export default function Register() {
router.push('/signup/onboarding');
}
}
}, [email, initialLoad, router, signupSuccess, profile]);
}, [email, router, signupSuccess, profile]);

const handleSubmit = async () => {
setLoading(true);
Expand Down
4 changes: 2 additions & 2 deletions src/components/account/account-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import type Stripe from 'stripe';
export interface AccountContextProps {
userDataLoading: boolean;
paymentMethods?: Stripe.PaymentMethod[];
profile: ProfileDB | null;
profile: ProfileDB | undefined;
user: User | null;
company?: CompanyWithProfilesInvitesAndUsage;
createInvite: (email: string, companyOwner: boolean) => void;
Expand All @@ -29,7 +29,7 @@ export const AccountContext = createContext<AccountContextProps>({
userDataLoading: false,

paymentMethods: undefined,
profile: null,
profile: undefined,
user: null,
company: undefined,
createInvite: () => {},
Expand Down
11 changes: 2 additions & 9 deletions src/components/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,16 @@ import { Sidebar } from 'src/components/sidebar';

import { useUser } from 'src/hooks/use-user';
import useOnOutsideClick from 'src/hooks/use-on-outside-click';
import { useSupabaseClient } from '@supabase/auth-helpers-react';

export const Layout = ({ children }: any) => {
const { t } = useTranslation();
const { profile, loading, refreshProfile } = useUser();
const { profile, loading, refreshProfile, logout } = useUser();

useEffect(() => {
// this fixes a bug where the profile is not loaded on the first page load when coming from signup
if (!loading && !profile?.id) refreshProfile();
}, [refreshProfile, profile, loading]);

const supabase = useSupabaseClient();
Copy link

Choose a reason for hiding this comment

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

Awesome to see supabase being abstracted away from here into an auth layer.

◽ Compliment

Image of Ryan B Ryan B


const [accountMenuOpen, setAccountMenuOpen] = useState(false);
const accountMenuRef = useRef(null);
const accountMenuButtonRef = useRef(null);
Expand Down Expand Up @@ -75,11 +72,7 @@ export const Layout = ({ children }: any) => {
<Button
className="px-4 py-2 text-sm hover:bg-gray-100 active:bg-gray-200"
variant="neutral"
onClick={async () => {
await supabase.auth.signOut();
// need to trigger a page reload to get the new auth state, so don't use router.push
window.location.href = '/signup';
}}
onClick={logout}
>
{t('navbar.logout')}
</Button>
Expand Down
Loading