Skip to content

Commit

Permalink
Handle location redirection URL error in getSharingInfo call in odsp …
Browse files Browse the repository at this point in the history
…driver (#21277)

## Description

ADO: https://dev.azure.com/fluidframework/internal/_workitems/edit/8006

After a tenant changes their domain, this leads existing workspaces on
the Loop app to stop getting sharing link to the newly created pages.
This is because after a redirect, the authorization headers are not sent
by the browser. And even if were to just retry with the headers set,
SharePoint requires for the auth tokens to be generated for that
particular domain.

This PR handle causes the browser to not handle the redirect implicitly
by supplying the redirect:manual header in the request and handles the
redirect by itself. It will successfully, return the share link to the
app.

On the load of the same container, we already have
resolveWithLocationRedirectionHandling which handles the redirect during
load and also signals the app about the new domain.

Now, I am using the common redirect header instead of prefer header for
VROOM Apis as getSharingInfo is not a VROOM api. Redirect header works
with all APIs.

---------

Co-authored-by: Jatin Garg <jatingarg@Jatins-MacBook-Pro-2.local>
  • Loading branch information
jatgarg and Jatin Garg authored Jun 13, 2024
1 parent 15ff63b commit a584932
Show file tree
Hide file tree
Showing 4 changed files with 260 additions and 2 deletions.
64 changes: 62 additions & 2 deletions packages/drivers/odsp-driver/src/getFileLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,18 @@
import type { ITelemetryBaseProperties } from "@fluidframework/core-interfaces";
import { assert } from "@fluidframework/core-utils/internal";
import { NonRetryableError, runWithRetry } from "@fluidframework/driver-utils/internal";
import { hasRedirectionLocation } from "@fluidframework/odsp-doclib-utils/internal";
import {
IOdspUrlParts,
OdspErrorTypes,
OdspResourceTokenFetchOptions,
TokenFetcher,
} from "@fluidframework/odsp-driver-definitions/internal";
import { ITelemetryLoggerExt, PerformanceEvent } from "@fluidframework/telemetry-utils/internal";
import {
ITelemetryLoggerExt,
PerformanceEvent,
isFluidError,
} from "@fluidframework/telemetry-utils/internal";

import { getUrlAndHeadersWithAuth } from "./getUrlAndHeadersWithAuth.js";
import {
Expand Down Expand Up @@ -55,7 +60,12 @@ export async function getFileLink(
fileLinkCore = await runWithRetry(
async () =>
runWithRetryForCoherencyAndServiceReadOnlyErrors(
async () => getFileLinkCore(getToken, odspUrlParts, logger),
async () =>
getFileLinkWithLocationRedirectionHandling(
getToken,
odspUrlParts,
logger,
),
"getFileLinkCore",
logger,
),
Expand Down Expand Up @@ -95,6 +105,54 @@ export async function getFileLink(
return fileLink;
}

/**
* Handles location redirection while fulfilling the getFilelink call. We don't want browser to handle
* the redirection as the browser will retry with same auth token which will not work as we need app
* to regenerate tokens for the new site domain. So when we will make the network calls below we will set
* the redirect:manual header to manually handle these redirects.
* Refer: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308
* @param getToken - token fetcher to fetch the token.
* @param odspUrlParts - parts of odsp resolved url.
* @param logger - logger to send events.
* @returns Response from the API call.
* @alpha
*/
async function getFileLinkWithLocationRedirectionHandling(
getToken: TokenFetcher<OdspResourceTokenFetchOptions>,
odspUrlParts: IOdspUrlParts,
logger: ITelemetryLoggerExt,
): Promise<string> {
// We can have chains of location redirection one after the other, so have a for loop
// so that we can keep handling the same type of error. Set max number of redirection to 5.
let lastError: unknown;
for (let count = 1; count <= 5; count++) {
try {
return await getFileLinkCore(getToken, odspUrlParts, logger);
} catch (error: unknown) {
lastError = error;
if (
isFluidError(error) &&
error.errorType === OdspErrorTypes.fileNotFoundOrAccessDeniedError &&
hasRedirectionLocation(error) &&
error.redirectLocation !== undefined
) {
const redirectLocation = error.redirectLocation;
logger.sendTelemetryEvent({
eventName: "LocationRedirectionErrorForGetOdspFileLink",
retryCount: count,
});
// Generate the new SiteUrl from the redirection location.
const newSiteDomain = new URL(redirectLocation).origin;
const newSiteUrl = `${newSiteDomain}${new URL(odspUrlParts.siteUrl).pathname}`;
odspUrlParts.siteUrl = newSiteUrl;
continue;
}
throw error;
}
}
throw lastError;
}

async function getFileLinkCore(
getToken: TokenFetcher<OdspResourceTokenFetchOptions>,
odspUrlParts: IOdspUrlParts,
Expand Down Expand Up @@ -138,6 +196,7 @@ async function getFileLinkCore(
headers: {
"Content-Type": "application/json;odata=verbose",
"Accept": "application/json;odata=verbose",
"redirect": "manual",
...headers,
},
};
Expand Down Expand Up @@ -219,6 +278,7 @@ async function getFileItemLite(
storageToken,
forceAccessTokenViaAuthorizationHeader,
);
headers.redirect = "manual";
const requestInit = { method: "GET", headers };
const response = await fetchHelper(url, requestInit);
additionalProps = response.propsToLog;
Expand Down
169 changes: 169 additions & 0 deletions packages/drivers/odsp-driver/src/test/getFileLink.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {

describe("getFileLink", () => {
const siteUrl = "https://microsoft.sharepoint-df.com/siteUrl";
const newSiteUrl = "https://microsoft.sharepoint.com/siteUrl";
const driveId = "driveId";
const logger = new MockLogger();
const storageTokenFetcher = async (): Promise<string> => "StorageToken";
Expand Down Expand Up @@ -145,4 +146,172 @@ describe("getFileLink", () => {
"did not retries 5 times",
);
});

it("should handle location redirection once", async () => {
const result = await mockFetchMultiple(
async () =>
getFileLink(
storageTokenFetcher,
{ siteUrl, driveId, itemId: "itemId8" },
logger.toTelemetryLogger(),
),
[
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
async (): Promise<MockResponse> => okResponse({}, fileItemResponse),
async (): Promise<MockResponse> =>
okResponse({}, { d: { directUrl: "sharelink" } }),
],
);
assert.strictEqual(
result,
"sharelink",
"File link should match url returned from sharing information",
);
// Should be present in cache now and subsequent calls should fetch from cache.
const sharelink2 = await getFileLink(
storageTokenFetcher,
{ siteUrl, driveId, itemId: "itemId8" },
logger.toTelemetryLogger(),
);
assert.strictEqual(
sharelink2,
"sharelink",
"File link should match url returned from sharing information from cache",
);
});

it("should handle location redirection multiple times", async () => {
const result = await mockFetchMultiple(
async () =>
getFileLink(
storageTokenFetcher,
{ siteUrl, driveId, itemId: "itemId9" },
logger.toTelemetryLogger(),
),
[
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
302,
),
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
307,
),
async (): Promise<MockResponse> => okResponse({}, fileItemResponse),
async (): Promise<MockResponse> =>
okResponse({}, { d: { directUrl: "sharelink" } }),
],
);
assert.strictEqual(
result,
"sharelink",
"File link should match url returned from sharing information",
);
// Should be present in cache now and subsequent calls should fetch from cache.
const sharelink2 = await getFileLink(
storageTokenFetcher,
{ siteUrl, driveId, itemId: "itemId9" },
logger.toTelemetryLogger(),
);
assert.strictEqual(
sharelink2,
"sharelink",
"File link should match url returned from sharing information from cache",
);
});

it("should handle location redirection max 5 times", async () => {
await assert.rejects(
mockFetchMultiple(async () => {
return getFileLink(
storageTokenFetcher,
{ siteUrl, driveId, itemId: "itemId10" },
logger.toTelemetryLogger(),
);
}, [
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
async (): Promise<MockResponse> =>
createResponse(
{ Location: newSiteUrl },
{
error: {
message: "locationMoved",
},
},
308,
),
]),
"File link should reject when not found",
);
});
});
1 change: 1 addition & 0 deletions packages/utils/odsp-doclib-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export {
fetchIncorrectResponse,
getSPOAndGraphRequestIdsFromResponse,
hasFacetCodes,
hasRedirectionLocation,
OdspErrorResponse,
OdspErrorResponseInnerError,
OdspRedirectError,
Expand Down
28 changes: 28 additions & 0 deletions packages/utils/odsp-doclib-utils/src/odspErrorUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,20 @@ export function createOdspNetworkError(
const driverProps = { driverVersion, statusCode, ...props };

switch (statusCode) {
// The location of file can move on Spo. If the redirect:manual header is added to network call
// it causes browser to not handle the redirects. Location header in such cases will contain the new location.
// Refer: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/308
case 301:
case 302:
case 303:
case 307:
case 308:
redirectLocation = response?.headers.get("Location") ?? undefined;
if (redirectLocation !== undefined) {
error = new OdspRedirectError(errorMessage, redirectLocation, driverProps);
break;
}
// Don't break here. Let it be a generic network error if redirectLocation is not there.
case 400:
if (innerMostErrorCode === "fluidInvalidSchema") {
error = new FluidInvalidSchemaError(errorMessage, driverProps);
Expand Down Expand Up @@ -446,3 +460,17 @@ function numberFromHeader(header: string | null): number | undefined {
export function hasFacetCodes(x: any): x is Pick<IOdspErrorAugmentations, "facetCodes"> {
return Array.isArray(x?.facetCodes);
}

/**
* @internal
*/
export function hasRedirectionLocation(
x: unknown,
): x is Pick<IOdspErrorAugmentations, "redirectLocation"> {
return (
x !== null &&
typeof x === "object" &&
"redirectLocation" in x &&
typeof x?.redirectLocation === "string"
);
}

0 comments on commit a584932

Please sign in to comment.