Skip to content

Commit

Permalink
[main > Release/Client/rc2]: Use tenant domain instead of siteUrl in …
Browse files Browse the repository at this point in the history
…/shares api in ODSP driver (#20738) (#20770)

## Description

[Task
Link](https://dev.azure.com/fluidframework/internal/_workitems/edit/7650)

To load a Fluid container, host/app provides a URL which is used to talk
to service to load the container. That URL can be a share link which
host/app specifies explicitly to Fluid that the URL is a share link.
Fluid then uses that link to fetch the snapshot for the container. User
sometimes don’t have permissions initially, so to save COGS, Fluid
supply this share link in body of trees latest call to the service, so
that in 1 network call, we can do the redeem and also fetch the
snapshot. Sometimes this redeem on service side fails (in case it is
first time sharing with that user), then the service throws on the trees
latest call. Fluid catches this exception and determines/guess if this
is because of redeem failure at service side and then fallback to doing
redeem by itself using the /shares API. Once the redeem fails even in
/shares API, the container load fails.

In this issue, for outlook Win32 the /shares API (Documentation link for
API:

[Link](https://docs.microsoft.com/en-us/onedrive/developer/rest-api/api/shares_get))
call is failing because of the length limitation on the API. There are 2
ways to call the /shares API(for Fluid use),

Method 1: site-url/_api/v2.0/shares/{shareID}
This has length limitation of about 400 characters.

Method 2: tenant-domain/_api/v2.0/shares/{shareID} Then the length
limitation is about 2048 characters. where this, share ID is generated
by encoding the share link supplied by the app in the above load
request. So, encoding longer URL will give longer shareID.

Tenant-domain can be extracted from site-url by doing: Const
tenant-domain = new URL(site-url).origin

This issue occurred for Outlook win 32, because MDO team started adding
some, additional long query params to the share link. So, we are
switching to use Method 1 instead of Method 2.
  • Loading branch information
jatgarg committed Apr 22, 2024
1 parent 90a4798 commit dc3f29c
Show file tree
Hide file tree
Showing 2 changed files with 159 additions and 25 deletions.
86 changes: 62 additions & 24 deletions packages/drivers/odsp-driver/src/fetchSnapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
isRuntimeMessage,
NonRetryableError,
} from "@fluidframework/driver-utils";
import { loggerToMonitoringContext } from "@fluidframework/telemetry-utils";
import {
fetchIncorrectResponse,
throwOdspNetworkError,
Expand Down Expand Up @@ -220,34 +221,71 @@ async function redeemSharingLink(
storageTokenFetcher: InstrumentedStorageTokenFetcher,
logger: ITelemetryLoggerExt,
forceAccessTokenViaAuthorizationHeader: boolean,
): Promise<IOdspResponse<unknown>> {
return PerformanceEvent.timedExecAsync(
): Promise<void> {
await PerformanceEvent.timedExecAsync(
logger,
{
eventName: "RedeemShareLink",
},
async () =>
getWithRetryForTokenRefresh(async (tokenFetchOptions) => {
assert(
!!odspResolvedUrl.shareLinkInfo?.sharingLinkToRedeem,
0x1ed /* "Share link should be present" */,
);
const storageToken = await storageTokenFetcher(
tokenFetchOptions,
"RedeemShareLink",
);
const encodedShareUrl = getEncodedShareUrl(
odspResolvedUrl.shareLinkInfo?.sharingLinkToRedeem,
);
const redeemUrl = `${odspResolvedUrl.siteUrl}/_api/v2.0/shares/${encodedShareUrl}`;
const { url, headers } = getUrlAndHeadersWithAuth(
redeemUrl,
storageToken,
forceAccessTokenViaAuthorizationHeader,
);
headers.prefer = "redeemSharingLink";
return fetchAndParseAsJSONHelper(url, { headers });
}),
async () => {
assert(
!!odspResolvedUrl.shareLinkInfo?.sharingLinkToRedeem,
0x1ed /* "Share link should be present" */,
);

const encodedShareUrl = getEncodedShareUrl(
odspResolvedUrl.shareLinkInfo?.sharingLinkToRedeem,
);

let redeemUrl: string | undefined;
async function callSharesAPI(baseUrl: string): Promise<void> {
await getWithRetryForTokenRefresh(async (tokenFetchOptions) => {
const storageToken = await storageTokenFetcher(
tokenFetchOptions,
"RedeemShareLink",
);
redeemUrl = `${baseUrl}/_api/v2.0/shares/${encodedShareUrl}`;
const { url, headers } = getUrlAndHeadersWithAuth(
redeemUrl,
storageToken,
forceAccessTokenViaAuthorizationHeader,
);
headers.prefer = "redeemSharingLink";
await fetchAndParseAsJSONHelper(url, { headers });
});
}

const disableUsingTenantDomain = loggerToMonitoringContext(logger).config.getBoolean(
"Fluid.Driver.Odsp.DisableUsingTenantDomainForSharesApi",
);
// There is an issue where if we use the siteUrl in /shares, then the allowed length of url is just a few hundred characters(300-400)
// and we fail to do the redeem. But if we use the tenant domain in the url, then the allowed length becomes 2048. So, first
// construct the url for /shares using tenant domain but to be on safer side, fallback to using the siteUrl. We get tenant domain
// by getting origin of the siteUrl.
if (!disableUsingTenantDomain) {
try {
await callSharesAPI(new URL(odspResolvedUrl.siteUrl).origin);
return;
} catch (error) {
logger.sendTelemetryEvent(
{
eventName: "ShareLinkRedeemFailedWithTenantDomain",
details: JSON.stringify({
length: redeemUrl?.length,
shareLinkUrlLength:
odspResolvedUrl.shareLinkInfo?.sharingLinkToRedeem.length,
queryParamsLength: new URL(
odspResolvedUrl.shareLinkInfo?.sharingLinkToRedeem,
).search.length,
useHeaders: forceAccessTokenViaAuthorizationHeader,
}),
},
error,
);
}
}
await callSharesAPI(odspResolvedUrl.siteUrl);
},
);
}

Expand Down
98 changes: 97 additions & 1 deletion packages/drivers/odsp-driver/src/test/fetchSnapshot.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ import { OdspDriverUrlResolver } from "../odspDriverUrlResolver.js";
import { ISnapshotRequestAndResponseOptions } from "../fetchSnapshot.js";
import { OdspDocumentStorageService } from "../odspDocumentStorageManager.js";
import { convertToCompactSnapshot } from "../compactSnapshotWriter.js";
import { createResponse } from "./mockFetch.js";
import {
createResponse,
mockFetchMultiple,
notFound,
okResponse,
type MockResponse,
} from "./mockFetch.js";

const createUtLocalCache = (): LocalPersistentCache => new LocalPersistentCache();

Expand Down Expand Up @@ -466,6 +472,96 @@ describe("Tests1 for snapshot fetch", () => {
"unexpected events",
);
});

it("RedeemFallback behavior when fallback succeeds with using tenant domain", async () => {
resolved.shareLinkInfo = {
sharingLinkToRedeem: "https://microsoft.sharepoint-df.com/sharelink",
};
hostPolicy.enableRedeemFallback = true;

const snapshot: ISnapshot = {
blobContents,
snapshotTree: snapshotTreeWithGroupId,
ops: [],
latestSequenceNumber: 0,
sequenceNumber: 0,
snapshotFormatV: 1,
};
const response = (await createResponse(
{ "x-fluid-epoch": "epoch1", "content-type": "application/ms-fluid" },
convertToCompactSnapshot(snapshot),
200,
)) as unknown as Response;

await assert.doesNotReject(
async () =>
mockFetchMultiple(
async () => service.getSnapshot({}),
[
notFound,
async (): Promise<MockResponse> => okResponse({}, {}),
async (): Promise<Response> => {
return response;
},
],
),
"Should succeed",
);
assert(
mockLogger.matchEvents([
{ eventName: "TreesLatest_cancel", shareLinkPresent: true },
{ eventName: "RedeemShareLink_end" },
{ eventName: "RedeemFallback", errorType: "fileNotFoundOrAccessDeniedError" },
{ eventName: "TreesLatest_end" },
]),
);
});

it("RedeemFallback behavior when fallback succeeds with using siteUrl", async () => {
resolved.shareLinkInfo = {
sharingLinkToRedeem: "https://microsoft.sharepoint-df.com/sharelink",
};
hostPolicy.enableRedeemFallback = true;

const snapshot: ISnapshot = {
blobContents,
snapshotTree: snapshotTreeWithGroupId,
ops: [],
latestSequenceNumber: 0,
sequenceNumber: 0,
snapshotFormatV: 1,
};
const response = (await createResponse(
{ "x-fluid-epoch": "epoch1", "content-type": "application/ms-fluid" },
convertToCompactSnapshot(snapshot),
200,
)) as unknown as Response;

await assert.doesNotReject(
async () =>
mockFetchMultiple(
async () => service.getSnapshot({}),
[
notFound,
notFound,
async (): Promise<MockResponse> => okResponse({}, {}),
async (): Promise<Response> => {
return response;
},
],
),
"Should succeed",
);
assert(
mockLogger.matchEvents([
{ eventName: "TreesLatest_cancel", shareLinkPresent: true },
{ eventName: "ShareLinkRedeemFailedWithTenantDomain", statusCode: 404 },
{ eventName: "RedeemShareLink_end" },
{ eventName: "RedeemFallback", errorType: "fileNotFoundOrAccessDeniedError" },
{ eventName: "TreesLatest_end" },
]),
);
});
});

const snapshotTreeWithGroupId: ISnapshotTree = {
Expand Down

0 comments on commit dc3f29c

Please sign in to comment.