feat: Hanko passkeys support #14741

merlindru · 2024-04-25T00:22:30Z

Note: This previously was #13127 but has since been reworked and simplified

What does this PR do?

Fixes #13342
Fixes CAL-2984

This PR adds passkey support for logging in through Hanko Passkeys.

Type of change

New feature (non-breaking change which adds functionality)
This change requires a documentation update (probably)

How should this be tested?

Go to cloud.hanko.io and create a "Passkey API" project
- Then create a new API key
  set it to HANKO_PASSKEYS_API_KEY="<api key>"
  (quotes important)
- Copy the tenant ID to NEXT_PUBLIC_HANKO_PASSKEYS_TENANT_ID=<tenant id>
Log in as normal (with password/google/...)
Go to settings > passkeys > create a new passkey
Log out
Go to login > sign in with a passkey

Mandatory Tasks

Make sure you have self-reviewed the code. A decent size PR without self-review might be rejected.

next-auth-options.ts

This file now includes a PasskeyProvider.

For its authorize function, I've collected all of the important checks I could find in the email+password provider (a.k.a. CredentialsProvider), such as rate-limiting, user.locked, belongsToActiveTeams, ...) and included them in the PasskeyProvider.

For some of these, the fields/relations returned by UserRepository.findById wasn't enough. Unfortunately .findTeamsByUserId doesn't return enough info either

I didn't want to touch any of the existing queries, so I've added UserRepository.findByIdAndIncludeProfilesAndPassword

...which is the exact same as the email version of this function (findByEmailAndInclude...). It just looks the user up by their ID instead.

Checklist

I haven't checked if my PR needs changes to the documentation (unlikely though)
I haven't added tests that prove my fix is effective or that my feature works (this is possible with Playwright)

UI

Note about images

These are the same images from the old PR. There's one minor change since then: since icon components aren't supported for Button, EmptyPage, ... anymore, I used Lucide's key-round icon

This is what key-round looks like:

Versus the passkey icon:

Adds passkey button to login page:

Adds "Passkeys" settings section to sidebar:

Adds passkeys settings page

Empty:

Clicking on the "add" button and "sign in with a passkey" buttons opens a browser dialog. This looks different for every browser, phone, etc.

For example:

With passkeys added:

The name is set by the Hanko Passkeys API (our backend). We'll improve the name soon so it's not just a semi-random string and more user friendly
If the passkey was used, the description says e.g. "Created 3 days ago • Last used 5 minutes ago"

Dropdown stolen straight from API settings page:

vercel · 2024-04-25T00:22:36Z

@merlindru is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

github-actions · 2024-04-25T00:22:43Z

Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link.

socket-security · 2024-04-25T00:23:42Z

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/js-tokens@4.0.0	None	`0`	15.1 kB	lydell
npm/object-assign@4.1.1	None	`0`	5.49 kB	sindresorhus
npm/regenerator-runtime@0.14.1	None	`0`	27.9 kB	benjamn
npm/typescript@5.4.5	None	`0`	32.4 MB	typescript-bot

🚮 Removed packages: npm/superjson@1.9.1

View full report↗︎

PeerRich · 2024-04-25T07:45:31Z

.env.example

@@ -116,15 +116,31 @@ NEXT_PUBLIC_FRESHCHAT_HOST=
 # @see https://support.google.com/cloud/answer/6158849#public-and-internal&zippy=%2Cpublic-and-internal-applications
 GOOGLE_LOGIN_ENABLED=false

-#   - GOOGLE CALENDAR/MEET/LOGIN
+# Hanko Passkeys Config


lets mention this lower in the file

yeah was unsure about that. wanted to place it below google auth stuff since i did that everywhere else, but agreed, for the env file its a little too high up

whops

merlindru · 2024-04-25T08:05:51Z

We'd really like to use the passkey icon (not part of lucide, it's an SVG)

however the components that made use of it (Button, EmptyPage, ...) have all been changed to only accept a lucide icon name (string)

Is there any way we still could use a custom component as an icon for these?

graphite-app · 2024-04-25T08:10:12Z

Graphite Automations

"Add foundation team as reviewer" took an action on this PR • (04/25/24)

1 reviewer was added to this PR based on Keith Williams's automation.

"Add consumer team as reviewer" took an action on this PR • (04/25/24)

1 reviewer was added to this PR based on Keith Williams's automation.

merlindru · 2024-04-25T08:12:34Z

While PASSKEY_LOGIN_ENABLED hides all passkey related UI on the login page, the settings page as well as the API routes to register and remove passkeys stay enabled

These aren't strictly related to logging in with Passkeys; do you feel like the env var should disable everything related to passkeys or just the ability to login?

emrysal · 2024-04-25T12:43:55Z

apps/web/package.json

@@ -62,6 +63,7 @@
    "@stripe/react-stripe-js": "^1.10.0",
    "@stripe/stripe-js": "^1.35.0",
    "@tanstack/react-query": "^5.17.15",
+    "@teamhanko/passkeys-next-auth-provider": "^0.2.7",


I wish Node had a better way to make optional dependencies 🤷‍♂️

I'm not sure since when npm has this, but there is optionalDependencies

https://docs.npmjs.com/cli/v10/configuring-npm/package-json#optionaldependencies

Was supported in yarn v1 so I assume v2+ has it too

Opted not to do this for now because optionalDependencies isn't in use for any other packages atm. Also I'm unsure if a build without installing optional deps would be successful, especially because of import behavior, haven't tested it

Do you want me to test this? What other not-strictly-needed dependencies are there that could be moved to optionalDependencies (at least in theory)?

emrysal · 2024-04-25T12:48:29Z

apps/web/pages/api/auth/passkeys/register/finalize.ts

+
+export default async function handler(req: NextApiRequest, res: NextApiResponse) {
+  if (!tenantId || !apiKey) {
+    return res.status(503).json({ message: "Passkey API not configured" });


Is 503 the best status code to use here?

Other routes use 403: https://github.com/calcom/cal.com/blob/main/apps/web/pages/api/auth/signup.ts#L23

However, MDN says

[...] 503 Service Unavailable server error response code indicates that the server is not ready to handle the request.

Common causes are a server that is down for maintenance or that is overloaded. This response should be used for temporary conditions [...]

While in my mind 403 has always been more for insufficient permissions etc

What do you think? Maybe 403 for consistency's sake?

I'd prefer 403 Forbidden, 503 implies a server issue but most of the time this'll be a client issue (don't access the route)

Don't think a 403 is correct here though because that leads the client to believe they are lacking permissions when in reality the functionality isn't supported at all due to a lack of server configuration. Would consider 501 Not Implemented or 405 Method Not Allowed instead.

See https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/501

If everyone is fine with it, will go with 501 - seems like the most appropriate IMO

emrysal · 2024-04-25T12:50:45Z

apps/web/pages/auth/login.tsx

+                    onClick={async (e) => {
+                      e.preventDefault();
+                      await signInWithPasskey({
+                        tenantId: process.env.NEXT_PUBLIC_HANKO_PASSKEYS_TENANT_ID!,


Forbidden non-null assertion needs an eslint comment to exclude or check to prevent

I'll check for it explicitly

emrysal · 2024-04-25T12:52:31Z

apps/web/pages/settings/security/passkeys.tsx

+  lastUsedAt?: string;
+}
+
+const PasskeyListItem = (props: Passkey & { onRemoved?: () => any }) => {


Try to avoid using any

Crap, I resolved this ☝️ but missed an onError: (e: any) => {} in another place. Sorry - I'll wait for more feedback before fixing this (so I don't make too many commits)

emrysal

While PASSKEY_LOGIN_ENABLED hides all passkey related UI on the login page, the settings page as well as the API routes to register and remove passkeys stay enabled

These aren't strictly related to logging in with Passkeys; do you feel like the env var should disable everything related to passkeys or just the ability to login?

In line with current practises it's not needed to fully "unregister" routes

We'd really like to use the passkey icon (not part of lucide, it's an SVG)

Feel free to modify the button component to allow StartIcon's that aren't strings, ideally in a way it doesn't accept lucide icons (strings should be used instead).

sean-brydon · 2024-04-28T09:27:20Z

packages/features/auth/lib/next-auth-options.ts

+const IS_PASSKEY_LOGIN_ENABLED = !!(
+  process.env.PASSKEY_LOGIN_ENABLED === "true" &&
+  process.env.NEXT_PUBLIC_HANKO_PASSKEYS_TENANT_ID &&
+  process.env.HANKO_PASSKEYS_API_KEY
+);
+


I think we already have this in constants - lets put it from there

We do, but the GOOGLE variables are also duplicated in next-auth-options, so I did the same to stay consistent. Do you know if there's any reason for this?

Otherwise we could import both from constants.ts.
(I don't think the google constants differ from the ones in next-aut-options.ts in any way)

Hanko passkeys support

eddbd4e

github-actions bot added app-store area: app store, apps, calendar integrations, google calendar, outlook, lark, apple calendar app-store-template Created by Linear-GitHub Sync community Created by Linear-GitHub Sync foundation Low priority Created by Linear-GitHub Sync labels Apr 25, 2024

github-actions bot added the ❗️ .env changes contains changes to env variables label Apr 25, 2024

PeerRich changed the title ~~Hanko passkeys support~~ feat: Hanko passkeys support Apr 25, 2024

PeerRich previously approved these changes Apr 25, 2024

View reviewed changes

merlindru marked this pull request as ready for review April 25, 2024 08:06

graphite-app bot requested review from a team April 25, 2024 08:06

dosubot bot added authentication area: authentication, auth, google sign in, password, SAML, password reset, can't log in ✨ feature New feature or request labels Apr 25, 2024

emrysal reviewed Apr 25, 2024

View reviewed changes

emrysal requested changes Apr 25, 2024

View reviewed changes

Passkeys revision round 1

df7482e

sean-brydon reviewed Apr 28, 2024

View reviewed changes

merlindru requested review from emrysal and keithwillcode April 29, 2024 17:11

keithwillcode added this to the v4.1 milestone May 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Hanko passkeys support #14741

feat: Hanko passkeys support #14741

merlindru commented Apr 25, 2024

vercel bot commented Apr 25, 2024

github-actions bot commented Apr 25, 2024 •

edited

socket-security bot commented Apr 25, 2024

PeerRich Apr 25, 2024

merlindru Apr 25, 2024

merlindru commented Apr 25, 2024

graphite-app bot commented Apr 25, 2024

merlindru commented Apr 25, 2024

emrysal Apr 25, 2024

merlindru Apr 25, 2024 •

edited

merlindru Apr 26, 2024

emrysal Apr 25, 2024

merlindru Apr 25, 2024 •

edited

emrysal Apr 25, 2024

keithwillcode Apr 25, 2024 •

edited

keithwillcode Apr 25, 2024

merlindru Apr 25, 2024

emrysal Apr 25, 2024

merlindru Apr 25, 2024

emrysal Apr 25, 2024

merlindru Apr 27, 2024

emrysal left a comment •

edited

sean-brydon Apr 28, 2024

merlindru Apr 29, 2024

feat: Hanko passkeys support #14741

Are you sure you want to change the base?

feat: Hanko passkeys support #14741

Conversation

merlindru commented Apr 25, 2024

What does this PR do?

Type of change

How should this be tested?

Mandatory Tasks

next-auth-options.ts

Checklist

UI

Note about images

Adds passkey button to login page:

Adds "Passkeys" settings section to sidebar:

Adds passkeys settings page

vercel bot commented Apr 25, 2024

github-actions bot commented Apr 25, 2024 • edited

socket-security bot commented Apr 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

merlindru commented Apr 25, 2024

graphite-app bot commented Apr 25, 2024

Graphite Automations

merlindru commented Apr 25, 2024

Choose a reason for hiding this comment

merlindru Apr 25, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

merlindru Apr 25, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

keithwillcode Apr 25, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emrysal left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Apr 25, 2024 •

edited

merlindru Apr 25, 2024 •

edited

merlindru Apr 25, 2024 •

edited

keithwillcode Apr 25, 2024 •

edited

emrysal left a comment •

edited