edgedb-js icon indicating copy to clipboard operation
edgedb-js copied to clipboard

Add webhook callback to auth helper libraries

Open scotttrinh opened this issue 9 months ago • 6 comments

We can validate the shape, verify the signature (if present), and present the developer with a nice API like:

type WebhookEvent =
  | EmailVerificationRequestedEvent
  | PasswordResetRequestedEvent;

// in `createAuthRouteHandlers`
async onWebhook(data: WebhookEvent) {
  switch (data.event_type) {
    case "EmailVerificationRequested": {
      await emails.sendVerification(data.email, data.verification_token);
      return;
    }
    case "PasswordResetRequested": {
      await emails.sendPasswordReset(data.email, data.reset_token);
      return;
    }
}

For Next.js we could even automatically wrap this in after so it doesn't block sending the webhook response.

scotttrinh avatar Feb 25 '25 15:02 scotttrinh

Hey, I am about to add the signature checking, and thought that maybe it will be in the library, which I think it needs to be. But I see a problem with the switch for the events. My implementation is something similar:

type Event =
  | PasswordResetRequestedWebhookEvent
  | EmailVerificationRequestedWebhookEvent;

export async function POST(req: NextRequest) {
  const event = (await req.json()) as Event;

  const emailFactor = await selectEmailFactorQuery.run(client, {
    id: event.email_factor_id,
  });

  if (!emailFactor) {
    return Response.json({ body: "Email factor not found" }, { status: 404 });
  }

  try {
    if (event.event_type === "EmailVerificationRequested") {
      await sendVerifyEmail(emailFactor, event);
    } else if (event.event_type === "PasswordResetRequested") {
      await sendPasswordReset(emailFactor, event);
    }
  } catch {
    console.error(
      `[POST /auth/webhook] failed to send email ${event.event_type}`,
    );
  }

  return Response.json({ body: "OK" });
}

So instead, I got the idea that the final api might be incorporated like this:

// /app/auth/[...auth]/route.ts

import { auth } from "@/gel";

const { GET, POST } = auth.createAuthRouteHandlers({
  webhook: {
    onEmailVerificationRequested({email, event}) {
      await sendVerifyEmail(email, event);
    }
    onPasswordResetRequested({email, event}) {
      await sendPasswordReset(email, event);
    }
  }
});

export { GET, POST };

@scotttrinh what do you think? Here we don't need to switch by the email names or import event types etc. We will get autocompletion for the handlers.

Also, note that the events contain only email_factor_id so I guess the handler will get the email by something like selectEmailFactorQuery (your code assumes that the event data has email while in practice it's not there)

MiroslavPetrik avatar Mar 31 '25 11:03 MiroslavPetrik

I am about to add the signature checking, and thought that maybe it will be in the library, which I think it needs to be.

Yes, absolutely!

So instead, I got the idea that the final api might be incorporated like this:

This is a nice API, especially if you do something different for every webhook request. Maybe we can have both (onAnyWebhook or something similar) so that if you're using webhooks for some kind of synchronization or logging/auditing thing you don't have to duplicate your logic in every handler.

note that the events contain only email_factor_id so I guess the handler will get the email by something like selectEmailFactorQuery (your code assumes that the event data has email while in practice it's not there)

Oh, good catch! I was trying to avoid weird race conditions with updates here but it looks like the upshot of that is requiring that the auth library makes it's own databse queries, which I think we've avoided until now. I'd like to keep it clean here: not every webhook with an email factor needs to send an email, but forcing the consumer to have to write the boilerplate seems unfriendly.

Maybe I'll consider denormalizing the email into the event payload itself: I believe in every case we already have it at event creation time. If users need to be more careful avoiding race conditions with updates, they can always re-request the email themselves manually by enqueuing the email lookup into their own handler code.

scotttrinh avatar Mar 31 '25 13:03 scotttrinh

Valid points, better to keep it as route handler, and not as event handler. More things can be reused beyond analytics.

For the signature verification, I got it working this way:

  const rawBody = await req.text(); // Get the raw body as a string
  const signatureHeader = req.headers.get("x-ext-auth-signature-sha256");

  if (!signatureHeader) {
    return Response.json({ body: "Missing signature header" }, { status: 400 });
  }

  // Generate HMAC digest using the raw body
  const hmac = crypto.createHmac("sha256", env.GEL_AUTH_WEBHOOK_SECRET);
  const digest = hmac.update(rawBody).digest("hex");

  // Compare the signature with the digest
  const validSignature = crypto.timingSafeEqual(
    Buffer.from(digest, "utf8"),
    Buffer.from(signatureHeader, "utf8"),
  );

  if (!validSignature) {
    return Response.json({ body: "Invalid signature" }, { status: 403 });
  }

  const event = JSON.parse(rawBody) as Event;

MiroslavPetrik avatar Mar 31 '25 15:03 MiroslavPetrik

Awesome, that's quite helpful thanks for doing some exploring there. I think for our general case, we'll want to lookup the secret (there goes my idea of avoiding a database roundtrip...), and only "throw" if you have one set and it doesn't match, or if you don't have one set and the request includes one.

PRs welcome in @gel/auth-core for a function that that takes in the body and the header value, and returns a JSON parse of the body.

scotttrinh avatar Mar 31 '25 15:03 scotttrinh

Hmm, you can query the secret? I don't get the signing_secret_key when I query the WebhookConfig:

select ext::auth::WebhookConfig {**};

Moreover, the result is duplicated (perhaps some view, where the secret is being filtered out?).

But I see you can check if you have it set:

select ext::auth::webhook_signing_key_exists(ext::auth::WebhookConfig);

MiroslavPetrik avatar Mar 31 '25 16:03 MiroslavPetrik

Hmm, you can query the secret?

Ahh, right, I forgot we made this a secret config, and indeed you are not able to query for a secret config value. Typically we'd add some functionality to ext::auth to validate it against the secret within the module (similar to our global ext::auth::ClientTokenIdentity computed), so maybe we can expose something like that here. I just want to avoid developers needing to know to save their secret somewhere, but for now it's fine to expect that the secret is shared already with the application via something that the developer can get access to.

scotttrinh avatar Mar 31 '25 17:03 scotttrinh