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

Add support for Mozilla Accounts prompt none auth flow #5104

Merged
merged 20 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from 12 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
4 changes: 4 additions & 0 deletions src/app/(proper_react)/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { getCountryCode } from "../functions/server/getCountryCode";
import { PageLoadEvent } from "../components/client/PageLoadEvent";
import { getExperimentationId } from "../functions/server/getExperimentationId";
import { getEnabledFeatureFlags } from "../../db/tables/featureFlags";
import { PromptNoneAuth } from "../components/client/PromptNoneAuth";
import { addClientIdForSubscriber } from "../../db/tables/google_analytics_clients";
import { logger } from "../functions/server/logging";

Expand Down Expand Up @@ -63,6 +64,9 @@ export default async function Layout({ children }: { children: ReactNode }) {
<L10nProvider bundleSources={l10nBundles}>
<ReactAriaI18nProvider locale={getLocale(l10nBundles)}>
<CountryCodeProvider countryCode={countryCode}>
{enabledFlags.includes("PromptNoneAuthFlow") && !session && (
Vinnl marked this conversation as resolved.
Show resolved Hide resolved
<PromptNoneAuth />
)}
{children}
<PageLoadEvent
experimentationId={getExperimentationId(session?.user ?? null)}
Expand Down
20 changes: 19 additions & 1 deletion src/app/api/auth/[...nextauth]/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,27 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { NextApiRequest, NextApiResponse } from "next";
import { NextResponse } from "next/server";
import NextAuth from "next-auth";
import { ResponseInternal } from "next-auth/core";
import { authOptions } from "../../utils/auth";

const handler = NextAuth(authOptions);
// There is currently no support for handling OAuth provider callback errors:
// https://github.com/nextauthjs/next-auth/discussions/8209
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the comment, hopefully they add this eventually.

const handler = async (req: NextApiRequest, res: NextApiResponse) => {
if (
req.method === "GET" &&
req.url?.startsWith(
`${process.env.SERVER_URL}/api/auth/callback/fxa?error=`,
)
) {
return NextResponse.redirect(`${process.env.SERVER_URL}/user/dashboard`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@flozia Shouldn't this be going to / rather than /user/dashboard/? Now, if I'm not signed in, I get redirected to FxA, then back to Monitor, then to the dashboard, where I see a flash of an error page, before being redirected back to FxA, but this time not a prompt=none flow, so I'm still asked to sign in. (If that's what we want, we could just not do a prompt=none flow, right?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Vinnl Redirecting to / might be safer in this case. The idea behind redirecting to /user/dashboard was to initiate authentication as a fallback, but there might be too many redirects — I’ve opened PR #5192.

}

return (await NextAuth(req, res, authOptions)) as Promise<
ResponseInternal<Body>
>;
};

export { handler as GET, handler as POST };
33 changes: 33 additions & 0 deletions src/app/components/client/PromptNoneAuth.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

"use client";

import { ReactNode, useEffect } from "react";
import { useSearchParams } from "next/navigation";
import { signIn } from "next-auth/react";
import { containsExpectedSearchParams } from "../../functions/universal/attributions";
import { CONST_MOZILLA_ACCOUNTS_SETTINGS_PROMO_SEARCH_PARAMS } from "../../../constants";

export const PromptNoneAuth = (): ReactNode => {
const searchParams = useSearchParams();

useEffect(() => {
const isPromptNoneAuthAttempt = containsExpectedSearchParams(
CONST_MOZILLA_ACCOUNTS_SETTINGS_PROMO_SEARCH_PARAMS,
searchParams,
);
if (isPromptNoneAuthAttempt) {
void signIn(
"fxa",
{ callbackUrl: "/user/dashboard" },
{ prompt: "none" },
);
}
// This effect should only run once
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);

return null;
};
79 changes: 79 additions & 0 deletions src/app/functions/universal/attributions.test.ts
flozia marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

import { it, expect } from "@jest/globals";
import { containsExpectedSearchParams } from "./attributions";

it("returns `true` when the actual search params are matching the expected params", () => {
expect(
containsExpectedSearchParams(
{
one: "1",
two: "2",
},
new URLSearchParams({
one: "1",
two: "2",
}),
),
).toBe(true);
});

it("returns `true` when the actual search params are a superset of the expected params", () => {
expect(
containsExpectedSearchParams(
{
one: "1",
two: "2",
},
new URLSearchParams({
one: "1",
two: "2",
three: "3",
}),
),
).toBe(true);
});

it("returns `false` if any expected param is missing from the actual search params", () => {
expect(
containsExpectedSearchParams(
{
one: "1",
two: "2",
},
new URLSearchParams({
two: "2",
}),
),
).toBe(false);
});

it("returns `false` if any of the actual search param keys are not matching the expected params", () => {
expect(
containsExpectedSearchParams(
{
one: "1",
three: "2",
},
new URLSearchParams({
two: "2",
}),
),
).toBe(false);
});

it("returns `false` if any of the actual search param values are not matching the expected params", () => {
expect(
containsExpectedSearchParams(
{
one: "1",
two: "3",
},
new URLSearchParams({
two: "2",
}),
),
).toBe(false);
});
9 changes: 9 additions & 0 deletions src/app/functions/universal/attributions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,12 @@ export function modifyAttributionsForUrlSearchParams(

return searchParams;
}

export const containsExpectedSearchParams = (
expectedSearchParams: Record<string, string>,
searchParams: URLSearchParams,
) =>
Object.keys(expectedSearchParams).every(
(searchParamKey) =>
searchParams.get(searchParamKey) === expectedSearchParams[searchParamKey],
);
5 changes: 5 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,8 @@ export const CONST_URL_MONITOR_GITHUB =
export const CONST_DAY_MILLISECONDS = 24 * 60 * 60 * 1000;
export const CONST_URL_MONITOR_LANDING_PAGE_ID =
"monitor.mozilla.org-monitor-product-page";
export const CONST_MOZILLA_ACCOUNTS_SETTINGS_PROMO_SEARCH_PARAMS = {
Vinnl marked this conversation as resolved.
Show resolved Hide resolved
utm_source: "moz-account",
utm_campaign: "settings-promo",
utm_content: "monitor-free",
} as const;
1 change: 1 addition & 0 deletions src/db/tables/featureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const featureFlagNames = [
"PetitionBannerCsatSurvey",
"MonthlyReportFreeUser",
"BreachEmailRedesign",
"PromptNoneAuthFlow",
"GA4SubscriptionEvents",
] as const;
export type FeatureFlagName = (typeof featureFlagNames)[number];
Expand Down
Loading