Skip to content

Add E2E tests for silent authentication #5150

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

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f7163a8
feat: Force feature flags for E2E tests
flozia Oct 8, 2024
8bc9e46
merge: mntor-3492 -> mntor-3492-e2e
flozia Oct 9, 2024
0e6d065
chore: Enable ability to force feature flags for all E2E test environ…
flozia Oct 9, 2024
4ae7859
chore: Enable forcing flags only for local environments
flozia Oct 9, 2024
0243bbf
Merge branch 'main' into mntor-3492-e2e
flozia Oct 10, 2024
f3f533c
Merge branch 'main' into mntor-3492-e2e
flozia Oct 11, 2024
032032f
Merge branch 'main' into mntor-3492-e2e
flozia Oct 17, 2024
9efbaec
Merge branch 'mntor-3492-e2e' of github.com:mozilla/blurts-server int…
flozia Oct 17, 2024
c1df6ca
Merge branch 'main' into mntor-3492-e2e
flozia Oct 22, 2024
431b289
fix: Silent authentication E2E test flow
flozia Oct 22, 2024
44e5826
Merge branch 'main' into mntor-3492-e2e
flozia Oct 22, 2024
93efd14
Merge branch 'main' into mntor-3492-e2e
flozia Oct 22, 2024
58453fd
Merge branch 'main' into mntor-3492-e2e
flozia Oct 24, 2024
e62b3be
wip: Debug post authentication state
flozia Oct 25, 2024
8b4f65d
Merge branch 'mntor-3492-e2e' of github.com:mozilla/blurts-server int…
flozia Oct 25, 2024
e9fee2c
Merge branch 'main' into mntor-3492-e2e
flozia Oct 30, 2024
7f3feea
chore: Set forced feature flags via setExtraHTTPHeaders
flozia Oct 30, 2024
66644b6
Merge branch 'main' into mntor-3492-e2e
flozia Oct 30, 2024
9769e0a
fix: Set timeout for landing page header to show
flozia Oct 30, 2024
0d3493d
Merge branch 'main' into mntor-3492-e2e
flozia Oct 30, 2024
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
22 changes: 19 additions & 3 deletions src/db/tables/featureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import createDbConnection from "../connect";
import { logger } from "../../app/functions/server/logging";
import { FeatureFlagRow } from "knex/types/tables";
import { headers } from "next/headers";

const knex = createDbConnection();

Expand Down Expand Up @@ -75,11 +76,26 @@ export async function getEnabledFeatureFlags(
return void subQuery;
});

const enabledFlagNames = await query;

return enabledFlagNames.map(
const enabledFlagNames = (await query).map(
(row: { name: string }) => row.name as FeatureFlagName,
);

// Force feature flags for E2E tests via URL query params
if (typeof process.env.E2E_TEST_ENV !== "undefined") {
const forcedFeatureFlags = headers().get("x-forced-feature-flags");
if (forcedFeatureFlags) {
const forcedFeatureFlagsFiltered = forcedFeatureFlags
.split(",")
.filter((forcedFeatureFlag) =>
featureFlagNames.includes(forcedFeatureFlag as FeatureFlagName),
);
return [
...new Set([...enabledFlagNames, ...forcedFeatureFlagsFiltered]),
] as FeatureFlagName[];
}
}

return enabledFlagNames;
}

/**
Expand Down
30 changes: 30 additions & 0 deletions src/e2e/pages/authPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,34 @@ export class AuthPage {
expect(verificationCode).toBeDefined();
await this.enterVerificationCode(verificationCode as string);
}

async signInToFxA(email: string, password: string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not clear to me (because I'm not terribly familiar with our existing e2e tests) why this extra method is needed - don't we already have methods to sign in?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This method is for logging explicitly in to the Monitor Accounts application and not Monitor.

await this.page.goto(process.env.FXA_SETTINGS_URL as string);
await this.page.context().clearCookies();
await this.page
.locator("//input[@type='password'] | //div/input[@type='email']")
.waitFor({ state: "visible" });
const visible = await this.useDifferentEmailButton.isVisible();
if (visible) {
await this.useDifferentEmailButton.click();
await this.page.waitForURL(/^(?!.*signin).*/);
}

// enter email
await this.emailInputField.fill(email);
await this.continueButton.click();
await this.page.waitForURL(/^(?!.*signin).*/);

// enter password
await this.passwordInputField.fill(password);
await this.continue({ waitForURL: process.env.FXA_SETTINGS_URL });
}

async initSilentAuth() {
await this.page.goto(
`${process.env.E2E_TEST_BASE_URL as string}/?feature_flags=PromptNoneAuthFlow&utm_source=moz-account&utm_campaign=settings-promo&utm_content=monitor-free`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

So as per my other comment, this would be:

Suggested change
await this.page.goto(
`${process.env.E2E_TEST_BASE_URL as string}/?feature_flags=PromptNoneAuthFlow&utm_source=moz-account&utm_campaign=settings-promo&utm_content=monitor-free`,
await this.page.setExtraHTTPHeaders({ "x-forced-feature-flags": "PromptNoneAuthFlow" });
await this.page.goto(
`${process.env.E2E_TEST_BASE_URL as string}/?utm_source=moz-account&utm_campaign=settings-promo&utm_content=monitor-free`,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! Update in 7f3feea.

);
// FxA can take a while to load on stage:
await this.page.waitForURL("**/oauth/**");
}
}
63 changes: 62 additions & 1 deletion src/e2e/specs/auth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ test.describe(`${process.env.E2E_TEST_ENV} - Authentication flow verification @s
landingPage,
dashboardPage,
}, testInfo) => {
// speed up test by ignore non necessary requests
// speed up test by ignoring non-necessary requests
await page.route(/(analytics)/, async (route) => {
await route.abort();
});
Expand All @@ -68,4 +68,65 @@ test.describe(`${process.env.E2E_TEST_ENV} - Authentication flow verification @s
},
);
});

test("Verify successful silent authentication with existing user", async ({
page,
authPage,
dashboardPage,
}, testInfo) => {
// speed up test by ignoring non-necessary requests
await page.route(/(analytics)/, async (route) => {
await route.abort();
});

// login to FxA
await authPage.signInToFxA(
process.env.E2E_TEST_ACCOUNT_EMAIL as string,
process.env.E2E_TEST_ACCOUNT_PASSWORD as string,
);

// start authentication flow
await authPage.initSilentAuth();

// assert successful login
await expect(dashboardPage.fixedTab).toBeVisible();
await expect(dashboardPage.actionNeededTab).toBeVisible();

await testInfo.attach(
`${process.env.E2E_TEST_ENV}-silent-authentication-monitor-dashboard.png`,
{
body: await page.screenshot(),
contentType: "image/png",
},
);
});

test("Verify failed silent authentication with existing user", async ({
page,
authPage,
dashboardPage,
}, testInfo) => {
// speed up test by ignoring non-necessary requests
await page.route(/(analytics)/, async (route) => {
await route.abort();
});

// start authentication flow
await authPage.initSilentAuth();

// sign in
await authPage.signIn(process.env.E2E_TEST_ACCOUNT_EMAIL as string);

// assert successful login
await expect(dashboardPage.fixedTab).toBeVisible();
await expect(dashboardPage.actionNeededTab).toBeVisible();

await testInfo.attach(
`${process.env.E2E_TEST_ENV}-silent-authentication-monitor-dashboard.png`,
{
body: await page.screenshot(),
contentType: "image/png",
},
);
});
});
7 changes: 7 additions & 0 deletions src/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@ export function middleware(request: NextRequest) {

const requestHeaders = new Headers(request.headers);
requestHeaders.set("x-nonce", nonce);

if (typeof process.env.E2E_TEST_ENV !== "undefined") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm worried that if this env var is set somehow in stage or prod, we would have this feature flag forced?

Would it be better to have more guards like && not in prod && not in stage?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking about this as well and had the condition first only to check for process.env.E2E_TEST_ENV === "local". Now you are mentioning it: Yes, let’s play it safe — changed in 4ae7859.

const forcedFeatureFlags =
request.nextUrl.searchParams.get("feature_flags");
requestHeaders.set("x-forced-feature-flags", forcedFeatureFlags ?? "");
}

// Add the CSP to the request headers - that will make Next.js detect it and
// add it to the inline `<script>` tags that it injects itself, as per
// https://github.com/vercel/next.js/discussions/51039#discussioncomment-6596642
Expand Down