wip

bluesky-social · Apr 15, 2024 · ee312a3 · ee312a3
1 parent 416b04d
commit ee312a3
Show file tree

Hide file tree

Showing 8 changed files with 71 additions and 95 deletions.
diff --git a/packages/oauth-provider/src/account/account-hooks.ts b/packages/oauth-provider/src/account/account-hooks.ts
@@ -1,27 +1,3 @@
-import { Client } from '../client/client.js'
-import { DeviceId } from '../device/device-id.js'
-import { ClientAuth, OAuthClientId } from '../token/token-store.js'
-import { Awaitable } from '../util/awaitable.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'
-
-/**
- * Allows disabling the call to {@link AccountStore.addAuthorizedClient} based
- * on the account, client and clientAuth (not all these info are available to
- * the store method).
- */
-export type AccountAddAuthorizedClient = (
-  deviceId: DeviceId,
-  account: Sub,
-  clientId: OAuthClientId,
-  data: {
-    client: Client
-    clientAuth: ClientAuth
-  },
-) => Awaitable<boolean>
-
 export type AccountHooks = {
-  onAccountAddAuthorizedClient?: AccountAddAuthorizedClient
+  //
 }
diff --git a/packages/oauth-provider/src/account/account-manager.ts b/packages/oauth-provider/src/account/account-manager.ts
@@ -1,5 +1,4 @@
-import { ClientAuth } from '../client/client-auth.js'
-import { Client } from '../client/client.js'
+import { OAuthClientId } from '@atproto/oauth-client-metadata'
 import { DeviceId } from '../device/device-id.js'
 import { InvalidRequestError } from '../oauth-errors.js'
 import { Sub } from '../oidc/sub.js'
@@ -10,7 +9,6 @@ import {
   DeviceAccount,
   LoginCredentials,
 } from './account-store.js'
-import { Account } from './account.js'
 
 const TIMING_ATTACK_MITIGATION_DELAY = 400
 
@@ -36,7 +34,7 @@ export class AccountManager {
     )
 
     const current = await this.store.readDeviceAccount(deviceId, account.sub)
-    if (current && !current.data.secondFactorRequired) {
+    if (current && !current.data.secondFactor) {
       return this.store.updateDeviceAccount(deviceId, account.sub, {
         remembered: remember,
         authenticatedAt: new Date(),
@@ -45,49 +43,52 @@ export class AccountManager {
 
     if (!secondFactors?.length) {
       return this.store.upsertDeviceAccount(deviceId, account.sub, {
-        ...current?.data,
         remembered: remember,
         authenticatedAt: new Date(),
-        secondFactorRequired: false,
-        authorizedClients: [],
+        secondFactor: null,
+        authorizedClients: current?.data.authorizedClients || [],
       })
     }
 
-    throw new Error('2FA not implemented')
+    return this.store.upsertDeviceAccount(deviceId, account.sub, {
+      remembered: remember,
+      authenticatedAt: new Date(),
+      secondFactor: {
+        methods: secondFactors,
+      },
+      authorizedClients: current?.data.authorizedClients || [],
+    })
   }
 
-  public async get(deviceId: DeviceId, sub: Sub): Promise<DeviceAccount> {
+  public async getAuthenticated(
+    deviceId: DeviceId,
+    sub: Sub,
+  ): Promise<DeviceAccount> {
     const result = await this.store.readDeviceAccount(deviceId, sub)
-    if (result) return result
 
-    throw new InvalidRequestError(`Account not found`)
-  }
+    if (!result) {
+      throw new InvalidRequestError(`Account not found`)
+    }
 
-  public async addAuthorizedClient(
-    deviceId: DeviceId,
-    account: Account,
-    client: Client,
-    clientAuth: ClientAuth,
-  ): Promise<void> {
-    if (this.hooks.onAccountAddAuthorizedClient) {
-      const shouldAdd = await this.hooks.onAccountAddAuthorizedClient(
-        deviceId,
-        account.sub,
-        client.id,
-        { client, clientAuth },
-      )
-      if (!shouldAdd) return
+    if (result.data.secondFactor) {
+      throw new InvalidRequestError(`Second factor required`)
     }
 
-    // TODO: refactor to use "updateDeviceAccount"
+    return result
+  }
 
-    // await this.store.addAuthorizedClient(deviceId, account.sub, client.id)
+  public async setAuthorizedClients(
+    deviceId: DeviceId,
+    sub: Sub,
+    authorizedClients: OAuthClientId[],
+  ): Promise<void> {
+    await this.store.updateDeviceAccount(deviceId, sub, { authorizedClients })
   }
 
   public async listActiveSessions(deviceId: DeviceId) {
     const results = await this.store.listDeviceAccounts(deviceId)
     return results.filter(
-      (result) => result.data.authenticatedAt != null && result.data.remembered,
+      (result) => result.data.remembered && result.data.secondFactor === null,
     )
   }
 }
diff --git a/packages/oauth-provider/src/account/account-store.ts b/packages/oauth-provider/src/account/account-store.ts
@@ -25,7 +25,9 @@ export type AuthenticationResult = {
 export type DeviceAccountData = {
   remembered: boolean
   authenticatedAt: Date
-  secondFactorRequired: boolean
+  secondFactor: null | {
+    methods: readonly SecondFactorMethod[]
+  }
   authorizedClients: readonly OAuthClientId[]
 }
 

diff --git a/packages/oauth-provider/src/oauth-provider.ts b/packages/oauth-provider/src/oauth-provider.ts
@@ -28,7 +28,6 @@ import { AccessTokenType } from './access-token/access-token-type.js'
 import { AccessToken } from './access-token/access-token.js'
 import { AccountManager } from './account/account-manager.js'
 import {
-  AccountInfo,
   AccountStore,
   DeviceAccount,
   LoginCredentials,
@@ -467,7 +466,7 @@ export class OAuthProvider extends OAuthVerifier {
             uri,
             deviceId,
             ssoSession.account,
-            ssoSession.info,
+            ssoSession.authenticatedAt,
           )
 
           return { issuer, client, parameters, redirect }
@@ -484,7 +483,7 @@ export class OAuthProvider extends OAuthVerifier {
                 uri,
                 deviceId,
                 ssoSession.account,
-                ssoSession.info,
+                ssoSession.authenticatedAt,
               )
 
               return { issuer, client, parameters, redirect }
@@ -523,6 +522,8 @@ export class OAuthProvider extends OAuthVerifier {
     {
       account: Account
 
+      authenticatedAt: Date
+
       selected: boolean
       loginRequired: boolean
       consentRequired: boolean
@@ -534,6 +535,8 @@ export class OAuthProvider extends OAuthVerifier {
     return sessions.map(({ account, data }) => ({
       account,
 
+      authenticatedAt: data.authenticatedAt,
+
       selected:
         parameters.prompt !== 'select_account' &&
         parameters.login_hint === account.sub,
@@ -566,14 +569,17 @@ export class OAuthProvider extends OAuthVerifier {
     const client = await this.clientManager.getClient(clientId)
 
     try {
-      const { parameters, clientAuth } = await this.requestManager.get(
+      const { parameters } = await this.requestManager.get(
         uri,
         clientId,
         deviceId,
       )
 
       try {
-        const { account, data } = await this.accountManager.get(deviceId, sub)
+        const { account, data } = await this.accountManager.getAuthenticated(
+          deviceId,
+          sub,
+        )
 
         // The user is trying to authorize without a fresh login
         if (this.loginRequired(client, parameters, data.authenticatedAt)) {
@@ -588,15 +594,16 @@ export class OAuthProvider extends OAuthVerifier {
           uri,
           deviceId,
           account,
-          data,
+          data.authenticatedAt,
         )
 
-        await this.accountManager.addAuthorizedClient(
-          deviceId,
-          account,
-          client,
-          clientAuth,
-        )
+        if (data.remembered && !data.authorizedClients.includes(clientId)) {
+          await this.accountManager.setAuthorizedClients(
+            deviceId,
+            account.sub,
+            [...data.authorizedClients, clientId],
+          )
+        }
 
         return { issuer, client, parameters, redirect }
       } catch (err) {
@@ -684,22 +691,20 @@ export class OAuthProvider extends OAuthVerifier {
         input.code,
       )
 
-      const { account, info } = await this.accountManager.get(deviceId, sub)
-
-      // User revoked consent while client was asking for a token (or store
-      // failed to persist the consent)
-      if (!info.authorizedClients.includes(client.id)) {
-        throw new AccessDeniedError(parameters, 'Client not trusted anymore')
-      }
+      const { account, data } = await this.accountManager.getAuthenticated(
+        deviceId,
+        sub,
+      )
 
       return await this.tokenManager.create(
         client,
         clientAuth,
+        deviceId,
         account,
-        { id: deviceId, info },
         parameters,
         input,
         dpopJkt,
+        data.authenticatedAt,
       )
     } catch (err) {
       // If a token is replayed, requestManager.findCode will throw. In that

diff --git a/packages/oauth-provider/src/request/request-manager.ts b/packages/oauth-provider/src/request/request-manager.ts
@@ -1,10 +1,6 @@
 import { OAuthClientId } from '@atproto/oauth-client-metadata'
 import { OAuthServerMetadata } from '@atproto/oauth-server-metadata'
 
-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'
@@ -356,7 +352,7 @@ export class RequestManager {
     uri: RequestUri,
     deviceId: DeviceId,
     account: Account,
-    deviceAccountData: DeviceAccountData,
+    authTime: Date,
   ): Promise<{ code?: Code; id_token?: string }> {
     const id = decodeRequestUri(uri)
 
@@ -402,7 +398,7 @@ export class RequestManager {
 
       const id_token = responseType.includes('id_token')
         ? await this.signer.idToken(client, data.parameters, account, {
-            auth_time: deviceAccountData.authenticatedAt,
+            auth_time: authTime,
             exp: this.createTokenExpiry(),
             code,
           })

diff --git a/packages/oauth-provider/src/token/token-data.ts b/packages/oauth-provider/src/token/token-data.ts
@@ -21,6 +21,7 @@ export type TokenData = {
   createdAt: Date
   updatedAt: Date
   expiresAt: Date
+  authTime: Date
   clientId: OAuthClientId
   clientAuth: ClientAuth
   deviceId: DeviceId | null

diff --git a/packages/oauth-provider/src/token/token-manager.ts b/packages/oauth-provider/src/token/token-manager.ts
@@ -4,7 +4,6 @@ import { isJwt } from '@atproto/jwk'
 
 import { AccessTokenType } from '../access-token/access-token-type.js'
 import { AccessToken } from '../access-token/access-token.js'
-import { 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'
@@ -35,6 +34,7 @@ import {
   isTokenId,
   tokenIdSchema,
 } from './token-id.js'
+import { TokenResponse } from './token-response.js'
 import { TokenInfo, TokenStore } from './token-store.js'
 import { TokenType } from './token-type.js'
 import { CodeGrantRequest, RefreshGrantRequest } from './types.js'
@@ -43,7 +43,6 @@ import {
   VerifyTokenClaimsResult,
   verifyTokenClaims,
 } from './verify-token-claims.js'
-import { TokenResponse } from './token-response.js'
 
 export type AuthenticateTokenIdResult = VerifyTokenClaimsResult & {
   tokenInfo: TokenInfo
@@ -73,11 +72,12 @@ export class TokenManager {
   async create(
     client: Client,
     clientAuth: ClientAuth,
+    deviceId: DeviceId,
     account: Account,
-    device: null | { id: DeviceId; info: DeviceAccountInfo },
     parameters: AuthorizationParameters,
     input: CodeGrantRequest,
     dpopJkt: null | string,
+    authTime: Date,
   ): Promise<TokenResponse> {
     if (client.metadata.dpop_bound_access_tokens && !dpopJkt) {
       throw new InvalidDpopProofError('DPoP proof required')
@@ -198,9 +198,10 @@ export class TokenManager {
       createdAt: now,
       updatedAt: now,
       expiresAt,
+      authTime,
       clientId: client.id,
       clientAuth,
-      deviceId: device?.id ?? null,
+      deviceId,
       sub: account.sub,
       parameters,
       details: authorizationDetails ?? null,
@@ -228,7 +229,7 @@ export class TokenManager {
           exp: expiresAt,
           iat: now,
           // If there is no deviceInfo, we are in a "password_grant" context
-          auth_time: device?.info.authenticatedAt || new Date(),
+          auth_time: authTime,
           access_token: accessToken,
           code,
         })
@@ -264,10 +265,6 @@ export class TokenManager {
       throw new InvalidGrantError(`Token was not issued to this client`)
     }
 
-    if (tokenInfo.info?.authorizedClients.includes(client.id) === false) {
-      throw new InvalidGrantError(`Client no longer trusted by user`)
-    }
-
     if (tokenInfo.data.clientAuth.method !== clientAuth.method) {
       throw new InvalidGrantError(`Client authentication method mismatch`)
     }
@@ -290,7 +287,7 @@ export class TokenManager {
       throw new InvalidGrantError(`Invalid refresh token`)
     }
 
-    const { account, info, data } = tokenInfo
+    const { account, data } = tokenInfo
     const { parameters } = data
 
     try {
@@ -375,7 +372,7 @@ export class TokenManager {
         ? await this.signer.idToken(client, parameters, account, {
             exp: expiresAt,
             iat: now,
-            auth_time: info?.authenticatedAt,
+            auth_time: data.authTime,
             access_token: accessToken,
           })
         : undefined