Skip to content

Commit

Permalink
minor tweak: do not send error param leftover from /login validatio…
Browse files Browse the repository at this point in the history
…n to `/authorize` route
  • Loading branch information
ldgit committed Sep 6, 2024
1 parent c867739 commit 135930c
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 4 deletions.
20 changes: 20 additions & 0 deletions e2e/authentication.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,26 @@ test("Login should preserve query parameters on validation error", async ({ page
expectQueryParametersToBePreserved(page.url());
});

test("Login should remove error query parameter once user inputs correct credentials", async ({
page,
}) => {
await page.goto(
"/login?client_id=123&response_type=code&redirect_uri=https://www.example.com&scope=openid",
);

// Cause the error by submitting invalid data.
await page.getByRole("button", { name: "Sign in" }).click();
await expect(page.getByText("Wrong username or password.")).toBeVisible();
expect(page.url()).toContain("error=1");

// Enter correct credentials.
await page.getByLabel(/Username/).fill("MarkS");
await page.getByLabel(/Password/).fill("test");
await page.getByRole("button", { name: "Sign in" }).click();
expectQueryParametersToBePreserved(page.url());
expect(page.url()).not.toContain("error=1");
});

function expectQueryParametersToBePreserved(url: string) {
const loginPageSearchParams = new URL(url).searchParams;
expect(loginPageSearchParams.get("client_id")).toEqual("123");
Expand Down
7 changes: 3 additions & 4 deletions routes/frontend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,11 @@ export default async function frontend(fastify: FastifyInstance) {
await signInUser(user.id, reply.setCookie.bind(reply) as SetCookieHandler);

// If there are Oauth2 parameters in the query string redirect user back to /authorize endpoint
// to approve/deny the request.
// so the user can approve or deny the authorization request.
// We check that query string parameters are valid there.
if (request.query.redirect_uri) {
// TODO do not send error param
// const { error, ...oauth2Params } = request.query;
return reply.redirect(`/authorize?${querystring.stringify(request.query)}`);
const { error, ...oauth2Params } = request.query; // Remove the error parameter if present.
return reply.redirect(`/authorize?${querystring.stringify(oauth2Params)}`);
}

return reply.redirect("/");
Expand Down

0 comments on commit 135930c

Please sign in to comment.