Skip to content

Commit

Permalink
Address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jaclarke committed Nov 6, 2023
1 parent 7d5f0ca commit 41daab2
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 77 deletions.
37 changes: 37 additions & 0 deletions packages/auth-core/genConsts.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
const fs = require("node:fs/promises");
const { createClient } = require("edgedb");

const client = createClient("_localdev");

(async function () {
const OAuthProviders = await client.query(`
with providers := (
select schema::ObjectType
filter .bases.name = 'ext::auth::OAuthProviderConfig'
)
select (
select providers.properties filter .name = 'name'
).default`);

await fs.writeFile(
"./src/consts.ts",
`// AUTOGENERATED - Run \`yarn gen-consts\` to re-generate.
export const builtinOAuthProviderNames = [
${OAuthProviders.sort()
.map((provider) => ` ${provider.replace(/^'|'$/g, '"')},`)
.join("\n")}
] as const;
export type BuiltinOAuthProviderNames =
(typeof builtinOAuthProviderNames)[number];
export const builtinLocalProviderNames = [
"builtin::local_emailpassword",
] as const;
export type BuiltinLocalProviderNames =
(typeof builtinLocalProviderNames)[number];
`
);

console.log('Generated into "src/consts.ts"');
})();
3 changes: 2 additions & 1 deletion packages/auth-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
],
"scripts": {
"test": "jest --detectOpenHandles",
"build": "tsc --project tsconfig.json"
"build": "tsc --project tsconfig.json",
"gen-consts": "node genConsts.js"
},
"devDependencies": {
"@types/node": "^20.8.4",
Expand Down
16 changes: 16 additions & 0 deletions packages/auth-core/src/consts.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// AUTOGENERATED - Run `yarn gen-consts` to re-generate.

export const builtinOAuthProviderNames = [
"builtin::oauth_apple",
"builtin::oauth_azure",
"builtin::oauth_github",
"builtin::oauth_google",
] as const;
export type BuiltinOAuthProviderNames =
(typeof builtinOAuthProviderNames)[number];

export const builtinLocalProviderNames = [
"builtin::local_emailpassword",
] as const;
export type BuiltinLocalProviderNames =
(typeof builtinLocalProviderNames)[number];
148 changes: 73 additions & 75 deletions packages/auth-core/src/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ 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 (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`

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

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)

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

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)

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

export interface TokenData {
auth_token: string;
Expand All @@ -11,21 +12,6 @@ export interface TokenData {
provider_refresh_token: string | null;
}

export const builtinOAuthProviderNames = [
"builtin::oauth_apple",
"builtin::oauth_azure",
"builtin::oauth_github",
"builtin::oauth_google",
] as const;
export type BuiltinOAuthProviderNames =
(typeof builtinOAuthProviderNames)[number];

export const builtinLocalProviderNames = [
"builtin::local_emailpassword",
] as const;
export type BuiltinLocalProviderNames =
(typeof builtinLocalProviderNames)[number];

export class Auth {
/** @internal */
public readonly baseUrl: string;
Expand All @@ -51,29 +37,26 @@ export class Auth {
}

/** @internal */
public async _fetch(
path: string,
method: "get",
searchParams?: Record<string, string>
): Promise<unknown>;
public async _fetch(
path: string,
method: "post",
searchParams?: Record<string, string>,
body?: any
): Promise<unknown>;
public async _fetch(
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 (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
const res = await fetch(new URL(path, this.baseUrl), {
method: "get",
});
if (!res.ok) {
throw new Error(await res.text());
}
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 (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
}

/** @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 (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
path: string,
method: "get" | "post",
searchParams?: Record<string, string>,
body?: any

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
) {
const url = `${this.baseUrl}/${path}${
searchParams ? "?" + new URLSearchParams(searchParams).toString() : ""
}`;
const res = await fetch(url, {
method,
// verbose: true,
): Promise<T> {
const res = await fetch(new URL(path, this.baseUrl), {
method: "post",
...(body != null
? {
body: JSON.stringify(body),
Expand All @@ -87,28 +70,30 @@ export class Auth {
if (res.headers.get("content-type")?.startsWith("application/json")) {
return res.json();
}
return null;
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 (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
}

createPKCESession() {
return new AuthPCKESession(this);
}

getToken(code: string, verifier: string): Promise<TokenData> {
return this._fetch("token", "get", {
code,
verifier,
}) as Promise<TokenData>;
return this._get<TokenData>(
`token?${new URLSearchParams({
code,
verifier,
}).toString()}`
);
}

async signinWithEmailPassword(email: string, password: string) {
const { challenge, verifier } = pkce.createVerifierChallengePair();
const { code } = (await this._fetch("authenticate", "post", undefined, {
const { code } = await this._post<{ code: string }>("authenticate", {
provider: "builtin::local_emailpassword",
challenge,
email,
password,
})) as { code: string };
});
return this.getToken(code, verifier);
}

Expand All @@ -121,13 +106,15 @@ export class Auth {
| { status: "verificationRequired"; verifier: string }
> {
const { challenge, verifier } = pkce.createVerifierChallengePair();
const result = (await this._fetch("register", "post", undefined, {
const result = await this._post<
{ code: string } | { verification_email_sent_at: string }
>("register", {
provider: "builtin::local_emailpassword",
challenge,
email,
password,
verify_url: verifyUrl,
})) as { code: string } | { verification_email_sent_at: string };
});
if ("code" in result) {
return {
status: "complete",
Expand All @@ -139,26 +126,26 @@ export class Auth {
}

async verifyEmailPasswordSignup(verificationToken: string, verifier: string) {
const { code } = (await this._fetch("verify", "post", undefined, {
const { code } = await this._post<{ code: string }>("verify", {
provider: "builtin::local_emailpassword",
verification_token: verificationToken,
})) as { code: string };
});
return this.getToken(code, verifier);
}

async resendVerificationEmail(verificationToken: string) {
await this._fetch("resend-verification-email", "post", undefined, {
await this._post("resend-verification-email", {
provider: "builtin::local_emailpassword",
verification_token: verificationToken,
});
}

async sendPasswordResetEmail(email: string, resetUrl: string) {
return this._fetch("send-reset-email", "post", undefined, {
return this._post<{ email_sent: string }>("send-reset-email", {
provider: "builtin::local_emailpassword",
email,
reset_url: resetUrl,
}) as Promise<{ email_sent: string }>;
});
}

static checkPasswordResetTokenValid(resetToken: string) {
Expand All @@ -179,22 +166,35 @@ export class Auth {
}

async resetPasswordWithResetToken(resetToken: string, password: string) {
return this._fetch("reset-password", "post", undefined, {
return this._post<TokenData>("reset-password", {
provider: "builtin::local_emailpassword",
reset_token: resetToken,
password,
}) as Promise<TokenData>;
});
}

async getProvidersInfo() {
// TODO: cache this data somehow?
const providers = (await this.client.query(`
with module ext::auth
select cfg::Config.extensions[is AuthConfig].providers {
_typename := .__type__.name,
name,
[is OAuthProviderConfig].display_name,
}`)) as { _typename: string; name: string; display_name: string | null }[];
// TODO: cache this data when we have a way to invalidate on config update
let providers: {
_typename: string;
name: string;
display_name: string | null;
}[];
try {
providers = await this.client.query(`
with module ext::auth
select cfg::Config.extensions[is AuthConfig].providers {
_typename := .__type__.name,
name,
[is OAuthProviderConfig].display_name,
}`);
} catch (err) {
if (err instanceof edgedb.InvalidReferenceError) {
throw new Error("auth extension is not enabled");
}
throw err;
}

const emailPasswordProvider = providers.find(
(p) => p.name === "builtin::local_emailpassword"
);
Expand Down Expand Up @@ -226,17 +226,17 @@ export class AuthPCKESession {
redirectTo: string,
redirectToOnSignup?: string
) {
const params = new URLSearchParams({
provider: providerName,
challenge: this.challenge,
redirect_to: redirectTo,
});
const url = new URL("authorize", this.auth.baseUrl);

url.searchParams.set("provider", providerName);
url.searchParams.set("challenge", this.challenge);
url.searchParams.set("redirect_to", redirectTo);

if (redirectToOnSignup) {
params.append("redirect_to_on_signup", redirectToOnSignup);
url.searchParams.set("redirect_to_on_signup", redirectToOnSignup);
}

return `${this.auth.baseUrl}/authorize?${params.toString()}`;
return url.toString();
}

// getEmailPasswordSigninFormActionUrl(
Expand Down Expand Up @@ -274,18 +274,16 @@ export class AuthPCKESession {
// }

getHostedUISigninUrl() {
const params = new URLSearchParams({
challenge: this.challenge,
});
const url = new URL("ui/signin", this.auth.baseUrl);
url.searchParams.set("challenge", this.challenge);

return `${this.auth.baseUrl}/ui/signin?${params.toString()}`;
return url.toString();
}

getHostedUISignupUrl() {
const params = new URLSearchParams({
challenge: this.challenge,
});
const url = new URL("ui/signup", this.auth.baseUrl);
url.searchParams.set("challenge", this.challenge);

return `${this.auth.baseUrl}/ui/signup?${params.toString()}`;
return url.toString();
}
}
1 change: 1 addition & 0 deletions packages/auth-core/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from "./core";
export * from "./pkce";
export * from "./consts";
2 changes: 1 addition & 1 deletion packages/auth-core/src/pkce.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ export function createVerifierChallengePair(): {
verifier: string;
challenge: string;
} {
const verifier = crypto.randomBytes(32).toString("hex");
const verifier = crypto.randomBytes(32).toString("base64url");
const challenge = crypto
.createHash("sha256")
.update(verifier)
Expand Down

0 comments on commit 41daab2

Please sign in to comment.