Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Identity] AuthenticationRequiredError options #17987

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -53,9 +53,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."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this one still silent auth?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep yep, this is after the user authenticates, we should be able to retrieve their information from memory to use it.

How browser auth works on Identity is that before the refresh, getToken needs to be called — after the refresh, it also needs to be called, but by that time, it reads from the cache.

We could rephrase this a bit if that helps.

});
}

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