From b267aff3ce5e321aeb4a7bf2a431ebe7963e25c3 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Fri, 18 Jun 2021 01:56:54 +0000 Subject: [PATCH 01/19] [Identity] Draft for the support for tenant Id Challenges / tenant discovery in ClientCredentials --- sdk/core/core-auth/review/core-auth.api.md | 1 + sdk/core/core-auth/src/tokenCredential.ts | 5 +++ sdk/identity/identity/review/identity.api.md | 22 ++++++++-- .../authorizationCodeCredential.browser.ts | 3 +- .../authorizationCodeCredential.ts | 12 ++++-- .../authorizationCodeCredentialOptions.ts | 15 +++++++ .../clientCertificateCredentialOptions.ts | 4 ++ .../clientSecretCredentialOptions.ts | 7 +++- .../managedIdentityCredential/index.ts | 41 ++++++++++++++++--- .../managedIdentityCredential/utils.ts | 16 ++++++++ sdk/identity/identity/src/index.ts | 6 ++- .../identity/src/msal/nodeFlows/nodeCommon.ts | 8 ++++ 12 files changed, 124 insertions(+), 16 deletions(-) create mode 100644 sdk/identity/identity/src/credentials/authorizationCodeCredentialOptions.ts diff --git a/sdk/core/core-auth/review/core-auth.api.md b/sdk/core/core-auth/review/core-auth.api.md index b9a56357328e..1512a1301a94 100644 --- a/sdk/core/core-auth/review/core-auth.api.md +++ b/sdk/core/core-auth/review/core-auth.api.md @@ -47,6 +47,7 @@ export interface GetTokenOptions { requestOptions?: { timeout?: number; }; + tenantId?: string; tracingOptions?: { spanOptions?: SpanOptions; tracingContext?: Context; diff --git a/sdk/core/core-auth/src/tokenCredential.ts b/sdk/core/core-auth/src/tokenCredential.ts index e0fbf5e66a5b..08ffdfc69be5 100644 --- a/sdk/core/core-auth/src/tokenCredential.ts +++ b/sdk/core/core-auth/src/tokenCredential.ts @@ -52,6 +52,11 @@ export interface GetTokenOptions { */ tracingContext?: Context; }; + + /** + * Allows specifying a tenantId. Useful to handle challenges that provide tenant Id hints. + */ + tenantId?: string; } /** diff --git a/sdk/identity/identity/review/identity.api.md b/sdk/identity/identity/review/identity.api.md index 4e92bcea784e..dc80a79b2d7d 100644 --- a/sdk/identity/identity/review/identity.api.md +++ b/sdk/identity/identity/review/identity.api.md @@ -51,11 +51,17 @@ export class AuthenticationRequiredError extends Error { // @public export class AuthorizationCodeCredential implements TokenCredential { - constructor(tenantId: string | "common", clientId: string, clientSecret: string, authorizationCode: string, redirectUri: string, options?: TokenCredentialOptions); - constructor(tenantId: string | "common", clientId: string, authorizationCode: string, redirectUri: string, options?: TokenCredentialOptions); + constructor(tenantId: string | "common", clientId: string, clientSecret: string, authorizationCode: string, redirectUri: string, options?: AuthorizationCodeCredentialOptions); + constructor(tenantId: string | "common", clientId: string, authorizationCode: string, redirectUri: string, options?: AuthorizationCodeCredentialOptions); getToken(scopes: string | string[], options?: GetTokenOptions): Promise; } +// @public +export interface AuthorizationCodeCredentialOptions extends TokenCredentialOptions { + allowMultiTenantAuthentication?: boolean; + tenantId?: string; +} + // @public export enum AzureAuthorityHosts { AzureChina = "https://login.chinacloudapi.cn", @@ -92,6 +98,7 @@ export class ClientCertificateCredential implements TokenCredential { // @public export interface ClientCertificateCredentialOptions extends TokenCredentialOptions { + allowMultiTenantAuthentication?: boolean; sendCertificateChain?: boolean; } @@ -103,6 +110,7 @@ export class ClientSecretCredential implements TokenCredential { // @public export interface ClientSecretCredentialOptions extends TokenCredentialOptions { + allowMultiTenantAuthentication?: boolean; } // @public @@ -205,11 +213,17 @@ export const logger: AzureLogger; // @public export class ManagedIdentityCredential implements TokenCredential { - constructor(clientId: string, options?: TokenCredentialOptions); - constructor(options?: TokenCredentialOptions); + constructor(clientId: string, options?: ManagedIdentityCredentialOptions); + constructor(options?: ManagedIdentityCredentialOptions); getToken(scopes: string | string[], options?: GetTokenOptions): Promise; } +// @public +export interface ManagedIdentityCredentialOptions extends TokenCredentialOptions { + allowMultiTenantAuthentication?: boolean; + tenantId?: string; +} + // @public export function serializeAuthenticationRecord(record: AuthenticationRecord): string; diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredential.browser.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredential.browser.ts index 44e9169b30b0..ba7a5e472fc3 100644 --- a/sdk/identity/identity/src/credentials/authorizationCodeCredential.browser.ts +++ b/sdk/identity/identity/src/credentials/authorizationCodeCredential.browser.ts @@ -4,6 +4,7 @@ import { TokenCredential, AccessToken } from "@azure/core-http"; import { TokenCredentialOptions } from "../client/identityClient"; import { credentialLogger, formatError } from "../util/logging"; +import { AuthorizationCodeCredentialOptions } from "./authorizationCodeCredentialOptions"; const BrowserNotSupportedError = new Error( "AuthorizationCodeCredential is not supported in the browser. InteractiveBrowserCredential is more appropriate for this use case." @@ -17,7 +18,7 @@ export class AuthorizationCodeCredential implements TokenCredential { clientSecret: string, authorizationCode: string, redirectUri: string, - options?: TokenCredentialOptions + options?: AuthorizationCodeCredentialOptions ); constructor( tenantId: string | "common", diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts index e976bfee7b30..77b03fab0b31 100644 --- a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts +++ b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts @@ -10,6 +10,8 @@ import { SpanStatusCode } from "@azure/core-tracing"; import { credentialLogger, formatSuccess, formatError } from "../util/logging"; import { getIdentityTokenEndpointSuffix } from "../util/identityTokenEndpoint"; import { checkTenantId } from "../util/checkTenantId"; +import { AuthorizationCodeCredentialOptions } from "./authorizationCodeCredentialOptions"; +import { validateMultiTenantRequest } from "./managedIdentityCredential/utils"; const logger = credentialLogger("AuthorizationCodeCredential"); @@ -28,6 +30,7 @@ export class AuthorizationCodeCredential implements TokenCredential { private authorizationCode: string; private redirectUri: string; private lastTokenResponse: TokenResponse | null = null; + private allowMultiTenantAuthentication: boolean = false; /** * Creates an instance of CodeFlowCredential with the details needed @@ -57,7 +60,7 @@ export class AuthorizationCodeCredential implements TokenCredential { clientSecret: string, authorizationCode: string, redirectUri: string, - options?: TokenCredentialOptions + options?: AuthorizationCodeCredentialOptions ); /** * Creates an instance of CodeFlowCredential with the details needed @@ -85,7 +88,7 @@ export class AuthorizationCodeCredential implements TokenCredential { clientId: string, authorizationCode: string, redirectUri: string, - options?: TokenCredentialOptions + options?: AuthorizationCodeCredentialOptions ); /** * @hidden @@ -97,12 +100,13 @@ export class AuthorizationCodeCredential implements TokenCredential { clientSecretOrAuthorizationCode: string, authorizationCodeOrRedirectUri: string, redirectUriOrOptions: string | TokenCredentialOptions | undefined, - options?: TokenCredentialOptions + options?: AuthorizationCodeCredentialOptions ) { checkTenantId(logger, tenantId); this.clientId = clientId; this.tenantId = tenantId; + this.allowMultiTenantAuthentication = Boolean(options?.allowMultiTenantAuthentication); if (typeof redirectUriOrOptions === "string") { // the clientId+clientSecret constructor @@ -135,6 +139,8 @@ export class AuthorizationCodeCredential implements TokenCredential { scopes: string | string[], options?: GetTokenOptions ): Promise { + validateMultiTenantRequest(this.allowMultiTenantAuthentication, this.tenantId, options); + const { span, updatedOptions } = createSpan("AuthorizationCodeCredential-getToken", options); try { let tokenResponse: TokenResponse | null = null; diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredentialOptions.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredentialOptions.ts new file mode 100644 index 000000000000..0047b784cd00 --- /dev/null +++ b/sdk/identity/identity/src/credentials/authorizationCodeCredentialOptions.ts @@ -0,0 +1,15 @@ +import { TokenCredentialOptions } from "../client/identityClient"; + +/** + * Options for the {@link AuthorizationCodeCredential} + */ +export interface AuthorizationCodeCredentialOptions extends TokenCredentialOptions { + /** + * Allows specifying a tenant ID + */ + tenantId?: string; + /** + * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. + */ + allowMultiTenantAuthentication?: boolean; +} diff --git a/sdk/identity/identity/src/credentials/clientCertificateCredentialOptions.ts b/sdk/identity/identity/src/credentials/clientCertificateCredentialOptions.ts index 41f02fea246c..7e2ff40d7dc9 100644 --- a/sdk/identity/identity/src/credentials/clientCertificateCredentialOptions.ts +++ b/sdk/identity/identity/src/credentials/clientCertificateCredentialOptions.ts @@ -12,4 +12,8 @@ export interface ClientCertificateCredentialOptions extends TokenCredentialOptio * Set this option to send base64 encoded public certificate in the client assertion header as an x5c claim */ sendCertificateChain?: boolean; + /** + * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. + */ + allowMultiTenantAuthentication?: boolean; } diff --git a/sdk/identity/identity/src/credentials/clientSecretCredentialOptions.ts b/sdk/identity/identity/src/credentials/clientSecretCredentialOptions.ts index 05c9f9542220..935f45425962 100644 --- a/sdk/identity/identity/src/credentials/clientSecretCredentialOptions.ts +++ b/sdk/identity/identity/src/credentials/clientSecretCredentialOptions.ts @@ -6,4 +6,9 @@ import { TokenCredentialOptions } from "../client/identityClient"; /** * Optional parameters for the {@link ClientSecretCredential} class. */ -export interface ClientSecretCredentialOptions extends TokenCredentialOptions {} +export interface ClientSecretCredentialOptions extends TokenCredentialOptions { + /** + * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. + */ + allowMultiTenantAuthentication?: boolean; +} diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts index a49723c57984..6b9826bd6783 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts @@ -7,7 +7,7 @@ import { createSpan } from "../../util/tracing"; import { AuthenticationError, CredentialUnavailableError } from "../../client/errors"; import { SpanStatusCode } from "@azure/core-tracing"; import { credentialLogger, formatSuccess, formatError } from "../../util/logging"; -import { mapScopesToResource } from "./utils"; +import { mapScopesToResource, validateMultiTenantRequest } from "./utils"; import { cloudShellMsi } from "./cloudShellMsi"; import { imdsMsi } from "./imdsMsi"; import { MSI } from "./models"; @@ -16,6 +16,20 @@ import { arcMsi } from "./arcMsi"; const logger = credentialLogger("ManagedIdentityCredential"); +/** + * Options for the {@link ManagedIdentityCredential} + */ +export interface ManagedIdentityCredentialOptions extends TokenCredentialOptions { + /** + * Allows specifying a tenant ID + */ + tenantId?: string; + /** + * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. + */ + allowMultiTenantAuthentication?: boolean; +} + /** * Attempts authentication using a managed identity that has been assigned * to the deployment environment. This authentication type works in Azure VMs, @@ -27,8 +41,10 @@ const logger = credentialLogger("ManagedIdentityCredential"); */ export class ManagedIdentityCredential implements TokenCredential { private identityClient: IdentityClient; - private clientId: string | undefined; private isEndpointUnavailable: boolean | null = null; + private clientId?: string; + private tenantId?: string; + private allowMultiTenantAuthentication: boolean = false; /** * Creates an instance of ManagedIdentityCredential with the client ID of a @@ -37,28 +53,39 @@ export class ManagedIdentityCredential implements TokenCredential { * @param clientId - The client ID of the user-assigned identity, or app registration (when working with AKS pod-identity). * @param options - Options for configuring the client which makes the access token request. */ - constructor(clientId: string, options?: TokenCredentialOptions); + constructor(clientId: string, options?: ManagedIdentityCredentialOptions); /** * Creates an instance of ManagedIdentityCredential * * @param options - Options for configuring the client which makes the access token request. */ - constructor(options?: TokenCredentialOptions); + constructor(options?: ManagedIdentityCredentialOptions); /** * @internal * @hidden */ constructor( - clientIdOrOptions: string | TokenCredentialOptions | undefined, - options?: TokenCredentialOptions + clientIdOrOptions: string | ManagedIdentityCredentialOptions | undefined, + options?: ManagedIdentityCredentialOptions ) { if (typeof clientIdOrOptions === "string") { // clientId, options constructor this.clientId = clientIdOrOptions; this.identityClient = new IdentityClient(options); + + if (options) { + this.tenantId = options.tenantId; + this.allowMultiTenantAuthentication = Boolean(options.allowMultiTenantAuthentication); + } } else { // options only constructor this.identityClient = new IdentityClient(clientIdOrOptions); + if (clientIdOrOptions) { + this.tenantId = clientIdOrOptions.tenantId; + this.allowMultiTenantAuthentication = Boolean( + clientIdOrOptions.allowMultiTenantAuthentication + ); + } } } @@ -128,6 +155,8 @@ export class ManagedIdentityCredential implements TokenCredential { scopes: string | string[], options?: GetTokenOptions ): Promise { + validateMultiTenantRequest(this.allowMultiTenantAuthentication, this.tenantId, options); + let result: AccessToken | null = null; const { span, updatedOptions } = createSpan("ManagedIdentityCredential-getToken", options); diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts index 6c54ec33a94b..268ee642bf6d 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts @@ -46,3 +46,19 @@ export async function msiGenericGetToken( return (tokenResponse && tokenResponse.accessToken) || null; } + +/** + * Verifies whether locally assigned tenants are equal to tenants received through getToken. + * @internal + */ +export function validateMultiTenantRequest( + allowMultiTenantAuthentication: boolean, + tenantId?: string, + getTokenOptions?: GetTokenOptions +) { + if (!allowMultiTenantAuthentication && getTokenOptions && getTokenOptions.tenantId !== tenantId) { + throw new Error( + "Multi-tenant authentication was attempted, but multi-tenant authentication was not enabled in this credential instance." + ); + } +} diff --git a/sdk/identity/identity/src/index.ts b/sdk/identity/identity/src/index.ts index ec431a1a5d42..a2dc936d9bf9 100644 --- a/sdk/identity/identity/src/index.ts +++ b/sdk/identity/identity/src/index.ts @@ -27,7 +27,10 @@ export { InteractiveBrowserCredentialBrowserOptions, BrowserLoginStyle } from "./credentials/interactiveBrowserCredentialOptions"; -export { ManagedIdentityCredential } from "./credentials/managedIdentityCredential"; +export { + ManagedIdentityCredential, + ManagedIdentityCredentialOptions +} from "./credentials/managedIdentityCredential"; export { DeviceCodeCredential } from "./credentials/deviceCodeCredential"; export { DeviceCodePromptCallback, @@ -37,6 +40,7 @@ export { DeviceCodeCredentialOptions } from "./credentials/deviceCodeCredentialO export { UsernamePasswordCredential } from "./credentials/usernamePasswordCredential"; export { UsernamePasswordCredentialOptions } from "./credentials/usernamePasswordCredentialOptions"; export { AuthorizationCodeCredential } from "./credentials/authorizationCodeCredential"; +export { AuthorizationCodeCredentialOptions } from "./credentials/authorizationCodeCredentialOptions"; export { AzurePowerShellCredential } from "./credentials/azurePowerShellCredential"; export { diff --git a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts index 62705e1b34a3..7b60689f64d0 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts @@ -20,6 +20,7 @@ import { msalToPublic, publicToMsal } from "../utils"; +import { validateMultiTenantRequest } from "../../credentials/managedIdentityCredential/utils"; /** * Union of the constructor parameters that all MSAL flow types for Node. @@ -27,6 +28,7 @@ import { */ export interface MsalNodeOptions extends MsalFlowOptions { tokenCredentialOptions: TokenCredentialOptions; + allowMultiTenantAuthentication?: Boolean; } /** @@ -43,12 +45,16 @@ export abstract class MsalNode extends MsalBaseUtilities implements MsalFlow { protected confidentialApp: msalNode.ConfidentialClientApplication | undefined; protected msalConfig: msalNode.Configuration; protected clientId: string; + protected tenantId: string; protected identityClient?: IdentityClient; protected requiresConfidential: boolean = false; + protected allowMultiTenantAuthentication: boolean = false; constructor(options: MsalNodeOptions) { super(options); + this.allowMultiTenantAuthentication = Boolean(options.allowMultiTenantAuthentication); this.msalConfig = this.defaultNodeMsalConfig(options); + this.tenantId = resolveTenantId(options.logger, options.tenantId, options.clientId); this.clientId = this.msalConfig.auth.clientId; } @@ -214,8 +220,10 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov scopes: string[], options: CredentialFlowGetTokenOptions = {} ): Promise { + validateMultiTenantRequest(this.allowMultiTenantAuthentication, this.tenantId, options); options.correlationId = options?.correlationId || this.generateUuid(); await this.init(options); + return this.getTokenSilent(scopes, options).catch((err) => { if (err.name !== "AuthenticationRequiredError") { throw err; From 3f1cbbfd84c2d5bfb5569571b8d45d76e3335fa6 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Fri, 18 Jun 2021 20:39:13 +0000 Subject: [PATCH 02/19] all the credentials now --- sdk/identity/identity/review/identity.api.md | 17 +++++++++++------ .../identity/src/client/identityClient.ts | 4 ++++ .../authorizationCodeCredentialOptions.ts | 14 ++++---------- .../src/credentials/azureCliCredential.ts | 16 ++++++++++++++++ .../credentials/azureCliCredentialOptions.ts | 18 ++++++++++++++++++ .../credentials/azurePowerShellCredential.ts | 16 ++++++++++++++++ .../azurePowerShellCredentialOptions.ts | 18 ++++++++++++++++++ .../clientCertificateCredentialOptions.ts | 4 ---- .../clientSecretCredentialOptions.ts | 7 +------ .../managedIdentityCredential/index.ts | 4 ---- .../managedIdentityCredential/utils.ts | 9 +++++++-- sdk/identity/identity/src/index.ts | 1 + .../identity/src/msal/nodeFlows/nodeCommon.ts | 2 +- 13 files changed, 97 insertions(+), 33 deletions(-) create mode 100644 sdk/identity/identity/src/credentials/azureCliCredentialOptions.ts create mode 100644 sdk/identity/identity/src/credentials/azurePowerShellCredentialOptions.ts diff --git a/sdk/identity/identity/review/identity.api.md b/sdk/identity/identity/review/identity.api.md index dc80a79b2d7d..b858fccde2b0 100644 --- a/sdk/identity/identity/review/identity.api.md +++ b/sdk/identity/identity/review/identity.api.md @@ -58,8 +58,6 @@ export class AuthorizationCodeCredential implements TokenCredential { // @public export interface AuthorizationCodeCredentialOptions extends TokenCredentialOptions { - allowMultiTenantAuthentication?: boolean; - tenantId?: string; } // @public @@ -72,13 +70,22 @@ export enum AzureAuthorityHosts { // @public export class AzureCliCredential implements TokenCredential { + constructor(options?: AzureCliCredentialOptions); getToken(scopes: string | string[], options?: GetTokenOptions): Promise; + } + +// @public +export interface AzureCliCredentialOptions extends TokenCredentialOptions { + allowMultiTenantAuthentication?: boolean; + tenantId?: string; } // @public export class AzurePowerShellCredential implements TokenCredential { + // Warning: (ae-forgotten-export) The symbol "AzurePowerShellCredentialOptions" needs to be exported by the entry point index.d.ts + constructor(options?: AzurePowerShellCredentialOptions); getToken(scopes: string | string[], options?: GetTokenOptions): Promise; -} + } // @public export type BrowserLoginStyle = "redirect" | "popup"; @@ -98,7 +105,6 @@ export class ClientCertificateCredential implements TokenCredential { // @public export interface ClientCertificateCredentialOptions extends TokenCredentialOptions { - allowMultiTenantAuthentication?: boolean; sendCertificateChain?: boolean; } @@ -110,7 +116,6 @@ export class ClientSecretCredential implements TokenCredential { // @public export interface ClientSecretCredentialOptions extends TokenCredentialOptions { - allowMultiTenantAuthentication?: boolean; } // @public @@ -220,7 +225,6 @@ export class ManagedIdentityCredential implements TokenCredential { // @public export interface ManagedIdentityCredentialOptions extends TokenCredentialOptions { - allowMultiTenantAuthentication?: boolean; tenantId?: string; } @@ -231,6 +235,7 @@ export { TokenCredential } // @public export interface TokenCredentialOptions extends PipelineOptions { + allowMultiTenantAuthentication?: boolean; authorityHost?: string; } diff --git a/sdk/identity/identity/src/client/identityClient.ts b/sdk/identity/identity/src/client/identityClient.ts index 063955a18d62..00a4012d337f 100644 --- a/sdk/identity/identity/src/client/identityClient.ts +++ b/sdk/identity/identity/src/client/identityClient.ts @@ -315,4 +315,8 @@ export interface TokenCredentialOptions extends PipelineOptions { * The default is "https://login.microsoftonline.com". */ authorityHost?: string; + /** + * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. + */ + allowMultiTenantAuthentication?: boolean; } diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredentialOptions.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredentialOptions.ts index 0047b784cd00..fea500baaac4 100644 --- a/sdk/identity/identity/src/credentials/authorizationCodeCredentialOptions.ts +++ b/sdk/identity/identity/src/credentials/authorizationCodeCredentialOptions.ts @@ -1,15 +1,9 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + import { TokenCredentialOptions } from "../client/identityClient"; /** * Options for the {@link AuthorizationCodeCredential} */ -export interface AuthorizationCodeCredentialOptions extends TokenCredentialOptions { - /** - * Allows specifying a tenant ID - */ - tenantId?: string; - /** - * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. - */ - allowMultiTenantAuthentication?: boolean; -} +export interface AuthorizationCodeCredentialOptions extends TokenCredentialOptions {} diff --git a/sdk/identity/identity/src/credentials/azureCliCredential.ts b/sdk/identity/identity/src/credentials/azureCliCredential.ts index 7fa8e9332f85..03fd76b2a27c 100644 --- a/sdk/identity/identity/src/credentials/azureCliCredential.ts +++ b/sdk/identity/identity/src/credentials/azureCliCredential.ts @@ -8,6 +8,8 @@ import { SpanStatusCode } from "@azure/core-tracing"; import { credentialLogger, formatSuccess, formatError } from "../util/logging"; import * as child_process from "child_process"; import { ensureValidScope, getScopeResource } from "../util/scopeUtils"; +import { AzureCliCredentialOptions } from "./azureCliCredentialOptions"; +import { validateMultiTenantRequest } from "./managedIdentityCredential/utils"; /** * Mockable reference to the CLI credential cliCredentialFunctions @@ -64,6 +66,19 @@ const logger = credentialLogger("AzureCliCredential"); * in via the 'az' tool using the command "az login" from the commandline. */ export class AzureCliCredential implements TokenCredential { + private tenantId?: string; + private allowMultiTenantAuthentication: boolean = false; + + /** + * Creates an instance of the {@link AzureCliCredential}. + * + * @param options - Options, to optionally allow multi-tenant requests. + */ + constructor(options?: AzureCliCredentialOptions) { + this.tenantId = options?.tenantId; + this.allowMultiTenantAuthentication = Boolean(options?.allowMultiTenantAuthentication); + } + /** * Authenticates with Azure Active Directory and returns an access token if * successful. If authentication cannot be performed at this time, this method may @@ -81,6 +96,7 @@ export class AzureCliCredential implements TokenCredential { const scope = typeof scopes === "string" ? scopes : scopes[0]; logger.getToken.info(`Using the scope ${scope}`); + validateMultiTenantRequest(this.allowMultiTenantAuthentication, this.tenantId, options); ensureValidScope(scope, logger); const resource = getScopeResource(scope); diff --git a/sdk/identity/identity/src/credentials/azureCliCredentialOptions.ts b/sdk/identity/identity/src/credentials/azureCliCredentialOptions.ts new file mode 100644 index 000000000000..891b9b6ed819 --- /dev/null +++ b/sdk/identity/identity/src/credentials/azureCliCredentialOptions.ts @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { TokenCredentialOptions } from "../client/identityClient"; + +/** + * Options for the {@link AzureCliCredential} + */ +export interface AzureCliCredentialOptions extends TokenCredentialOptions { + /** + * Allows specifying a tenant ID + */ + tenantId?: string; + /** + * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. + */ + allowMultiTenantAuthentication?: boolean; +} diff --git a/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts b/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts index eb1783bd3746..593a737f174f 100644 --- a/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts +++ b/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts @@ -7,6 +7,8 @@ import { credentialLogger, formatSuccess, formatError } from "../util/logging"; import { trace } from "../util/tracing"; import { ensureValidScope, getScopeResource } from "../util/scopeUtils"; import { processUtils } from "../util/processUtils"; +import { AzurePowerShellCredentialOptions } from "./azurePowerShellCredentialOptions"; +import { validateMultiTenantRequest } from "./managedIdentityCredential/utils"; const logger = credentialLogger("AzurePowerShellCredential"); @@ -91,6 +93,19 @@ if (isWindows) { * `Connect-AzAccount` from the command line. */ export class AzurePowerShellCredential implements TokenCredential { + private tenantId?: string; + private allowMultiTenantAuthentication: boolean = false; + + /** + * Creates an instance of the {@link AzurePowershellCredential}. + * + * @param options - Options, to optionally allow multi-tenant requests. + */ + constructor(options?: AzurePowerShellCredentialOptions) { + this.tenantId = options?.tenantId; + this.allowMultiTenantAuthentication = Boolean(options?.allowMultiTenantAuthentication); + } + /** * Gets the access token from Azure PowerShell * @param resource - The resource to use when getting the token @@ -150,6 +165,7 @@ export class AzurePowerShellCredential implements TokenCredential { logger.getToken.info(`Using the scope ${scope}`); + validateMultiTenantRequest(this.allowMultiTenantAuthentication, this.tenantId, options); ensureValidScope(scope, logger); const resource = getScopeResource(scope); diff --git a/sdk/identity/identity/src/credentials/azurePowerShellCredentialOptions.ts b/sdk/identity/identity/src/credentials/azurePowerShellCredentialOptions.ts new file mode 100644 index 000000000000..9c3ffbba6990 --- /dev/null +++ b/sdk/identity/identity/src/credentials/azurePowerShellCredentialOptions.ts @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { TokenCredentialOptions } from "../client/identityClient"; + +/** + * Options for the {@link AzurePowerShellCredential} + */ +export interface AzurePowerShellCredentialOptions extends TokenCredentialOptions { + /** + * Allows specifying a tenant ID + */ + tenantId?: string; + /** + * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. + */ + allowMultiTenantAuthentication?: boolean; +} diff --git a/sdk/identity/identity/src/credentials/clientCertificateCredentialOptions.ts b/sdk/identity/identity/src/credentials/clientCertificateCredentialOptions.ts index 7e2ff40d7dc9..41f02fea246c 100644 --- a/sdk/identity/identity/src/credentials/clientCertificateCredentialOptions.ts +++ b/sdk/identity/identity/src/credentials/clientCertificateCredentialOptions.ts @@ -12,8 +12,4 @@ export interface ClientCertificateCredentialOptions extends TokenCredentialOptio * Set this option to send base64 encoded public certificate in the client assertion header as an x5c claim */ sendCertificateChain?: boolean; - /** - * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. - */ - allowMultiTenantAuthentication?: boolean; } diff --git a/sdk/identity/identity/src/credentials/clientSecretCredentialOptions.ts b/sdk/identity/identity/src/credentials/clientSecretCredentialOptions.ts index 935f45425962..05c9f9542220 100644 --- a/sdk/identity/identity/src/credentials/clientSecretCredentialOptions.ts +++ b/sdk/identity/identity/src/credentials/clientSecretCredentialOptions.ts @@ -6,9 +6,4 @@ import { TokenCredentialOptions } from "../client/identityClient"; /** * Optional parameters for the {@link ClientSecretCredential} class. */ -export interface ClientSecretCredentialOptions extends TokenCredentialOptions { - /** - * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. - */ - allowMultiTenantAuthentication?: boolean; -} +export interface ClientSecretCredentialOptions extends TokenCredentialOptions {} diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts index 6b9826bd6783..9c0005ae31f7 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts @@ -24,10 +24,6 @@ export interface ManagedIdentityCredentialOptions extends TokenCredentialOptions * Allows specifying a tenant ID */ tenantId?: string; - /** - * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. - */ - allowMultiTenantAuthentication?: boolean; } /** diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts index 268ee642bf6d..193ad6a8ea1b 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts @@ -55,8 +55,13 @@ export function validateMultiTenantRequest( allowMultiTenantAuthentication: boolean, tenantId?: string, getTokenOptions?: GetTokenOptions -) { - if (!allowMultiTenantAuthentication && getTokenOptions && getTokenOptions.tenantId !== tenantId) { +): void { + if ( + !allowMultiTenantAuthentication && + getTokenOptions && + tenantId && + getTokenOptions.tenantId !== tenantId + ) { throw new Error( "Multi-tenant authentication was attempted, but multi-tenant authentication was not enabled in this credential instance." ); diff --git a/sdk/identity/identity/src/index.ts b/sdk/identity/identity/src/index.ts index a2dc936d9bf9..6f97abc246fe 100644 --- a/sdk/identity/identity/src/index.ts +++ b/sdk/identity/identity/src/index.ts @@ -21,6 +21,7 @@ export { ClientSecretCredentialOptions } from "./credentials/clientSecretCredent export { ClientCertificateCredential } from "./credentials/clientCertificateCredential"; export { ClientCertificateCredentialOptions } from "./credentials/clientCertificateCredentialOptions"; export { AzureCliCredential } from "./credentials/azureCliCredential"; +export { AzureCliCredentialOptions } from "./credentials/azureCliCredentialOptions"; export { InteractiveBrowserCredential } from "./credentials/interactiveBrowserCredential"; export { InteractiveBrowserCredentialOptions, diff --git a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts index 7b60689f64d0..560269696ed8 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts @@ -28,7 +28,7 @@ import { validateMultiTenantRequest } from "../../credentials/managedIdentityCre */ export interface MsalNodeOptions extends MsalFlowOptions { tokenCredentialOptions: TokenCredentialOptions; - allowMultiTenantAuthentication?: Boolean; + allowMultiTenantAuthentication?: boolean; } /** From b1f2e64f44273b9bd90cd8b5e44e6b4236984de7 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Fri, 18 Jun 2021 20:42:48 +0000 Subject: [PATCH 03/19] forgot about AzurePowerShellCredentialOptions --- sdk/identity/identity/review/identity.api.md | 7 ++++++- sdk/identity/identity/src/index.ts | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/sdk/identity/identity/review/identity.api.md b/sdk/identity/identity/review/identity.api.md index b858fccde2b0..b6c2a8aa8397 100644 --- a/sdk/identity/identity/review/identity.api.md +++ b/sdk/identity/identity/review/identity.api.md @@ -82,11 +82,16 @@ export interface AzureCliCredentialOptions extends TokenCredentialOptions { // @public export class AzurePowerShellCredential implements TokenCredential { - // Warning: (ae-forgotten-export) The symbol "AzurePowerShellCredentialOptions" needs to be exported by the entry point index.d.ts constructor(options?: AzurePowerShellCredentialOptions); getToken(scopes: string | string[], options?: GetTokenOptions): Promise; } +// @public +export interface AzurePowerShellCredentialOptions extends TokenCredentialOptions { + allowMultiTenantAuthentication?: boolean; + tenantId?: string; +} + // @public export type BrowserLoginStyle = "redirect" | "popup"; diff --git a/sdk/identity/identity/src/index.ts b/sdk/identity/identity/src/index.ts index 6f97abc246fe..0e985568cdd9 100644 --- a/sdk/identity/identity/src/index.ts +++ b/sdk/identity/identity/src/index.ts @@ -43,6 +43,7 @@ export { UsernamePasswordCredentialOptions } from "./credentials/usernamePasswor export { AuthorizationCodeCredential } from "./credentials/authorizationCodeCredential"; export { AuthorizationCodeCredentialOptions } from "./credentials/authorizationCodeCredentialOptions"; export { AzurePowerShellCredential } from "./credentials/azurePowerShellCredential"; +export { AzurePowerShellCredentialOptions } from "./credentials/azurePowerShellCredentialOptions"; export { AuthenticationError, From 334df0128dff6168ba6ba3483eba8ad790cd84c1 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Mon, 21 Jun 2021 19:20:00 +0000 Subject: [PATCH 04/19] a unit test --- .../managedIdentityCredential/utils.ts | 11 +++-- .../identity/test/internal/utils.spec.ts | 41 +++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 sdk/identity/identity/test/internal/utils.spec.ts diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts index 193ad6a8ea1b..b4889014038b 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts @@ -47,6 +47,13 @@ export async function msiGenericGetToken( return (tokenResponse && tokenResponse.accessToken) || null; } +/** + * @internal + */ +export const multiTenantError = new Error( + "Multi-tenant authentication was attempted, but multi-tenant authentication was not enabled in this credential instance." +); + /** * Verifies whether locally assigned tenants are equal to tenants received through getToken. * @internal @@ -62,8 +69,6 @@ export function validateMultiTenantRequest( tenantId && getTokenOptions.tenantId !== tenantId ) { - throw new Error( - "Multi-tenant authentication was attempted, but multi-tenant authentication was not enabled in this credential instance." - ); + throw multiTenantError; } } diff --git a/sdk/identity/identity/test/internal/utils.spec.ts b/sdk/identity/identity/test/internal/utils.spec.ts new file mode 100644 index 000000000000..a52116a16435 --- /dev/null +++ b/sdk/identity/identity/test/internal/utils.spec.ts @@ -0,0 +1,41 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import assert from "assert"; +import { + multiTenantError, + validateMultiTenantRequest +} from "../../src/credentials/managedIdentityCredential/utils"; + +describe("Identity utilities", function() { + describe("validateMultiTenantRequest", function() { + it("throws if multi-tenant authentication is disallowed, and the tenants don't match", async function() { + let error: Error | undefined; + try { + validateMultiTenantRequest(false, "credential-options-tenant-id", { + tenantId: "get-token-options-tenant-id" + }); + } catch (e) { + error = e; + } + assert.ok( + error, + "validateMultiTenantRequest should throw if multi-tenant is disallowed and the tenants don't match" + ); + assert.equal(error!.message, multiTenantError.message); + }); + + it("doesn't throw if multi-tenant authentication is allowed regardless of the value of the tenants", async function() { + assert.equal( + validateMultiTenantRequest(true, "credential-options-tenant-id", { + tenantId: "get-token-options-tenant-id" + }), + undefined + ); + assert.equal( + validateMultiTenantRequest(true, "same-tenant", { tenantId: "same-tenant" }), + undefined + ); + }); + }); +}); From dc1ea9e19468336326c75669a855a997be0b0ac1 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Tue, 22 Jun 2021 20:10:32 +0000 Subject: [PATCH 05/19] moved allowMultiTenantAuthentication to the GetTokenOptions --- sdk/core/core-auth/CHANGELOG.md | 1 + sdk/core/core-auth/review/core-auth.api.md | 1 + sdk/core/core-auth/src/tokenCredential.ts | 5 ++++ sdk/identity/identity/CHANGELOG.md | 1 + sdk/identity/identity/review/identity.api.md | 1 - .../identity/src/client/identityClient.ts | 4 --- .../authorizationCodeCredential.ts | 6 ++--- .../src/credentials/azureCliCredential.ts | 6 ++--- .../credentials/azurePowerShellCredential.ts | 6 ++--- .../managedIdentityCredential/index.ts | 10 +++---- .../managedIdentityCredential/utils.ts | 26 ------------------ .../identity/src/msal/nodeFlows/nodeCommon.ts | 6 ++--- .../identity/src/util/validateMultiTenant.ts | 27 +++++++++++++++++++ .../identity/test/internal/utils.spec.ts | 5 +--- 14 files changed, 47 insertions(+), 58 deletions(-) create mode 100644 sdk/identity/identity/src/util/validateMultiTenant.ts diff --git a/sdk/core/core-auth/CHANGELOG.md b/sdk/core/core-auth/CHANGELOG.md index 8babbff10b65..729e8af5f823 100644 --- a/sdk/core/core-auth/CHANGELOG.md +++ b/sdk/core/core-auth/CHANGELOG.md @@ -2,6 +2,7 @@ ## 1.3.1 (Unreleased) +- Added `tenantId` and `allowMultiTenantAuthentication` to `GetTokenOptions`, which is used to enable multi-tenant authentication. ## 1.3.0 (2021-03-30) diff --git a/sdk/core/core-auth/review/core-auth.api.md b/sdk/core/core-auth/review/core-auth.api.md index 1512a1301a94..fc76c681034c 100644 --- a/sdk/core/core-auth/review/core-auth.api.md +++ b/sdk/core/core-auth/review/core-auth.api.md @@ -44,6 +44,7 @@ export interface Context { // @public export interface GetTokenOptions { abortSignal?: AbortSignalLike; + allowMultiTenantAuthentication?: boolean; requestOptions?: { timeout?: number; }; diff --git a/sdk/core/core-auth/src/tokenCredential.ts b/sdk/core/core-auth/src/tokenCredential.ts index 08ffdfc69be5..194c18f8ffc0 100644 --- a/sdk/core/core-auth/src/tokenCredential.ts +++ b/sdk/core/core-auth/src/tokenCredential.ts @@ -57,6 +57,11 @@ export interface GetTokenOptions { * Allows specifying a tenantId. Useful to handle challenges that provide tenant Id hints. */ tenantId?: string; + + /** + * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. + */ + allowMultiTenantAuthentication?: boolean; } /** diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md index 7961335e8f85..7b26e632b119 100644 --- a/sdk/identity/identity/CHANGELOG.md +++ b/sdk/identity/identity/CHANGELOG.md @@ -26,6 +26,7 @@ - Added `regionalAuthority` property to `ClientSecretCredentialOptions` and `ClientCertificateCredentialOptions`. - If instead of a region, `AutoDiscoverRegion` is specified as the value for `regionalAuthority`, MSAL will be used to attempt to discover the region. - A region can also be specified through the `AZURE_REGIONAL_AUTHORITY_NAME` environment variable. +- All of the credentials now accept a `tenantId`, and they now work with the `GetTokenOptions` new properties: `tenantId` and `allowMultiTenantAuthentication`. If a tenant is specified, any `getToken` request that receives a `tenantId` that doesn't match the credential's `tenantId` will throw an error, unless `getToken` gets called with the `allowMultiTenantAuthentication` property set to true. ## 2.0.0-beta.3 (2021-05-12) diff --git a/sdk/identity/identity/review/identity.api.md b/sdk/identity/identity/review/identity.api.md index ee7786a42d93..9616928db0c3 100644 --- a/sdk/identity/identity/review/identity.api.md +++ b/sdk/identity/identity/review/identity.api.md @@ -318,7 +318,6 @@ export { TokenCredential } // @public export interface TokenCredentialOptions extends PipelineOptions { - allowMultiTenantAuthentication?: boolean; authorityHost?: string; } diff --git a/sdk/identity/identity/src/client/identityClient.ts b/sdk/identity/identity/src/client/identityClient.ts index da4d20fe400d..7b107439a1d0 100644 --- a/sdk/identity/identity/src/client/identityClient.ts +++ b/sdk/identity/identity/src/client/identityClient.ts @@ -313,8 +313,4 @@ export interface TokenCredentialOptions extends PipelineOptions { * The default is "https://login.microsoftonline.com". */ authorityHost?: string; - /** - * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. - */ - allowMultiTenantAuthentication?: boolean; } diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts index 95d327cea92f..0f1d0d90ecab 100644 --- a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts +++ b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts @@ -13,7 +13,7 @@ import { credentialLogger, formatSuccess, formatError } from "../util/logging"; import { getIdentityTokenEndpointSuffix } from "../util/identityTokenEndpoint"; import { checkTenantId } from "../util/checkTenantId"; import { AuthorizationCodeCredentialOptions } from "./authorizationCodeCredentialOptions"; -import { validateMultiTenantRequest } from "./managedIdentityCredential/utils"; +import { validateMultiTenantRequest } from "../util/validateMultiTenant"; const logger = credentialLogger("AuthorizationCodeCredential"); @@ -32,7 +32,6 @@ export class AuthorizationCodeCredential implements TokenCredential { private authorizationCode: string; private redirectUri: string; private lastTokenResponse: TokenResponse | null = null; - private allowMultiTenantAuthentication: boolean = false; /** * Creates an instance of CodeFlowCredential with the details needed @@ -108,7 +107,6 @@ export class AuthorizationCodeCredential implements TokenCredential { this.clientId = clientId; this.tenantId = tenantId; - this.allowMultiTenantAuthentication = Boolean(options?.allowMultiTenantAuthentication); if (typeof redirectUriOrOptions === "string") { // the clientId+clientSecret constructor @@ -141,7 +139,7 @@ export class AuthorizationCodeCredential implements TokenCredential { scopes: string | string[], options?: GetTokenOptions ): Promise { - validateMultiTenantRequest(this.allowMultiTenantAuthentication, this.tenantId, options); + validateMultiTenantRequest(options?.allowMultiTenantAuthentication, this.tenantId, options); const { span, updatedOptions } = createSpan("AuthorizationCodeCredential-getToken", options); try { diff --git a/sdk/identity/identity/src/credentials/azureCliCredential.ts b/sdk/identity/identity/src/credentials/azureCliCredential.ts index c0a1c4394963..57b551f5041a 100644 --- a/sdk/identity/identity/src/credentials/azureCliCredential.ts +++ b/sdk/identity/identity/src/credentials/azureCliCredential.ts @@ -10,7 +10,7 @@ import { credentialLogger, formatSuccess, formatError } from "../util/logging"; import * as child_process from "child_process"; import { ensureValidScope, getScopeResource } from "../util/scopeUtils"; import { AzureCliCredentialOptions } from "./azureCliCredentialOptions"; -import { validateMultiTenantRequest } from "./managedIdentityCredential/utils"; +import { validateMultiTenantRequest } from "../util/validateMultiTenant"; /** * Mockable reference to the CLI credential cliCredentialFunctions @@ -68,7 +68,6 @@ const logger = credentialLogger("AzureCliCredential"); */ export class AzureCliCredential implements TokenCredential { private tenantId?: string; - private allowMultiTenantAuthentication: boolean = false; /** * Creates an instance of the {@link AzureCliCredential}. @@ -77,7 +76,6 @@ export class AzureCliCredential implements TokenCredential { */ constructor(options?: AzureCliCredentialOptions) { this.tenantId = options?.tenantId; - this.allowMultiTenantAuthentication = Boolean(options?.allowMultiTenantAuthentication); } /** @@ -97,7 +95,7 @@ export class AzureCliCredential implements TokenCredential { const scope = typeof scopes === "string" ? scopes : scopes[0]; logger.getToken.info(`Using the scope ${scope}`); - validateMultiTenantRequest(this.allowMultiTenantAuthentication, this.tenantId, options); + validateMultiTenantRequest(options?.allowMultiTenantAuthentication, this.tenantId, options); ensureValidScope(scope, logger); const resource = getScopeResource(scope); diff --git a/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts b/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts index 452d746a95a5..903beba71e48 100644 --- a/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts +++ b/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts @@ -9,7 +9,7 @@ import { trace } from "../util/tracing"; import { ensureValidScope, getScopeResource } from "../util/scopeUtils"; import { processUtils } from "../util/processUtils"; import { AzurePowerShellCredentialOptions } from "./azurePowerShellCredentialOptions"; -import { validateMultiTenantRequest } from "./managedIdentityCredential/utils"; +import { validateMultiTenantRequest } from "../util/validateMultiTenant"; const logger = credentialLogger("AzurePowerShellCredential"); @@ -95,7 +95,6 @@ if (isWindows) { */ export class AzurePowerShellCredential implements TokenCredential { private tenantId?: string; - private allowMultiTenantAuthentication: boolean = false; /** * Creates an instance of the {@link AzurePowershellCredential}. @@ -104,7 +103,6 @@ export class AzurePowerShellCredential implements TokenCredential { */ constructor(options?: AzurePowerShellCredentialOptions) { this.tenantId = options?.tenantId; - this.allowMultiTenantAuthentication = Boolean(options?.allowMultiTenantAuthentication); } /** @@ -166,7 +164,7 @@ export class AzurePowerShellCredential implements TokenCredential { logger.getToken.info(`Using the scope ${scope}`); - validateMultiTenantRequest(this.allowMultiTenantAuthentication, this.tenantId, options); + validateMultiTenantRequest(options.allowMultiTenantAuthentication, this.tenantId, options); ensureValidScope(scope, logger); const resource = getScopeResource(scope); diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts index d3c25af78681..9bb5e8b5ebc5 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts @@ -8,12 +8,13 @@ import { createSpan } from "../../util/tracing"; import { AuthenticationError, CredentialUnavailableError } from "../../client/errors"; import { SpanStatusCode } from "@azure/core-tracing"; import { credentialLogger, formatSuccess, formatError } from "../../util/logging"; -import { mapScopesToResource, validateMultiTenantRequest } from "./utils"; +import { mapScopesToResource } from "./utils"; import { cloudShellMsi } from "./cloudShellMsi"; import { imdsMsi } from "./imdsMsi"; import { MSI } from "./models"; import { appServiceMsi2017 } from "./appServiceMsi2017"; import { arcMsi } from "./arcMsi"; +import { validateMultiTenantRequest } from "../../util/validateMultiTenant"; const logger = credentialLogger("ManagedIdentityCredential"); @@ -41,7 +42,6 @@ export class ManagedIdentityCredential implements TokenCredential { private isEndpointUnavailable: boolean | null = null; private clientId?: string; private tenantId?: string; - private allowMultiTenantAuthentication: boolean = false; /** * Creates an instance of ManagedIdentityCredential with the client ID of a @@ -72,16 +72,12 @@ export class ManagedIdentityCredential implements TokenCredential { if (options) { this.tenantId = options.tenantId; - this.allowMultiTenantAuthentication = Boolean(options.allowMultiTenantAuthentication); } } else { // options only constructor this.identityClient = new IdentityClient(clientIdOrOptions); if (clientIdOrOptions) { this.tenantId = clientIdOrOptions.tenantId; - this.allowMultiTenantAuthentication = Boolean( - clientIdOrOptions.allowMultiTenantAuthentication - ); } } } @@ -152,7 +148,7 @@ export class ManagedIdentityCredential implements TokenCredential { scopes: string | string[], options?: GetTokenOptions ): Promise { - validateMultiTenantRequest(this.allowMultiTenantAuthentication, this.tenantId, options); + validateMultiTenantRequest(options?.allowMultiTenantAuthentication, this.tenantId, options); let result: AccessToken | null = null; diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts index 831944b103e5..ef2336c6a9c7 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/utils.ts @@ -48,29 +48,3 @@ export async function msiGenericGetToken( return (tokenResponse && tokenResponse.accessToken) || null; } - -/** - * @internal - */ -export const multiTenantError = new Error( - "Multi-tenant authentication was attempted, but multi-tenant authentication was not enabled in this credential instance." -); - -/** - * Verifies whether locally assigned tenants are equal to tenants received through getToken. - * @internal - */ -export function validateMultiTenantRequest( - allowMultiTenantAuthentication: boolean, - tenantId?: string, - getTokenOptions?: GetTokenOptions -): void { - if ( - !allowMultiTenantAuthentication && - getTokenOptions && - tenantId && - getTokenOptions.tenantId !== tenantId - ) { - throw multiTenantError; - } -} diff --git a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts index bedc857971a5..106f85863a3e 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts @@ -22,9 +22,9 @@ import { msalToPublic, publicToMsal } from "../utils"; -import { validateMultiTenantRequest } from "../../credentials/managedIdentityCredential/utils"; import { TokenCachePersistenceOptions } from "./tokenCachePersistenceOptions"; import { RegionalAuthority } from "../../regionalAuthority"; +import { validateMultiTenantRequest } from "../../util/validateMultiTenant"; /** * Union of the constructor parameters that all MSAL flow types for Node. @@ -77,13 +77,11 @@ export abstract class MsalNode extends MsalBaseUtilities implements MsalFlow { protected tenantId: string; protected identityClient?: IdentityClient; protected requiresConfidential: boolean = false; - protected allowMultiTenantAuthentication: boolean = false; protected azureRegion?: string; protected createCachePlugin: (() => Promise) | undefined; constructor(options: MsalNodeOptions) { super(options); - this.allowMultiTenantAuthentication = Boolean(options.allowMultiTenantAuthentication); this.msalConfig = this.defaultNodeMsalConfig(options); this.tenantId = resolveTenantId(options.logger, options.tenantId, options.clientId); this.clientId = this.msalConfig.auth.clientId; @@ -266,7 +264,7 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov scopes: string[], options: CredentialFlowGetTokenOptions = {} ): Promise { - validateMultiTenantRequest(this.allowMultiTenantAuthentication, this.tenantId, options); + validateMultiTenantRequest(options?.allowMultiTenantAuthentication, this.tenantId, options); options.correlationId = options?.correlationId || this.generateUuid(); await this.init(options); diff --git a/sdk/identity/identity/src/util/validateMultiTenant.ts b/sdk/identity/identity/src/util/validateMultiTenant.ts new file mode 100644 index 000000000000..aaad3534f5d6 --- /dev/null +++ b/sdk/identity/identity/src/util/validateMultiTenant.ts @@ -0,0 +1,27 @@ +import { GetTokenOptions } from "@azure/core-auth"; + +/** + * @internal + */ +export const multiTenantError = new Error( + "Multi-tenant authentication was attempted, but multi-tenant authentication was not enabled in this credential instance." +); + +/** + * Verifies whether locally assigned tenants are equal to tenants received through getToken. + * @internal + */ +export function validateMultiTenantRequest( + allowMultiTenantAuthentication?: boolean, + tenantId?: string, + getTokenOptions?: GetTokenOptions +): void { + if ( + !allowMultiTenantAuthentication && + getTokenOptions?.tenantId && + tenantId && + getTokenOptions.tenantId !== tenantId + ) { + throw multiTenantError; + } +} diff --git a/sdk/identity/identity/test/internal/utils.spec.ts b/sdk/identity/identity/test/internal/utils.spec.ts index a52116a16435..a4744e01aa83 100644 --- a/sdk/identity/identity/test/internal/utils.spec.ts +++ b/sdk/identity/identity/test/internal/utils.spec.ts @@ -2,10 +2,7 @@ // Licensed under the MIT license. import assert from "assert"; -import { - multiTenantError, - validateMultiTenantRequest -} from "../../src/credentials/managedIdentityCredential/utils"; +import { multiTenantError, validateMultiTenantRequest } from "../../src/util/validateMultiTenant"; describe("Identity utilities", function() { describe("validateMultiTenantRequest", function() { From c8adf94b6af7744280cce451b24d32e4eaf40661 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Tue, 22 Jun 2021 20:12:33 +0000 Subject: [PATCH 06/19] added validateMultiTenantRequest on browserCommon --- sdk/identity/identity/src/msal/browserFlows/browserCommon.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts b/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts index 5e06a9ea4dbd..86a675bc9451 100644 --- a/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts +++ b/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts @@ -14,6 +14,7 @@ import { AuthenticationRecord } from "../types"; import { CredentialFlowGetTokenOptions } from "../credentials"; import { AuthenticationRequiredError } from "../errors"; import { CredentialUnavailableError } from "../../client/errors"; +import { validateMultiTenantRequest } from "../../util/validateMultiTenant"; /** * Union of the constructor parameters that all MSAL flow types take. @@ -139,6 +140,8 @@ export abstract class MsalBrowser extends MsalBaseUtilities implements MsalBrows scopes: string[], options?: CredentialFlowGetTokenOptions ): Promise { + validateMultiTenantRequest(options?.allowMultiTenantAuthentication, this.tenantId, options); + // We ensure that redirection is handled at this point. await this.handleRedirect(); From 7be77565361d2d8cc025508dc8b05759e4c2493b Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Thu, 24 Jun 2021 18:41:57 +0000 Subject: [PATCH 07/19] lint fix --- sdk/identity/identity/src/util/validateMultiTenant.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sdk/identity/identity/src/util/validateMultiTenant.ts b/sdk/identity/identity/src/util/validateMultiTenant.ts index aaad3534f5d6..4b0b05cfb0fd 100644 --- a/sdk/identity/identity/src/util/validateMultiTenant.ts +++ b/sdk/identity/identity/src/util/validateMultiTenant.ts @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + import { GetTokenOptions } from "@azure/core-auth"; /** From 0dacea85be140489901a56ae205ae64101181f39 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Fri, 25 Jun 2021 21:59:22 +0000 Subject: [PATCH 08/19] asiggning the tenantId where it should go --- .../src/credentials/authorizationCodeCredential.ts | 4 ++-- .../identity/src/credentials/azureCliCredential.ts | 5 ++--- .../src/credentials/azurePowerShellCredential.ts | 8 +++----- .../credentials/managedIdentityCredential/index.ts | 4 ++-- .../identity/src/msal/browserFlows/browserCommon.ts | 4 ++-- .../identity/src/msal/nodeFlows/nodeCommon.ts | 4 ++-- .../identity/src/util/validateMultiTenant.ts | 12 ++++++++---- sdk/identity/identity/test/internal/utils.spec.ts | 13 +++++++++---- 8 files changed, 30 insertions(+), 24 deletions(-) diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts index d72174f65082..78dcc9990a83 100644 --- a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts +++ b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts @@ -13,7 +13,7 @@ import { credentialLogger, formatSuccess, formatError } from "../util/logging"; import { getIdentityTokenEndpointSuffix } from "../util/identityTokenEndpoint"; import { checkTenantId } from "../util/checkTenantId"; import { AuthorizationCodeCredentialOptions } from "./authorizationCodeCredentialOptions"; -import { validateMultiTenantRequest } from "../util/validateMultiTenant"; +import { processMultiTenantRequest } from "../util/validateMultiTenant"; const logger = credentialLogger("AuthorizationCodeCredential"); @@ -137,7 +137,7 @@ export class AuthorizationCodeCredential implements TokenCredential { scopes: string | string[], options?: GetTokenOptions ): Promise { - validateMultiTenantRequest(options?.allowMultiTenantAuthentication, this.tenantId, options); + this.tenantId = processMultiTenantRequest(this.tenantId, options)!; const { span, updatedOptions } = createSpan("AuthorizationCodeCredential-getToken", options); try { diff --git a/sdk/identity/identity/src/credentials/azureCliCredential.ts b/sdk/identity/identity/src/credentials/azureCliCredential.ts index 2fb6d228c5d8..6e5e0ba61b9d 100644 --- a/sdk/identity/identity/src/credentials/azureCliCredential.ts +++ b/sdk/identity/identity/src/credentials/azureCliCredential.ts @@ -10,7 +10,7 @@ import { credentialLogger, formatSuccess, formatError } from "../util/logging"; import * as child_process from "child_process"; import { ensureValidScope, getScopeResource } from "../util/scopeUtils"; import { AzureCliCredentialOptions } from "./azureCliCredentialOptions"; -import { validateMultiTenantRequest } from "../util/validateMultiTenant"; +import { processMultiTenantRequest } from "../util/validateMultiTenant"; /** * Mockable reference to the CLI credential cliCredentialFunctions @@ -90,10 +90,9 @@ export class AzureCliCredential implements TokenCredential { scopes: string | string[], options?: GetTokenOptions ): Promise { + this.tenantId = processMultiTenantRequest(this.tenantId, options); const scope = typeof scopes === "string" ? scopes : scopes[0]; logger.getToken.info(`Using the scope ${scope}`); - - validateMultiTenantRequest(options?.allowMultiTenantAuthentication, this.tenantId, options); ensureValidScope(scope, logger); const resource = getScopeResource(scope); diff --git a/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts b/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts index 10e7e78a974a..a07c8d868235 100644 --- a/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts +++ b/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts @@ -9,7 +9,7 @@ import { trace } from "../util/tracing"; import { ensureValidScope, getScopeResource } from "../util/scopeUtils"; import { processUtils } from "../util/processUtils"; import { AzurePowerShellCredentialOptions } from "./azurePowerShellCredentialOptions"; -import { validateMultiTenantRequest } from "../util/validateMultiTenant"; +import { processMultiTenantRequest } from "../util/validateMultiTenant"; const logger = credentialLogger("AzurePowerShellCredential"); @@ -158,12 +158,10 @@ export class AzurePowerShellCredential implements TokenCredential { options: GetTokenOptions = {} ): Promise { return trace(`${this.constructor.name}.getToken`, options, async () => { + this.tenantId = processMultiTenantRequest(this.tenantId, options); const scope = typeof scopes === "string" ? scopes : scopes[0]; - - logger.getToken.info(`Using the scope ${scope}`); - - validateMultiTenantRequest(options.allowMultiTenantAuthentication, this.tenantId, options); ensureValidScope(scope, logger); + logger.getToken.info(`Using the scope ${scope}`); const resource = getScopeResource(scope); try { diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts index 95354fde38e0..05f6f4f70a7f 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts @@ -14,7 +14,7 @@ import { imdsMsi } from "./imdsMsi"; import { MSI } from "./models"; import { appServiceMsi2017 } from "./appServiceMsi2017"; import { arcMsi } from "./arcMsi"; -import { validateMultiTenantRequest } from "../../util/validateMultiTenant"; +import { processMultiTenantRequest } from "../../util/validateMultiTenant"; const logger = credentialLogger("ManagedIdentityCredential"); @@ -147,7 +147,7 @@ export class ManagedIdentityCredential implements TokenCredential { scopes: string | string[], options?: GetTokenOptions ): Promise { - validateMultiTenantRequest(options?.allowMultiTenantAuthentication, this.tenantId, options); + this.tenantId = processMultiTenantRequest(this.tenantId, options); let result: AccessToken | null = null; diff --git a/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts b/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts index 86a675bc9451..ad0923c8372f 100644 --- a/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts +++ b/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts @@ -14,7 +14,7 @@ import { AuthenticationRecord } from "../types"; import { CredentialFlowGetTokenOptions } from "../credentials"; import { AuthenticationRequiredError } from "../errors"; import { CredentialUnavailableError } from "../../client/errors"; -import { validateMultiTenantRequest } from "../../util/validateMultiTenant"; +import { processMultiTenantRequest } from "../../util/validateMultiTenant"; /** * Union of the constructor parameters that all MSAL flow types take. @@ -140,7 +140,7 @@ export abstract class MsalBrowser extends MsalBaseUtilities implements MsalBrows scopes: string[], options?: CredentialFlowGetTokenOptions ): Promise { - validateMultiTenantRequest(options?.allowMultiTenantAuthentication, this.tenantId, options); + this.tenantId = processMultiTenantRequest(this.tenantId, options)!; // We ensure that redirection is handled at this point. await this.handleRedirect(); diff --git a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts index 106f85863a3e..90b294a06270 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts @@ -24,7 +24,7 @@ import { } from "../utils"; import { TokenCachePersistenceOptions } from "./tokenCachePersistenceOptions"; import { RegionalAuthority } from "../../regionalAuthority"; -import { validateMultiTenantRequest } from "../../util/validateMultiTenant"; +import { processMultiTenantRequest } from "../../util/validateMultiTenant"; /** * Union of the constructor parameters that all MSAL flow types for Node. @@ -264,7 +264,7 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov scopes: string[], options: CredentialFlowGetTokenOptions = {} ): Promise { - validateMultiTenantRequest(options?.allowMultiTenantAuthentication, this.tenantId, options); + this.tenantId = processMultiTenantRequest(this.tenantId, options)!; options.correlationId = options?.correlationId || this.generateUuid(); await this.init(options); diff --git a/sdk/identity/identity/src/util/validateMultiTenant.ts b/sdk/identity/identity/src/util/validateMultiTenant.ts index 4b0b05cfb0fd..ebe1d1d1bd81 100644 --- a/sdk/identity/identity/src/util/validateMultiTenant.ts +++ b/sdk/identity/identity/src/util/validateMultiTenant.ts @@ -12,19 +12,23 @@ export const multiTenantError = new Error( /** * Verifies whether locally assigned tenants are equal to tenants received through getToken. + * Returns the appropriate tenant. * @internal */ -export function validateMultiTenantRequest( - allowMultiTenantAuthentication?: boolean, +export function processMultiTenantRequest( tenantId?: string, getTokenOptions?: GetTokenOptions -): void { +): string | undefined { if ( - !allowMultiTenantAuthentication && + !getTokenOptions?.allowMultiTenantAuthentication && getTokenOptions?.tenantId && tenantId && getTokenOptions.tenantId !== tenantId ) { throw multiTenantError; } + if (getTokenOptions?.allowMultiTenantAuthentication && getTokenOptions?.tenantId) { + return getTokenOptions.tenantId; + } + return tenantId; } diff --git a/sdk/identity/identity/test/internal/utils.spec.ts b/sdk/identity/identity/test/internal/utils.spec.ts index a4744e01aa83..62ad044cd4e0 100644 --- a/sdk/identity/identity/test/internal/utils.spec.ts +++ b/sdk/identity/identity/test/internal/utils.spec.ts @@ -2,14 +2,15 @@ // Licensed under the MIT license. import assert from "assert"; -import { multiTenantError, validateMultiTenantRequest } from "../../src/util/validateMultiTenant"; +import { multiTenantError, processMultiTenantRequest } from "../../src/util/validateMultiTenant"; describe("Identity utilities", function() { describe("validateMultiTenantRequest", function() { it("throws if multi-tenant authentication is disallowed, and the tenants don't match", async function() { let error: Error | undefined; try { - validateMultiTenantRequest(false, "credential-options-tenant-id", { + processMultiTenantRequest("credential-options-tenant-id", { + allowMultiTenantAuthentication: false, tenantId: "get-token-options-tenant-id" }); } catch (e) { @@ -24,13 +25,17 @@ describe("Identity utilities", function() { it("doesn't throw if multi-tenant authentication is allowed regardless of the value of the tenants", async function() { assert.equal( - validateMultiTenantRequest(true, "credential-options-tenant-id", { + processMultiTenantRequest("credential-options-tenant-id", { + allowMultiTenantAuthentication: true, tenantId: "get-token-options-tenant-id" }), undefined ); assert.equal( - validateMultiTenantRequest(true, "same-tenant", { tenantId: "same-tenant" }), + processMultiTenantRequest("same-tenant", { + allowMultiTenantAuthentication: true, + tenantId: "same-tenant" + }), undefined ); }); From d252fbe8cebf3b227bdeb8b69fd56a8d0aa18b54 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Fri, 25 Jun 2021 22:12:52 +0000 Subject: [PATCH 09/19] better tests --- .../identity/test/internal/utils.spec.ts | 52 +++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/sdk/identity/identity/test/internal/utils.spec.ts b/sdk/identity/identity/test/internal/utils.spec.ts index 62ad044cd4e0..1d2f11471371 100644 --- a/sdk/identity/identity/test/internal/utils.spec.ts +++ b/sdk/identity/identity/test/internal/utils.spec.ts @@ -23,20 +23,66 @@ describe("Identity utilities", function() { assert.equal(error!.message, multiTenantError.message); }); - it("doesn't throw if multi-tenant authentication is allowed regardless of the value of the tenants", async function() { + it("throws if multi-tenant authentication is undefined, and the tenants don't match", async function() { + let error: Error | undefined; + try { + processMultiTenantRequest("credential-options-tenant-id", { + allowMultiTenantAuthentication: undefined, + tenantId: "get-token-options-tenant-id" + }); + } catch (e) { + error = e; + } + assert.ok( + error, + "validateMultiTenantRequest should throw if multi-tenant is disallowed and the tenants don't match" + ); + assert.equal(error!.message, multiTenantError.message); + }); + + it("If allowMultiTenantAuthentication is disallowed, it shouldn't throw if the tenant received is the same as the tenant we already had, that same tenant should be returned", async function() { + assert.equal( + processMultiTenantRequest("same-tenant", { + allowMultiTenantAuthentication: false, + tenantId: "same-tenant" + }), + "same-tenant" + ); + assert.equal( + processMultiTenantRequest("same-tenant", { + allowMultiTenantAuthentication: undefined, + tenantId: "same-tenant" + }), + "same-tenant" + ); + }); + + it("If we had a tenant and the options have another tenant, we pick the tenant from the options", async function() { assert.equal( processMultiTenantRequest("credential-options-tenant-id", { allowMultiTenantAuthentication: true, tenantId: "get-token-options-tenant-id" }), - undefined + "get-token-options-tenant-id" ); + }); + + it("If we had a tenant and there is no tenant in the options, we pick the tenant we already had", async function() { + assert.equal( + processMultiTenantRequest("credential-options-tenant-id", { + allowMultiTenantAuthentication: true + }), + "credential-options-tenant-id" + ); + }); + + it("If the tenant received is the same as the tenant we already had, that same tenant should be returned", async function() { assert.equal( processMultiTenantRequest("same-tenant", { allowMultiTenantAuthentication: true, tenantId: "same-tenant" }), - undefined + "same-tenant" ); }); }); From fae2ab64032d05d688b2f5a001994363b3d6b4de Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Mon, 28 Jun 2021 23:56:30 +0000 Subject: [PATCH 10/19] feedback from Schaab --- sdk/core/core-auth/CHANGELOG.md | 2 +- sdk/core/core-auth/review/core-auth.api.md | 1 - sdk/core/core-auth/src/tokenCredential.ts | 5 --- sdk/identity/identity/CHANGELOG.md | 8 +++-- sdk/identity/identity/review/identity.api.md | 22 ++++--------- .../identity/src/client/identityClient.ts | 5 +++ .../authorizationCodeCredential.browser.ts | 3 +- .../authorizationCodeCredential.ts | 21 ++++++++----- .../authorizationCodeCredentialOptions.ts | 9 ------ .../src/credentials/azureCliCredential.ts | 27 +++++++++++++--- .../credentials/azureCliCredentialOptions.ts | 4 --- .../credentials/azurePowerShellCredential.ts | 20 +++++++++--- .../azurePowerShellCredentialOptions.ts | 4 --- .../managedIdentityCredential/imdsMsi.ts | 13 ++++++-- .../managedIdentityCredential/index.ts | 31 +++---------------- .../credentials/visualStudioCodeCredential.ts | 13 ++++++-- sdk/identity/identity/src/index.ts | 6 +--- .../src/msal/browserFlows/browserCommon.ts | 20 +++++++++--- .../src/msal/browserFlows/msalAuthCode.ts | 4 +-- sdk/identity/identity/src/msal/credentials.ts | 4 +++ .../msal/nodeFlows/msalClientCertificate.ts | 3 +- .../src/msal/nodeFlows/msalClientSecret.ts | 3 +- .../src/msal/nodeFlows/msalDeviceCode.ts | 3 +- .../src/msal/nodeFlows/msalOpenBrowser.ts | 21 +++++++++---- .../msal/nodeFlows/msalUsernamePassword.ts | 3 +- .../identity/src/msal/nodeFlows/nodeCommon.ts | 28 +++++++++++------ .../identity/src/util/validateMultiTenant.ts | 5 +-- .../identity/test/internal/utils.spec.ts | 22 +++++-------- 28 files changed, 172 insertions(+), 138 deletions(-) delete mode 100644 sdk/identity/identity/src/credentials/authorizationCodeCredentialOptions.ts diff --git a/sdk/core/core-auth/CHANGELOG.md b/sdk/core/core-auth/CHANGELOG.md index 729e8af5f823..71b4e87e0204 100644 --- a/sdk/core/core-auth/CHANGELOG.md +++ b/sdk/core/core-auth/CHANGELOG.md @@ -2,7 +2,7 @@ ## 1.3.1 (Unreleased) -- Added `tenantId` and `allowMultiTenantAuthentication` to `GetTokenOptions`, which is used to enable multi-tenant authentication. +- Added `tenantId`, which is used to enable multi-tenant authentication. ## 1.3.0 (2021-03-30) diff --git a/sdk/core/core-auth/review/core-auth.api.md b/sdk/core/core-auth/review/core-auth.api.md index fc76c681034c..1512a1301a94 100644 --- a/sdk/core/core-auth/review/core-auth.api.md +++ b/sdk/core/core-auth/review/core-auth.api.md @@ -44,7 +44,6 @@ export interface Context { // @public export interface GetTokenOptions { abortSignal?: AbortSignalLike; - allowMultiTenantAuthentication?: boolean; requestOptions?: { timeout?: number; }; diff --git a/sdk/core/core-auth/src/tokenCredential.ts b/sdk/core/core-auth/src/tokenCredential.ts index 194c18f8ffc0..08ffdfc69be5 100644 --- a/sdk/core/core-auth/src/tokenCredential.ts +++ b/sdk/core/core-auth/src/tokenCredential.ts @@ -57,11 +57,6 @@ export interface GetTokenOptions { * Allows specifying a tenantId. Useful to handle challenges that provide tenant Id hints. */ tenantId?: string; - - /** - * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. - */ - allowMultiTenantAuthentication?: boolean; } /** diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md index 47fd53b82874..2d471a3ce585 100644 --- a/sdk/identity/identity/CHANGELOG.md +++ b/sdk/identity/identity/CHANGELOG.md @@ -26,8 +26,12 @@ - Added `regionalAuthority` property to `ClientSecretCredentialOptions` and `ClientCertificateCredentialOptions`. - If instead of a region, `AutoDiscoverRegion` is specified as the value for `regionalAuthority`, MSAL will be used to attempt to discover the region. - A region can also be specified through the `AZURE_REGIONAL_AUTHORITY_NAME` environment variable. -- All of the credentials now accept a `tenantId`, and they now work with the `GetTokenOptions` new properties: `tenantId` and `allowMultiTenantAuthentication`. If a tenant is specified, any `getToken` request that receives a `tenantId` that doesn't match the credential's `tenantId` will throw an error, unless `getToken` gets called with the `allowMultiTenantAuthentication` property set to true. - +- The `AzureCliCredential` now receives an `AzureCliCredentialOptions` object, which has the same structure of the `TokenCredentialOptions`, but with an extra optional `tenantId`. +- The `AzurePowerShellCredential` now receives an `AzurePowerShellCredentialOptions` object, which has the same structure of the `TokenCredentialOptions`, but with an extra optional `tenantId`. +- All of the credentials now accept `allowMultiTenantAuthentication`, and as long as they allow configuring a `tenantId`, they now also work with the `GetTokenOptions` new property: `tenantId`. + - If a tenant is specified, any `getToken` request that receives a `tenantId` that doesn't match the credential's `tenantId` will throw an error, unless `getToken` gets called with the `allowMultiTenantAuthentication` property set to true. + - If multi-tenant authentication is allowed, a `tenantId` received through `GetTokenOptions` will be used for that specific request for token instead of the `tenantId` configured when the credential was initialized. + ## 2.0.0-beta.3 (2021-05-12) ### New features diff --git a/sdk/identity/identity/review/identity.api.md b/sdk/identity/identity/review/identity.api.md index 9616928db0c3..719cbbd5ffb3 100644 --- a/sdk/identity/identity/review/identity.api.md +++ b/sdk/identity/identity/review/identity.api.md @@ -51,15 +51,11 @@ export class AuthenticationRequiredError extends Error { // @public export class AuthorizationCodeCredential implements TokenCredential { - constructor(tenantId: string | "common", clientId: string, clientSecret: string, authorizationCode: string, redirectUri: string, options?: AuthorizationCodeCredentialOptions); - constructor(tenantId: string | "common", clientId: string, authorizationCode: string, redirectUri: string, options?: AuthorizationCodeCredentialOptions); + constructor(tenantId: string | "common", clientId: string, clientSecret: string, authorizationCode: string, redirectUri: string, options?: TokenCredentialOptions); + constructor(tenantId: string | "common", clientId: string, authorizationCode: string, redirectUri: string, options?: TokenCredentialOptions); getToken(scopes: string | string[], options?: GetTokenOptions): Promise; } -// @public -export interface AuthorizationCodeCredentialOptions extends TokenCredentialOptions { -} - // @public export enum AzureAuthorityHosts { AzureChina = "https://login.chinacloudapi.cn", @@ -76,7 +72,6 @@ export class AzureCliCredential implements TokenCredential { // @public export interface AzureCliCredentialOptions extends TokenCredentialOptions { - allowMultiTenantAuthentication?: boolean; tenantId?: string; } @@ -88,7 +83,6 @@ export class AzurePowerShellCredential implements TokenCredential { // @public export interface AzurePowerShellCredentialOptions extends TokenCredentialOptions { - allowMultiTenantAuthentication?: boolean; tenantId?: string; } @@ -237,16 +231,11 @@ export const logger: AzureLogger; // @public export class ManagedIdentityCredential implements TokenCredential { - constructor(clientId: string, options?: ManagedIdentityCredentialOptions); - constructor(options?: ManagedIdentityCredentialOptions); + constructor(clientId: string, options?: TokenCredentialOptions); + constructor(options?: TokenCredentialOptions); getToken(scopes: string | string[], options?: GetTokenOptions): Promise; } -// @public -export interface ManagedIdentityCredentialOptions extends TokenCredentialOptions { - tenantId?: string; -} - // @public export enum RegionalAuthority { AsiaEast = "eastasia", @@ -318,6 +307,7 @@ export { TokenCredential } // @public export interface TokenCredentialOptions extends PipelineOptions { + allowMultiTenantAuthentication?: boolean; authorityHost?: string; } @@ -337,7 +327,7 @@ export interface UsernamePasswordCredentialOptions extends TokenCredentialOption // @public export class VisualStudioCodeCredential implements TokenCredential { constructor(options?: VisualStudioCodeCredentialOptions); - getToken(scopes: string | string[], _options?: GetTokenOptions): Promise; + getToken(scopes: string | string[], options?: GetTokenOptions): Promise; } // @public diff --git a/sdk/identity/identity/src/client/identityClient.ts b/sdk/identity/identity/src/client/identityClient.ts index 7b107439a1d0..a85099691fa5 100644 --- a/sdk/identity/identity/src/client/identityClient.ts +++ b/sdk/identity/identity/src/client/identityClient.ts @@ -313,4 +313,9 @@ export interface TokenCredentialOptions extends PipelineOptions { * The default is "https://login.microsoftonline.com". */ authorityHost?: string; + + /** + * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. + */ + allowMultiTenantAuthentication?: boolean; } diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredential.browser.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredential.browser.ts index c461896ad702..364afd8b852f 100644 --- a/sdk/identity/identity/src/credentials/authorizationCodeCredential.browser.ts +++ b/sdk/identity/identity/src/credentials/authorizationCodeCredential.browser.ts @@ -5,7 +5,6 @@ import { TokenCredential, AccessToken } from "@azure/core-auth"; import { TokenCredentialOptions } from "../client/identityClient"; import { credentialLogger, formatError } from "../util/logging"; -import { AuthorizationCodeCredentialOptions } from "./authorizationCodeCredentialOptions"; const BrowserNotSupportedError = new Error( "AuthorizationCodeCredential is not supported in the browser. InteractiveBrowserCredential is more appropriate for this use case." @@ -19,7 +18,7 @@ export class AuthorizationCodeCredential implements TokenCredential { clientSecret: string, authorizationCode: string, redirectUri: string, - options?: AuthorizationCodeCredentialOptions + options?: TokenCredentialOptions ); constructor( tenantId: string | "common", diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts index 78dcc9990a83..09928ae5c670 100644 --- a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts +++ b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts @@ -12,7 +12,6 @@ import { SpanStatusCode } from "@azure/core-tracing"; import { credentialLogger, formatSuccess, formatError } from "../util/logging"; import { getIdentityTokenEndpointSuffix } from "../util/identityTokenEndpoint"; import { checkTenantId } from "../util/checkTenantId"; -import { AuthorizationCodeCredentialOptions } from "./authorizationCodeCredentialOptions"; import { processMultiTenantRequest } from "../util/validateMultiTenant"; const logger = credentialLogger("AuthorizationCodeCredential"); @@ -32,6 +31,7 @@ export class AuthorizationCodeCredential implements TokenCredential { private authorizationCode: string; private redirectUri: string; private lastTokenResponse: TokenResponse | null = null; + private allowMultiTenantAuthentication?: boolean; /** * Creates an instance of CodeFlowCredential with the details needed @@ -61,7 +61,7 @@ export class AuthorizationCodeCredential implements TokenCredential { clientSecret: string, authorizationCode: string, redirectUri: string, - options?: AuthorizationCodeCredentialOptions + options?: TokenCredentialOptions ); /** * Creates an instance of CodeFlowCredential with the details needed @@ -89,7 +89,7 @@ export class AuthorizationCodeCredential implements TokenCredential { clientId: string, authorizationCode: string, redirectUri: string, - options?: AuthorizationCodeCredentialOptions + options?: TokenCredentialOptions ); /** * @hidden @@ -101,12 +101,13 @@ export class AuthorizationCodeCredential implements TokenCredential { clientSecretOrAuthorizationCode: string, authorizationCodeOrRedirectUri: string, redirectUriOrOptions: string | TokenCredentialOptions | undefined, - options?: AuthorizationCodeCredentialOptions + options?: TokenCredentialOptions ) { checkTenantId(logger, tenantId); this.clientId = clientId; this.tenantId = tenantId; + this.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication; if (typeof redirectUriOrOptions === "string") { // the clientId+clientSecret constructor @@ -137,7 +138,11 @@ export class AuthorizationCodeCredential implements TokenCredential { scopes: string | string[], options?: GetTokenOptions ): Promise { - this.tenantId = processMultiTenantRequest(this.tenantId, options)!; + const tenantId = processMultiTenantRequest( + this.tenantId, + this.allowMultiTenantAuthentication, + options + )!; const { span, updatedOptions } = createSpan("AuthorizationCodeCredential-getToken", options); try { @@ -150,7 +155,7 @@ export class AuthorizationCodeCredential implements TokenCredential { // Try to use the refresh token first if (this.lastTokenResponse && this.lastTokenResponse.refreshToken) { tokenResponse = await this.identityClient.refreshAccessToken( - this.tenantId, + tenantId, this.clientId, scopeString, this.lastTokenResponse.refreshToken, @@ -161,9 +166,9 @@ export class AuthorizationCodeCredential implements TokenCredential { } if (tokenResponse === null) { - const urlSuffix = getIdentityTokenEndpointSuffix(this.tenantId); + const urlSuffix = getIdentityTokenEndpointSuffix(tenantId); const webResource = this.identityClient.createWebResource({ - url: `${this.identityClient.authorityHost}/${this.tenantId}/${urlSuffix}`, + url: `${this.identityClient.authorityHost}/${tenantId}/${urlSuffix}`, method: "POST", disableJsonStringifyOnBody: true, deserializationMapper: undefined, diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredentialOptions.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredentialOptions.ts deleted file mode 100644 index fea500baaac4..000000000000 --- a/sdk/identity/identity/src/credentials/authorizationCodeCredentialOptions.ts +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT license. - -import { TokenCredentialOptions } from "../client/identityClient"; - -/** - * Options for the {@link AuthorizationCodeCredential} - */ -export interface AuthorizationCodeCredentialOptions extends TokenCredentialOptions {} diff --git a/sdk/identity/identity/src/credentials/azureCliCredential.ts b/sdk/identity/identity/src/credentials/azureCliCredential.ts index 6e5e0ba61b9d..43f441d10933 100644 --- a/sdk/identity/identity/src/credentials/azureCliCredential.ts +++ b/sdk/identity/identity/src/credentials/azureCliCredential.ts @@ -37,13 +37,26 @@ export const cliCredentialInternals = { * @internal */ async getAzureCliAccessToken( - resource: string + resource: string, + tenantId?: string ): Promise<{ stdout: string; stderr: string; error: Error | null }> { + let tenantSection: string[] = []; + if (tenantId) { + tenantSection = ["--tenant", tenantId]; + } return new Promise((resolve, reject) => { try { child_process.execFile( "az", - ["account", "get-access-token", "--output", "json", "--resource", resource], + [ + "account", + "get-access-token", + "--output", + "json", + "--resource", + ...tenantSection, + resource + ], { cwd: cliCredentialInternals.getSafeWorkingDir() }, (error, stdout, stderr) => { resolve({ stdout: stdout, stderr: stderr, error }); @@ -68,6 +81,7 @@ const logger = credentialLogger("AzureCliCredential"); */ export class AzureCliCredential implements TokenCredential { private tenantId?: string; + private allowMultiTenantAuthentication?: boolean; /** * Creates an instance of the {@link AzureCliCredential}. @@ -76,6 +90,7 @@ export class AzureCliCredential implements TokenCredential { */ constructor(options?: AzureCliCredentialOptions) { this.tenantId = options?.tenantId; + this.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication; } /** @@ -90,7 +105,11 @@ export class AzureCliCredential implements TokenCredential { scopes: string | string[], options?: GetTokenOptions ): Promise { - this.tenantId = processMultiTenantRequest(this.tenantId, options); + const tenantId = processMultiTenantRequest( + this.tenantId, + this.allowMultiTenantAuthentication, + options + ); const scope = typeof scopes === "string" ? scopes : scopes[0]; logger.getToken.info(`Using the scope ${scope}`); ensureValidScope(scope, logger); @@ -101,7 +120,7 @@ export class AzureCliCredential implements TokenCredential { const { span } = createSpan("AzureCliCredential-getToken", options); try { - const obj = await cliCredentialInternals.getAzureCliAccessToken(resource); + const obj = await cliCredentialInternals.getAzureCliAccessToken(resource, tenantId); if (obj.stderr) { const isLoginError = obj.stderr.match("(.*)az login(.*)"); const isNotInstallError = diff --git a/sdk/identity/identity/src/credentials/azureCliCredentialOptions.ts b/sdk/identity/identity/src/credentials/azureCliCredentialOptions.ts index 891b9b6ed819..b50807eab973 100644 --- a/sdk/identity/identity/src/credentials/azureCliCredentialOptions.ts +++ b/sdk/identity/identity/src/credentials/azureCliCredentialOptions.ts @@ -11,8 +11,4 @@ export interface AzureCliCredentialOptions extends TokenCredentialOptions { * Allows specifying a tenant ID */ tenantId?: string; - /** - * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. - */ - allowMultiTenantAuthentication?: boolean; } diff --git a/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts b/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts index a07c8d868235..49ad615248f6 100644 --- a/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts +++ b/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts @@ -95,6 +95,7 @@ if (isWindows) { */ export class AzurePowerShellCredential implements TokenCredential { private tenantId?: string; + private allowMultiTenantAuthentication?: boolean; /** * Creates an instance of the {@link AzurePowershellCredential}. @@ -103,6 +104,7 @@ export class AzurePowerShellCredential implements TokenCredential { */ constructor(options?: AzurePowerShellCredentialOptions) { this.tenantId = options?.tenantId; + this.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication; } /** @@ -110,7 +112,8 @@ export class AzurePowerShellCredential implements TokenCredential { * @param resource - The resource to use when getting the token */ private async getAzurePowerShellAccessToken( - resource: string + resource: string, + tenantId?: string ): Promise<{ Token: string; ExpiresOn: string }> { // Clone the stack to avoid mutating it while iterating for (const powerShellCommand of [...commandStack]) { @@ -122,6 +125,11 @@ export class AzurePowerShellCredential implements TokenCredential { continue; } + let tenantSection = ""; + if (tenantId) { + tenantSection = `-TenantId "${tenantId}"`; + } + const results = await runCommands([ [ powerShellCommand, @@ -131,7 +139,7 @@ export class AzurePowerShellCredential implements TokenCredential { [ powerShellCommand, "-Command", - `Get-AzAccessToken -ResourceUrl "${resource}" | ConvertTo-Json` + `Get-AzAccessToken ${tenantSection} -ResourceUrl "${resource}" | ConvertTo-Json` ] ]); @@ -158,14 +166,18 @@ export class AzurePowerShellCredential implements TokenCredential { options: GetTokenOptions = {} ): Promise { return trace(`${this.constructor.name}.getToken`, options, async () => { - this.tenantId = processMultiTenantRequest(this.tenantId, options); + const tenantId = processMultiTenantRequest( + this.tenantId, + this.allowMultiTenantAuthentication, + options + ); const scope = typeof scopes === "string" ? scopes : scopes[0]; ensureValidScope(scope, logger); logger.getToken.info(`Using the scope ${scope}`); const resource = getScopeResource(scope); try { - const response = await this.getAzurePowerShellAccessToken(resource); + const response = await this.getAzurePowerShellAccessToken(resource, tenantId); logger.getToken.info(formatSuccess(scopes)); return { token: response.Token, diff --git a/sdk/identity/identity/src/credentials/azurePowerShellCredentialOptions.ts b/sdk/identity/identity/src/credentials/azurePowerShellCredentialOptions.ts index 9c3ffbba6990..aa48462dbd10 100644 --- a/sdk/identity/identity/src/credentials/azurePowerShellCredentialOptions.ts +++ b/sdk/identity/identity/src/credentials/azurePowerShellCredentialOptions.ts @@ -11,8 +11,4 @@ export interface AzurePowerShellCredentialOptions extends TokenCredentialOptions * Allows specifying a tenant ID */ tenantId?: string; - /** - * If set to true, allows authentication flows to change the tenantId of the request if a different tenantId is received from a challenge or through a direct getToken call. - */ - allowMultiTenantAuthentication?: boolean; } diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/imdsMsi.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/imdsMsi.ts index f27c43d12f73..0445d15c764d 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/imdsMsi.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/imdsMsi.ts @@ -44,7 +44,7 @@ function prepareRequestOptions(resource?: string, clientId?: string): RequestPre } return { - url: imdsEndpoint, + url: process.env.AZURE_POD_IDENTITY_TOKEN_URL ?? imdsEndpoint, method: "GET", queryParameters, headers: { @@ -73,6 +73,11 @@ export const imdsMsi: MSI = { getTokenOptions ); + // if the PodIdenityEndpoint environment variable was set no need to probe the endpoint, it can be assumed to exist + if (process.env.AZURE_POD_IDENTITY_TOKEN_URL) { + return true; + } + const request = prepareRequestOptions(resource, clientId); // This will always be populated, but let's make TypeScript happy @@ -90,7 +95,11 @@ export const imdsMsi: MSI = { // not having a "Metadata" header should cause an error to be // returned quickly from the endpoint, proving its availability. const webResource = identityClient.createWebResource(request); - webResource.timeout = updatedOptions?.requestOptions?.timeout || 500; + + // In Kubernetes pods, node-fetch (used by core-http) takes longer than 2 seconds to begin sending the network request, + // So smaller timeouts will cause this credential to be immediately aborted. + // This won't be a problem once we move Identity to core-rest-pipeline. + webResource.timeout = updatedOptions?.requestOptions?.timeout || 3000; try { logger.info(`Pinging IMDS endpoint`); diff --git a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts index 05f6f4f70a7f..49a95cd31cf2 100644 --- a/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts +++ b/sdk/identity/identity/src/credentials/managedIdentityCredential/index.ts @@ -14,20 +14,9 @@ import { imdsMsi } from "./imdsMsi"; import { MSI } from "./models"; import { appServiceMsi2017 } from "./appServiceMsi2017"; import { arcMsi } from "./arcMsi"; -import { processMultiTenantRequest } from "../../util/validateMultiTenant"; const logger = credentialLogger("ManagedIdentityCredential"); -/** - * Options for the {@link ManagedIdentityCredential} - */ -export interface ManagedIdentityCredentialOptions extends TokenCredentialOptions { - /** - * Allows specifying a tenant ID - */ - tenantId?: string; -} - /** * Attempts authentication using a managed identity that has been assigned * to the deployment environment. This authentication type works in Azure VMs, @@ -39,9 +28,8 @@ export interface ManagedIdentityCredentialOptions extends TokenCredentialOptions */ export class ManagedIdentityCredential implements TokenCredential { private identityClient: IdentityClient; + private clientId: string | undefined; private isEndpointUnavailable: boolean | null = null; - private clientId?: string; - private tenantId?: string; /** * Creates an instance of ManagedIdentityCredential with the client ID of a @@ -50,35 +38,28 @@ export class ManagedIdentityCredential implements TokenCredential { * @param clientId - The client ID of the user-assigned identity, or app registration (when working with AKS pod-identity). * @param options - Options for configuring the client which makes the access token request. */ - constructor(clientId: string, options?: ManagedIdentityCredentialOptions); + constructor(clientId: string, options?: TokenCredentialOptions); /** * Creates an instance of ManagedIdentityCredential * * @param options - Options for configuring the client which makes the access token request. */ - constructor(options?: ManagedIdentityCredentialOptions); + constructor(options?: TokenCredentialOptions); /** * @internal * @hidden */ constructor( - clientIdOrOptions: string | ManagedIdentityCredentialOptions | undefined, - options?: ManagedIdentityCredentialOptions + clientIdOrOptions: string | TokenCredentialOptions | undefined, + options?: TokenCredentialOptions ) { if (typeof clientIdOrOptions === "string") { // clientId, options constructor this.clientId = clientIdOrOptions; this.identityClient = new IdentityClient(options); - - if (options) { - this.tenantId = options.tenantId; - } } else { // options only constructor this.identityClient = new IdentityClient(clientIdOrOptions); - if (clientIdOrOptions) { - this.tenantId = clientIdOrOptions.tenantId; - } } } @@ -147,8 +128,6 @@ export class ManagedIdentityCredential implements TokenCredential { scopes: string | string[], options?: GetTokenOptions ): Promise { - this.tenantId = processMultiTenantRequest(this.tenantId, options); - let result: AccessToken | null = null; const { span, updatedOptions } = createSpan("ManagedIdentityCredential-getToken", options); diff --git a/sdk/identity/identity/src/credentials/visualStudioCodeCredential.ts b/sdk/identity/identity/src/credentials/visualStudioCodeCredential.ts index c12d7dde7c9b..d3e3ebb1911d 100644 --- a/sdk/identity/identity/src/credentials/visualStudioCodeCredential.ts +++ b/sdk/identity/identity/src/credentials/visualStudioCodeCredential.ts @@ -12,6 +12,7 @@ import { IdentityClient, TokenCredentialOptions } from "../client/identityClient import { AzureAuthorityHosts } from "../constants"; import { checkTenantId } from "../util/checkTenantId"; import { credentialLogger, formatError, formatSuccess } from "../util/logging"; +import { processMultiTenantRequest } from "../util/validateMultiTenant"; import { VSCodeCredentialFinder } from "./visualStudioCodeCredentialExtension"; const CommonTenantId = "common"; @@ -102,6 +103,7 @@ export class VisualStudioCodeCredential implements TokenCredential { private identityClient: IdentityClient; private tenantId: string; private cloudName: VSCodeCloudNames; + private allowMultiTenantAuthentication?: boolean; /** * Creates an instance of VisualStudioCodeCredential to use for automatically authenticating via VSCode. @@ -127,6 +129,7 @@ export class VisualStudioCodeCredential implements TokenCredential { } else { this.tenantId = CommonTenantId; } + this.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication; checkUnsupportedTenant(this.tenantId); } @@ -168,9 +171,15 @@ export class VisualStudioCodeCredential implements TokenCredential { */ public async getToken( scopes: string | string[], - _options?: GetTokenOptions + options?: GetTokenOptions ): Promise { await this.prepareOnce(); + const tenantId = processMultiTenantRequest( + this.tenantId, + this.allowMultiTenantAuthentication, + options + )!; + if (findCredentials === undefined) { throw new CredentialUnavailableError( "No implementation of VisualStudioCodeCredential is available (do you need to install and use the `@azure/identity-vscode` extension package?)" @@ -206,7 +215,7 @@ export class VisualStudioCodeCredential implements TokenCredential { if (refreshToken) { const tokenResponse = await this.identityClient.refreshAccessToken( - this.tenantId, + tenantId, AzureAccountClientId, scopeString, refreshToken, diff --git a/sdk/identity/identity/src/index.ts b/sdk/identity/identity/src/index.ts index 476f6f108512..55ff135f90e7 100644 --- a/sdk/identity/identity/src/index.ts +++ b/sdk/identity/identity/src/index.ts @@ -37,10 +37,7 @@ export { InteractiveBrowserCredentialBrowserOptions, BrowserLoginStyle } from "./credentials/interactiveBrowserCredentialOptions"; -export { - ManagedIdentityCredential, - ManagedIdentityCredentialOptions -} from "./credentials/managedIdentityCredential"; +export { ManagedIdentityCredential } from "./credentials/managedIdentityCredential"; export { DeviceCodeCredential } from "./credentials/deviceCodeCredential"; export { DeviceCodePromptCallback, @@ -50,7 +47,6 @@ export { DeviceCodeCredentialOptions } from "./credentials/deviceCodeCredentialO export { UsernamePasswordCredential } from "./credentials/usernamePasswordCredential"; export { UsernamePasswordCredentialOptions } from "./credentials/usernamePasswordCredentialOptions"; export { AuthorizationCodeCredential } from "./credentials/authorizationCodeCredential"; -export { AuthorizationCodeCredentialOptions } from "./credentials/authorizationCodeCredentialOptions"; export { AzurePowerShellCredential } from "./credentials/azurePowerShellCredential"; export { AzurePowerShellCredentialOptions } from "./credentials/azurePowerShellCredentialOptions"; diff --git a/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts b/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts index ad0923c8372f..b263c628ba4f 100644 --- a/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts +++ b/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts @@ -23,6 +23,7 @@ import { processMultiTenantRequest } from "../../util/validateMultiTenant"; export interface MsalBrowserFlowOptions extends MsalFlowOptions { redirectUri?: string; loginStyle: BrowserLoginStyle; + allowMultiTenantAuthentication?: boolean; } /** @@ -42,12 +43,12 @@ export function defaultBrowserMsalConfig( options: MsalBrowserFlowOptions ): msalBrowser.Configuration { const tenantId = options.tenantId || DefaultTenantId; - const authorityHost = getAuthorityHost(tenantId, options.authorityHost); + const authority = getAuthorityHost(tenantId, options.authorityHost); return { auth: { clientId: options.clientId!, - authority: authorityHost, - knownAuthorities: getKnownAuthorities(tenantId, authorityHost), + authority, + knownAuthorities: getKnownAuthorities(tenantId, authority), // If the users picked redirect as their login style, // but they didn't provide a redirectUri, // we can try to use the current page we're in as a default value. @@ -69,6 +70,8 @@ export abstract class MsalBrowser extends MsalBaseUtilities implements MsalBrows protected loginStyle: BrowserLoginStyle; protected clientId: string; protected tenantId: string; + protected allowMultiTenantAuthentication?: boolean; + protected authorityHost?: string; protected account: AuthenticationRecord | undefined; protected msalConfig: msalBrowser.Configuration; protected disableAutomaticAuthentication?: boolean; @@ -83,6 +86,8 @@ export abstract class MsalBrowser extends MsalBaseUtilities implements MsalBrows } this.clientId = options.clientId; this.tenantId = resolveTenantId(this.logger, options.tenantId, options.clientId); + this.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication; + this.authorityHost = options.authorityHost; this.msalConfig = defaultBrowserMsalConfig(options); this.disableAutomaticAuthentication = options.disableAutomaticAuthentication; @@ -138,9 +143,14 @@ export abstract class MsalBrowser extends MsalBaseUtilities implements MsalBrows */ public async getToken( scopes: string[], - options?: CredentialFlowGetTokenOptions + options: CredentialFlowGetTokenOptions = {} ): Promise { - this.tenantId = processMultiTenantRequest(this.tenantId, options)!; + const tenantId = processMultiTenantRequest( + this.tenantId, + this.allowMultiTenantAuthentication, + options + )!; + options.authority = getAuthorityHost(tenantId, this.authorityHost); // We ensure that redirection is handled at this point. await this.handleRedirect(); diff --git a/sdk/identity/identity/src/msal/browserFlows/msalAuthCode.ts b/sdk/identity/identity/src/msal/browserFlows/msalAuthCode.ts index 9c8ee4317b26..9e225ab4671a 100644 --- a/sdk/identity/identity/src/msal/browserFlows/msalAuthCode.ts +++ b/sdk/identity/identity/src/msal/browserFlows/msalAuthCode.ts @@ -159,7 +159,7 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov } const parameters: msalBrowser.SilentRequest = { - authority: this.msalConfig.auth.authority!, + authority: options?.authority || this.msalConfig.auth.authority!, correlationId: options?.correlationId, account: publicToMsal(account), forceRefresh: false, @@ -188,7 +188,7 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov } const parameters: msalBrowser.RedirectRequest = { - authority: this.msalConfig.auth.authority!, + authority: options?.authority || this.msalConfig.auth.authority!, correlationId: options?.correlationId, account: publicToMsal(account), scopes diff --git a/sdk/identity/identity/src/msal/credentials.ts b/sdk/identity/identity/src/msal/credentials.ts index 6b80f45b96f1..55a2305db4de 100644 --- a/sdk/identity/identity/src/msal/credentials.ts +++ b/sdk/identity/identity/src/msal/credentials.ts @@ -19,6 +19,10 @@ export interface CredentialFlowGetTokenOptions extends GetTokenOptions { * Makes getToken throw if a manual authentication is necessary. */ disableAutomaticAuthentication?: boolean; + /** + * Authority, to overwrite the default one, if necessary. + */ + authority?: string; } /** diff --git a/sdk/identity/identity/src/msal/nodeFlows/msalClientCertificate.ts b/sdk/identity/identity/src/msal/nodeFlows/msalClientCertificate.ts index 3b5131f5725e..c8fb88139c8f 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/msalClientCertificate.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/msalClientCertificate.ts @@ -109,7 +109,8 @@ export class MsalClientCertificate extends MsalNode { const result = await this.confidentialApp!.acquireTokenByClientCredential({ scopes, correlationId: options.correlationId, - azureRegion: this.azureRegion + azureRegion: this.azureRegion, + authority: options.authority }); // Even though we're providing the same default in memory persistence cache that we use for DeviceCodeCredential, // The Client Credential flow does not return the account information from the authentication service, diff --git a/sdk/identity/identity/src/msal/nodeFlows/msalClientSecret.ts b/sdk/identity/identity/src/msal/nodeFlows/msalClientSecret.ts index eafbc6e70be8..dbea908575b9 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/msalClientSecret.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/msalClientSecret.ts @@ -33,7 +33,8 @@ export class MsalClientSecret extends MsalNode { const result = await this.confidentialApp!.acquireTokenByClientCredential({ scopes, correlationId: options.correlationId, - azureRegion: this.azureRegion + azureRegion: this.azureRegion, + authority: options.authority }); // The Client Credential flow does not return an account, // so each time getToken gets called, we will have to acquire a new token through the service. diff --git a/sdk/identity/identity/src/msal/nodeFlows/msalDeviceCode.ts b/sdk/identity/identity/src/msal/nodeFlows/msalDeviceCode.ts index 391726b786fb..323b3c980354 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/msalDeviceCode.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/msalDeviceCode.ts @@ -38,7 +38,8 @@ export class MsalDeviceCode extends MsalNode { deviceCodeCallback: this.userPromptCallback, scopes, cancel: false, - correlationId: options?.correlationId + correlationId: options?.correlationId, + authority: options?.authority }; const promise = this.publicApp!.acquireTokenByDeviceCode(requestOptions); // TODO: diff --git a/sdk/identity/identity/src/msal/nodeFlows/msalOpenBrowser.ts b/sdk/identity/identity/src/msal/nodeFlows/msalOpenBrowser.ts index 8b584bbc99a4..80d6d705fdd9 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/msalOpenBrowser.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/msalOpenBrowser.ts @@ -3,7 +3,7 @@ import * as msalNode from "@azure/msal-node"; -import { AccessToken, GetTokenOptions } from "@azure/core-auth"; +import { AccessToken } from "@azure/core-auth"; import { Socket } from "net"; import http from "http"; @@ -14,6 +14,7 @@ import { credentialLogger, formatError, formatSuccess } from "../../util/logging import { MsalNodeOptions, MsalNode } from "./nodeCommon"; import { msalToPublic } from "../utils"; import { CredentialUnavailableError } from "../../client/errors"; +import { CredentialFlowGetTokenOptions } from "../credentials"; /** * Options that can be passed to configure MSAL to handle authentication through opening a browser window. @@ -60,7 +61,10 @@ export class MsalOpenBrowser extends MsalNode { return this.publicApp!.acquireTokenByCode(request); } - protected doGetToken(scopes: string[], options?: GetTokenOptions): Promise { + protected doGetToken( + scopes: string[], + options?: CredentialFlowGetTokenOptions + ): Promise { return new Promise((resolve, reject) => { const socketToDestroy: Socket[] = []; @@ -87,7 +91,8 @@ export class MsalOpenBrowser extends MsalNode { const tokenRequest: msalNode.AuthorizationCodeRequest = { code: url.searchParams.get("code")!, redirectUri: this.redirectUri, - scopes: scopes + scopes: scopes, + authority: options?.authority }; this.acquireTokenByCode(tokenRequest) @@ -167,7 +172,7 @@ export class MsalOpenBrowser extends MsalNode { app.on("connection", (socket) => socketToDestroy.push(socket)); app.on("listening", () => { - const openPromise = this.openAuthCodeUrl(scopes); + const openPromise = this.openAuthCodeUrl(scopes, options); const abortSignal = options?.abortSignal; if (abortSignal) { @@ -185,10 +190,14 @@ export class MsalOpenBrowser extends MsalNode { }); } - private async openAuthCodeUrl(scopeArray: string[]): Promise { + private async openAuthCodeUrl( + scopeArray: string[], + options?: CredentialFlowGetTokenOptions + ): Promise { const authCodeUrlParameters: msalNode.AuthorizationUrlRequest = { scopes: scopeArray, - redirectUri: this.redirectUri + redirectUri: this.redirectUri, + authority: options?.authority }; const response = await this.publicApp!.getAuthCodeUrl(authCodeUrlParameters); diff --git a/sdk/identity/identity/src/msal/nodeFlows/msalUsernamePassword.ts b/sdk/identity/identity/src/msal/nodeFlows/msalUsernamePassword.ts index 93f71eed2a7c..5db432850074 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/msalUsernamePassword.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/msalUsernamePassword.ts @@ -40,7 +40,8 @@ export class MsalUsernamePassword extends MsalNode { scopes, username: this.username, password: this.password, - correlationId: options?.correlationId + correlationId: options?.correlationId, + authority: options?.authority }; const result = await this.publicApp!.acquireTokenByUsernamePassword(requestOptions); return this.handleResult(scopes, this.clientId, result || undefined); diff --git a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts index 90b294a06270..664a108c72a2 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts @@ -75,6 +75,8 @@ export abstract class MsalNode extends MsalBaseUtilities implements MsalFlow { protected msalConfig: msalNode.Configuration; protected clientId: string; protected tenantId: string; + protected allowMultiTenantAuthentication?: boolean; + protected authorityHost?: string; protected identityClient?: IdentityClient; protected requiresConfidential: boolean = false; protected azureRegion?: string; @@ -84,6 +86,7 @@ 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.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication; this.clientId = this.msalConfig.auth.clientId; // If persistence has been configured @@ -107,19 +110,19 @@ export abstract class MsalNode extends MsalBaseUtilities implements MsalFlow { protected defaultNodeMsalConfig(options: MsalNodeOptions): msalNode.Configuration { const clientId = options.clientId || DeveloperSignOnClientId; const tenantId = resolveTenantId(options.logger, options.tenantId, options.clientId); - const authorityHost = getAuthorityHost( - tenantId, - options.authorityHost || process.env.AZURE_AUTHORITY_HOST - ); + + this.authorityHost = options.authorityHost || process.env.AZURE_AUTHORITY_HOST; + const authority = getAuthorityHost(tenantId, this.authorityHost); + this.identityClient = new IdentityClient({ ...options.tokenCredentialOptions, - authorityHost + authorityHost: authority }); return { auth: { clientId, - authority: authorityHost, - knownAuthorities: getKnownAuthorities(tenantId, authorityHost) + authority, + knownAuthorities: getKnownAuthorities(tenantId, authority) }, // Cache is defined in this.prepare(); system: { @@ -237,7 +240,8 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov // To be able to re-use the account, the Token Cache must also have been provided. account: publicToMsal(this.account), correlationId: options?.correlationId, - scopes + scopes, + authority: options?.authority }; try { @@ -264,7 +268,13 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov scopes: string[], options: CredentialFlowGetTokenOptions = {} ): Promise { - this.tenantId = processMultiTenantRequest(this.tenantId, options)!; + const tenantId = processMultiTenantRequest( + this.tenantId, + this.allowMultiTenantAuthentication, + options + )!; + options.authority = getAuthorityHost(tenantId, this.authorityHost); + options.correlationId = options?.correlationId || this.generateUuid(); await this.init(options); diff --git a/sdk/identity/identity/src/util/validateMultiTenant.ts b/sdk/identity/identity/src/util/validateMultiTenant.ts index ebe1d1d1bd81..27ec12207db4 100644 --- a/sdk/identity/identity/src/util/validateMultiTenant.ts +++ b/sdk/identity/identity/src/util/validateMultiTenant.ts @@ -17,17 +17,18 @@ export const multiTenantError = new Error( */ export function processMultiTenantRequest( tenantId?: string, + allowMultiTenantAuthentication?: boolean, getTokenOptions?: GetTokenOptions ): string | undefined { if ( - !getTokenOptions?.allowMultiTenantAuthentication && + !allowMultiTenantAuthentication && getTokenOptions?.tenantId && tenantId && getTokenOptions.tenantId !== tenantId ) { throw multiTenantError; } - if (getTokenOptions?.allowMultiTenantAuthentication && getTokenOptions?.tenantId) { + if (allowMultiTenantAuthentication && getTokenOptions?.tenantId) { return getTokenOptions.tenantId; } return tenantId; diff --git a/sdk/identity/identity/test/internal/utils.spec.ts b/sdk/identity/identity/test/internal/utils.spec.ts index 1d2f11471371..fe93a041c7c1 100644 --- a/sdk/identity/identity/test/internal/utils.spec.ts +++ b/sdk/identity/identity/test/internal/utils.spec.ts @@ -9,8 +9,7 @@ describe("Identity utilities", function() { it("throws if multi-tenant authentication is disallowed, and the tenants don't match", async function() { let error: Error | undefined; try { - processMultiTenantRequest("credential-options-tenant-id", { - allowMultiTenantAuthentication: false, + processMultiTenantRequest("credential-options-tenant-id", false, { tenantId: "get-token-options-tenant-id" }); } catch (e) { @@ -26,8 +25,7 @@ describe("Identity utilities", function() { it("throws if multi-tenant authentication is undefined, and the tenants don't match", async function() { let error: Error | undefined; try { - processMultiTenantRequest("credential-options-tenant-id", { - allowMultiTenantAuthentication: undefined, + processMultiTenantRequest("credential-options-tenant-id", undefined, { tenantId: "get-token-options-tenant-id" }); } catch (e) { @@ -42,15 +40,13 @@ describe("Identity utilities", function() { it("If allowMultiTenantAuthentication is disallowed, it shouldn't throw if the tenant received is the same as the tenant we already had, that same tenant should be returned", async function() { assert.equal( - processMultiTenantRequest("same-tenant", { - allowMultiTenantAuthentication: false, + processMultiTenantRequest("same-tenant", false, { tenantId: "same-tenant" }), "same-tenant" ); assert.equal( - processMultiTenantRequest("same-tenant", { - allowMultiTenantAuthentication: undefined, + processMultiTenantRequest("same-tenant", undefined, { tenantId: "same-tenant" }), "same-tenant" @@ -59,8 +55,7 @@ describe("Identity utilities", function() { it("If we had a tenant and the options have another tenant, we pick the tenant from the options", async function() { assert.equal( - processMultiTenantRequest("credential-options-tenant-id", { - allowMultiTenantAuthentication: true, + processMultiTenantRequest("credential-options-tenant-id", true, { tenantId: "get-token-options-tenant-id" }), "get-token-options-tenant-id" @@ -69,17 +64,14 @@ describe("Identity utilities", function() { it("If we had a tenant and there is no tenant in the options, we pick the tenant we already had", async function() { assert.equal( - processMultiTenantRequest("credential-options-tenant-id", { - allowMultiTenantAuthentication: true - }), + processMultiTenantRequest("credential-options-tenant-id", true, {}), "credential-options-tenant-id" ); }); it("If the tenant received is the same as the tenant we already had, that same tenant should be returned", async function() { assert.equal( - processMultiTenantRequest("same-tenant", { - allowMultiTenantAuthentication: true, + processMultiTenantRequest("same-tenant", true, { tenantId: "same-tenant" }), "same-tenant" From e39736e1f25fe0a7f5bda36f15c44d5a63ba2734 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Rodr=C3=ADguez?= Date: Tue, 29 Jun 2021 12:49:19 -0400 Subject: [PATCH 11/19] Update sdk/identity/identity/CHANGELOG.md Co-authored-by: Scott Schaab --- sdk/identity/identity/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md index 977ce4a7008c..d2645727e31b 100644 --- a/sdk/identity/identity/CHANGELOG.md +++ b/sdk/identity/identity/CHANGELOG.md @@ -17,7 +17,7 @@ - The `AzureCliCredential` now receives an `AzureCliCredentialOptions` object, which has the same structure of the `TokenCredentialOptions`, but with an extra optional `tenantId`. - The `AzurePowerShellCredential` now receives an `AzurePowerShellCredentialOptions` object, which has the same structure of the `TokenCredentialOptions`, but with an extra optional `tenantId`. - All of the credentials now accept `allowMultiTenantAuthentication`, and as long as they allow configuring a `tenantId`, they now also work with the `GetTokenOptions` new property: `tenantId`. - - If a tenant is specified, any `getToken` request that receives a `tenantId` that doesn't match the credential's `tenantId` will throw an error, unless `getToken` gets called with the `allowMultiTenantAuthentication` property set to true. + - If a tenant is specified, any `getToken` request that receives a `tenantId` that doesn't match the credential's `tenantId` will throw an error, unless the credential has been constructed with the `allowMultiTenantAuthentication` option set to true. - If multi-tenant authentication is allowed, a `tenantId` received through `GetTokenOptions` will be used for that specific request for token instead of the `tenantId` configured when the credential was initialized. ### Breaking Changes From 525730504bc5aec684f021819994cb70c2194c7a Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Tue, 29 Jun 2021 17:03:24 +0000 Subject: [PATCH 12/19] core-auth CHANGELOG update --- sdk/core/core-auth/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/core-auth/CHANGELOG.md b/sdk/core/core-auth/CHANGELOG.md index 71b4e87e0204..aeed19e42657 100644 --- a/sdk/core/core-auth/CHANGELOG.md +++ b/sdk/core/core-auth/CHANGELOG.md @@ -2,7 +2,7 @@ ## 1.3.1 (Unreleased) -- Added `tenantId`, which is used to enable multi-tenant authentication. +- Added `tenantId` optional property to the `GetTokenOptions` interface. If `tenantId` is set, credentials will be able to use multi-tenant authentication, in the cases when it's enabled. ## 1.3.0 (2021-03-30) From b3e30cd0495e6267b3ed62c8cb1d1e962ce6b42b Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Tue, 29 Jun 2021 17:37:31 +0000 Subject: [PATCH 13/19] getAuthorityHost -> getAuthority --- .../identity/src/msal/browserFlows/browserCommon.ts | 6 +++--- sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts | 6 +++--- sdk/identity/identity/src/msal/utils.ts | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts b/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts index b263c628ba4f..610389465ea8 100644 --- a/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts +++ b/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts @@ -8,7 +8,7 @@ import { AccessToken } from "@azure/core-auth"; import { DefaultTenantId } from "../../constants"; import { resolveTenantId } from "../../util/resolveTenantId"; import { BrowserLoginStyle } from "../../credentials/interactiveBrowserCredentialOptions"; -import { getAuthorityHost, getKnownAuthorities, MsalBaseUtilities } from "../utils"; +import { getAuthority, getKnownAuthorities, MsalBaseUtilities } from "../utils"; import { MsalFlow, MsalFlowOptions } from "../flows"; import { AuthenticationRecord } from "../types"; import { CredentialFlowGetTokenOptions } from "../credentials"; @@ -43,7 +43,7 @@ export function defaultBrowserMsalConfig( options: MsalBrowserFlowOptions ): msalBrowser.Configuration { const tenantId = options.tenantId || DefaultTenantId; - const authority = getAuthorityHost(tenantId, options.authorityHost); + const authority = getAuthority(tenantId, options.authorityHost); return { auth: { clientId: options.clientId!, @@ -150,7 +150,7 @@ export abstract class MsalBrowser extends MsalBaseUtilities implements MsalBrows this.allowMultiTenantAuthentication, options )!; - options.authority = getAuthorityHost(tenantId, this.authorityHost); + options.authority = getAuthority(tenantId, this.authorityHost); // We ensure that redirection is handled at this point. await this.handleRedirect(); diff --git a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts index 664a108c72a2..ceaec0e3c656 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts @@ -16,7 +16,7 @@ import { AuthenticationRequiredError } from "../errors"; import { AuthenticationRecord } from "../types"; import { defaultLoggerCallback, - getAuthorityHost, + getAuthority, getKnownAuthorities, MsalBaseUtilities, msalToPublic, @@ -112,7 +112,7 @@ export abstract class MsalNode extends MsalBaseUtilities implements MsalFlow { const tenantId = resolveTenantId(options.logger, options.tenantId, options.clientId); this.authorityHost = options.authorityHost || process.env.AZURE_AUTHORITY_HOST; - const authority = getAuthorityHost(tenantId, this.authorityHost); + const authority = getAuthority(tenantId, this.authorityHost); this.identityClient = new IdentityClient({ ...options.tokenCredentialOptions, @@ -273,7 +273,7 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov this.allowMultiTenantAuthentication, options )!; - options.authority = getAuthorityHost(tenantId, this.authorityHost); + options.authority = getAuthority(tenantId, this.authorityHost); options.correlationId = options?.correlationId || this.generateUuid(); await this.init(options); diff --git a/sdk/identity/identity/src/msal/utils.ts b/sdk/identity/identity/src/msal/utils.ts index ccf0a5170257..007bc315b30a 100644 --- a/sdk/identity/identity/src/msal/utils.ts +++ b/sdk/identity/identity/src/msal/utils.ts @@ -51,10 +51,10 @@ export function ensureValidMsalToken( } /** - * Generates a valid authorityHost by combining a host with a tenantId. + * Generates a valid authority by combining a host with a tenantId. * @internal */ -export function getAuthorityHost(tenantId: string, host: string = DefaultAuthorityHost): string { +export function getAuthority(tenantId: string, host: string = DefaultAuthorityHost): string { if (host.endsWith("/")) { return host + tenantId; } else { @@ -204,7 +204,7 @@ export function publicToMsal(account: AuthenticationRecord): msalCommon.AccountI export function msalToPublic(clientId: string, account: MsalAccountInfo): AuthenticationRecord { const record = { - authority: getAuthorityHost(account.tenantId, account.environment), + authority: getAuthority(account.tenantId, account.environment), homeAccountId: account.homeAccountId, tenantId: account.tenantId || DefaultTenantId, username: account.username, From 09a7b569a602be88f96641b2aff09ae540e4f60f Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Wed, 30 Jun 2021 17:31:55 +0000 Subject: [PATCH 14/19] options.authority feedback from Will --- sdk/identity/identity/src/msal/browserFlows/browserCommon.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts b/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts index 900e1b690698..6383344611da 100644 --- a/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts +++ b/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts @@ -151,7 +151,10 @@ export abstract class MsalBrowser extends MsalBaseUtilities implements MsalBrows this.allowMultiTenantAuthentication, options )!; - options.authority = getAuthority(tenantId, this.authorityHost); + + if (!options.authority) { + options.authority = getAuthority(tenantId, this.authorityHost); + } // We ensure that redirection is handled at this point. await this.handleRedirect(); From b329866a6cbcd9f307d81868ce1afcf9a63d7019 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Wed, 30 Jun 2021 18:24:35 +0000 Subject: [PATCH 15/19] validating tenants if provided to the CLI and the powershell credential --- sdk/identity/identity/src/credentials/azureCliCredential.ts | 5 +++++ .../identity/src/credentials/azurePowerShellCredential.ts | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/sdk/identity/identity/src/credentials/azureCliCredential.ts b/sdk/identity/identity/src/credentials/azureCliCredential.ts index 43f441d10933..1b45cc79e462 100644 --- a/sdk/identity/identity/src/credentials/azureCliCredential.ts +++ b/sdk/identity/identity/src/credentials/azureCliCredential.ts @@ -11,6 +11,7 @@ import * as child_process from "child_process"; import { ensureValidScope, getScopeResource } from "../util/scopeUtils"; import { AzureCliCredentialOptions } from "./azureCliCredentialOptions"; import { processMultiTenantRequest } from "../util/validateMultiTenant"; +import { checkTenantId } from "../util/checkTenantId"; /** * Mockable reference to the CLI credential cliCredentialFunctions @@ -110,6 +111,10 @@ export class AzureCliCredential implements TokenCredential { this.allowMultiTenantAuthentication, options ); + if (tenantId) { + checkTenantId(logger, tenantId); + } + const scope = typeof scopes === "string" ? scopes : scopes[0]; logger.getToken.info(`Using the scope ${scope}`); ensureValidScope(scope, logger); diff --git a/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts b/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts index 49ad615248f6..b888e989460c 100644 --- a/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts +++ b/sdk/identity/identity/src/credentials/azurePowerShellCredential.ts @@ -10,6 +10,7 @@ import { ensureValidScope, getScopeResource } from "../util/scopeUtils"; import { processUtils } from "../util/processUtils"; import { AzurePowerShellCredentialOptions } from "./azurePowerShellCredentialOptions"; import { processMultiTenantRequest } from "../util/validateMultiTenant"; +import { checkTenantId } from "../util/checkTenantId"; const logger = credentialLogger("AzurePowerShellCredential"); @@ -171,6 +172,10 @@ export class AzurePowerShellCredential implements TokenCredential { this.allowMultiTenantAuthentication, options ); + if (tenantId) { + checkTenantId(logger, tenantId); + } + const scope = typeof scopes === "string" ? scopes : scopes[0]; ensureValidScope(scope, logger); logger.getToken.info(`Using the scope ${scope}`); From 0983549b7f111ec0d0ee00f938aa7122af27f982 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Wed, 30 Jun 2021 18:34:38 +0000 Subject: [PATCH 16/19] validateMultiTenant error message --- sdk/identity/identity/src/util/validateMultiTenant.ts | 7 +++---- sdk/identity/identity/test/internal/utils.spec.ts | 9 ++++++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/sdk/identity/identity/src/util/validateMultiTenant.ts b/sdk/identity/identity/src/util/validateMultiTenant.ts index 27ec12207db4..7976b0011f14 100644 --- a/sdk/identity/identity/src/util/validateMultiTenant.ts +++ b/sdk/identity/identity/src/util/validateMultiTenant.ts @@ -6,9 +6,8 @@ import { GetTokenOptions } from "@azure/core-auth"; /** * @internal */ -export const multiTenantError = new Error( - "Multi-tenant authentication was attempted, but multi-tenant authentication was not enabled in this credential instance." -); +export const multiTenantErrorMessage = + "A getToken request was attempted with a tenant different than the tenant configured at the initialization of the credential, but multi-tenant authentication was not enabled in this credential instance."; /** * Verifies whether locally assigned tenants are equal to tenants received through getToken. @@ -26,7 +25,7 @@ export function processMultiTenantRequest( tenantId && getTokenOptions.tenantId !== tenantId ) { - throw multiTenantError; + throw new Error(multiTenantErrorMessage); } if (allowMultiTenantAuthentication && getTokenOptions?.tenantId) { return getTokenOptions.tenantId; diff --git a/sdk/identity/identity/test/internal/utils.spec.ts b/sdk/identity/identity/test/internal/utils.spec.ts index fe93a041c7c1..c3189bc81e4f 100644 --- a/sdk/identity/identity/test/internal/utils.spec.ts +++ b/sdk/identity/identity/test/internal/utils.spec.ts @@ -2,7 +2,10 @@ // Licensed under the MIT license. import assert from "assert"; -import { multiTenantError, processMultiTenantRequest } from "../../src/util/validateMultiTenant"; +import { + multiTenantErrorMessage, + processMultiTenantRequest +} from "../../src/util/validateMultiTenant"; describe("Identity utilities", function() { describe("validateMultiTenantRequest", function() { @@ -19,7 +22,7 @@ describe("Identity utilities", function() { error, "validateMultiTenantRequest should throw if multi-tenant is disallowed and the tenants don't match" ); - assert.equal(error!.message, multiTenantError.message); + assert.equal(error!.message, multiTenantErrorMessage); }); it("throws if multi-tenant authentication is undefined, and the tenants don't match", async function() { @@ -35,7 +38,7 @@ describe("Identity utilities", function() { error, "validateMultiTenantRequest should throw if multi-tenant is disallowed and the tenants don't match" ); - assert.equal(error!.message, multiTenantError.message); + assert.equal(error!.message, multiTenantErrorMessage); }); it("If allowMultiTenantAuthentication is disallowed, it shouldn't throw if the tenant received is the same as the tenant we already had, that same tenant should be returned", async function() { From f49f32b0d7b9856afc667b052ac042e9d1b6930e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Rodr=C3=ADguez?= Date: Wed, 30 Jun 2021 16:20:25 -0400 Subject: [PATCH 17/19] Update sdk/identity/identity/CHANGELOG.md Co-authored-by: chradek <51000525+chradek@users.noreply.github.com> --- sdk/identity/identity/CHANGELOG.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md index 37cba9adcfdc..9d9e1c2d9570 100644 --- a/sdk/identity/identity/CHANGELOG.md +++ b/sdk/identity/identity/CHANGELOG.md @@ -17,11 +17,8 @@ - Added `regionalAuthority` property to `ClientSecretCredentialOptions` and `ClientCertificateCredentialOptions`. - If instead of a region, `AutoDiscoverRegion` is specified as the value for `regionalAuthority`, MSAL will be used to attempt to discover the region. - A region can also be specified through the `AZURE_REGIONAL_AUTHORITY_NAME` environment variable. -- The `AzureCliCredential` now receives an `AzureCliCredentialOptions` object, which has the same structure of the `TokenCredentialOptions`, but with an extra optional `tenantId`. -- The `AzurePowerShellCredential` now receives an `AzurePowerShellCredentialOptions` object, which has the same structure of the `TokenCredentialOptions`, but with an extra optional `tenantId`. -- All of the credentials now accept `allowMultiTenantAuthentication`, and as long as they allow configuring a `tenantId`, they now also work with the `GetTokenOptions` new property: `tenantId`. - - If a tenant is specified, any `getToken` request that receives a `tenantId` that doesn't match the credential's `tenantId` will throw an error, unless the credential has been constructed with the `allowMultiTenantAuthentication` option set to true. - - If multi-tenant authentication is allowed, a `tenantId` received through `GetTokenOptions` will be used for that specific request for token instead of the `tenantId` configured when the credential was initialized. +- `AzureCliCredential` and `AzurePowerShellCredential` now allow specifying a `tenantId`. +- All credentials except `ManagedIdentityCredential` support enabling multi tenant authentication via the `allowMultiTenantAuthentication` option. ### Breaking Changes From 723f82535a8e5f1bd9ec4f33d0efa3455534ce4f Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Wed, 30 Jun 2021 20:25:40 +0000 Subject: [PATCH 18/19] feedback by Christopher Radek --- .../identity/src/credentials/authorizationCodeCredential.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts index 09928ae5c670..27931429580c 100644 --- a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts +++ b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts @@ -107,7 +107,6 @@ export class AuthorizationCodeCredential implements TokenCredential { this.clientId = clientId; this.tenantId = tenantId; - this.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication; if (typeof redirectUriOrOptions === "string") { // the clientId+clientSecret constructor @@ -123,6 +122,7 @@ export class AuthorizationCodeCredential implements TokenCredential { options = redirectUriOrOptions as TokenCredentialOptions; } + this.allowMultiTenantAuthentication = options?.allowMultiTenantAuthentication; this.identityClient = new IdentityClient(options); } From e63a14bb24d66ec64b3b7d01632dc76c547a26c8 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Wed, 30 Jun 2021 21:13:16 +0000 Subject: [PATCH 19/19] safer than non-null assertions --- .../src/credentials/authorizationCodeCredential.ts | 8 +++----- .../src/credentials/visualStudioCodeCredential.ts | 9 ++++----- .../identity/src/msal/browserFlows/browserCommon.ts | 8 +++----- sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts | 9 ++++----- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts index 27931429580c..acb08813efc8 100644 --- a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts +++ b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts @@ -138,11 +138,9 @@ export class AuthorizationCodeCredential implements TokenCredential { scopes: string | string[], options?: GetTokenOptions ): Promise { - const tenantId = processMultiTenantRequest( - this.tenantId, - this.allowMultiTenantAuthentication, - options - )!; + const tenantId = + processMultiTenantRequest(this.tenantId, this.allowMultiTenantAuthentication, options) || + this.tenantId; const { span, updatedOptions } = createSpan("AuthorizationCodeCredential-getToken", options); try { diff --git a/sdk/identity/identity/src/credentials/visualStudioCodeCredential.ts b/sdk/identity/identity/src/credentials/visualStudioCodeCredential.ts index d3e3ebb1911d..aa479c67876f 100644 --- a/sdk/identity/identity/src/credentials/visualStudioCodeCredential.ts +++ b/sdk/identity/identity/src/credentials/visualStudioCodeCredential.ts @@ -174,11 +174,10 @@ export class VisualStudioCodeCredential implements TokenCredential { options?: GetTokenOptions ): Promise { await this.prepareOnce(); - const tenantId = processMultiTenantRequest( - this.tenantId, - this.allowMultiTenantAuthentication, - options - )!; + + const tenantId = + processMultiTenantRequest(this.tenantId, this.allowMultiTenantAuthentication, options) || + this.tenantId; if (findCredentials === undefined) { throw new CredentialUnavailableError( diff --git a/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts b/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts index 6383344611da..6f8a91127f04 100644 --- a/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts +++ b/sdk/identity/identity/src/msal/browserFlows/browserCommon.ts @@ -146,11 +146,9 @@ export abstract class MsalBrowser extends MsalBaseUtilities implements MsalBrows scopes: string[], options: CredentialFlowGetTokenOptions = {} ): Promise { - const tenantId = processMultiTenantRequest( - this.tenantId, - this.allowMultiTenantAuthentication, - options - )!; + const tenantId = + processMultiTenantRequest(this.tenantId, this.allowMultiTenantAuthentication, options) || + this.tenantId; if (!options.authority) { options.authority = getAuthority(tenantId, this.authorityHost); diff --git a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts index ceaec0e3c656..6ed484c075b9 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts @@ -268,11 +268,10 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov scopes: string[], options: CredentialFlowGetTokenOptions = {} ): Promise { - const tenantId = processMultiTenantRequest( - this.tenantId, - this.allowMultiTenantAuthentication, - options - )!; + const tenantId = + processMultiTenantRequest(this.tenantId, this.allowMultiTenantAuthentication, options) || + this.tenantId; + options.authority = getAuthority(tenantId, this.authorityHost); options.correlationId = options?.correlationId || this.generateUuid();