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.

Sign up for GitHub

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

Jump to bottom

Add webauth & magic links to sveltekit #973

Merged
merged 5 commits into from
May 17, 2024
Merged

Add webauth & magic links to sveltekit #973

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
06d1773
Add webauth & magic links to sveltekit
diksipav Apr 18, 2024
7e5b7bb
Set cookies w setVerifierCookie & setAuthCookie
diksipav May 8, 2024
0135023
Use set/del cookie funcs instead of class methods
diksipav May 9, 2024
7c58a83
Update set cookie functions
diksipav May 17, 2024
653af7f
Add class methods for setting/deleting cookies
diksipav May 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
137 changes: 54 additions & 83 deletions packages/auth-sveltekit/src/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,34 @@ export default function serverAuth(client: Client, options: AuthOptions) {
};
}

function setCookie(
cookies: Cookies,
value: string,
config: AuthConfig,
isAuth = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I prefer two separate functions here than a boolean with a default value. If you want to share the base configuration object between both functions, that's fine! Something like:

const BASE_COOKIE_CONFIG = {
  httpOnly: true,
  sameSite: "lax",
  path: "/",
};

then in the functions:

function setVerifierCookie(cookies: Cookies, config: AuthConfig, value: string): void {
  const expires = new Date(Date.now() + 1000 * 60 * 24 * 7); // in 7 days
  cookies.set(config.pkceVerifierCookieName, value, {
    ...BASE_COOKIE_CONFIG,
    expires,
    secure: config.baseUrl.startsWith("https"),
  });
}

Then in the class you can:

private setVerifierCookie(verifier: string) {
  setVerifierCookie(this.cookies, this.config, verifier);
}

The advantage here is that you can look at the call site and know which cookie it's setting without knowing if the optional true means verifier or auth token, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I agree with all of these, will update : )

) {
const isSecure = config.baseUrl.startsWith("https");
const name = isAuth ? config.authCookieName : config.pkceVerifierCookieName;

const expires = isAuth
? Auth.getTokenExpiration(value)
: new Date(Date.now() + 1000 * 60 * 24 * 7); // In 7 days

cookies.set(name, value, {
httpOnly: true,
sameSite: "lax",
path: "/",
expires: expires ?? undefined,
secure: isSecure,
});
}

function deleteCookie(cookies: Cookies, name: string) {
cookies.set(name, "", {
path: "/",
});
}

export class ServerRequestAuth extends ClientAuth {
private readonly client: Client;
private readonly core: Promise<Auth>;
Expand Down Expand Up @@ -141,28 +169,6 @@ export class ServerRequestAuth extends ClientAuth {
return Auth.checkPasswordResetTokenValid(resetToken);
}

private setVerifierCookie(verifier: string) {
const expires = new Date(Date.now() + 1000 * 60 * 24 * 7); // In 7 days
this.cookies.set(this.config.pkceVerifierCookieName, verifier, {
httpOnly: true,
sameSite: "lax",
path: "/",
expires,
secure: this.isSecure,
});
}

private setAuthCookie(authToken: string) {
const expires = Auth.getTokenExpiration(authToken);
this.cookies.set(this.config.authCookieName, authToken, {
httpOnly: true,
sameSite: "lax",
path: "/",
expires: expires ?? undefined,
secure: this.isSecure,
});
}
Comment on lines -135 to -155
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a blocker, but the change here would be smaller and easier to follow for future endpoints if we kept these class methods and just had them call the new function, so that other methods could continue to just call this.setAuthCookie(tokenData.auth_token), etc.

  private setVerifierCookie(verifier: string) {
    setVerifierCookie(this.cookies, this.config, verifier);
  }

  private setAuthCookie(authToken: string) {
    setAuthCookie(this.cookies, this.config, authToken);
  }

And for bonus points doing a similar thing for the delete cookie functionality:

  private deleteVerifierCookie() {
    deleteCookie(this.cookies, this.config.pkceVerifierCookieName);
  }

  private deleteAuthCookie() {
    deleteCookie(this.cookies, this.config.authCookieName);
  }

Copy link
Contributor Author

@diksipav diksipav May 17, 2024 •
edited

Choose a reason for hiding this comment

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

hmm yes, this is even more cleaner (for cookies inside class methods), will update : )
but still we have to use functions for cookies outside the class


async getProvidersInfo() {
return (await this.core).getProvidersInfo();
}
Expand All @@ -184,12 +190,12 @@ export class ServerRequestAuth extends ClientAuth {
`${this.config.authRoute}/emailpassword/verify`
);

this.setVerifierCookie(result.verifier);
setCookie(this.cookies, result.verifier, this.config);

if (result.status === "complete") {
const tokenData = result.tokenData;

this.setAuthCookie(tokenData.auth_token);
setCookie(this.cookies, tokenData.auth_token, this.config, true);

return { tokenData };
}
Expand Down Expand Up @@ -225,7 +231,7 @@ export class ServerRequestAuth extends ClientAuth {
`${this.config.authRoute}/emailpassword/verify`
);

this.setVerifierCookie(verifier);
setCookie(this.cookies, verifier, this.config);
} else {
throw new InvalidDataError(
"expected 'verification_token' or 'email' in data"
Expand All @@ -246,7 +252,7 @@ export class ServerRequestAuth extends ClientAuth {
await this.core
).signinWithEmailPassword(email, password);

this.setAuthCookie(tokenData.auth_token);
setCookie(this.cookies, tokenData.auth_token, this.config, true);

return { tokenData };
}
Expand All @@ -267,7 +273,7 @@ export class ServerRequestAuth extends ClientAuth {
new URL(this.config.passwordResetPath, this.config.baseUrl).toString()
);

this.setVerifierCookie(verifier);
setCookie(this.cookies, verifier, this.config);
}

async emailPasswordResetPassword(
Expand All @@ -289,11 +295,10 @@ export class ServerRequestAuth extends ClientAuth {
await this.core
).resetPasswordWithResetToken(resetToken, verifier, password);

this.setAuthCookie(tokenData.auth_token);
setCookie(this.cookies, tokenData.auth_token, this.config, true);

deleteCookie(this.cookies, this.config.pkceVerifierCookieName);

this.cookies.delete(this.config.pkceVerifierCookieName, {
path: "/",
});
return { tokenData };
}

Expand All @@ -313,7 +318,7 @@ export class ServerRequestAuth extends ClientAuth {
new URL(this.config.magicLinkFailurePath, this.config.baseUrl).toString()
);

this.setVerifierCookie(verifier);
setCookie(this.cookies, verifier, this.config);
}

async magicLinkSend(data: { email: string } | FormData): Promise<void> {
Expand All @@ -332,7 +337,7 @@ export class ServerRequestAuth extends ClientAuth {
new URL(this.config.magicLinkFailurePath, this.config.baseUrl).toString()
);

this.setVerifierCookie(verifier);
setCookie(this.cookies, verifier, this.config);
}

async webAuthnSignIn(data: {
Expand All @@ -345,7 +350,7 @@ export class ServerRequestAuth extends ClientAuth {
await this.core
).signinWithWebAuthn(email, assertion);

this.setAuthCookie(tokenData.auth_token);
setCookie(this.cookies, tokenData.auth_token, this.config, true);

return { tokenData };
}
Expand All @@ -362,12 +367,12 @@ export class ServerRequestAuth extends ClientAuth {
await this.core
).signupWithWebAuthn(email, credentials, verify_url, user_handle);

this.setVerifierCookie(result.verifier);
setCookie(this.cookies, result.verifier, this.config);

if (result.status === "complete") {
const tokenData = result.tokenData;

this.setAuthCookie(tokenData.auth_token);
setCookie(this.cookies, tokenData.auth_token, this.config, true);

return { tokenData };
}
Expand All @@ -376,7 +381,7 @@ export class ServerRequestAuth extends ClientAuth {
}

async signout(): Promise<void> {
this.cookies.delete(this.config.authCookieName, { path: "/" });
deleteCookie(this.cookies, this.config.authCookieName);
}
}

Expand Down Expand Up @@ -466,10 +471,7 @@ async function handleAuthRoutes(
const redirectUrl = `${config.authRoute}/oauth/callback`;
const pkceSession = await core.then((core) => core.createPKCESession());

cookies.set(config.pkceVerifierCookieName, pkceSession.verifier, {
httpOnly: true,
path: "/",
});
setCookie(cookies, pkceSession.verifier, config);

return redirect(
307,
Expand Down Expand Up @@ -518,16 +520,9 @@ async function handleAuthRoutes(
});
}

cookies.set(config.authCookieName, tokenData.auth_token, {
httpOnly: true,
sameSite: "lax",
path: "/",
});
setCookie(cookies, tokenData.auth_token, config, true);

cookies.set(config.pkceVerifierCookieName, "", {
maxAge: 0,
path: "/",
});
deleteCookie(cookies, config.pkceVerifierCookieName);

return onOAuthCallback({
error: null,
Expand Down Expand Up @@ -584,16 +579,9 @@ async function handleAuthRoutes(
});
}

cookies.set(config.authCookieName, tokenData.auth_token, {
httpOnly: true,
sameSite: "strict",
path: "/",
});
setCookie(cookies, tokenData.auth_token, config, true);

cookies.set(config.pkceVerifierCookieName, "", {
maxAge: 0,
path: "/",
});
deleteCookie(cookies, config.pkceVerifierCookieName);

return onBuiltinUICallback({
error: null,
Expand All @@ -607,10 +595,7 @@ async function handleAuthRoutes(
case "builtin/signup": {
const pkceSession = await core.then((core) => core.createPKCESession());

cookies.set(config.pkceVerifierCookieName, pkceSession.verifier, {
httpOnly: true,
path: "/",
});
deleteCookie(cookies, config.pkceVerifierCookieName);

return redirect(
307,
Expand Down Expand Up @@ -651,11 +636,7 @@ async function handleAuthRoutes(
});
}

cookies.set(config.authCookieName, tokenData.auth_token, {
httpOnly: true,
sameSite: "strict",
path: "/",
});
setCookie(cookies, tokenData.auth_token, config, true);

return onEmailVerify({
error: null,
Expand Down Expand Up @@ -702,16 +683,9 @@ async function handleAuthRoutes(
});
}

cookies.set(config.authCookieName, tokenData.auth_token, {
httpOnly: true,
sameSite: "lax",
path: "/",
});
setCookie(cookies, tokenData.auth_token, config, true);

cookies.set(config.pkceVerifierCookieName, "", {
maxAge: 0,
path: "/",
});
deleteCookie(cookies, config.pkceVerifierCookieName);

return onMagicLinkCallback({
error: null,
Expand Down Expand Up @@ -769,11 +743,7 @@ async function handleAuthRoutes(
});
}

cookies.set(config.authCookieName, tokenData.auth_token, {
httpOnly: true,
sameSite: "strict",
path: "/",
});
setCookie(cookies, tokenData.auth_token, config, true);

return onEmailVerify({
error: null,
Expand All @@ -787,7 +757,8 @@ async function handleAuthRoutes(
`'onSignout' auth route handler not configured`
);
}
cookies.delete(config.authCookieName, { path: "/" });

deleteCookie(cookies, config.authCookieName);
return onSignout();
}

Expand Down