Sensitivity label parsing fix in ODSP join session call (microsoft#25186)

## Description

Fixes parsing of sensitivity label information in the ODSP join session
response. So this was double-parsed, leading to runtime errors when
sensitivity label information was actually present.

In practice, sensitivity label information hasn't been present, because
it wasn't rolled out ODSP side and we didn't sent the
`Return-Sensitivity-Labels` Prefer header. We're now setting this
header, but have control over it via a feature gate. Once ODSP rollout
is done, we can test and roll out on our end, and eventually delete the
feature gate again.

## Breaking Changes

This breaks the legacy/alpha API of sensitivityLabelsInfo in
ISocketStorageDiscovery, switching from string to a more specific object
type. At runtime, when sensitivity label information had been present,
this would have been an object all along. However, in production this as
not been present, and shouldn't have been used by any consumer. So this
makes sensitivityLabelsInfo correct and usable for the first time.

Author: steffenloesch

.changeset/dry-frogs-change.md

@@ -0,0 +1,8 @@
+---
+"@fluidframework/odsp-driver": minor
+"@fluidframework/odsp-driver-definitions": minor
+"__section": fix
+---
+Fixed bug in parsing sensitivity label information from the ODSP join session response
+
+Fixes parsing of sensitivity label information in the ODSP join session response. If there had been sensitivity label data present in the ODSP response, this data would have been double-parsed, leading to runtime errors. This issue was so far not hit in practice, because ODSP did not roll out sensitivity labels in the response yet. This bug fix gets odsp-driver ready for that rollout, which is planned to happen soon.

packages/drivers/odsp-driver-definitions/api-report/odsp-driver-definitions.legacy.alpha.api.md

@@ -150,6 +150,20 @@ export interface IRelaySessionAwareDriverFactory extends IProvideSessionAwareDri
     getRelayServiceSessionInfo(resolvedUrl: IResolvedUrl): Promise<ISocketStorageDiscovery | undefined>;
 }
 
+// @alpha @legacy
+export interface ISensitivityLabel {
+    appliedByUserEmail: string;
+    assignmentMethod: string;
+    sensitivityLabelId: string;
+    tenantId: string;
+}
+
+// @alpha @legacy
+export interface ISensitivityLabelsInfo {
+    labels: ISensitivityLabel[];
+    timestamp: string;
+}
+
 // @alpha @legacy
 export interface ISharingLink extends ISharingLinkKind {
     // (undocumented)
@@ -187,7 +201,7 @@ export interface ISocketStorageDiscovery {
     refreshSessionDurationSeconds?: number;
     // (undocumented)
     runtimeTenantId?: string;
-    sensitivityLabelsInfo?: string;
+    sensitivityLabelsInfo?: ISensitivityLabelsInfo;
     // (undocumented)
     snapshotStorageUrl: string;
     socketToken?: string;

packages/drivers/odsp-driver-definitions/package.json

@@ -105,7 +105,12 @@
 		"typescript": "~5.4.5"
 	},
 	"typeValidation": {
-		"broken": {},
+		"broken": {
+			"Interface_ISocketStorageDiscovery": {
+				"forwardCompat": false,
+				"backCompat": false
+			}
+		},
 		"entrypoint": "legacy"
 	}
 }

packages/drivers/odsp-driver-definitions/src/index.ts

@@ -45,5 +45,7 @@ export {
 export {
 	IProvideSessionAwareDriverFactory,
 	IRelaySessionAwareDriverFactory,
+	ISensitivityLabel,
+	ISensitivityLabelsInfo,
 	ISocketStorageDiscovery,
 } from "./sessionProvider.js";

packages/drivers/odsp-driver-definitions/src/sessionProvider.ts

@@ -45,7 +45,35 @@ export interface ISocketStorageDiscovery {
 	 * response will contain empty labels when the file has no labels, so this field will be there
 	 * even if file has no labels when the service will implement this contract.
 	 */
-	sensitivityLabelsInfo?: string;
+	sensitivityLabelsInfo?: ISensitivityLabelsInfo;
+}
+
+/**
+ * Sensitivity labels information for a file, part of the socket storage discovery response.
+ * @legacy
+ * @alpha
+ */
+export interface ISensitivityLabelsInfo {
+	/** ISO format timestamp when the label info snapshot was generated. */
+	timestamp: string;
+	/** List of applied sensitivity labels. Empty if none. */
+	labels: ISensitivityLabel[];
+}
+
+/**
+ * A single sensitivity label applied to a document, part of the socket storage discovery response.
+ * @legacy
+ * @alpha
+ */
+export interface ISensitivityLabel {
+	/** Unique identifier of the sensitivity label. */
+	sensitivityLabelId: string;
+	/** Tenant under which the label is defined. */
+	tenantId: string;
+	/** How the label was assigned, for example "standard". */
+	assignmentMethod: string;
+	/** Email of the user who applied the label. */
+	appliedByUserEmail: string;
 }
 
 /**

packages/drivers/odsp-driver-definitions/src/test/types/validateOdspDriverDefinitionsPrevious.generated.ts

@@ -346,6 +346,7 @@ declare type current_as_old_for_Interface_ISnapshotOptions = requireAssignableTo
  * typeValidation.broken:
  * "Interface_ISocketStorageDiscovery": {"forwardCompat": false}
  */
+// @ts-expect-error compatibility expected to be broken
 declare type old_as_current_for_Interface_ISocketStorageDiscovery = requireAssignableTo<TypeOnly<old.ISocketStorageDiscovery>, TypeOnly<current.ISocketStorageDiscovery>>
 
 /*
@@ -355,6 +356,7 @@ declare type old_as_current_for_Interface_ISocketStorageDiscovery = requireAssig
  * typeValidation.broken:
  * "Interface_ISocketStorageDiscovery": {"backCompat": false}
  */
+// @ts-expect-error compatibility expected to be broken
 declare type current_as_old_for_Interface_ISocketStorageDiscovery = requireAssignableTo<TypeOnly<current.ISocketStorageDiscovery>, TypeOnly<old.ISocketStorageDiscovery>>
 
 /*

packages/drivers/odsp-driver/src/odspDelayLoadedDeltaStream.ts

@@ -25,6 +25,7 @@ import {
 	type IOdspError,
 	type IOdspResolvedUrl,
 	type ISocketStorageDiscovery,
+	type ISensitivityLabelsInfo,
 	type InstrumentedStorageTokenFetcher,
 	OdspErrorTypes,
 	type TokenFetchOptions,
@@ -189,9 +190,7 @@ export class OdspDelayLoadedDeltaStream {
 				);
 			}
 			if (websocketEndpoint.sensitivityLabelsInfo !== undefined) {
-				this.emitMetaDataUpdateEvent({
-					sensitivityLabelsInfo: websocketEndpoint.sensitivityLabelsInfo,
-				});
+				this.emitSensitivityLabelUpdateEvent(websocketEndpoint.sensitivityLabelsInfo);
 			}
 
 			const connectionId = uuid();
@@ -283,9 +282,9 @@ export class OdspDelayLoadedDeltaStream {
 					// Drop error
 				}
 				if (envelope?.contents?.type === policyLabelsUpdatesSignalType) {
-					this.emitMetaDataUpdateEvent({
-						sensitivityLabelsInfo: JSON.stringify(envelope.contents.content),
-					});
+					this.emitSensitivityLabelUpdateEvent(
+						envelope.contents.content as ISensitivityLabelsInfo,
+					);
 				}
 			}
 		}
@@ -416,6 +415,9 @@ export class OdspDelayLoadedDeltaStream {
 		const disableJoinSessionRefresh = this.mc.config.getBoolean(
 			"Fluid.Driver.Odsp.disableJoinSessionRefresh",
 		);
+		const setSensitivityLabelHeader = this.mc.config.getBoolean(
+			"Fluid.Driver.Odsp.setSensitivityLabelHeader",
+		);
 		const executeFetch = async (): Promise<{
 			entryTime: number;
 			joinSessionResponse: ISocketStorageDiscovery;
@@ -430,14 +432,13 @@ export class OdspDelayLoadedDeltaStream {
 				requestSocketToken,
 				options,
 				disableJoinSessionRefresh,
+				setSensitivityLabelHeader,
 				isRefreshingJoinSession,
 				displayName,
 			);
 			// Emit event only in case it is fetched from the network.
 			if (joinSessionResponse.sensitivityLabelsInfo !== undefined) {
-				this.emitMetaDataUpdateEvent({
-					sensitivityLabelsInfo: joinSessionResponse.sensitivityLabelsInfo,
-				});
+				this.emitSensitivityLabelUpdateEvent(joinSessionResponse.sensitivityLabelsInfo);
 			}
 			return {
 				entryTime: Date.now(),
@@ -507,17 +508,15 @@ export class OdspDelayLoadedDeltaStream {
 		return response.joinSessionResponse;
 	}
 
-	private emitMetaDataUpdateEvent(metadata: Record<string, string>): void {
-		const label = JSON.parse(metadata.sensitivityLabelsInfo) as {
-			labels: unknown;
-			timestamp: number;
-		};
-		const time = label.timestamp;
-		assert(time > 0, 0x8e0 /* time should be positive */);
-		if (time > this.labelUpdateTimestamp) {
-			this.labelUpdateTimestamp = time;
+	private emitSensitivityLabelUpdateEvent(
+		sensitivityLabelsInfo: ISensitivityLabelsInfo,
+	): void {
+		const createdTimestamp = Date.parse(sensitivityLabelsInfo.timestamp);
+		assert(createdTimestamp > 0, 0x8e0 /* time should be positive */);
+		if (createdTimestamp > this.labelUpdateTimestamp) {
+			this.labelUpdateTimestamp = createdTimestamp;
 			this.metadataUpdateHandler({
-				sensitivityLabelsInfo: metadata.sensitivityLabelsInfo,
+				sensitivityLabelsInfo: JSON.stringify(sensitivityLabelsInfo),
 			});
 		}
 	}

packages/drivers/odsp-driver/src/test/socketTests/deltaConnectionUpdateTests.spec.ts

@@ -7,7 +7,10 @@ import { strict as assert } from "node:assert";
 
 import type { IClient } from "@fluidframework/driver-definitions";
 import type { ISignalMessage } from "@fluidframework/driver-definitions/internal";
-import type { ISocketStorageDiscovery } from "@fluidframework/odsp-driver-definitions/internal";
+import type {
+	ISensitivityLabel,
+	ISocketStorageDiscovery,
+} from "@fluidframework/odsp-driver-definitions/internal";
 import {
 	type ITelemetryLoggerExt,
 	MockLogger,
@@ -77,17 +80,26 @@ describe("DeltaConnectionMetadata update tests", () => {
 		await yieldEventLoop();
 	}
 
-	function addJoinSessionStub(label: string): SinonStub {
-		joinSessionResponse.sensitivityLabelsInfo = JSON.stringify({
-			labels: label,
-			timestamp: Date.now(),
-		});
+	function addJoinSessionStub(label: ISensitivityLabel): SinonStub {
+		joinSessionResponse.sensitivityLabelsInfo = {
+			timestamp: new Date().toISOString(),
+			labels: [label],
+		};
 		const joinSessionStub = stub(fetchJoinSession, mockify.key).callsFake(
 			async () => joinSessionResponse,
 		);
 		return joinSessionStub;
 	}
 
+	function testSensitivityLabelObjectWithId(id: string): ISensitivityLabel {
+		return {
+			sensitivityLabelId: id,
+			tenantId: "tenantId",
+			assignmentMethod: "standard",
+			appliedByUserEmail: "fakeemail@microsoft.com",
+		};
+	}
+
 	before(async () => {
 		hashedDocumentId = await getHashedDocumentId(driveId, itemId);
 	});
@@ -98,7 +110,11 @@ describe("DeltaConnectionMetadata update tests", () => {
 			async (_options) => "token",
 			async (_options) => "token",
 			new LocalPersistentCache(2000),
-			{ snapshotOptions: { timeout: 2000 } },
+			{
+				snapshotOptions: { timeout: 2000 },
+				// Ensure each test uses its own socket reuse namespace and won’t share sockets.
+				isolateSocketCache: true,
+			},
 		);
 		const locator: OdspFluidDataStoreLocator = {
 			driveId,
@@ -134,25 +150,79 @@ describe("DeltaConnectionMetadata update tests", () => {
 		}
 	}
 
-	it("Delta connection metadata should be updated via Fluid signals and join session response", async () => {
+	it("Join session response parsing", async () => {
 		await tickClock(1);
 		socket = new ClientSocketMock();
 		let eventRaised = false;
-		let content: Record<string, string>;
+
+		const exampleSensitivityLabelsInfo = `{
+			"timestamp":"2025-05-28T14:56:21-07:00",
+			"labels":[
+				{"sensitivityLabelId":"sensitivityLabelId",
+				"tenantId":"tenantId",
+				"assignmentMethod":"standard",
+				"appliedByUserEmail":"fakeemail@microsoft.com"}
+			]}`;
+
+		const joinSessionResponseString = `
+			{
+				"@odata.context":"https://microsoft.sharepoint-df.com/_api/v2.1/$metadata#oneDrive.session",
+				"deltaStorageUrl":"https://fake/deltaStorageUrl",
+				"deltaStreamSocketUrl":"https://localhost:3001",
+				"id":"id",
+				"refreshSessionDurationSeconds":100,
+				"runtimeTenantId":"1!1!3",
+				"snapshotStorageUrl":"https://fake/snapshotStorageUrl",
+				"socketToken":"",
+				"sensitivityLabelsInfo": ${exampleSensitivityLabelsInfo}
+			}`;
+
+		const parsedResponse = JSON.parse(joinSessionResponseString) as ISocketStorageDiscovery;
 
 		const handler = (metadata: Record<string, string>): void => {
 			eventRaised = true;
-			assert.strictEqual(
-				// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
-				JSON.parse(metadata.sensitivityLabelsInfo).labels,
-				content.labels,
-				"label via event should match",
+			assert.deepStrictEqual(
+				JSON.parse(metadata.sensitivityLabelsInfo),
+				parsedResponse.sensitivityLabelsInfo,
+				"sensitivity info via event should match",
 			);
 		};
 
-		let joinSessionStub = addJoinSessionStub("label1");
+		const joinSessionStub = stub(fetchJoinSession, mockify.key).callsFake(
+			async () => parsedResponse,
+		);
+
+		service.on("metadataUpdate", handler);
+		const connection = (await mockSocket(socket as unknown as Socket, async () =>
+			service.connectToDeltaStream(client),
+		)) as OdspDocumentDeltaConnection;
+		assert(eventRaised, "event1 should have been raised");
+		service.off("metadataUpdate", handler);
+		joinSessionStub.restore();
+
+		assert(!connection.disposed, "connection should not be disposed");
+
+		assert(joinSessionStub.calledOnce, "Should called once on first try");
+	});
+
+	it("Delta connection metadata should be updated via Fluid signals and join session response", async () => {
+		await tickClock(1);
+		socket = new ClientSocketMock();
+		let eventRaised = false;
+		let content: Record<string, ISensitivityLabel[]>;
+
+		const handler = (metadata: Record<string, string>): void => {
+			eventRaised = true;
+			// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access
+			const arg1 = JSON.parse(metadata.sensitivityLabelsInfo).labels;
+			const arg2: ISensitivityLabel[] | undefined = content.labels;
+			assert.deepStrictEqual(arg1, arg2, "label via event should match");
+		};
+
+		const label1Object = testSensitivityLabelObjectWithId("label1");
+		let joinSessionStub = addJoinSessionStub(label1Object);
 
-		content = { labels: "label1" };
+		content = { labels: [label1Object] };
 		service.on("metadataUpdate", handler);
 		const connection = (await mockSocket(socket as unknown as Socket, async () =>
 			service.connectToDeltaStream(client),
@@ -170,8 +240,9 @@ describe("DeltaConnectionMetadata update tests", () => {
 		// Now change label through signal and listen as event on service.
 		// eslint-disable-next-line require-atomic-updates
 		eventRaised = false;
-		content = { labels: "label2" };
-		const signalContent1 = { labels: "label2", timestamp: Date.now() };
+		const label2Object = testSensitivityLabelObjectWithId("label2");
+		content = { labels: [label2Object] };
+		const signalContent1 = { labels: [label2Object], timestamp: new Date().toISOString() };
 		const signalMessage1: ISignalMessage = {
 			clientId: null,
 			content: JSON.stringify({
@@ -193,16 +264,18 @@ describe("DeltaConnectionMetadata update tests", () => {
 		// Now update through join session response
 		// eslint-disable-next-line require-atomic-updates
 		eventRaised = false;
-		content = { labels: "label3" };
+		const label3Object = testSensitivityLabelObjectWithId("label3");
+		content = { labels: [label3Object] };
 		service.on("metadataUpdate", handler);
-		joinSessionStub = addJoinSessionStub("label3");
+		joinSessionStub = addJoinSessionStub(label3Object);
 		await tickClock(70000);
 		joinSessionStub.restore();
 		assert(eventRaised, "event3 should have been raised");
 		service.off("metadataUpdate", handler);
 	});
 
 	afterEach(async () => {
+		service?.dispose();
 		clock.reset();
 		socket?.close();
 		await epochTracker.removeEntries().catch(() => {});