chore(electric): Swap to using sub claim in jwt, backwards compatib…

…le with `user_id` (#692)

The standard `sub` "subject" claim is where most auth providers will
place any pk for the user, we should default to that with backwards
compatibility with our `user_id` claim.

---------

Co-authored-by: Oleksii Sholik <[email protected]>

clients/typescript/src/auth/secure/index.ts

@@ -33,5 +33,5 @@ export function mockSecureAuthToken(
   const mockIss = iss ?? 'dev.electric-sql.com'
   const mockKey = key ?? 'integration-tests-signing-key-example'
 
-  return secureAuthToken({ user_id: 'test-user' }, mockIss, mockKey)
+  return secureAuthToken({ sub: 'test-user' }, mockIss, mockKey)
 }

clients/typescript/test/auth/insecure.test.ts

@@ -4,15 +4,15 @@ import { decodeJwt } from 'jose'
 import { insecureAuthToken } from '../../src/auth'
 
 test('insecureAuthToken generates expected token', async (t) => {
-  const token = insecureAuthToken({ user_id: 'dummy-user' })
+  const token = insecureAuthToken({ sub: 'dummy-user' })
 
   const claims = decodeJwt(token)
-  t.deepEqual(claims, { user_id: 'dummy-user' })
+  t.deepEqual(claims, { sub: 'dummy-user' })
 })
 
 test('insecureAuthToken supports non-latin characters', async (t) => {
-  const token = insecureAuthToken({ user_id: '⚡' })
+  const token = insecureAuthToken({ sub: '⚡' })
 
   const claims = decodeJwt(token)
-  t.deepEqual(claims, { user_id: '⚡' })
+  t.deepEqual(claims, { sub: '⚡' })
 })

components/electric/lib/electric/satellite/auth/insecure.ex

@@ -3,8 +3,8 @@ defmodule Electric.Satellite.Auth.Insecure do
   Implementation module of the "insecure" auth mode.
 
   This mode does not do any signature verification and treats the standard "iat", "exp", and "nbf" claims as optional.
-  The only claim it does require is "user_id" which can be either a top-level claim or nested under a configurable
-  namespace.
+  The only claim it does require is "sub" or "user_id" which can be either a top-level claim or nested under a
+  configurable namespace.
 
   You must opt in to using the "insecure" mode. We do not recommend to use it outside of development or local testing.
   As soon as you're ready to deploy Electric in any capacity, make sure to switch to the "secure" auth mode.
@@ -23,8 +23,8 @@ defmodule Electric.Satellite.Auth.Insecure do
 
   ## Options
 
-    * `namespace: <string>` - optional namespace under which the "user_id" claim will be looked up. If omitted,
-      "user_id" must be a top-level claim.
+    * `namespace: <string>` - optional namespace under which the "sub" or "user_id" claim will be looked up. If omitted,
+      "sub" or "user_id" must be a top-level claim.
   """
   @spec build_config(Access.t()) :: map
   def build_config(opts) do

components/electric/lib/electric/satellite/auth/jwt_util.ex

@@ -7,7 +7,14 @@ defmodule Electric.Satellite.Auth.JWTUtil do
   alias Electric.Satellite.Auth.TokenError
 
   # The name of the claim which holds the User ID in a JWT.
-  @user_id_key "user_id"
+  # `sub` is "subject" according to the JWT spec:
+  # https://www.rfc-editor.org/rfc/rfc7519#section-4.1.2
+  @user_id_key "sub"
+
+  # We used to use `user_id` as the key, but we changed it to `sub` to be more
+  # compliant with the JWT spec. However, we still want to support reading the
+  # old `user_id` key for backwards compatibility.
+  @legacy_user_id_key "user_id"
 
   @doc """
   Fetch the User ID from the given map of claims.
@@ -25,9 +32,11 @@ defmodule Electric.Satellite.Auth.JWTUtil do
   end
 
   defp get_user_id(claims, namespace) when is_binary(namespace) and namespace != "",
-    do: get_in(claims, [namespace, @user_id_key])
+    do:
+      get_in(claims, [namespace, @user_id_key]) ||
+        get_in(claims, [namespace, @legacy_user_id_key])
 
-  defp get_user_id(claims, _), do: claims[@user_id_key]
+  defp get_user_id(claims, _), do: claims[@user_id_key] || claims[@legacy_user_id_key]
 
   defp validate_user_id(user_id) do
     if is_binary(user_id) and String.trim(user_id) != "" do

components/electric/lib/electric/satellite/auth/secure.ex

@@ -3,8 +3,8 @@ defmodule Electric.Satellite.Auth.Secure do
   Implementation module of the "secure" auth mode.
 
   This mode requires auth tokens to be signed. It also checks for the presence of at least "iat" and "exp" claims. If
-  you include values for "iss" and/or "aud" claims in your configuration, those will also be enforced. The "user_id"
-  claims must also be present, either at the top level or under a configurable namespace.
+  you include values for "iss" and/or "aud" claims in your configuration, those will also be enforced. A "sub" or
+  "user_id" claim must also be present, either at the top level or under a configurable namespace.
 
   Auth tokens must use the same signing algorithm as the one configured in Electric.
 
@@ -45,8 +45,8 @@ defmodule Electric.Satellite.Auth.Secure do
 
          "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQY..."
 
-    * `namespace: <string>` - optional namespace under which the "user_id" claim will be looked up. If omitted,
-      "user_id" must be a top-level claim.
+    * `namespace: <string>` - optional namespace under which the "sub" or "user_id" claim will be looked up. If omitted,
+      "sub" or "user_id" must be a top-level claim.
 
     * `iss: <string>` - optional issuer string to check all auth tokens with. If this is configured, JWTs without an
       "iss" claim will be considered invalid.