feat(auth): pre-persist registration webhook so rejected signups never leave orphaned Kratos identities

dev/ory/kratos.yml

@@ -111,25 +111,30 @@ selfservice:
       after:
         password:
           hooks:
-            # we are not sure if we need this hook yet.
-            # this could be used to check if the user is already registered in the backend
-            # before creating the user in kratos
-            # otherwise response: parse: false happens after kratos user creation
-            #
-            #
-            # - hook: web_hook
-            #   config:
-            #     url: http://bats-tests:4012/kratos/preregistration
-            #     method: POST
-            #     response:
-            #       parse: true
-            #     body: file:///home/ory/body.jsonnet # TODO: use a base64 encoding instead
-            #     auth:
-            #       type: api_key
-            #       config:
-            #         name: Authorization
-            #         value: The-Value-of-My-Key
-            #         in: header
+            # Pre-persist validation. `response.parse: true` makes Kratos run
+            # this hook BEFORE the identity is written (web_hook.go,
+            # ExecutePostRegistrationPrePersistHook): a 4xx with a `messages`
+            # body aborts the sign-up and nothing is persisted. The api only
+            # validates here (phone, carrier metadata, phone not already bound)
+            # and must not write — the account is created by the post-persist
+            # /registration hook below, once the identity exists. Note
+            # ctx.identity.id is the nil uuid at this point.
+            - hook: web_hook
+              config:
+                url: http://bats-tests:4012/kratos/preregistration
+                method: POST
+                response:
+                  parse: true
+                body: file:///home/ory/body.jsonnet # TODO: use a base64 encoding instead
+                auth:
+                  type: api_key
+                  config:
+                    name: Authorization
+                    value: The-Value-of-My-Key
+                    in: header
+            # Post-persist account creation (unchanged). parse: false → runs
+            # after the identity is committed; a failure here leaves an identity
+            # without an account, which the session middleware self-heals.
             - hook: web_hook
               config:
                 url: http://bats-tests:4012/kratos/registration

docker-compose.yml

@@ -39,6 +39,7 @@ services:
     depends_on:
       - redis
       - mongodb
+      - kratos
       # - bitcoind
       # - bitcoind-signer
       # - stablesats
@@ -177,6 +178,14 @@ services:
       - ${TMP_ENV_CI:-.env.ci}
     volumes:
       - ./:/repo
+    # dev/ory/kratos.yml points the registration web hooks at
+    # http://bats-tests:4012 (the bats container; host-gateway under the local
+    # override). The integration suite serves those hooks from inside jest, so
+    # Kratos must resolve that name to this container too.
+    networks:
+      default:
+        aliases:
+          - bats-tests
   oathkeeper:
     image: oryd/oathkeeper:v0.40.4-distroless
     ports: []

quickstart/bin/re-render.sh

@@ -42,6 +42,11 @@ rewrite_flash_quickstart_hosts() {
 
 rewrite_flash_quickstart_hosts
 
+# Upstream galoy ships the pre-persist registration hook commented out; the
+# api needs it live or rejected sign-ups leave orphaned identities (see the
+# script header). Must run after the host rewrite: it anchors on the flash host.
+"${REPO_ROOT}/quickstart/bin/splice-kratos-preregistration-hook.sh" dev/ory/kratos.yml
+
 ytt -f ./docker-compose.tmpl.yml -f ${GALOY_ROOT_DIR}/docker-compose.yml -f ${GALOY_ROOT_DIR}/docker-compose.override.yml > docker-compose.yml
 
 pushd ${GALOY_ROOT_DIR}

quickstart/bin/splice-kratos-preregistration-hook.sh

@@ -0,0 +1,113 @@
+#!/bin/bash
+#
+# Splice the pre-persist registration web hook into a freshly vendir-synced
+# quickstart/dev/ory/kratos.yml.
+#
+# Upstream galoy ships that hook commented out ("we are not sure if we need
+# this hook yet"). The api's POST /kratos/preregistration route is what keeps a
+# rejected sign-up from leaving an orphaned Kratos identity behind, and Kratos
+# only calls it when told to, with `response.parse: true` (pre-persist), ahead
+# of the post-persist /registration hook. The root dev/ory/kratos.yml carries
+# the real entry for the integration suite; the quickstart copy is regenerated
+# from upstream by re-render.sh, so a hand edit there is lost on the next
+# `make re-render` -- which is why the entry is spliced in here instead.
+#
+# Usage: splice-kratos-preregistration-hook.sh <path/to/kratos.yml>
+#   Run after the bats-tests -> flash host rewrite; it anchors on the flash host.
+#
+# Exits 0 and leaves the file alone when an uncommented preregistration hook is
+# already there. Exits 1 when the /registration hook cannot be found in the
+# expected shape, so a re-render can never silently produce a config without
+# the hook. test/flash/unit/dev/kratos-registration-hooks.spec.ts covers it.
+
+set -e
+set -o pipefail
+
+file=${1:?usage: $0 <kratos.yml>}
+
+registration_url='http://flash:4012/kratos/registration'
+preregistration_url='http://flash:4012/kratos/preregistration'
+
+existing_line=$(grep -nE "^[[:space:]]*url: ${preregistration_url}[[:space:]]*\$" "${file}" | head -1 | cut -d: -f1 || true)
+if [ -n "${existing_line}" ]; then
+  # A hook at that url only does its job pre-persist. If upstream ever ships
+  # it with `parse: false` (or drops `response` entirely) this must fail
+  # loudly, not report "already present".
+  if sed -n "${existing_line},$((existing_line + 4))p" "${file}" | grep -Eq '^[[:space:]]*parse: true[[:space:]]*$'; then
+    echo "${file}: pre-persist registration hook already present, nothing to splice" >&2
+    exit 0
+  fi
+  echo "${file}: a /kratos/preregistration hook is present but is not pre-persist (no 'response.parse: true' within 4 lines of line ${existing_line}); fix it by hand" >&2
+  exit 1
+fi
+
+# Anchor on the post-persist hook, expected as three consecutive lines:
+#   - hook: web_hook
+#     config:
+#       url: http://flash:4012/kratos/registration
+url_lines=$(grep -nE "^[[:space:]]*url: ${registration_url}[[:space:]]*\$" "${file}" | cut -d: -f1 || true)
+if [ "$(printf '%s\n' "${url_lines}" | grep -c .)" -ne 1 ]; then
+  echo "${file}: expected exactly one 'url: ${registration_url}' line, got: '${url_lines}'" >&2
+  exit 1
+fi
+url_line=${url_lines}
+hook_line=$((url_line - 2))
+if [ "${hook_line}" -lt 1 ] \
+  || ! sed -n "${hook_line}p" "${file}" | grep -Eq '^[[:space:]]*- hook: web_hook[[:space:]]*$' \
+  || ! sed -n "$((url_line - 1))p" "${file}" | grep -Eq '^[[:space:]]*config:[[:space:]]*$'; then
+  echo "${file}: the /registration hook (line ${url_line}) is not laid out as '- hook: web_hook' / 'config:' / 'url:'; update $(basename "$0")" >&2
+  exit 1
+fi
+
+indent=$(sed -n "${hook_line}p" "${file}" | sed -E 's/^([[:space:]]*).*/\1/')
+
+# Upstream keeps its commented-out draft of this very hook right above the
+# /registration entry. The real entry supersedes it, so drop the run of comment
+# lines leading into the anchor when it mentions the hook; keep any other
+# comment.
+drop_from=${hook_line}
+while [ "${drop_from}" -gt 1 ] \
+  && sed -n "$((drop_from - 1))p" "${file}" | grep -Eq '^[[:space:]]*(#.*)?$'; do
+  drop_from=$((drop_from - 1))
+done
+if [ "${drop_from}" -lt "${hook_line}" ] \
+  && ! sed -n "${drop_from},$((hook_line - 1))p" "${file}" | grep -q 'kratos/preregistration'; then
+  drop_from=${hook_line}
+fi
+
+# Mirrors the entry in dev/ory/kratos.yml at the repo root, host rewritten.
+block=$(cat <<'BLOCK'
+# Pre-persist validation, spliced in by quickstart/bin/re-render.sh (upstream
+# ships it commented out). `response.parse: true` makes Kratos run this hook
+# BEFORE the identity is written: a 4xx with a `messages` body aborts the
+# sign-up and nothing is persisted. See dev/ory/kratos.yml at the repo root.
+- hook: web_hook
+  config:
+    url: http://flash:4012/kratos/preregistration
+    method: POST
+    response:
+      parse: true
+    body: file:///home/ory/body.jsonnet
+    auth:
+      type: api_key
+      config:
+        name: Authorization
+        value: The-Value-of-My-Key
+        in: header
+BLOCK
+)
+
+tmp="${file}.splice.tmp"
+# The block goes through the environment: awk -v would interpret escapes and
+# some awks choke on embedded newlines there.
+SPLICE_BLOCK="${block}" awk \
+  -v hook_line="${hook_line}" -v drop_from="${drop_from}" -v indent="${indent}" '
+  FNR >= drop_from && FNR < hook_line && $0 ~ /^[[:space:]]*(#.*)?$/ { next }
+  FNR == hook_line {
+    n = split(ENVIRON["SPLICE_BLOCK"], lines, "\n")
+    for (i = 1; i <= n; i++) print indent lines[i]
+  }
+  { print }
+' "${file}" > "${tmp}"
+mv "${tmp}" "${file}"
+echo "${file}: spliced pre-persist /kratos/preregistration hook ahead of /kratos/registration" >&2

quickstart/dev/ory/kratos.yml

@@ -111,25 +111,23 @@ selfservice:
       after:
         password:
           hooks:
-            # we are not sure if we need this hook yet.
-            # this could be used to check if the user is already registered in the backend
-            # before creating the user in kratos
-            # otherwise response: parse: false happens after kratos user creation
-            #
-            #
-            # - hook: web_hook
-            #   config:
-            #     url: http://flash:4012/kratos/preregistration
-            #     method: POST
-            #     response:
-            #       parse: true
-            #     body: file:///home/ory/body.jsonnet # TODO: use a base64 encoding instead
-            #     auth:
-            #       type: api_key
-            #       config:
-            #         name: Authorization
-            #         value: The-Value-of-My-Key
-            #         in: header
+            # Pre-persist validation, spliced in by quickstart/bin/re-render.sh (upstream
+            # ships it commented out). `response.parse: true` makes Kratos run this hook
+            # BEFORE the identity is written: a 4xx with a `messages` body aborts the
+            # sign-up and nothing is persisted. See dev/ory/kratos.yml at the repo root.
+            - hook: web_hook
+              config:
+                url: http://flash:4012/kratos/preregistration
+                method: POST
+                response:
+                  parse: true
+                body: file:///home/ory/body.jsonnet
+                auth:
+                  type: api_key
+                  config:
+                    name: Authorization
+                    value: The-Value-of-My-Key
+                    in: header
             - hook: web_hook
               config:
                 url: http://flash:4012/kratos/registration

src/app/authentication/index.ts

@@ -7,3 +7,4 @@ export * from "./logout"
 export * from "./phone"
 export * from "./request-code"
 export * from "./totp"
+export * from "./validate-preregistration-payload"

src/app/authentication/validate-preregistration-payload.ts

@@ -0,0 +1,56 @@
+import { KRATOS_CALLBACK_API_KEY } from "@config"
+
+import { CallbackSecretValidator } from "@domain/authentication/secret-validator"
+import { PreRegistrationPayloadValidator } from "@domain/authentication/registration-payload-validator"
+import { PhoneAlreadyRegisteredError } from "@domain/authentication/errors"
+import { CouldNotFindUserFromPhoneError } from "@domain/errors"
+
+import { addAttributesToCurrentSpan } from "@services/tracing"
+import { SchemaIdType } from "@services/kratos"
+import { UsersRepository } from "@services/mongoose"
+
+// Pre-persist half of registration. Kratos calls this BEFORE it commits the
+// identity (web_hook with `response.parse: true`); a rejection here aborts the
+// sign-up with nothing written anywhere. It therefore must not write anything
+// either: every check is read-only, and the account itself is still created by
+// the post-persist /registration hook once the identity exists.
+//
+// `identity_id` is absent/nil at this point and is never inspected.
+export const validatePreRegistrationPayload = async ({
+  secret,
+  body,
+}: {
+  secret: string | undefined
+  body: {
+    identity_id?: string | null
+    phone?: string
+    schema_id?: string
+    transient_payload?: { phoneMetadata?: RawPhoneMetadataPayload } | null
+    flow_id?: string | null
+    flow_type?: string | null
+  }
+}): Promise<true | ApplicationError> => {
+  addAttributesToCurrentSpan({
+    "preregistration.schema_id": body.schema_id,
+  })
+
+  const isValidKey = CallbackSecretValidator(KRATOS_CALLBACK_API_KEY).authorize(secret)
+  if (isValidKey instanceof Error) {
+    return isValidKey
+  }
+
+  const payload = PreRegistrationPayloadValidator(
+    SchemaIdType.PhoneNoPasswordV0,
+  ).validate(body)
+  if (payload instanceof Error) return payload
+
+  // The post-persist hook upserts the users document by phone, which has a
+  // unique index. A document already holding this phone is exactly the
+  // DuplicateKeyForPersistError that used to strand the identity; reject it
+  // here while nothing has been committed.
+  const existing = await UsersRepository().findByPhone(payload.phone)
+  if (existing instanceof CouldNotFindUserFromPhoneError) return true
+  if (existing instanceof Error) return existing
+
+  return new PhoneAlreadyRegisteredError()
+}

src/domain/authentication/errors.ts

@@ -6,6 +6,15 @@ export class LikelyUserAlreadyExistError extends AuthenticationError {}
 
 export class AccountHasPositiveBalanceError extends AuthenticationError {}
 export class PhoneAlreadyExistsError extends AuthenticationError {}
+// The pre-persist registration hook found the phone already bound to a users
+// document. Raised on the sign-up path, where the caller has no account and no
+// session — so it must not read as "one phone per account"
+// (PhoneAlreadyExistsError belongs to the add-phone-to-account flow).
+export class PhoneAlreadyRegisteredError extends AuthenticationError {}
+// The pre-persist registration hook rejected the phone (unparsable, or its
+// carrier metadata failed validation). Distinct from "already registered": the
+// user cannot fix it by logging in instead.
+export class PhoneNotAllowedForRegistrationError extends AuthenticationError {}
 
 export class EmailCodeInvalidError extends AuthenticationError {}
 export class EmailUnverifiedError extends AuthenticationError {}

src/domain/authentication/index.types.d.ts

@@ -75,15 +75,35 @@ type RegistrationPayload = {
   phone: PhoneNumber
   phoneMetadata: PhoneMetadata | undefined
 }
+type RawPhoneMetadataPayload = Record<string, string | Record<string, string>>
+
 type RegistrationPayloadValidator = {
   validate(rawBody: {
     identity_id?: string
     phone?: string
     schema_id?: string
-    transient_payload?: { phoneMetadata?: Record<string, Record<string, string>> }
+    transient_payload?: { phoneMetadata?: RawPhoneMetadataPayload }
   }): RegistrationPayload | ValidationError
 }
 
+// What the pre-persist registration hook can know: the identity has no id yet
+// (Kratos sends the nil uuid), so only the phone and its metadata are checked.
+type PreRegistrationPayload = {
+  phone: PhoneNumber
+  phoneMetadata: PhoneMetadata | undefined
+}
+
+type PreRegistrationPayloadValidator = {
+  validate(rawBody: {
+    identity_id?: string | null
+    phone?: string
+    schema_id?: string
+    transient_payload?: { phoneMetadata?: RawPhoneMetadataPayload } | null
+    flow_id?: string | null
+    flow_type?: string | null
+  }): PreRegistrationPayload | ValidationError
+}
+
 interface IAuthWithPhonePasswordlessService {
   loginToken(args: {
     phone: PhoneNumber