Skip to content

Commit

Permalink
refactor(odsp-client): Upgrade eslint config from minimal to `recom…
Browse files Browse the repository at this point in the history
…mended` and fix violations (microsoft#19271)

Now that we've published a beta version of this package, it seems the
right time to tighten up eslint rules in this package. This PR upgrades
the config from "minimal" to "recommended" and fixes the resulting
violations.
  • Loading branch information
Josmithr authored Jan 19, 2024
1 parent b9cdc86 commit 38dfecc
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 27 deletions.
5 changes: 4 additions & 1 deletion packages/drivers/odsp-driver-definitions/src/tokenFetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ export interface TokenResponse {
/** Token value */
token: string;

/** Flag indicating whether token was obtained from local cache */
/**
* Whether or not the token was obtained from local cache.
* @remarks `undefined` indicates that it could not be determined whether or not the token was obtained this way.
*/
fromCache?: boolean;
}

Expand Down
11 changes: 10 additions & 1 deletion packages/service-clients/odsp-client/.eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

module.exports = {
extends: [require.resolve("@fluidframework/eslint-config-fluid/minimal"), "prettier"],
extends: [require.resolve("@fluidframework/eslint-config-fluid"), "prettier"],
parserOptions: {
project: ["./tsconfig.json", "./src/test/tsconfig.json"],
},
Expand All @@ -14,4 +14,13 @@ module.exports = {
// This library is used in the browser, so we don't want dependencies on most node libraries.
"import/no-nodejs-modules": ["error", { allow: ["events"] }],
},
overrides: [
{
files: ["src/test/**"],
rules: {
// It's fine for tests to use Node.js modules
"import/no-nodejs-modules": "off",
},
},
],
};
1 change: 1 addition & 0 deletions packages/service-clients/odsp-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@
"@microsoft/api-extractor": "^7.38.3",
"@types/mocha": "^9.1.1",
"@types/node": "^18.19.0",
"@types/uuid": "^9.0.2",
"c8": "^8.0.1",
"copyfiles": "^2.4.1",
"cross-env": "^7.0.3",
Expand Down
44 changes: 25 additions & 19 deletions packages/service-clients/odsp-client/src/odspClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,29 @@ import {
OdspContainerAttachProps,
} from "./interfaces";
import { createOdspAudienceMember } from "./odspAudience";
import { type IOdspTokenProvider } from "./token";

async function getStorageToken(
options: OdspResourceTokenFetchOptions,
tokenProvider: IOdspTokenProvider,
): Promise<TokenResponse> {
const tokenResponse: TokenResponse = await tokenProvider.fetchStorageToken(
options.siteUrl,
options.refresh,
);
return tokenResponse;
}

async function getWebsocketToken(
options: OdspResourceTokenFetchOptions,
tokenProvider: IOdspTokenProvider,
): Promise<TokenResponse> {
const tokenResponse: TokenResponse = await tokenProvider.fetchWebsocketToken(
options.siteUrl,
options.refresh,
);
return tokenResponse;
}

/**
* OdspClient provides the ability to have a Fluid object backed by the ODSP service within the context of Microsoft 365 (M365) tenants.
Expand All @@ -51,26 +74,9 @@ export class OdspClient {
private readonly urlResolver: OdspDriverUrlResolver;

public constructor(private readonly properties: OdspClientProps) {
const getStorageToken = async (options: OdspResourceTokenFetchOptions) => {
const tokenResponse: TokenResponse =
await this.properties.connection.tokenProvider.fetchStorageToken(
options.siteUrl,
options.refresh,
);
return tokenResponse;
};

const getWebsocketToken = async (options: OdspResourceTokenFetchOptions) => {
const tokenResponse: TokenResponse =
await this.properties.connection.tokenProvider.fetchWebsocketToken(
options.siteUrl,
options.refresh,
);
return tokenResponse;
};
this.documentServiceFactory = new OdspDocumentServiceFactory(
getStorageToken,
getWebsocketToken,
async (options) => getStorageToken(options, this.properties.connection.tokenProvider),
async (options) => getWebsocketToken(options, this.properties.connection.tokenProvider),
);

this.urlResolver = new OdspDriverUrlResolver();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
* Licensed under the MIT License.
*/

// eslint-disable-next-line import/no-nodejs-modules
import { strict as assert } from "assert";
import { strict as assert } from "node:assert";
import { type ContainerSchema } from "@fluidframework/fluid-static";
import { SharedMap } from "@fluidframework/map";
import { AttachState } from "@fluidframework/container-definitions";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Licensed under the MIT License.
*/

import { assert } from "@fluidframework/core-utils";
import { TokenResponse } from "@fluidframework/odsp-driver-definitions";
import {
IClientConfig,
Expand All @@ -27,7 +28,7 @@ export class OdspTestTokenProvider implements IOdspTokenProvider {
const pushScope = "offline_access https://pushchannel.1drv.ms/PushChannel.ReadWrite.All";
const tokens = await this.fetchTokens(siteUrl, pushScope);
return {
fromCache: true,
fromCache: false,
token: tokens.accessToken,
};
}
Expand All @@ -36,12 +37,18 @@ export class OdspTestTokenProvider implements IOdspTokenProvider {
const sharePointScopes = `${siteUrl}/Container.Selected`;
const tokens = await this.fetchTokens(siteUrl, sharePointScopes);
return {
fromCache: true,
fromCache: false,
token: tokens.accessToken,
};
}

private async fetchTokens(siteUrl: string, scope: string) {
private async fetchTokens(
siteUrl: string,
scope: string,
): Promise<{
accessToken: string;
refreshToken?: string;
}> {
const server = new URL(siteUrl).host;
const clientConfig: IClientConfig = {
clientId: this.creds.clientId,
Expand All @@ -60,9 +67,19 @@ export class OdspTestTokenProvider implements IOdspTokenProvider {
};
const response = await unauthPostAsync(getFetchTokenUrl(server), new URLSearchParams(body));

const parsedResponse = await response.json();
const parsedResponse = (await response.json()) as Record<string, unknown>;

const accessToken = parsedResponse.access_token;
assert(accessToken !== undefined, 'Response did not include "access_token".');
assert(typeof accessToken === "string", '"access_token" was malformed. Expected a string.');

const refreshToken = parsedResponse.refresh_token;
if (refreshToken !== undefined) {
assert(
typeof refreshToken === "string",
'"refreshToken" was malformed. Expected a string.',
);
}

return { accessToken, refreshToken };
}
Expand Down
2 changes: 2 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 38dfecc

Please sign in to comment.