diff --git a/packages/agents-hosting-storage-blob/src/blobsStorage.ts b/packages/agents-hosting-storage-blob/src/blobsStorage.ts index cff998c13..d52f8ece5 100644 --- a/packages/agents-hosting-storage-blob/src/blobsStorage.ts +++ b/packages/agents-hosting-storage-blob/src/blobsStorage.ts @@ -3,11 +3,12 @@ import StreamConsumers from 'stream/consumers' import { isTokenCredential, TokenCredential } from '@azure/core-auth' import { AnonymousCredential, + BlobRequestConditions, ContainerClient, StoragePipelineOptions, StorageSharedKeyCredential, } from '@azure/storage-blob' -import { Storage, StoreItems } from '@microsoft/agents-hosting' +import { Storage, StorageWriteOptions, StoreItems } from '@microsoft/agents-hosting' import { ExceptionHelper } from '@microsoft/agents-activity' import { Errors } from './errorHelper' import { sanitizeBlobKey } from './blobsTranscriptStore' @@ -136,30 +137,50 @@ export class BlobsStorage implements Storage { * @returns A promise that resolves when the write operation is complete * @throws Will throw if there's a validation error, eTag conflict, or other storage error */ - async write (changes: StoreItems): Promise { + async write (changes: StoreItems, options?: StorageWriteOptions): Promise { z.record(z.unknown()).parse(changes) await this._initialize() - await Promise.all( - Object.entries(changes).map(async ([key, { eTag = '', ...change }]) => { + const entries = Object.entries(changes) + const results = await Promise.all( + entries.map(async ([key, value]) => { + const { eTag = '', ...change } = value + const conditions: BlobRequestConditions = {} + + if (options?.ifNotExists) { + logger.debug(`ifNoneMatch=* condition applied for key: ${key}`) + conditions.ifNoneMatch = '*' + } else if (typeof eTag === 'string' && eTag !== '*') { + logger.debug(`ifMatch=${eTag} condition applied for key: ${key}`) + conditions.ifMatch = eTag + } + try { const blob = this._containerClient.getBlockBlobClient(sanitizeBlobKey(key)) const serialized = JSON.stringify(change) logger.debug(`Writing blob: ${key}, eTag: ${eTag}, size: ${serialized.length}`) - return await blob.upload(serialized, serialized.length, { - conditions: typeof eTag === 'string' && eTag !== '*' ? { ifMatch: eTag } : {}, + const item = await blob.upload(serialized, serialized.length, { + conditions, blobHTTPHeaders: { blobContentType: 'application/json' }, }) - } catch (err: any) { - if (err.statusCode === 412) { - throw ExceptionHelper.generateException(Error, Errors.ETagConflict) - } else { - throw err + + return { key, eTag: item.etag } + } catch (cause: any) { + if (cause.statusCode === 409) { + throw ExceptionHelper.generateException(Error, Errors.ItemAlreadyExists, cause, { key }) } + + if (cause.statusCode === 412) { + throw ExceptionHelper.generateException(Error, Errors.ETagConflict, cause, { key }) + } + + throw cause } }) ) + + return results.reduce((acc, { key, eTag }) => ({ ...acc, [key]: { eTag } }), {}) } /** diff --git a/packages/agents-hosting-storage-blob/src/errorHelper.ts b/packages/agents-hosting-storage-blob/src/errorHelper.ts index d33ece1f0..3679a367b 100644 --- a/packages/agents-hosting-storage-blob/src/errorHelper.ts +++ b/packages/agents-hosting-storage-blob/src/errorHelper.ts @@ -36,10 +36,18 @@ export const Errors: { [key: string]: AgentErrorDefinition } = { }, /** - * Error thrown when there is an eTag conflict during storage write. + * Error thrown when there is an eTag conflict during a storage operation. */ ETagConflict: { code: -160002, - description: 'Storage: error writing "{key}" due to eTag conflict.' + description: 'Unable to write \'{key}\' due to eTag conflict.' + }, + + /** + * Error thrown when attempting to create an item that already exists in storage. + */ + ItemAlreadyExists: { + code: -160003, + description: 'Unable to write \'{key}\' because it already exists.' } } diff --git a/packages/agents-hosting-storage-cosmos/CosmosDBErrorCodes.md b/packages/agents-hosting-storage-cosmos/CosmosDBErrorCodes.md index d0b4cf8ea..eecafd9b8 100644 --- a/packages/agents-hosting-storage-cosmos/CosmosDBErrorCodes.md +++ b/packages/agents-hosting-storage-cosmos/CosmosDBErrorCodes.md @@ -2,7 +2,7 @@ This document provides detailed information about error codes in the Microsoft 365 Agents SDK for JavaScript CosmosDB storage package. Each error includes a description, context, and likely fixes. -## Storage - CosmosDB Errors (-100000 to -100019) +## Storage - CosmosDB Errors (-100000 to -100021) ### -100000 #### Missing CosmosDB Storage Options @@ -472,3 +472,55 @@ const state = { ``` 4. **Check for accidental object references** that create circular dependencies in your state data. + +### -100020 +#### ETag Conflict + +Unable to write '{key}' due to eTag conflict. + +**Description & Context:** This error occurs during write operations when an eTag conflict is detected. An eTag (entity tag) is a version identifier used for optimistic concurrency control in Cosmos DB. When you read a document, it includes an eTag value. If you then attempt to write that document back with a specific eTag (rather than using the wildcard '*' for last-write-wins), Cosmos DB checks if the eTag still matches the current version of the document. If another process has modified the document since you read it, the eTag will no longer match, and the write operation fails with a conflict. This error indicates that the state document was modified by another concurrent operation between the time you read it and when you attempted to write your changes. + +**Likely Fix:** Choose one of two approaches: + +1. **Use last-write-wins** (simplest): Set eTag to `'*'` to overwrite regardless of current version: +```typescript +const changes: StoreItems = { + 'conversation-id': { + userState: { /* your state */ }, + eTag: '*' + } +}; +await storage.write(changes); +``` + +2. **Retry with re-read**: Read the latest version and retry with its current eTag: +```typescript +const items = await storage.read(['conversation-id']); +const item = items['conversation-id']; +const changes: StoreItems = { + 'conversation-id': { + userState: { /* your state */ }, + eTag: item?.eTag || '*' + } +}; +await storage.write(changes); +``` + +### -100021 +#### Item Already Exists + +Unable to write '{key}' because it already exists. + +**Description & Context:** This error occurs during write operations when attempting to create a new item in storage using a key that already exists in Cosmos DB. This error is typically thrown when using conditional write operations that enforce a "create-only" semantic (only allowing new documents to be created, not updates to existing ones). This can happen when the storage implementation has specific logic to prevent accidental overwrites of existing state, or when using Cosmos DB's conditional request logic with preconditions. The error indicates that the document you're trying to create with the given key is already present in the container. + +**Likely Fix:** This error is expected when using `StorageWriteOptions.ifNotExists`. To avoid it, don't use the `ifNotExists` parameter: + +```typescript +// Instead of: +// await storage.write(changes, { ifNotExists: true }); + +// Use: +await storage.write(changes); +``` + +This performs an upsert operation that handles both create and update scenarios. diff --git a/packages/agents-hosting-storage-cosmos/src/cosmosDbPartitionedStorage.ts b/packages/agents-hosting-storage-cosmos/src/cosmosDbPartitionedStorage.ts index 79c5943ac..0d5a08731 100644 --- a/packages/agents-hosting-storage-cosmos/src/cosmosDbPartitionedStorage.ts +++ b/packages/agents-hosting-storage-cosmos/src/cosmosDbPartitionedStorage.ts @@ -1,11 +1,11 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { Container, CosmosClient } from '@azure/cosmos' +import { Container, CosmosClient, ItemDefinition, ItemResponse, RequestOptions } from '@azure/cosmos' import { CosmosDbKeyEscape } from './cosmosDbKeyEscape' import { DocumentStoreItem } from './documentStoreItem' import { CosmosDbPartitionedStorageOptions } from './cosmosDbPartitionedStorageOptions' -import { Storage, StoreItems } from '@microsoft/agents-hosting' +import { Storage, StorageWriteOptions, StoreItems } from '@microsoft/agents-hosting' import { ExceptionHelper } from '@microsoft/agents-activity' import { Errors } from './errorHelper' @@ -170,47 +170,61 @@ export class CosmosDbPartitionedStorage implements Storage { * Writes items to storage. * @param changes The items to write. */ - async write (changes: StoreItems): Promise { + async write (changes: StoreItems, options?: StorageWriteOptions): Promise { if (!changes) { throw ExceptionHelper.generateException( ReferenceError, Errors.MissingWriteChanges ) - } else if (changes.length === 0) { - return + } else if (Object.keys(changes).length === 0) { + return {} } await this.initialize() - await Promise.all( - Object.entries(changes).map(async ([key, { eTag, ...change }]): Promise => { - const document = new DocumentStoreItem({ - id: CosmosDbKeyEscape.escapeKey( - key, - this.cosmosDbStorageOptions.keySuffix, - this.cosmosDbStorageOptions.compatibilityMode - ), - realId: key, - document: change, - }) + const writePromises = Object.entries(changes).map(async ([key, value]) => { + const { eTag, ...change } = value + const requestOptions: RequestOptions = {} - const accessCondition = - eTag !== '*' && eTag != null && eTag.length > 0 - ? { accessCondition: { type: 'IfMatch', condition: eTag } } - : undefined + if (eTag !== '*' && eTag != null && eTag.length > 0) { + requestOptions.accessCondition = { type: 'IfMatch', condition: eTag } + } - try { - await this.container.items.upsert(document, accessCondition) - } catch (err: any) { - this.checkForNestingError(change, err) - throw ExceptionHelper.generateException( - Error, - Errors.DocumentUpsertError, - err - ) - } + const document = new DocumentStoreItem({ + id: CosmosDbKeyEscape.escapeKey( + key, + this.cosmosDbStorageOptions.keySuffix, + this.cosmosDbStorageOptions.compatibilityMode + ), + realId: key, + document: change, }) - ) + + try { + let item: ItemResponse + if (options?.ifNotExists) { + requestOptions.accessCondition = { type: 'IfNoneMatch', condition: '*' } + item = await this.container.items.create(document, requestOptions) + } else { + item = await this.container.items.upsert(document, requestOptions) + } + return { key, eTag: item.etag } + } catch (cause: any) { + if (cause.code === 409) { + throw ExceptionHelper.generateException(Error, Errors.ItemAlreadyExists, cause, { key }) + } + + if (cause.code === 412) { + throw ExceptionHelper.generateException(Error, Errors.ETagConflict, cause, { key }) + } + + this.checkForNestingError(change, cause) + throw ExceptionHelper.generateException(Error, Errors.DocumentUpsertError, cause) + } + }) + + const results = await Promise.all(writePromises) + return results.reduce((acc, { key, eTag }) => ({ ...acc, [key]: { eTag } }), {}) } /** diff --git a/packages/agents-hosting-storage-cosmos/src/errorHelper.ts b/packages/agents-hosting-storage-cosmos/src/errorHelper.ts index db2068080..0ee454b3d 100644 --- a/packages/agents-hosting-storage-cosmos/src/errorHelper.ts +++ b/packages/agents-hosting-storage-cosmos/src/errorHelper.ts @@ -177,5 +177,21 @@ export const Errors: { [key: string]: AgentErrorDefinition } = { MaxNestingDepthExceeded: { code: -100019, description: 'Maximum nesting depth of {maxDepth} exceeded. {additionalMessage}' + }, + + /** + * Error thrown when there is an eTag conflict during a storage operation. + */ + ETagConflict: { + code: -100020, + description: 'Unable to write \'{key}\' due to eTag conflict.' + }, + + /** + * Error thrown when attempting to create an item that already exists in storage. + */ + ItemAlreadyExists: { + code: -100021, + description: 'Unable to write \'{key}\' because it already exists.' } } diff --git a/packages/agents-hosting-storage-cosmos/test/errorHelper.test.ts b/packages/agents-hosting-storage-cosmos/test/errorHelper.test.ts index 33aced67d..ba1fd4a03 100644 --- a/packages/agents-hosting-storage-cosmos/test/errorHelper.test.ts +++ b/packages/agents-hosting-storage-cosmos/test/errorHelper.test.ts @@ -33,10 +33,10 @@ describe('Errors tests', () => { val => val && typeof val === 'object' && 'code' in val && 'description' in val ) as AgentErrorDefinition[] - // All error codes should be negative and in the range -100000 to -100019 + // All error codes should be negative and in the range -100000 to -100021 errorDefinitions.forEach(errorDef => { assert.ok(errorDef.code < 0, `Error code ${errorDef.code} should be negative`) - assert.ok(errorDef.code >= -100019, `Error code ${errorDef.code} should be >= -100019`) + assert.ok(errorDef.code >= -100021, `Error code ${errorDef.code} should be >= -100021`) assert.ok(errorDef.code <= -100000, `Error code ${errorDef.code} should be <= -100000`) }) }) diff --git a/packages/agents-hosting/src/app/auth/handlerStorage.ts b/packages/agents-hosting/src/app/auth/handlerStorage.ts index e0842e633..f0b93b285 100644 --- a/packages/agents-hosting/src/app/auth/handlerStorage.ts +++ b/packages/agents-hosting/src/app/auth/handlerStorage.ts @@ -41,14 +41,14 @@ export class HandlerStorage { + public write (data: TActiveHandler) { return this.storage.write({ [this.key]: data }) } /** * Deletes handler state from storage. */ - public async delete (): Promise { + public async delete () { try { await this.storage.delete([this.key]) } catch (error) { diff --git a/packages/agents-hosting/src/app/auth/handlers/azureBotAuthorization.ts b/packages/agents-hosting/src/app/auth/handlers/azureBotAuthorization.ts index ab982139f..05ae064d3 100644 --- a/packages/agents-hosting/src/app/auth/handlers/azureBotAuthorization.ts +++ b/packages/agents-hosting/src/app/auth/handlers/azureBotAuthorization.ts @@ -229,7 +229,11 @@ export class AzureBotAuthorization implements AuthorizationHandler { * @param callback The callback function to be invoked on sign-in failure. */ onFailure (callback: (context: TurnContext, reason?: string) => Promise | void): void { - this._onFailure = callback + this._onFailure = async (context, reason) => { + // Clear any token exchange state on failure. + await this.deleteTokenExchange(context) + return callback(context, reason) + } } /** @@ -274,6 +278,7 @@ export class AzureBotAuthorization implements AuthorizationHandler { logger.debug(this.prefix(`Signing out User '${user}' from => Channel: '${channel}', Connection: '${connection}'`), context.activity) const userTokenClient = await this.getUserTokenClient(context) await userTokenClient.signOut(user, connection, channel) + await this.deleteTokenExchange(context) return true } @@ -403,6 +408,8 @@ export class AzureBotAuthorization implements AuthorizationHandler { const oCard = CardFactory.oauthCard(this._options.name!, this._options.title!, this._options.text!, signInResource, this._options.enableSso) await context.sendActivity(MessageFactory.attachment(oCard)) await storage.write({ activity, id: this.id, ...(active ?? {}), attemptsLeft: this.maxAttempts }) + // Clear any previous token exchange state when starting a new sign-in process. + await this.deleteTokenExchange(context) return AuthorizationHandlerStatus.PENDING } @@ -461,6 +468,16 @@ export class AzureBotAuthorization implements AuthorizationHandler { return AuthorizationHandlerStatus.PENDING } + // Duplication check is done after successful token exchange to allow MS Teams show the consent prompt per platform (e.g., web, mobile) in case of failing the token exchange. + // If the duplication check is done before, only one platform will show the consent prompt. + // Note: in case this check needs to be done before token exchange, consider adding the isSsoUserConsentFlow === undefined flag, + // to allow multiple token exchanges when the flag is set (indicating user consent flow), duplicated across platforms will still apply (showing one consent prompt). + if (await this.isTokenExchangeDuplicated(context)) { + logger.debug('Skipping duplicated signin/tokenExchange invoke activity.') + // Using PENDING state to avoid deleting the active session, as the deletion will be done by the first token exchange activity. + return AuthorizationHandlerStatus.PENDING + } + await this.sendInvokeResponse(context, { status: 200, body: { id: tokenExchangeInvokeRequest.id, connectionName: this._options.name! } @@ -491,12 +508,7 @@ export class AzureBotAuthorization implements AuthorizationHandler { /** * Verifies the magic code provided by the user. */ - private async codeVerification (storage: HandlerStorage, context: TurnContext, active?: AzureBotActiveHandler): Promise<{ status: AuthorizationHandlerStatus, code?: string }> { - if (!active) { - logger.debug(this.prefix('No active session found. Skipping code verification.'), context.activity) - return { status: AuthorizationHandlerStatus.IGNORED } - } - + private async codeVerification (storage: HandlerStorage, context: TurnContext, active: AzureBotActiveHandler): Promise<{ status: AuthorizationHandlerStatus, code?: string }> { const { activity } = context let state: string | undefined = activity.text @@ -603,4 +615,54 @@ export class AzureBotAuthorization implements AuthorizationHandler { return acc }, []) ?? [] } + + /** + * Generates a storage key for persisting token exchange state. + * @param context The turn context containing activity information. + * @returns The storage key string in format: `auth/azurebot/exchange/{channelId}/{userId}`. + * @throws Will throw an error if required activity properties are missing. + */ + private getTokenExchangeKey (context: TurnContext): string { + const channelId = context.activity.channelId + const userId = context.activity.from?.id + if (!channelId || !userId) { + throw new Error('Both \'activity.channelId\' and \'activity.from.id\' are required to generate the Token Exchange Storage key.') + } + return `auth/azurebot/exchange/${channelId}/${userId}` + } + + /** + * Checks if a token exchange request is duplicated. + * This is used to prevent duplicating responses when receiving multiple identical token exchange requests + * from the same user across different platforms (e.g., web and mobile). + * @param context The turn context. + * @returns True if the token exchange request is duplicated, false otherwise. + */ + private async isTokenExchangeDuplicated (context: TurnContext) { + const key = this.getTokenExchangeKey(context) + try { + await this.settings.storage.write({ [key]: { exchanged: true, timestamp: Date.now() } }, { ifNotExists: true }) + return false + } catch (error) { + return true + } + } + + /** + * Deletes the token exchange state from storage. + * @param context The turn context. + * @returns A promise that resolves when the state is deleted. + */ + private async deleteTokenExchange (context: TurnContext) { + try { + await this.settings.storage.delete([this.getTokenExchangeKey(context)]) + } catch (error) { + if ((error as Error).message?.toLowerCase().includes('not found')) { + logger.debug(this.prefix('Token exchange state not found for deletion.')) + return + } + + throw error + } + } } diff --git a/packages/agents-hosting/src/app/turnState.ts b/packages/agents-hosting/src/app/turnState.ts index 2c6087576..73037d9c0 100644 --- a/packages/agents-hosting/src/app/turnState.ts +++ b/packages/agents-hosting/src/app/turnState.ts @@ -344,7 +344,7 @@ export class TurnState< } if (storage) { - const promises: Promise[] = [] + const promises: Promise[] = [] if (changes) { promises.push(storage.write(changes)) } diff --git a/packages/agents-hosting/src/storage/eTagConflictError.ts b/packages/agents-hosting/src/storage/eTagConflictError.ts new file mode 100644 index 000000000..d7402817f --- /dev/null +++ b/packages/agents-hosting/src/storage/eTagConflictError.ts @@ -0,0 +1,11 @@ +/** + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +/** + * Error thrown when there is an eTag conflict during a storage operation. + */ +export class ETagConflictError extends Error { + public readonly code: number = 412 +} diff --git a/packages/agents-hosting/src/storage/fileStorage.ts b/packages/agents-hosting/src/storage/fileStorage.ts index e9653fd15..946cb1c98 100644 --- a/packages/agents-hosting/src/storage/fileStorage.ts +++ b/packages/agents-hosting/src/storage/fileStorage.ts @@ -5,7 +5,8 @@ import path from 'path' import fs from 'fs' -import { Storage, StoreItem } from './storage' +import { Storage, StorageWriteOptions, StoreItem, StoreItems } from './storage' +import { ItemAlreadyExistsError } from './itemAlreadyExistsError' /** * A file-based storage implementation that persists data to the local filesystem. @@ -50,7 +51,7 @@ import { Storage, StoreItem } from './storage' */ export class FileStorage implements Storage { private _folder: string - private _stateFile: Record + private _stateFile: Record /** * Creates a new FileStorage instance that stores data in the specified folder. @@ -123,13 +124,17 @@ export class FileStorage implements Storage { * > Any eTag values in the changes object are ignored. * */ - write (changes: StoreItem) : Promise { + write (changes: StoreItems, options?: StorageWriteOptions) : Promise { const keys = Object.keys(changes) for (const key of keys) { + if (options?.ifNotExists && key in this._stateFile) { + throw new ItemAlreadyExistsError(`The key '${key}' already exists in storage.`) + } this._stateFile[key] = changes[key] } + fs.writeFileSync(this._folder + '/state.json', JSON.stringify(this._stateFile, null, 2)) - return Promise.resolve() + return Promise.resolve(changes) } /** diff --git a/packages/agents-hosting/src/storage/index.ts b/packages/agents-hosting/src/storage/index.ts index 2a57b10cf..1ec117b7b 100644 --- a/packages/agents-hosting/src/storage/index.ts +++ b/packages/agents-hosting/src/storage/index.ts @@ -1,3 +1,5 @@ export * from './storage' export * from './memoryStorage' export * from './fileStorage' +export * from './eTagConflictError' +export * from './itemAlreadyExistsError' diff --git a/packages/agents-hosting/src/storage/itemAlreadyExistsError.ts b/packages/agents-hosting/src/storage/itemAlreadyExistsError.ts new file mode 100644 index 000000000..15a666ab0 --- /dev/null +++ b/packages/agents-hosting/src/storage/itemAlreadyExistsError.ts @@ -0,0 +1,11 @@ +/** + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +/** + * Error thrown when attempting to create an item that already exists in storage. + */ +export class ItemAlreadyExistsError extends Error { + public readonly code: number = 409 +} diff --git a/packages/agents-hosting/src/storage/memoryStorage.ts b/packages/agents-hosting/src/storage/memoryStorage.ts index 5adc3f193..22212fe4f 100644 --- a/packages/agents-hosting/src/storage/memoryStorage.ts +++ b/packages/agents-hosting/src/storage/memoryStorage.ts @@ -3,7 +3,9 @@ * Licensed under the MIT License. */ -import { Storage, StoreItem } from './storage' +import { ETagConflictError } from './eTagConflictError' +import { ItemAlreadyExistsError } from './itemAlreadyExistsError' +import { Storage, StorageWriteOptions, StoreItem, StoreItems } from './storage' import { debug } from '@microsoft/agents-activity/logger' const logger = debug('agents:memory-storage') @@ -90,25 +92,35 @@ export class MemoryStorage implements Storage { * has the same eTag. If an item has an eTag of '*' or no eTag, it will * always be written regardless of the current state. */ - async write (changes: StoreItem): Promise { - if (!changes || changes.length === 0) { + async write (changes: StoreItems, options?: StorageWriteOptions): Promise { + if (!changes || Object.keys(changes).length === 0) { throw new ReferenceError('Changes are required when writing.') } + const result: StoreItems = {} for (const [key, newItem] of Object.entries(changes)) { + if (options?.ifNotExists && key in this.memory) { + throw new ItemAlreadyExistsError(`The key '${key}' already exists in storage.`) + } + logger.debug(`Writing key: ${key}`) const oldItemStr = this.memory[key] + let savedItem: StoreItem if (!oldItemStr || newItem.eTag === '*' || !newItem.eTag) { - this.saveItem(key, newItem) + savedItem = this.saveItem(key, newItem) } else { const oldItem = JSON.parse(oldItemStr) if (newItem.eTag === oldItem.eTag) { - this.saveItem(key, newItem) + savedItem = this.saveItem(key, newItem) } else { - throw new Error(`Storage: error writing "${key}" due to eTag conflict.`) + throw new ETagConflictError(`Unable to write '${key}' due to eTag conflict.`, { cause: { oldItem, newItem } }) } } + + result[key] = { eTag: savedItem.eTag } } + + return result } /** @@ -138,8 +150,9 @@ export class MemoryStorage implements Storage { * * @private */ - private saveItem (key: string, item: unknown): void { + private saveItem (key: string, item: unknown): StoreItem { const clone = Object.assign({}, item, { eTag: (this.etag++).toString() }) this.memory[key] = JSON.stringify(clone) + return clone } } diff --git a/packages/agents-hosting/src/storage/storage.ts b/packages/agents-hosting/src/storage/storage.ts index 7f6e1f834..fe61ddc90 100644 --- a/packages/agents-hosting/src/storage/storage.ts +++ b/packages/agents-hosting/src/storage/storage.ts @@ -62,6 +62,23 @@ export interface StoreItems { */ export type StorageKeyFactory = (context: TurnContext) => string | Promise +/** + * Options for storage write operations. + */ +export interface StorageWriteOptions { + /** + * If true, the write operation will only succeed if the item does not already exist in storage. + * + * @remarks + * This is useful for scenarios where you want to ensure that you are creating a new item + * and do not want to overwrite any existing data. If the item already exists, the write + * operation will fail with an error. + * + * The default value is false, meaning that the write operation will overwrite existing items. + */ + ifNotExists?: boolean; +} + /** * Defines the interface for storage operations in the Agents platform. * @@ -82,16 +99,17 @@ export interface Storage { * @returns A promise that resolves to the store items. Items that don't exist in storage will not be included in the result. * @throws If the keys array is empty or undefined */ - read: (keys: string[]) => Promise; + read: (keys: string[]) => Promise; /** * Writes store items to storage. * * @param changes The items to write to storage, indexed by key - * @returns A promise that resolves when the write operation is complete + * @param options Optional settings for the write operation + * @returns A promise that resolves to the written store items * @throws If the changes object is empty or undefined, or if an eTag conflict occurs and optimistic concurrency is enabled */ - write: (changes: StoreItem) => Promise; + write: (changes: StoreItems, options?: StorageWriteOptions) => Promise; /** * Deletes store items from storage. diff --git a/packages/agents-hosting/test/hosting/app/authorization.test.ts b/packages/agents-hosting/test/hosting/app/authorization.test.ts index dbad865b8..9ec72e6b1 100644 --- a/packages/agents-hosting/test/hosting/app/authorization.test.ts +++ b/packages/agents-hosting/test/hosting/app/authorization.test.ts @@ -1,9 +1,13 @@ import { strict as assert } from 'assert' import { describe, it } from 'node:test' +import { createStubInstance } from 'sinon' import { AgentApplication } from './../../../src/app' import { MemoryStorage } from '../../../src/storage' -import { AzureBotAuthorizationOptions } from '../../../src/app/auth/handlers' +import { CloudAdapter, MsalConnectionManager, TurnContext, UserTokenClient } from '../../../src' +import { Activity, ActivityTypes } from '@microsoft/agents-activity' +import { AzureBotActiveHandler, AzureBotAuthorization, AzureBotAuthorizationOptions } from '../../../src/app/auth/handlers' +import { AuthorizationHandlerStatus } from '../../../src/app/auth/types' describe('AgentApplication', () => { it('should intitalize with underfined authorization', () => { @@ -115,4 +119,34 @@ describe('AgentApplication', () => { await app.authorization.getToken({} as any, 'nonExistinghandler') }, { message: "Cannot find auth handler with ID 'nonExistinghandler'. Ensure it is configured in the agent application options." }) }) + + it('should handle duplicate token exchange requests', async () => { + const adapter = new CloudAdapter() + const storage = new MemoryStorage() + const connections = new MsalConnectionManager() + const exchangeActivity = Activity.fromObject({ + type: ActivityTypes.Invoke, + channelId: 'msteams', + from: { id: 'user1' }, + recipient: { id: 'bot' }, + conversation: { id: 'convo1' }, + name: 'signin/tokenExchange', + value: { id: 'testId', token: 'incoming-token', connectionName: 'connectionName' } + }) + const context = new TurnContext(adapter, exchangeActivity) + const auth = new AzureBotAuthorization('handlerId', { name: 'connectionName' }, { connections, storage }) + + const userTokenClient = createStubInstance(UserTokenClient) + userTokenClient.exchangeTokenAsync.resolves({ token: 'exchanged-token' }) + context.turnState.set(context.adapter.UserTokenClientKey, userTokenClient) + + // Provide an active session to hit the token exchange path + const active: AzureBotActiveHandler = { id: auth.id, activity: exchangeActivity, attemptsLeft: 2 } + const first = await auth.signin(context, active) + const second = await auth.signin(context, active) + + assert.equal(first, AuthorizationHandlerStatus.APPROVED) + // Second request should be pending to discard the duplicated exchange. + assert.equal(second, AuthorizationHandlerStatus.PENDING) + }) }) diff --git a/packages/agents-hosting/test/hosting/storage/memoryStorage.test.ts b/packages/agents-hosting/test/hosting/storage/memoryStorage.test.ts index 74485fcfa..78602bdcf 100644 --- a/packages/agents-hosting/test/hosting/storage/memoryStorage.test.ts +++ b/packages/agents-hosting/test/hosting/storage/memoryStorage.test.ts @@ -86,7 +86,7 @@ describe('MemoryStorage', () => { await memoryStorage.write({ key1: { value: 'conflict', eTag: 'invalid' } }), { name: 'Error', - message: 'Storage: error writing "key1" due to eTag conflict.' + message: 'Unable to write \'key1\' due to eTag conflict.' } ) })