Skip to content

Commit

Permalink
[identity] Fix MSAL Flow additionallyAllowedTenantIds to pass through (
Browse files Browse the repository at this point in the history
  • Loading branch information
mpodwysocki authored Nov 4, 2022
1 parent 5c6bff2 commit 5a6c70c
Show file tree
Hide file tree
Showing 8 changed files with 25 additions and 12 deletions.
18 changes: 14 additions & 4 deletions sdk/identity/identity/src/msal/nodeFlows/msalNodeCommon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,29 @@ import {
publicToMsal,
} from "../utils";
import { MsalFlow, MsalFlowOptions } from "../flows";
import {
processMultiTenantRequest,
resolveAddionallyAllowedTenantIds,
resolveTenantId,
} from "../../util/tenantIdUtils";
import { AbortSignalLike } from "@azure/abort-controller";
import { AuthenticationRecord } from "../types";
import { AuthenticationRequiredError } from "../../errors";
import { CredentialFlowGetTokenOptions } from "../credentials";
import { DeveloperSignOnClientId } from "../../constants";
import { IdentityClient } from "../../client/identityClient";
import { LogPolicyOptions } from "@azure/core-rest-pipeline";
import { MultiTenantTokenCredentialOptions } from "../../credentials/multiTenantTokenCredentialOptions";
import { RegionalAuthority } from "../../regionalAuthority";
import { TokenCachePersistenceOptions } from "./tokenCachePersistenceOptions";
import { TokenCredentialOptions } from "../../tokenCredentialOptions";
import { processMultiTenantRequest, resolveTenantId } from "../../util/tenantIdUtils";

/**
* Union of the constructor parameters that all MSAL flow types for Node.
* @internal
*/
export interface MsalNodeOptions extends MsalFlowOptions {
tokenCachePersistenceOptions?: TokenCachePersistenceOptions;
tokenCredentialOptions: TokenCredentialOptions;
tokenCredentialOptions: MultiTenantTokenCredentialOptions;
/**
* Specifies a regional authority. Please refer to the {@link RegionalAuthority} type for the accepted values.
* If {@link RegionalAuthority.AutoDiscoverRegion} is specified, we will try to discover the regional authority endpoint.
Expand Down Expand Up @@ -79,6 +83,7 @@ export abstract class MsalNode extends MsalBaseUtilities implements MsalFlow {
protected msalConfig: msalNode.Configuration;
protected clientId: string;
protected tenantId: string;
private additionallyAllowedTenantIds: string[];
protected authorityHost?: string;
protected identityClient?: IdentityClient;
protected requiresConfidential: boolean = false;
Expand All @@ -97,6 +102,9 @@ export abstract class MsalNode extends MsalBaseUtilities implements MsalFlow {
super(options);
this.msalConfig = this.defaultNodeMsalConfig(options);
this.tenantId = resolveTenantId(options.logger, options.tenantId, options.clientId);
this.additionallyAllowedTenantIds = resolveAddionallyAllowedTenantIds(
options?.tokenCredentialOptions?.additionallyAllowedTenants
);
this.clientId = this.msalConfig.auth.clientId;
if (options?.getAssertion) {
this.getAssertion = options.getAssertion;
Expand Down Expand Up @@ -303,7 +311,9 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
scopes: string[],
options: CredentialFlowGetTokenOptions = {}
): Promise<AccessToken> {
const tenantId = processMultiTenantRequest(this.tenantId, options) || this.tenantId;
const tenantId =
processMultiTenantRequest(this.tenantId, options, this.additionallyAllowedTenantIds) ||
this.tenantId;

options.authority = getAuthority(tenantId, this.authorityHost);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ export class MsalOpenBrowser extends MsalNode {
});
}

openPromise.then().catch((e) => {
openPromise.catch((e) => {
cleanup();
reject(e);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import { GetTokenOptions } from "@azure/core-auth";

function createConfigurationErrorMessage(tenantId: string) {
function createConfigurationErrorMessage(tenantId: string): string {
return `The current credential is not configured to acquire tokens for tenant ${tenantId}. To enable acquiring tokens for this tenant add it to the AdditionallyAllowedTenants on the credential options, or add "*" to AdditionallyAllowedTenants to allow acquiring tokens for any tenant.`;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

import { GetTokenOptions } from "@azure/core-auth";

function createConfigurationErrorMessage(tenantId: string) {
function createConfigurationErrorMessage(tenantId: string): string {
return `The current credential is not configured to acquire tokens for tenant ${tenantId}. To enable acquiring tokens for this tenant add it to the AdditionallyAllowedTenants on the credential options, or add "*" to AdditionallyAllowedTenants to allow acquiring tokens for any tenant.`;
}

Expand Down
2 changes: 1 addition & 1 deletion sdk/identity/identity/test/httpRequests.browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export class IdentityTestContext implements IdentityTestContextInterface {
}
}

private _trackRequest(url: RequestInfo, request?: RequestInit) {
private _trackRequest(url: RequestInfo, request?: RequestInit): void {
const headers = new Headers(request?.headers);
const rawHeaders: Record<string, string> = {};

Expand Down
5 changes: 4 additions & 1 deletion sdk/identity/identity/test/httpRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,10 @@ export class IdentityTestContext implements IdentityTestContextInterface {
const totalOptions: http.RequestOptions[] = [];

try {
const fakeRequest = (options: string | URL | http.RequestOptions, resolve: any) => {
const fakeRequest = (
options: string | URL | http.RequestOptions,
resolve: any
): http.ClientRequest => {
totalOptions.push(options as http.RequestOptions);

if (!responses.length) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ async function createJWTTokenFromCertificate(
authorityHost: string,
clientId: string,
certificatePath: string
) {
): Promise<string> {
const privateKeyPemCert = fs.readFileSync(certificatePath);
const audience = `${authorityHost}/v2.0`;
const secureContext = tls.createSecureContext({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ describe("OnBehalfOfCredential", function () {
userAssertionToken: "user-assertion",
});

const newMSALClientLogs = () =>
const newMSALClientLogs = (): number =>
testContext.logMessages.filter((message) =>
message.match("Initialized MSAL's On-Behalf-Of flow")
).length;
Expand Down Expand Up @@ -62,7 +62,7 @@ describe("OnBehalfOfCredential", function () {
userAssertionToken: "user-assertion",
});

const newMSALClientLogs = () =>
const newMSALClientLogs = (): number =>
testContext.logMessages.filter((message) =>
message.match("Initialized MSAL's On-Behalf-Of flow")
).length;
Expand Down

0 comments on commit 5a6c70c

Please sign in to comment.