Add webauth & magic links to sveltekit

[diksipav]

I tested this with example sveltekit and it works.

I retested and it works, but not sure is the setCookie function definition the best, especially the params it needs, doesn't look very pretty.

[scotttrinh]

Looks great! A couple of small things once you rebase this on the latest master.

[scotttrinh]

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.

[diksipav]

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

[scotttrinh]

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);
  }

[diksipav]

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

[scotttrinh]

🏆