Skip to content

Commit

Permalink
[Identity] AuthenticationRequiredError options (#17987)
Browse files Browse the repository at this point in the history
This PR comes from the feedback from @xirzec on the API review for the upcoming Identity release. The feedback stated that these optional parameters to AuthenticationRequiredError should be in an options bag, and that the `getTokenOptions` public property in that error class should be optional.

This PR:
- Moves the optional parameters of AuthenticationRequiredError to an options bag.
- And also moves the two errors we expose into the same file, in a more convenient location.

Feedback appreciated 🙏
  • Loading branch information
sadasant authored Oct 11, 2021
1 parent 0d80df2 commit 7a12b14
Show file tree
Hide file tree
Showing 20 changed files with 121 additions and 76 deletions.
2 changes: 2 additions & 0 deletions sdk/identity/identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ Azure Service Fabric support hasn't been added on the initial version 2 of Ident
- Renamed the `ApplicationCredential` to `AzureApplicationCredential`.
- Removed the `CredentialPersistenceOptions` from `DefaultAzureCredential` and `EnvironmentCredential`.
- Merged the configuration and the options bag on the `OnBehalfOfCredential` into a single options bag.
- `AuthenticationRequiredError` (introduced in 2.0.0-beta.1) now has its parameters into a single options bag.
- `AuthenticationRequiredError` (introduced in 2.0.0-beta.1) now has its parameters in a single options bag, `AuthenticationRequiredErrorOptions`.

### Bugs Fixed

Expand Down
12 changes: 9 additions & 3 deletions sdk/identity/identity/review/identity.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,15 @@ export interface AuthenticationRecord {
// @public
export class AuthenticationRequiredError extends Error {
constructor(
scopes: string[],
getTokenOptions?: GetTokenOptions, message?: string);
getTokenOptions: GetTokenOptions;
options: AuthenticationRequiredErrorOptions);
getTokenOptions?: GetTokenOptions;
scopes: string[];
}

// @public
export interface AuthenticationRequiredErrorOptions {
getTokenOptions?: GetTokenOptions;
message?: string;
scopes: string[];
}

Expand Down
2 changes: 1 addition & 1 deletion sdk/identity/identity/src/client/identityClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
PipelineRequest
} from "@azure/core-rest-pipeline";
import { AbortController, AbortSignalLike } from "@azure/abort-controller";
import { AuthenticationError, AuthenticationErrorName } from "./errors";
import { AuthenticationError, AuthenticationErrorName } from "../errors";
import { getIdentityTokenEndpointSuffix } from "../util/identityTokenEndpoint";
import { DefaultAuthorityHost } from "../constants";
import { createSpan } from "../util/tracing";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { TokenCredential, GetTokenOptions, AccessToken } from "@azure/core-auth";

import { createSpan } from "../util/tracing";
import { CredentialUnavailableError } from "../client/errors";
import { CredentialUnavailableError } from "../errors";
import { SpanStatusCode } from "@azure/core-tracing";
import { credentialLogger, formatSuccess, formatError } from "../util/logging";
import child_process from "child_process";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

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

import { CredentialUnavailableError } from "../client/errors";
import { CredentialUnavailableError } from "../errors";
import { credentialLogger, formatSuccess, formatError } from "../util/logging";
import { trace } from "../util/tracing";
import { ensureValidScope, getScopeResource } from "../util/scopeUtils";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

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

import { AggregateAuthenticationError, CredentialUnavailableError } from "../client/errors";
import { AggregateAuthenticationError, CredentialUnavailableError } from "../errors";
import { createSpan } from "../util/tracing";
import { SpanStatusCode } from "@azure/core-tracing";
import { credentialLogger, formatSuccess, formatError } from "../util/logging";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { AccessToken, TokenCredential, GetTokenOptions } from "@azure/core-auth"
import { credentialLogger, processEnvVars, formatSuccess, formatError } from "../util/logging";
import { TokenCredentialOptions } from "../client/identityClient";
import { ClientSecretCredential } from "./clientSecretCredential";
import { AuthenticationError, CredentialUnavailableError } from "../client/errors";
import { AuthenticationError, CredentialUnavailableError } from "../errors";
import { checkTenantId } from "../util/checkTenantId";
import { trace } from "../util/tracing";
import { ClientCertificateCredential } from "./clientCertificateCredential";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { credentialLogger } from "../../util/logging";
import { IdentityClient } from "../../client/identityClient";
import { mapScopesToResource, msiGenericGetToken } from "./utils";
import { azureArcAPIVersion } from "./constants";
import { AuthenticationError } from "../../client/errors";
import { AuthenticationError } from "../../errors";

const msiName = "ManagedIdentityCredential - Azure Arc MSI";
const logger = credentialLogger(msiName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { createSpan } from "../../util/tracing";
import { imdsApiVersion, imdsEndpointPath, imdsHost } from "./constants";
import { MSI, MSIConfiguration } from "./models";
import { mapScopesToResource, msiGenericGetToken } from "./utils";
import { AuthenticationError } from "../../client/errors";
import { AuthenticationError } from "../../errors";

const msiName = "ManagedIdentityCredential - IMDS";
const logger = credentialLogger(msiName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { SpanStatusCode } from "@azure/core-tracing";
import { AccessToken, GetTokenOptions, TokenCredential } from "@azure/core-auth";

import { IdentityClient, TokenCredentialOptions } from "../../client/identityClient";
import { AuthenticationError, CredentialUnavailableError } from "../../client/errors";
import { AuthenticationError, CredentialUnavailableError } from "../../errors";
import { credentialLogger, formatSuccess, formatError } from "../../util/logging";
import { appServiceMsi2017 } from "./appServiceMsi2017";
import { createSpan } from "../../util/tracing";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import fs from "fs";
import os from "os";
import path from "path";

import { CredentialUnavailableError } from "../client/errors";
import { CredentialUnavailableError } from "../errors";
import { IdentityClient, TokenCredentialOptions } from "../client/identityClient";
import { AzureAuthorityHosts } from "../constants";
import { checkTenantId } from "../util/checkTenantId";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

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

/**
* See the official documentation for more details:
*
Expand Down Expand Up @@ -182,3 +184,47 @@ function convertOAuthErrorResponseToErrorResponse(errorBody: OAuthErrorResponse)
traceId: errorBody.trace_id
};
}

/**
* Optional parameters to the {@link AuthenticationRequiredError}
*/
export interface AuthenticationRequiredErrorOptions {
/**
* The list of scopes for which the token will have access.
*/
scopes: string[];
/**
* The options passed to the getToken request.
*/
getTokenOptions?: GetTokenOptions;
/**
* The message of the error.
*/
message?: string;
}

/**
* Error used to enforce authentication after trying to retrieve a token silently.
*/
export class AuthenticationRequiredError extends Error {
/**
* The list of scopes for which the token will have access.
*/
public scopes: string[];
/**
* The options passed to the getToken request.
*/
public getTokenOptions?: GetTokenOptions;

constructor(
/**
* Optional parameters. A message can be specified. The {@link GetTokenOptions} of the request can also be specified to more easily associate the error with the received parameters.
*/
options: AuthenticationRequiredErrorOptions
) {
super(options.message);
this.scopes = options.scopes;
this.getTokenOptions = options.getTokenOptions;
this.name = "AuthenticationRequiredError";
}
}
23 changes: 12 additions & 11 deletions sdk/identity/identity/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,19 @@ export { IdentityPlugin } from "./plugins/provider";
import { TokenCredential } from "@azure/core-auth";
import { DefaultAzureCredential } from "./credentials/defaultAzureCredential";

export {
AuthenticationError,
ErrorResponse,
AggregateAuthenticationError,
AuthenticationErrorName,
AggregateAuthenticationErrorName,
CredentialUnavailableError,
CredentialUnavailableErrorName,
AuthenticationRequiredError,
AuthenticationRequiredErrorOptions
} from "./errors";

export { AuthenticationRecord } from "./msal/types";
export { AuthenticationRequiredError } from "./msal/errors";
export { serializeAuthenticationRecord, deserializeAuthenticationRecord } from "./msal/utils";
export { TokenCredentialOptions } from "./client/identityClient";

Expand Down Expand Up @@ -71,16 +82,6 @@ export {

export { TokenCachePersistenceOptions } from "./msal/nodeFlows/tokenCachePersistenceOptions";

export {
AuthenticationError,
ErrorResponse,
AggregateAuthenticationError,
AuthenticationErrorName,
AggregateAuthenticationErrorName,
CredentialUnavailableError,
CredentialUnavailableErrorName
} from "./client/errors";

export { TokenCredential, GetTokenOptions, AccessToken } from "@azure/core-auth";
export { logger } from "./util/logging";

Expand Down
14 changes: 7 additions & 7 deletions sdk/identity/identity/src/msal/browserFlows/browserCommon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@ import { AccessToken } from "@azure/core-auth";

import { DefaultTenantId } from "../../constants";
import { resolveTenantId } from "../../util/resolveTenantId";
import { processMultiTenantRequest } from "../../util/validateMultiTenant";
import { BrowserLoginStyle } from "../../credentials/interactiveBrowserCredentialOptions";
import { AuthenticationRequiredError, CredentialUnavailableError } from "../../errors";
import { getAuthority, getKnownAuthorities, MsalBaseUtilities } from "../utils";
import { MsalFlow, MsalFlowOptions } from "../flows";
import { AuthenticationRecord } from "../types";
import { CredentialFlowGetTokenOptions } from "../credentials";
import { AuthenticationRequiredError } from "../errors";
import { CredentialUnavailableError } from "../../client/errors";
import { processMultiTenantRequest } from "../../util/validateMultiTenant";

/**
* Union of the constructor parameters that all MSAL flow types take.
Expand Down Expand Up @@ -160,11 +159,12 @@ export abstract class MsalBrowser extends MsalBaseUtilities implements MsalBrows
throw err;
}
if (options?.disableAutomaticAuthentication) {
throw new AuthenticationRequiredError(
throw new AuthenticationRequiredError({
scopes,
options,
"Automatic authentication has been disabled. You may call the authentication() method."
);
getTokenOptions: options,
message:
"Automatic authentication has been disabled. You may call the authentication() method."
});
}
this.logger.info(
`Silent authentication failed, falling back to interactive method ${this.loginStyle}`
Expand Down
18 changes: 14 additions & 4 deletions sdk/identity/identity/src/msal/browserFlows/msalAuthCode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ import * as msalBrowser from "@azure/msal-browser";

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

import { MsalBrowserFlowOptions, MsalBrowser } from "./browserCommon";
import { AuthenticationRequiredError } from "../../errors";
import { defaultLoggerCallback, msalToPublic, publicToMsal } from "../utils";
import { AuthenticationRecord } from "../types";
import { AuthenticationRequiredError } from "../errors";
import { CredentialFlowGetTokenOptions } from "../credentials";
import { MsalBrowserFlowOptions, MsalBrowser } from "./browserCommon";

// We keep a copy of the redirect hash.
const redirectHash = self.location.hash;
Expand Down Expand Up @@ -158,7 +158,12 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
): Promise<AccessToken> {
const account = await this.getActiveAccount();
if (!account) {
throw new AuthenticationRequiredError(scopes, options);
throw new AuthenticationRequiredError({
scopes,
getTokenOptions: options,
message:
"Silent authentication failed. We couldn't retrieve an active account from the cache."
});
}

const parameters: msalBrowser.SilentRequest = {
Expand Down Expand Up @@ -187,7 +192,12 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
): Promise<AccessToken> {
const account = await this.getActiveAccount();
if (!account) {
throw new AuthenticationRequiredError(scopes, options);
throw new AuthenticationRequiredError({
scopes,
getTokenOptions: options,
message:
"Silent authentication failed. We couldn't retrieve an active account from the cache."
});
}

const parameters: msalBrowser.RedirectRequest = {
Expand Down
24 changes: 0 additions & 24 deletions sdk/identity/identity/src/msal/errors.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import open from "open";
import stoppable from "stoppable";

import { credentialLogger, formatError, formatSuccess } from "../../util/logging";
import { CredentialUnavailableError } from "../../errors";
import { MsalNodeOptions, MsalNode } from "./nodeCommon";
import { msalToPublic } from "../utils";
import { CredentialUnavailableError } from "../../client/errors";
import { CredentialFlowGetTokenOptions } from "../credentials";

/**
Expand Down
18 changes: 12 additions & 6 deletions sdk/identity/identity/src/msal/nodeFlows/nodeCommon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ import { AbortSignalLike } from "@azure/abort-controller";
import { DeveloperSignOnClientId } from "../../constants";
import { IdentityClient, TokenCredentialOptions } from "../../client/identityClient";
import { resolveTenantId } from "../../util/resolveTenantId";
import { AuthenticationRequiredError } from "../../errors";
import { CredentialFlowGetTokenOptions } from "../credentials";
import { MsalFlow, MsalFlowOptions } from "../flows";
import { AuthenticationRequiredError } from "../errors";
import { AuthenticationRecord } from "../types";
import {
defaultLoggerCallback,
Expand Down Expand Up @@ -242,7 +242,12 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
): Promise<AccessToken> {
await this.getActiveAccount();
if (!this.account) {
throw new AuthenticationRequiredError(scopes, options);
throw new AuthenticationRequiredError({
scopes,
getTokenOptions: options,
message:
"Silent authentication failed. We couldn't retrieve an active account from the cache."
});
}

const silentRequest: msalNode.SilentFlowRequest = {
Expand Down Expand Up @@ -291,11 +296,12 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
throw err;
}
if (options?.disableAutomaticAuthentication) {
throw new AuthenticationRequiredError(
throw new AuthenticationRequiredError({
scopes,
options,
"Automatic authentication has been disabled. You may call the authentication() method."
);
getTokenOptions: options,
message:
"Automatic authentication has been disabled. You may call the authentication() method."
});
}
this.logger.info(`Silent authentication failed, falling back to interactive method.`);
return this.doGetToken(scopes, options);
Expand Down
11 changes: 5 additions & 6 deletions sdk/identity/identity/src/msal/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@ import { AbortError } from "@azure/abort-controller";

import { v4 as uuidv4 } from "uuid";
import { CredentialLogger, formatError, formatSuccess } from "../util/logging";
import { CredentialUnavailableError } from "../client/errors";
import { CredentialUnavailableError, AuthenticationRequiredError } from "../errors";
import { DefaultAuthorityHost, DefaultTenantId } from "../constants";
import { AuthenticationRecord, MsalAccountInfo, MsalResult, MsalToken } from "./types";
import { AuthenticationRequiredError } from "./errors";
import { MsalFlowOptions } from "./flows";

/**
Expand All @@ -32,11 +31,11 @@ export function ensureValidMsalToken(
): void {
const error = (message: string): Error => {
logger.getToken.info(message);
return new AuthenticationRequiredError(
Array.isArray(scopes) ? scopes : [scopes],
return new AuthenticationRequiredError({
scopes: Array.isArray(scopes) ? scopes : [scopes],
getTokenOptions,
message
);
});
};
if (!msalToken) {
throw error("No response");
Expand Down Expand Up @@ -190,7 +189,7 @@ export class MsalBaseUtilities {
) {
return error;
}
return new AuthenticationRequiredError(scopes, getTokenOptions, error.message);
return new AuthenticationRequiredError({ scopes, getTokenOptions, message: error.message });
}
}

Expand Down
Loading

0 comments on commit 7a12b14

Please sign in to comment.