Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Identity] Precise Typechecking #31870

Merged
merged 10 commits into from
Dec 4, 2024
14 changes: 9 additions & 5 deletions common/tools/dev-tool/src/commands/run/testVitest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ export const commandInfo = makeCommandInfo(
default: false,
description: "whether to use browser to run tests",
},
esm: {
kind: "boolean",
default: false,
description: "whether to use esm to run tests",
},
"relay-server": {
shortName: "rs",
description:
Expand Down Expand Up @@ -64,12 +69,11 @@ export default leafCommand(commandInfo, async (options) => {

let args = "";
// Only set if we didn't provide a config file path
if (
options["browser"] &&
updatedArgs?.indexOf("-c") === -1 &&
updatedArgs?.indexOf("--config") === -1
) {
const providedConfig = updatedArgs?.find((arg) => arg === "-c" || arg === "--config");
if (options["browser"] && !providedConfig) {
args = "-c vitest.browser.config.ts";
} else if (options["esm"] && !providedConfig) {
args = "-c vitest.esm.config.ts";
}

const vitestArgs = updatedArgs?.length ? updatedArgs.join(" ") : "";
Expand Down
16 changes: 16 additions & 0 deletions sdk/identity/identity/eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,20 @@ export default [
"@typescript-eslint/no-unused-vars": "off",
},
},
{
files: ["**/*.ts", "**/*.cts", "**/*.mts"],
languageOptions: {
parserOptions: {
project: ["./tsconfig.src.json", "./tsconfig.tests.json"],
},
},
},
{
files: ["*.md/*.ts"],
languageOptions: {
parserOptions: {
project: null,
},
},
},
];
5 changes: 3 additions & 2 deletions sdk/identity/identity/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"integration-test": "npm run integration-test:node && npm run integration-test:browser",
"integration-test:browser": "echo skipped",
"integration-test:managed-identity": "dev-tool run test:vitest -- --test-timeout 180000 --config ./vitest.managed-identity.config.ts",
"integration-test:node": "dev-tool run test:vitest -- --test-timeout 180000 'test/public/node/*.spec.ts' 'test/internal/node/*.spec.ts'",
"integration-test:node": "dev-tool run test:vitest --esm",
"lint": "eslint package.json api-extractor.json src test",
"lint:fix": "eslint package.json api-extractor.json src test --fix --fix-type [problem,suggestion]",
"pack": "npm pack 2>&1",
Expand All @@ -40,7 +40,7 @@
"unit-test": "npm run unit-test:node && npm run unit-test:browser",
"unit-test:browser": "npm run clean && dev-tool run build-package && dev-tool run build-test && dev-tool run test:vitest --browser",
"unit-test:node": "dev-tool run test:vitest",
"unit-test:node:no-timeouts": "dev-tool run test:node-ts-input -- --timeout Infinite --exclude 'test/snippets.spec.ts' --exclude 'test/**/browser/**/*.spec.ts' 'test/**/**/*.spec.ts'",
"unit-test:node:no-timeouts": "dev-tool run test:vitest -- --test-timeout=0",
"update-snippets": "dev-tool run update-snippets"
},
"files": [
Expand Down Expand Up @@ -119,6 +119,7 @@
},
"type": "module",
"tshy": {
"project": "./tsconfig.src.json",
"exports": {
"./package.json": "./package.json",
".": "./src/index.ts"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type { GetTokenOptions } from "@azure/core-auth";
import { credentialLogger } from "../../util/logging.js";
import { mapScopesToResource } from "./utils.js";
import { tracingClient } from "../../util/tracing.js";
import { IdentityClient } from "../../client/identityClient.js";
import type { IdentityClient } from "../../client/identityClient.js";

const msiName = "ManagedIdentityCredential - IMDS";
const logger = credentialLogger(msiName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ import { IdentityClient } from "../../client/identityClient.js";
import { AuthenticationRequiredError, CredentialUnavailableError } from "../../errors.js";
import { getMSALLogLevel, defaultLoggerCallback } from "../../msal/utils.js";
import { imdsRetryPolicy } from "./imdsRetryPolicy.js";
import { MSIConfiguration } from "./models.js";
import type { MSIConfiguration } from "./models.js";
import { formatSuccess, formatError, credentialLogger } from "../../util/logging.js";
import { tracingClient } from "../../util/tracing.js";
import { imdsMsi } from "./imdsMsi.js";
import { tokenExchangeMsi } from "./tokenExchangeMsi.js";
import { mapScopesToResource } from "./utils.js";
import { MsalToken, ValidMsalToken } from "../../msal/types.js";
import type { MsalToken, ValidMsalToken } from "../../msal/types.js";

const logger = credentialLogger("ManagedIdentityCredential");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const redirectHash = self.location.hash;
* @internal
*/
export class MSALAuthCode extends MsalBrowser {
protected app?: msalBrowser.IPublicClientApplication;
private loginHint?: string;

/**
Expand Down
2 changes: 2 additions & 0 deletions sdk/identity/identity/src/plugins/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ export interface CachePluginControl {
setPersistence(
persistenceFactory: (
options?: TokenCachePersistenceOptions,
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
) => Promise<import("@azure/msal-node").ICachePlugin>,
): void;
}

export interface NativeBrokerPluginControl {
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
setNativeBroker(nativeBroker: import("@azure/msal-node").INativeBrokerPlugin): void;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { AzureApplicationCredential } from "../../../src/credentials/azureApplic
import {
createDefaultHttpClient,
createHttpHeaders,
HttpClient,
type HttpClient,
RestError,
} from "@azure/core-rest-pipeline";
import { ManagedIdentityApplication } from "@azure/msal-node";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,8 +189,7 @@ describe("AzurePowerShellCredential", function () {
Type: "Bearer",
};

const stub = vi
.spyOn(processUtils, "execFile")
vi.spyOn(processUtils, "execFile")
.mockResolvedValueOnce("") // The first call checks that the command is available.
.mockResolvedValueOnce(JSON.stringify(tokenResponse));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import type { AccessToken, TokenCredential } from "../../../src/index.js";
import { ChainedTokenCredential } from "../../../src/index.js";
import { logger as chainedTokenCredentialLogger } from "../../../src/credentials/chainedTokenCredential.js";
import { describe, it, assert, expect, vi, beforeEach, afterEach } from "vitest";
import { describe, it, assert, expect, vi, afterEach } from "vitest";

class TestMockCredential implements TokenCredential {
constructor(public returnPromise: Promise<AccessToken | null>) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { ClientAssertionCredential } from "../../../src/index.js";
import { ConfidentialClientApplication } from "@azure/msal-node";
import { createJWTTokenFromCertificate } from "../../public/node/utils/utils.js";
import { env } from "@azure-tools/test-recorder";
import { describe, it, assert, expect, vi, beforeEach, afterEach, MockInstance } from "vitest";
import { describe, it, assert, expect, vi, beforeEach, afterEach, type MockInstance } from "vitest";

describe("ClientAssertionCredential (internal)", function () {
let cleanup: MsalTestCleanup;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { env } from "@azure-tools/test-recorder";

import { ClientCertificateCredential } from "../../../src/index.js";
import { parseCertificate } from "../../../src/credentials/clientCertificateCredential.js";
import { describe, it, assert, expect, vi, beforeEach, afterEach } from "vitest";
import { describe, it, assert, beforeEach, afterEach } from "vitest";

const ASSET_PATH = "assets";

Expand Down Expand Up @@ -100,7 +100,7 @@ describe("ClientCertificateCredential (internal)", function () {
);
});

it("throws when given a file that doesn't contain a PEM-formatted certificate", async function (ctx) {
it("throws when given a file that doesn't contain a PEM-formatted certificate", async function () {
const fullPath = path.resolve("./clientCertificateCredential.spec.ts");
const credential = new ClientCertificateCredential("tenant", "client", {
certificatePath: fullPath,
Expand All @@ -117,7 +117,7 @@ describe("ClientCertificateCredential (internal)", function () {
assert.deepEqual(error?.message, `ENOENT: no such file or directory, open '${fullPath}'`);
});

it("throws when given a certificate that isn't PEM-formatted", async function (ctx) {
it("throws when given a certificate that isn't PEM-formatted", async function () {
const credential = new ClientCertificateCredential("tenant", "client", {
certificate: "not-pem-formatted",
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { env, isLiveMode, isPlaybackMode } from "@azure-tools/test-recorder";
import { ClientSecretCredential } from "../../../src/index.js";
import { ConfidentialClientApplication } from "@azure/msal-node";
import type { GetTokenOptions } from "@azure/core-auth";
import { describe, it, assert, expect, vi, beforeEach, afterEach, MockInstance } from "vitest";
import { describe, it, assert, expect, vi, beforeEach, afterEach, type MockInstance } from "vitest";

describe("ClientSecretCredential (internal)", function () {
let cleanup: MsalTestCleanup;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { Recorder } from "@azure-tools/test-recorder";
import { env, isLiveMode } from "@azure-tools/test-recorder";
import { DeviceCodeCredential } from "../../../src/index.js";
import { PublicClientApplication } from "@azure/msal-node";
import { describe, it, expect, vi, beforeEach, afterEach, MockInstance } from "vitest";
import { describe, it, expect, vi, beforeEach, afterEach, type MockInstance } from "vitest";

describe("DeviceCodeCredential (internal)", function () {
let cleanup: MsalTestCleanup;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
declare global {
namespace NodeJS {
interface Global {
// eslint-disable-next-line @typescript-eslint/consistent-type-imports
URL: typeof import("url").URL;
}
}
Expand Down Expand Up @@ -50,7 +51,7 @@ describe("InteractiveBrowserCredential (internal)", function () {

const scope = "https://vault.azure.net/.default";

it("Throws an expected error if no browser is available", async function (ctx) {
it("Throws an expected error if no browser is available", async function () {
const credential = new InteractiveBrowserCredential(
recorder.configureClientOptions({
redirectUri: "http://localhost:8081",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import { imdsMsi } from "../../../../src/credentials/managedIdentityCredential/i
import { RestError } from "@azure/core-rest-pipeline";
import { AuthenticationRequiredError, CredentialUnavailableError } from "../../../../src/errors.js";
import type { AccessToken, GetTokenOptions } from "@azure/core-auth";
import { describe, it, assert, expect, vi, beforeEach, afterEach, MockInstance } from "vitest";
import { IdentityClient } from "../../../../src/client/identityClient.js";
import { describe, it, assert, expect, vi, beforeEach, afterEach, type MockInstance } from "vitest";
import type { IdentityClient } from "../../../../src/client/identityClient.js";

describe("ManagedIdentityCredential (MSAL)", function () {
let acquireTokenStub: MockInstance<
Expand Down
6 changes: 3 additions & 3 deletions sdk/identity/identity/test/internal/node/msalClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ describe("MsalClient", function () {
});
});

it("supports cancellation", async function (ctx) {
it("supports cancellation", async function () {
const client = msalClient.createMsalClient(clientId, tenantId);

const scopes = ["https://vault.azure.net/.default"];
Expand All @@ -465,7 +465,7 @@ describe("MsalClient", function () {
});

describe("cross-tenant federation", function () {
it("allows passing an authority host", async function (ctx) {
it("allows passing an authority host", async function () {
const tenantIdOne = "tenantOne";
const tenantIdTwo = "tenantTwo";
const authorityHost = "https://custom.authority.com";
Expand All @@ -490,7 +490,7 @@ describe("MsalClient", function () {
assert.equal(requestAuthority, expectedAuthority);
});

it("allows using the AZURE_AUTHORITY_HOST environment variable", async function (ctx) {
it("allows using the AZURE_AUTHORITY_HOST environment variable", async function () {
const tenantIdOne = "tenantOne";
const tenantIdTwo = "tenantTwo";
const authorityHost = "https://custom.authority.com";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from "../../../src/msal/nodeFlows/msalPlugins.js";

import type { MsalClientOptions } from "../../../src/msal/nodeFlows/msalClient.js";
import { describe, it, assert, expect, vi, beforeEach, afterEach } from "vitest";
import { describe, it, assert, vi, beforeEach, afterEach } from "vitest";

describe("#generatePluginConfiguration", function () {
let options: MsalClientOptions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { isPlaybackMode } from "@azure-tools/test-recorder";
import { PublicClientApplication } from "@azure/msal-node";
import { UsernamePasswordCredential } from "../../../src/index.js";
import { getUsernamePasswordStaticResources } from "../../msalTestUtils.js";
import { describe, it, assert, expect, vi, beforeEach, afterEach, MockInstance } from "vitest";
import { describe, it, assert, expect, vi, beforeEach, afterEach, type MockInstance } from "vitest";

describe("UsernamePasswordCredential (internal)", function () {
let cleanup: MsalTestCleanup;
Expand Down Expand Up @@ -58,7 +58,7 @@ describe("UsernamePasswordCredential (internal)", function () {
);
});

it("Authenticates silently after the initial request", async function (ctx) {
it("Authenticates silently after the initial request", async function () {
const { clientId, password, tenantId, username } = getUsernamePasswordStaticResources();
const credential = new UsernamePasswordCredential(
tenantId,
Expand All @@ -82,7 +82,7 @@ describe("UsernamePasswordCredential (internal)", function () {
).toHaveBeenCalledOnce();
});

it("Authenticates with tenantId on getToken", async function (ctx) {
it("Authenticates with tenantId on getToken", async function () {
const { clientId, password, tenantId, username } = getUsernamePasswordStaticResources();
const credential = new UsernamePasswordCredential(
tenantId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe("WorkloadIdentityCredential", function () {
await cleanup();
});

it("authenticates with WorkloadIdentity Credential", async function (ctx) {
it("authenticates with WorkloadIdentity Credential", async function () {
const credential = new WorkloadIdentityCredential({
tenantId,
clientId,
Expand All @@ -67,7 +67,7 @@ describe("WorkloadIdentityCredential", function () {
});
});

it("authenticates with ManagedIdentity Credential", async function (ctx) {
it("authenticates with ManagedIdentity Credential", async function () {
vi.stubEnv("AZURE_FEDERATED_TOKEN_FILE", tokenFilePath);
vi.stubEnv("AZURE_CLIENT_ID", clientId);
vi.stubEnv("AZURE_TENANT_ID", tenantId);
Expand All @@ -77,7 +77,7 @@ describe("WorkloadIdentityCredential", function () {
assert.ok(token?.expiresOnTimestamp! > Date.now());
});

it("authenticates with DefaultAzure Credential", async function (ctx) {
it("authenticates with DefaultAzure Credential", async function () {
const credential = new DefaultAzureCredential();
try {
const { token, successfulCredential } = await credential["getTokenInternal"](scope);
Expand All @@ -98,7 +98,7 @@ describe("WorkloadIdentityCredential", function () {
vi.restoreAllMocks();
}
});
it("authenticates with DefaultAzure Credential and client ID", async function (ctx) {
it("authenticates with DefaultAzure Credential and client ID", async function () {
const credential = new DefaultAzureCredential({
managedIdentityClientId: "managedIdentityClientId",
workloadIdentityClientId: "workloadIdentityClientId",
Expand Down
2 changes: 1 addition & 1 deletion sdk/identity/identity/test/node/msalNodeTestSetup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import type { AuthenticationResult } from "@azure/msal-node";
import { ConfidentialClientApplication, PublicClientApplication } from "@azure/msal-node";
import { PlaybackTenantId } from "../msalTestUtils.js";
import { Recorder, VitestTestContext } from "@azure-tools/test-recorder";
import { Recorder, type VitestTestContext } from "@azure-tools/test-recorder";
import { vi } from "vitest";

export type MsalTestCleanup = () => Promise<void>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { msalNodeTestSetup } from "../../node/msalNodeTestSetup.js";
import type { Recorder } from "@azure-tools/test-recorder";
import { env } from "@azure-tools/test-recorder";
import { ClientSecretCredential } from "../../../src/index.js";
import { describe, it, assert, expect, vi, beforeEach, afterEach } from "vitest";
import { describe, it, assert, beforeEach, afterEach } from "vitest";

describe("AuthorityValidation", function () {
let cleanup: MsalTestCleanup;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import { AzurePipelinesCredential } from "../../../src/index.js";
import { isLiveMode } from "@azure-tools/test-recorder";
import { describe, it, assert, expect, vi, beforeEach, afterEach } from "vitest";
import { describe, it, assert, expect } from "vitest";

describe("AzurePipelinesCredential", function () {
const scope = "https://vault.azure.net/.default";
Expand Down
6 changes: 3 additions & 3 deletions sdk/identity/identity/test/public/node/caeARM.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
import { DeveloperSignOnClientId } from "../../../src/constants.js";
import { IdentityClient } from "../../../src/client/identityClient.js";
import { authorizeRequestOnClaimChallenge } from "@azure/core-client";
import { describe, it, assert, expect, vi, beforeEach, afterEach } from "vitest";
import { describe, it, assert, beforeEach, afterEach } from "vitest";

/**
* Sequence of events needed to test the CAE challenges on the Graph endpoint.
Expand Down Expand Up @@ -145,7 +145,7 @@ describe.skip("CAE", function () {
await cleanup();
});

it("DeviceCodeCredential", async function (ctx) {
it("DeviceCodeCredential", async function () {
const [firstAccessToken, finalAccessToken] = await challengeFlow(
new DeviceCodeCredential(recorder.configureClientOptions({ tenantId: env.AZURE_TENANT_ID })),
recorder,
Expand All @@ -154,7 +154,7 @@ describe.skip("CAE", function () {
assert.notDeepEqual(firstAccessToken, finalAccessToken);
});

it("UsernamePasswordCredential", async function (ctx) {
it("UsernamePasswordCredential", async function () {
// Important: Recording this test may only work in certain tenants.

const [firstAccessToken, finalAccessToken] = await challengeFlow(
Expand Down
Loading
Loading