Skip to content

Commit

Permalink
Change shares api path to match other use cases (#23434)
Browse files Browse the repository at this point in the history
## Description

odsp-driver calls '/shares' API to redeem access to file referenced by
redeemable sharing link. I am changing API path to include '/driveItem'
in it. Technically it is not required for executing redeem operation.
However, we have other cases that use '/shares' API and do require to
specify '/driveItem' in order to get specific drive item properties. The
reason this matter is when caller of this API must possess logical
permissions to call this API (for instance, this will be the case when
call is made with app-only token) then two separate logical permissions
are needed for the '/shares' call with and without '/driveItem'.

## Reviewer Guidance

The review process is outlined on [this wiki
page](https://github.com/microsoft/FluidFramework/wiki/PR-Guidelines#guidelines).

---------

Co-authored-by: Andrei Zenkovitch <andreize@microsoft.com>
  • Loading branch information
AndreiZe and Andrei Zenkovitch authored Jan 8, 2025
1 parent fdd88e4 commit 352834c
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 11 deletions.
15 changes: 7 additions & 8 deletions packages/drivers/odsp-driver/src/fetchSnapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,13 +150,8 @@ export async function fetchSnapshotWithRedeem(
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
if (enableRedeemFallback && isRedeemSharingLinkError(odspResolvedUrl, error)) {
// Execute the redeem fallback
await redeemSharingLink(odspResolvedUrl, storageTokenFetcher, logger);

await redeemSharingLink(
odspResolvedUrl,
storageTokenFetcher,
logger,
forceAccessTokenViaAuthorizationHeader,
);
const odspResolvedUrlWithoutShareLink: IOdspResolvedUrl = {
...odspResolvedUrl,
shareLinkInfo: {
Expand Down Expand Up @@ -213,7 +208,6 @@ async function redeemSharingLink(
odspResolvedUrl: IOdspResolvedUrl,
getAuthHeader: InstrumentedStorageTokenFetcher,
logger: ITelemetryLoggerExt,
forceAccessTokenViaAuthorizationHeader: boolean,
): Promise<void> {
await PerformanceEvent.timedExecAsync(
logger,
Expand All @@ -233,7 +227,12 @@ async function redeemSharingLink(
let redeemUrl: string | undefined;
async function callSharesAPI(baseUrl: string): Promise<void> {
await getWithRetryForTokenRefresh(async (tokenFetchOptions) => {
redeemUrl = `${baseUrl}/_api/v2.0/shares/${encodedShareUrl}`;
// IMPORTANT: Note that redeemUrl has '/driveItem' in it. Technically it is not required for executing redeem operation.
// However, we have other cases that use '/shares' API and do require to specify '/driveItem' in order to get specific
// drive item properties. The reason this matters is when caller of this API must possess logical permissions to call
// this API (for instance, this will be the case when call is made with app-only token) then two separate logical
// permissions are needed for the '/shares' call with and without '/driveItem'.
redeemUrl = `${baseUrl}/_api/v2.0/shares/${encodedShareUrl}/driveItem`;
const url = redeemUrl;
const method = "GET";
const authHeader = await getAuthHeader(
Expand Down
5 changes: 2 additions & 3 deletions packages/drivers/odsp-driver/src/getFileLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export async function getFileLink(
}

/**
* Handles location redirection while fulfilling the getFilelink call. We don't want browser to handle
* 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.
Expand All @@ -125,7 +125,7 @@ async function getFileLinkWithLocationRedirectionHandling(
let locationRedirected = false;
for (let count = 1; count <= 5; count++) {
try {
const fileItem = await getFileItemLite(getToken, resolvedUrl, logger, true);
const fileItem = await getFileItemLite(getToken, resolvedUrl, logger);
// Sometimes the siteUrl in the actual file is different from the siteUrl in the resolvedUrl due to location
// redirection. This creates issues in the getSharingInformation call. So we need to update the siteUrl in the
// resolvedUrl to the siteUrl in the fileItem which is the updated siteUrl.
Expand Down Expand Up @@ -260,7 +260,6 @@ async function getFileItemLite(
getToken: TokenFetcher<OdspResourceTokenFetchOptions>,
odspUrlParts: IOdspUrlParts,
logger: ITelemetryLoggerExt,
forceAccessTokenViaAuthorizationHeader: boolean,
): Promise<FileItemLite> {
return PerformanceEvent.timedExecAsync(
logger,
Expand Down

0 comments on commit 352834c

Please sign in to comment.