wip

packages/oauth-provider/src/account/account-hooks.ts

@@ -1,8 +1,8 @@
 import { Client } from '../client/client.js'
 import { DeviceId } from '../device/device-id.js'
-import { ClientAuth } from '../token/token-store.js'
+import { ClientAuth, OAuthClientId } from '../token/token-store.js'
 import { Awaitable } from '../util/awaitable.js'
-import { Account } from './account-store.js'
+import { Sub } from './account-store.js'
 // https://github.com/typescript-eslint/typescript-eslint/issues/8902
 // eslint-disable-next-line
 import { AccountStore } from './account-store.js'
@@ -13,10 +13,11 @@ import { AccountStore } from './account-store.js'
  * the store method).
  */
 export type AccountAddAuthorizedClient = (
-  client: Client,
+  deviceId: DeviceId,
+  account: Sub,
+  clientId: OAuthClientId,
   data: {
-    deviceId: DeviceId
-    account: Account
+    client: Client
     clientAuth: ClientAuth
   },
 ) => Awaitable<boolean>

packages/oauth-provider/src/account/account-manager.ts

@@ -1,16 +1,16 @@
+import { ClientAuth } from '../client/client-auth.js'
 import { Client } from '../client/client.js'
 import { DeviceId } from '../device/device-id.js'
 import { InvalidRequestError } from '../oauth-errors.js'
 import { Sub } from '../oidc/sub.js'
-import { ClientAuth } from '../token/token-store.js'
 import { constantTime } from '../util/time.js'
 import { AccountHooks } from './account-hooks.js'
 import {
-  Account,
-  AccountInfo,
   AccountStore,
+  DeviceAccount,
   LoginCredentials,
 } from './account-store.js'
+import { Account } from './account.js'
 
 const TIMING_ATTACK_MITIGATION_DELAY = 400
 
@@ -21,19 +21,43 @@ export class AccountManager {
   ) {}
 
   public async signIn(
-    credentials: LoginCredentials,
     deviceId: DeviceId,
-  ): Promise<AccountInfo> {
-    return constantTime(TIMING_ATTACK_MITIGATION_DELAY, async () => {
-      const result = await this.store.authenticateAccount(credentials, deviceId)
-      if (result) return result
+    credentials: LoginCredentials,
+    remember: boolean,
+  ): Promise<DeviceAccount> {
+    const { account, secondFactors } = await constantTime(
+      TIMING_ATTACK_MITIGATION_DELAY,
+      async () => {
+        const result = await this.store.authenticateAccount(credentials)
+        if (result) return result
+
+        throw new InvalidRequestError('Invalid credentials')
+      },
+    )
+
+    const current = await this.store.readDeviceAccount(deviceId, account.sub)
+    if (current && !current.data.secondFactorRequired) {
+      return this.store.updateDeviceAccount(deviceId, account.sub, {
+        remembered: remember,
+        authenticatedAt: new Date(),
+      })
+    }
 
-      throw new InvalidRequestError('Invalid credentials')
-    })
+    if (!secondFactors?.length) {
+      return this.store.upsertDeviceAccount(deviceId, account.sub, {
+        ...current?.data,
+        remembered: remember,
+        authenticatedAt: new Date(),
+        secondFactorRequired: false,
+        authorizedClients: [],
+      })
+    }
+
+    throw new Error('2FA not implemented')
   }
 
-  public async get(deviceId: DeviceId, sub: Sub): Promise<AccountInfo> {
-    const result = await this.store.getDeviceAccount(deviceId, sub)
+  public async get(deviceId: DeviceId, sub: Sub): Promise<DeviceAccount> {
+    const result = await this.store.readDeviceAccount(deviceId, sub)
     if (result) return result
 
     throw new InvalidRequestError(`Account not found`)
@@ -46,19 +70,24 @@ export class AccountManager {
     clientAuth: ClientAuth,
   ): Promise<void> {
     if (this.hooks.onAccountAddAuthorizedClient) {
-      const shouldAdd = await this.hooks.onAccountAddAuthorizedClient(client, {
+      const shouldAdd = await this.hooks.onAccountAddAuthorizedClient(
         deviceId,
-        account,
-        clientAuth,
-      })
+        account.sub,
+        client.id,
+        { client, clientAuth },
+      )
       if (!shouldAdd) return
     }
 
-    await this.store.addAuthorizedClient(deviceId, account.sub, client.id)
+    // TODO: refactor to use "updateDeviceAccount"
+
+    // await this.store.addAuthorizedClient(deviceId, account.sub, client.id)
   }
 
-  public async list(deviceId: DeviceId): Promise<AccountInfo[]> {
+  public async listActiveSessions(deviceId: DeviceId) {
     const results = await this.store.listDeviceAccounts(deviceId)
-    return results.filter((result) => result.info.remembered)
+    return results.filter(
+      (result) => result.data.authenticatedAt != null && result.data.remembered,
+    )
   }
 }

packages/oauth-provider/src/account/account-store.ts

@@ -1,67 +1,80 @@
 import { OAuthClientId } from '@atproto/oauth-client-metadata'
 
 import { DeviceId } from '../device/device-id.js'
+import { DeviceData } from '../device/device-data.js'
 import { Sub } from '../oidc/sub.js'
 import { Awaitable } from '../util/awaitable.js'
 import { Account } from './account.js'
 
 export type LoginCredentials = {
   username: string
   password: string
+}
 
-  /**
-   * If false, the account must not be returned from
-   * {@link AccountStore.listDeviceAccounts}. Note that this only makes sense when
-   * used with a device ID.
-   */
-  remember?: boolean
+export type SecondFactorMethod = {
+  id: string
+  type: 'email' | 'totp' | 'webauthn'
+  data?: unknown
 }
 
-export type DeviceAccountInfo = {
+export type AuthenticationResult = {
+  account: Account
+  secondFactors?: readonly SecondFactorMethod[]
+}
+
+export type DeviceAccountData = {
   remembered: boolean
   authenticatedAt: Date
+  secondFactorRequired: boolean
   authorizedClients: readonly OAuthClientId[]
 }
 
 // Export all types needed to implement the AccountStore interface
-export type { Account, DeviceId, Sub }
+export type { Account, DeviceId, DeviceData, Sub }
 
-export type AccountInfo = {
+export type DeviceAccount = {
+  device: DeviceData
   account: Account
-  info: DeviceAccountInfo
+
+  data: DeviceAccountData
 }
 
 export interface AccountStore {
   authenticateAccount(
     credentials: LoginCredentials,
+  ): Awaitable<AuthenticationResult | null>
+
+  upsertDeviceAccount(
     deviceId: DeviceId,
-  ): Awaitable<AccountInfo | null>
+    sub: Sub,
+    data: DeviceAccountData,
+  ): Awaitable<DeviceAccount>
+
+  readDeviceAccount(
+    deviceId: DeviceId,
+    sub: Sub,
+  ): Awaitable<DeviceAccount | null>
 
-  addAuthorizedClient(
+  updateDeviceAccount(
     deviceId: DeviceId,
     sub: Sub,
-    clientId: OAuthClientId,
-  ): Awaitable<void>
+    data: Partial<DeviceAccountData>,
+  ): Awaitable<DeviceAccount>
 
-  getDeviceAccount(deviceId: DeviceId, sub: Sub): Awaitable<AccountInfo | null>
-  removeDeviceAccount(deviceId: DeviceId, sub: Sub): Awaitable<void>
+  deleteDeviceAccount(deviceId: DeviceId, sub: Sub): Awaitable<void>
 
-  /**
-   * @note Only the accounts that where logged in with `remember: true` need to
-   * be returned. The others will be ignored.
-   */
-  listDeviceAccounts(deviceId: DeviceId): Awaitable<AccountInfo[]>
+  listDeviceAccounts(deviceId: DeviceId): Awaitable<DeviceAccount[]>
 }
 
 export function isAccountStore(
   implementation: Record<string, unknown> & Partial<AccountStore>,
 ): implementation is Record<string, unknown> & AccountStore {
   return (
     typeof implementation.authenticateAccount === 'function' &&
-    typeof implementation.getDeviceAccount === 'function' &&
+    typeof implementation.readDeviceAccount === 'function' &&
     typeof implementation.addAuthorizedClient === 'function' &&
     typeof implementation.listDeviceAccounts === 'function' &&
-    typeof implementation.removeDeviceAccount === 'function'
+    typeof implementation.deleteDeviceAccount === 'function'
   )
 }
 

packages/oauth-provider/src/oauth-provider.ts

@@ -30,7 +30,7 @@ import { AccountManager } from './account/account-manager.js'
 import {
   AccountInfo,
   AccountStore,
-  DeviceAccountInfo,
+  DeviceAccount,
   LoginCredentials,
   asAccountStore,
 } from './account/account-store.js'
@@ -228,11 +228,11 @@ export class OAuthProvider extends OAuthVerifier {
   protected loginRequired(
     client: Client,
     parameters: AuthorizationParameters,
-    info: DeviceAccountInfo,
+    authenticatedAt: Date,
   ) {
     const authAge = Math.max(
       0, // Prevent negative values (fool proof)
-      (Date.now() - info.authenticatedAt.getTime()) / 1e3,
+      (Date.now() - authenticatedAt.getTime()) / 1e3,
     )
     const maxAge = Math.max(
       0, // Prevent negative values (fool proof)
@@ -522,7 +522,6 @@ export class OAuthProvider extends OAuthVerifier {
   ): Promise<
     {
       account: Account
-      info: DeviceAccountInfo
 
       selected: boolean
       loginRequired: boolean
@@ -531,23 +530,19 @@ export class OAuthProvider extends OAuthVerifier {
       matchesHint: boolean
     }[]
   > {
-    const accounts = await this.accountManager.list(deviceId)
-
-    return accounts.map(({ account, info }) => ({
+    const sessions = await this.accountManager.listActiveSessions(deviceId)
+    return sessions.map(({ account, data }) => ({
       account,
-      info,
 
       selected:
         parameters.prompt !== 'select_account' &&
         parameters.login_hint === account.sub,
       loginRequired:
         parameters.prompt === 'login' ||
-        this.loginRequired(client, parameters, info),
+        this.loginRequired(client, parameters, data.authenticatedAt),
       consentRequired:
-        parameters.prompt === 'login' ||
         parameters.prompt === 'consent' ||
-        !info.authorizedClients.includes(client.id),
-
+        !data.authorizedClients.includes(client.id),
       matchesHint:
         parameters.login_hint === account.sub || parameters.login_hint == null,
     }))
@@ -556,8 +551,9 @@ export class OAuthProvider extends OAuthVerifier {
   protected async signIn(
     deviceId: DeviceId,
     credentials: LoginCredentials,
-  ): Promise<AccountInfo> {
-    return this.accountManager.signIn(credentials, deviceId)
+    remember: boolean,
+  ): Promise<DeviceAccount> {
+    return this.accountManager.signIn(deviceId, credentials, remember)
   }
 
   protected async acceptRequest(
@@ -577,10 +573,10 @@ export class OAuthProvider extends OAuthVerifier {
       )
 
       try {
-        const { account, info } = await this.accountManager.get(deviceId, sub)
+        const { account, data } = await this.accountManager.get(deviceId, sub)
 
         // The user is trying to authorize without a fresh login
-        if (this.loginRequired(client, parameters, info)) {
+        if (this.loginRequired(client, parameters, data.authenticatedAt)) {
           throw new LoginRequiredError(
             parameters,
             'Account authentication required.',
@@ -592,7 +588,7 @@ export class OAuthProvider extends OAuthVerifier {
           uri,
           deviceId,
           account,
-          info,
+          data,
         )
 
         await this.accountManager.addAuthorizedClient(
@@ -1165,14 +1161,18 @@ export class OAuthProvider extends OAuthVerifier {
 
       const { deviceId } = await deviceManager.load(req, res)
 
-      const { account, info } = await server.signIn(deviceId, input.credentials)
+      const { account, data } = await server.signIn(
+        deviceId,
+        input.credentials,
+        input.credentials.remember,
+      )
 
       // Prevent fixation attacks
       await deviceManager.rotate(req, res, deviceId)
 
       return writeJson(res, {
         account,
-        consentRequired: !info.authorizedClients.includes(input.client_id),
+        consentRequired: !data.authorizedClients.includes(input.client_id),
       })
     })
 

packages/oauth-provider/src/output/send-authorize-page.ts

@@ -2,7 +2,6 @@ import { IncomingMessage, ServerResponse } from 'node:http'
 
 import { cssCode, html } from '@atproto/html'
 
-import { DeviceAccountInfo } from '../account/account-store.js'
 import { Account } from '../account/account.js'
 import { getAsset } from '../assets/index.js'
 import { Client } from '../client/client.js'
@@ -23,7 +22,6 @@ export type AuthorizationResultAuthorize = {
     uri: RequestUri
     sessions: readonly {
       account: Account
-      info: DeviceAccountInfo
 
       selected: boolean
       loginRequired: boolean

packages/oauth-provider/src/request/request-manager.ts

@@ -1,7 +1,10 @@
 import { OAuthClientId } from '@atproto/oauth-client-metadata'
 import { OAuthServerMetadata } from '@atproto/oauth-server-metadata'
 
-import { DeviceAccountInfo } from '../account/account-store.js'
+import {
+  DeviceAccountData,
+  DeviceAccountInfo,
+} from '../account/account-store.js'
 import { Account } from '../account/account.js'
 import { ClientAuth } from '../client/client-auth.js'
 import { Client } from '../client/client.js'
@@ -353,7 +356,7 @@ export class RequestManager {
     uri: RequestUri,
     deviceId: DeviceId,
     account: Account,
-    info: DeviceAccountInfo,
+    deviceAccountData: DeviceAccountData,
   ): Promise<{ code?: Code; id_token?: string }> {
     const id = decodeRequestUri(uri)
 
@@ -399,7 +402,7 @@ export class RequestManager {
 
       const id_token = responseType.includes('id_token')
         ? await this.signer.idToken(client, data.parameters, account, {
-            auth_time: info.authenticatedAt,
+            auth_time: deviceAccountData.authenticatedAt,
             exp: this.createTokenExpiry(),
             code,
           })