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 missing verifier from password reset flow #781

Merged
merged 1 commit into from
Nov 20, 2023
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
26 changes: 18 additions & 8 deletions packages/auth-core/src/core.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import jwtDecode from "jwt-decode";
import * as edgedb from "edgedb";
import { ResolvedConnectConfig } from "edgedb/dist/conUtils";

Check failure on line 3 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (18, ubuntu-latest, stable)

All imports in the declaration are only used as types. Use `import type`

Check failure on line 3 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (20, ubuntu-latest, stable)

All imports in the declaration are only used as types. Use `import type`

Check failure on line 3 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, nightly)

All imports in the declaration are only used as types. Use `import type`

Check failure on line 3 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 3)

All imports in the declaration are only used as types. Use `import type`

Check failure on line 3 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 2)

All imports in the declaration are only used as types. Use `import type`

Check failure on line 3 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (macos-latest, 18, stable)

All imports in the declaration are only used as types. Use `import type`

import * as pkce from "./pkce";
import { BuiltinOAuthProviderNames, emailPasswordProviderName } from "./consts";

Check failure on line 6 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (18, ubuntu-latest, stable)

Import "BuiltinOAuthProviderNames" is only used as types

Check failure on line 6 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (20, ubuntu-latest, stable)

Import "BuiltinOAuthProviderNames" is only used as types

Check failure on line 6 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, nightly)

Import "BuiltinOAuthProviderNames" is only used as types

Check failure on line 6 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 3)

Import "BuiltinOAuthProviderNames" is only used as types

Check failure on line 6 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 2)

Import "BuiltinOAuthProviderNames" is only used as types

Check failure on line 6 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (macos-latest, 18, stable)

Import "BuiltinOAuthProviderNames" is only used as types

export interface TokenData {
auth_token: string;
Expand All @@ -25,7 +25,7 @@

static async create(client: edgedb.Client) {
const connectConfig: ResolvedConnectConfig = (
await (client as any).pool._getNormalizedConnectConfig()

Check warning on line 28 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (18, ubuntu-latest, stable)

Unexpected any. Specify a different type

Check warning on line 28 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (20, ubuntu-latest, stable)

Unexpected any. Specify a different type

Check warning on line 28 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, nightly)

Unexpected any. Specify a different type

Check warning on line 28 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 3)

Unexpected any. Specify a different type

Check warning on line 28 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 2)

Unexpected any. Specify a different type

Check warning on line 28 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (macos-latest, 18, stable)

Unexpected any. Specify a different type
).connectionParams;

const [host, port] = connectConfig.address;
Expand All @@ -37,7 +37,7 @@
}

/** @internal */
public async _get<T extends any = unknown>(path: string): Promise<T> {

Check warning on line 40 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (18, ubuntu-latest, stable)

Constraining the generic type `T` to `any` does nothing and is unnecessary

Check warning on line 40 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (18, ubuntu-latest, stable)

Unexpected any. Specify a different type

Check warning on line 40 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (20, ubuntu-latest, stable)

Constraining the generic type `T` to `any` does nothing and is unnecessary

Check warning on line 40 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (20, ubuntu-latest, stable)

Unexpected any. Specify a different type

Check warning on line 40 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, nightly)

Constraining the generic type `T` to `any` does nothing and is unnecessary

Check warning on line 40 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, nightly)

Unexpected any. Specify a different type

Check warning on line 40 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 3)

Constraining the generic type `T` to `any` does nothing and is unnecessary

Check warning on line 40 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 3)

Unexpected any. Specify a different type

Check warning on line 40 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 2)

Constraining the generic type `T` to `any` does nothing and is unnecessary

Check warning on line 40 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 2)

Unexpected any. Specify a different type

Check warning on line 40 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (macos-latest, 18, stable)

Constraining the generic type `T` to `any` does nothing and is unnecessary

Check warning on line 40 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (macos-latest, 18, stable)

Unexpected any. Specify a different type
const res = await fetch(new URL(path, this.baseUrl), {
method: "get",
});
Expand All @@ -47,13 +47,13 @@
if (res.headers.get("content-type")?.startsWith("application/json")) {
return res.json();
}
return null as any;

Check warning on line 50 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (18, ubuntu-latest, stable)

Unexpected any. Specify a different type

Check warning on line 50 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (20, ubuntu-latest, stable)

Unexpected any. Specify a different type

Check warning on line 50 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, nightly)

Unexpected any. Specify a different type

Check warning on line 50 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 3)

Unexpected any. Specify a different type

Check warning on line 50 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 2)

Unexpected any. Specify a different type

Check warning on line 50 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (macos-latest, 18, stable)

Unexpected any. Specify a different type
}

/** @internal */
public async _post<T extends any = unknown>(

Check warning on line 54 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (18, ubuntu-latest, stable)

Constraining the generic type `T` to `any` does nothing and is unnecessary

Check warning on line 54 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (18, ubuntu-latest, stable)

Unexpected any. Specify a different type

Check warning on line 54 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (20, ubuntu-latest, stable)

Constraining the generic type `T` to `any` does nothing and is unnecessary

Check warning on line 54 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (20, ubuntu-latest, stable)

Unexpected any. Specify a different type

Check warning on line 54 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, nightly)

Constraining the generic type `T` to `any` does nothing and is unnecessary

Check warning on line 54 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, nightly)

Unexpected any. Specify a different type

Check warning on line 54 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 3)

Constraining the generic type `T` to `any` does nothing and is unnecessary

Check warning on line 54 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 3)

Unexpected any. Specify a different type

Check warning on line 54 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 2)

Constraining the generic type `T` to `any` does nothing and is unnecessary

Check warning on line 54 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 2)

Unexpected any. Specify a different type

Check warning on line 54 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (macos-latest, 18, stable)

Constraining the generic type `T` to `any` does nothing and is unnecessary

Check warning on line 54 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (macos-latest, 18, stable)

Unexpected any. Specify a different type
path: string,
body?: any

Check warning on line 56 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (18, ubuntu-latest, stable)

Unexpected any. Specify a different type

Check warning on line 56 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (20, ubuntu-latest, stable)

Unexpected any. Specify a different type

Check warning on line 56 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, nightly)

Unexpected any. Specify a different type

Check warning on line 56 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 3)

Unexpected any. Specify a different type

Check warning on line 56 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 2)

Unexpected any. Specify a different type

Check warning on line 56 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (macos-latest, 18, stable)

Unexpected any. Specify a different type
): Promise<T> {
const res = await fetch(new URL(path, this.baseUrl), {
method: "post",
Expand All @@ -70,7 +70,7 @@
if (res.headers.get("content-type")?.startsWith("application/json")) {
return res.json();
}
return null as any;

Check warning on line 73 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (18, ubuntu-latest, stable)

Unexpected any. Specify a different type

Check warning on line 73 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (20, ubuntu-latest, stable)

Unexpected any. Specify a different type

Check warning on line 73 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, nightly)

Unexpected any. Specify a different type

Check warning on line 73 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 3)

Unexpected any. Specify a different type

Check warning on line 73 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (ubuntu-latest, 18, 2)

Unexpected any. Specify a different type

Check warning on line 73 in packages/auth-core/src/core.ts

View workflow job for this annotation

GitHub Actions / test (macos-latest, 18, stable)

Unexpected any. Specify a different type
}

createPKCESession() {
Expand Down Expand Up @@ -141,11 +141,16 @@
}

async sendPasswordResetEmail(email: string, resetUrl: string) {
return this._post<{ email_sent: string }>("send-reset-email", {
provider: emailPasswordProviderName,
email,
reset_url: resetUrl,
});
const { challenge, verifier } = pkce.createVerifierChallengePair();
return {
verifier,
...(await this._post<{ email_sent: string }>("send-reset-email", {
provider: emailPasswordProviderName,
challenge,
email,
reset_url: resetUrl,
})),
};
}

static checkPasswordResetTokenValid(resetToken: string) {
Expand All @@ -165,19 +170,24 @@
}
}

async resetPasswordWithResetToken(resetToken: string, password: string) {
return this._post<TokenData>("reset-password", {
async resetPasswordWithResetToken(
resetToken: string,
verifier: string,
password: string
) {
const { code } = await this._post<{ code: string }>("reset-password", {
provider: emailPasswordProviderName,
reset_token: resetToken,
password,
});
return this.getToken(code, verifier);
}

async getProvidersInfo() {
// TODO: cache this data when we have a way to invalidate on config update
try {
return await this.client.queryRequiredSingle<{
oauth: { name: string; display_name: string }[];
oauth: { name: BuiltinOAuthProviderNames; display_name: string }[];
emailPassword: boolean;
}>(`
with
Expand Down
41 changes: 34 additions & 7 deletions packages/auth-nextjs/src/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,10 +354,15 @@ export class NextAppAuth extends NextAuth {
["email"],
"email missing from request body"
);
(await this.core).sendPasswordResetEmail(
email,
this.options.passwordResetUrl
);
const { verifier } = await (
await this.core
).sendPasswordResetEmail(email, this.options.passwordResetUrl);
cookies().set({
name: this.options.pkceVerifierCookieName,
value: verifier,
httpOnly: true,
sameSite: "strict",
});
return new Response(null, { status: 204 });
}
case "emailpassword/reset-password": {
Expand All @@ -368,6 +373,14 @@ export class NextAppAuth extends NextAuth {
}
let tokenData: TokenData;
try {
const verifier = req.cookies.get(
this.options.pkceVerifierCookieName
)?.value;
if (!verifier) {
return onEmailPasswordReset({
error: new Error("no pkce verifier cookie found"),
});
}
const [resetToken, password] = _extractParams(
await _getReqBody(req),
["reset_token", "password"],
Expand All @@ -376,7 +389,7 @@ export class NextAppAuth extends NextAuth {

tokenData = await (
await this.core
).resetPasswordWithResetToken(resetToken, password);
).resetPasswordWithResetToken(resetToken, verifier, password);
} catch (err) {
return onEmailPasswordReset({
error: err instanceof Error ? err : new Error(String(err)),
Expand All @@ -388,6 +401,7 @@ export class NextAppAuth extends NextAuth {
httpOnly: true,
sameSite: "strict",
});
cookies().delete(this.options.pkceVerifierCookieName);
return onEmailPasswordReset({ error: null, tokenData });
}
case "emailpassword/resend-verification-email": {
Expand Down Expand Up @@ -472,30 +486,43 @@ export class NextAppAuth extends NextAuth {
throw new Error(`'passwordResetUrl' option not configured`);
}
const [email] = _extractParams(data, ["email"], "email missing");
await (
const { verifier } = await (
await this.core
).sendPasswordResetEmail(
email,
`${this.options.baseUrl}/${this.options.passwordResetUrl}`
);
cookies().set({
name: this.options.pkceVerifierCookieName,
value: verifier,
httpOnly: true,
sameSite: "strict",
});
},
emailPasswordResetPassword: async (
data: FormData | { resetToken: string; password: string }
) => {
const verifier = cookies().get(
this.options.pkceVerifierCookieName
)?.value;
if (!verifier) {
throw new Error("no pkce verifier cookie found");
}
const [resetToken, password] = _extractParams(
data,
["reset_token", "password"],
"reset_token or password missing"
);
const tokenData = await (
await this.core
).resetPasswordWithResetToken(resetToken, password);
).resetPasswordWithResetToken(resetToken, verifier, password);
cookies().set({
name: this.options.authCookieName,
value: tokenData.auth_token,
httpOnly: true,
sameSite: "strict",
});
cookies().delete(this.options.pkceVerifierCookieName);
return tokenData;
},
emailPasswordResendVerificationEmail: async (
Expand Down
Loading