From fd3bd640517a8a15ca133c2762c6b336c368eb16 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Mon, 12 Apr 2021 17:32:37 +0000 Subject: [PATCH 1/3] [Identity] UsernamePasswordCredential API alignment --- sdk/identity/identity/review/identity.api.md | 4 +-- .../credentials/usernamePasswordCredential.ts | 31 +------------------ .../usernamePasswordCredentialOptions.ts | 17 ++++++++-- 3 files changed, 18 insertions(+), 34 deletions(-) diff --git a/sdk/identity/identity/review/identity.api.md b/sdk/identity/identity/review/identity.api.md index 4a37110f3c03..2487257d87c7 100644 --- a/sdk/identity/identity/review/identity.api.md +++ b/sdk/identity/identity/review/identity.api.md @@ -232,12 +232,12 @@ export interface TokenCredentialOptions extends PipelineOptions { // @public export class UsernamePasswordCredential implements TokenCredential { constructor(tenantId: string, clientId: string, username: string, password: string, options?: UsernamePasswordCredentialOptions); - authenticate(scopes: string | string[], options?: GetTokenOptions): Promise; getToken(scopes: string | string[], options?: GetTokenOptions): Promise; } // @public -export interface UsernamePasswordCredentialOptions extends InteractiveCredentialOptions { +export interface UsernamePasswordCredentialOptions extends TokenCredentialOptions { + tokenCachePersistenceOptions?: TokenCachePersistenceOptions; } // @public diff --git a/sdk/identity/identity/src/credentials/usernamePasswordCredential.ts b/sdk/identity/identity/src/credentials/usernamePasswordCredential.ts index eb2a0763680b..f7ffeefc0883 100644 --- a/sdk/identity/identity/src/credentials/usernamePasswordCredential.ts +++ b/sdk/identity/identity/src/credentials/usernamePasswordCredential.ts @@ -6,7 +6,6 @@ import { credentialLogger } from "../util/logging"; import { MsalUsernamePassword } from "../msal/nodeFlows/msalUsernamePassword"; import { MsalFlow } from "../msal/flows"; import { trace } from "../util/tracing"; -import { AuthenticationRecord } from "../msal/types"; import { UsernamePasswordCredentialOptions } from "./usernamePasswordCredentialOptions"; const logger = credentialLogger("UsernamePasswordCredential"); @@ -21,7 +20,6 @@ const logger = credentialLogger("UsernamePasswordCredential"); // to reduce the number of times we send the password over the network. export class UsernamePasswordCredential implements TokenCredential { private msalFlow: MsalFlow; - private disableAutomaticAuthentication?: boolean; /** * Creates an instance of the UsernamePasswordCredential with the details @@ -50,7 +48,6 @@ export class UsernamePasswordCredential implements TokenCredential { password, tokenCredentialOptions: options || {} }); - this.disableAutomaticAuthentication = options?.disableAutomaticAuthentication; } /** @@ -70,33 +67,7 @@ export class UsernamePasswordCredential implements TokenCredential { async getToken(scopes: string | string[], options: GetTokenOptions = {}): Promise { return trace(`${this.constructor.name}.getToken`, options, async (newOptions) => { const arrayScopes = Array.isArray(scopes) ? scopes : [scopes]; - return this.msalFlow.getToken(arrayScopes, { - ...newOptions, - disableAutomaticAuthentication: this.disableAutomaticAuthentication - }); - }); - } - - /** - * Authenticates with Azure Active Directory and returns an access token if - * successful. If authentication cannot be performed at this time, this method may - * return null. If an error occurs during authentication, an {@link AuthenticationError} - * containing failure details will be thrown. - * - * If the token can't be retrieved silently, this method will require user interaction to retrieve the token. - * - * @param scopes - The list of scopes for which the token will have access. - * @param options - The options used to configure any requests this - * TokenCredential implementation might make. - */ - async authenticate( - scopes: string | string[], - options: GetTokenOptions = {} - ): Promise { - return trace(`${this.constructor.name}.authenticate`, options, async (newOptions) => { - const arrayScopes = Array.isArray(scopes) ? scopes : [scopes]; - await this.msalFlow.getToken(arrayScopes, newOptions); - return this.msalFlow.getActiveAccount(); + return this.msalFlow.getToken(arrayScopes, newOptions); }); } } diff --git a/sdk/identity/identity/src/credentials/usernamePasswordCredentialOptions.ts b/sdk/identity/identity/src/credentials/usernamePasswordCredentialOptions.ts index 430e8e96adfd..2a50bd732186 100644 --- a/sdk/identity/identity/src/credentials/usernamePasswordCredentialOptions.ts +++ b/sdk/identity/identity/src/credentials/usernamePasswordCredentialOptions.ts @@ -1,9 +1,22 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { InteractiveCredentialOptions } from "./interactiveCredentialOptions"; +import { TokenCredentialOptions } from "../client/identityClient"; +import { TokenCachePersistenceOptions } from "../tokenCache/persistencePlatforms"; /** * Defines options for the {@link UsernamePasswordCredential} class. */ -export interface UsernamePasswordCredentialOptions extends InteractiveCredentialOptions {} +export interface UsernamePasswordCredentialOptions extends TokenCredentialOptions { + /** + * To provide a persistence layer to store the credentials, + * we allow users to optionally specify {@link TokenCachePersistenceOptions} for their credential. + * + * This feature is not currently available on Node 8 or earlier versions of Node JS. + * + * This persistence layer uses DPAPI on Windows. + * On OSX (Darwin) it tries to use the system's Keychain, otherwise if the property `allowUnencryptedStorage` is set to true, it uses an unencrypted file. + * On Linux it tries to use the system's Keyring, otherwise if the property `allowUnencryptedStorage` is set to true, it uses an unencrypted file. + */ + tokenCachePersistenceOptions?: TokenCachePersistenceOptions; +} From 95380a8236859c6d00c7df144a1cda7e70c00bd8 Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Mon, 12 Apr 2021 17:34:30 +0000 Subject: [PATCH 2/3] CHANGELOG entry --- sdk/identity/identity/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md index 64570f20644c..db417b9a091f 100644 --- a/sdk/identity/identity/CHANGELOG.md +++ b/sdk/identity/identity/CHANGELOG.md @@ -2,6 +2,7 @@ ## 2.0.0-beta.3 (Unreleased) +- Removed `authenticationRecord`, `disableAutomaticAuthentication` and `authenticate()` from the credential `UsernamePasswordCredential`. While MSAL does support this, allowing `authenticationRecord` arguably could result in users authenticating through an account other than the one they're specifying with the username and the password. - The feature of persistence caching of credentials (introduced in 2.0.0-beta.1) is now supported on Node.js 15 as well. - `AuthenticationRequiredError` (introduced in 2.0.0-beta.1) now has the same impact on `ChainedTokenCredential` as the `CredentialUnavailableError` which is to allow the next credential in the chain to be tried. From 8e548eb6b800d491a6a3d6dc1f2fd3ab698500ae Mon Sep 17 00:00:00 2001 From: Daniel Rodriguez Date: Mon, 12 Apr 2021 22:58:30 +0000 Subject: [PATCH 3/3] separate section for breaking changes from 2.0.0-beta.1 --- sdk/identity/identity/CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md index db417b9a091f..2a185e12eae1 100644 --- a/sdk/identity/identity/CHANGELOG.md +++ b/sdk/identity/identity/CHANGELOG.md @@ -2,10 +2,15 @@ ## 2.0.0-beta.3 (Unreleased) -- Removed `authenticationRecord`, `disableAutomaticAuthentication` and `authenticate()` from the credential `UsernamePasswordCredential`. While MSAL does support this, allowing `authenticationRecord` arguably could result in users authenticating through an account other than the one they're specifying with the username and the password. +### New features + - The feature of persistence caching of credentials (introduced in 2.0.0-beta.1) is now supported on Node.js 15 as well. - `AuthenticationRequiredError` (introduced in 2.0.0-beta.1) now has the same impact on `ChainedTokenCredential` as the `CredentialUnavailableError` which is to allow the next credential in the chain to be tried. +### Breaking changes from 2.0.0-beta.1 + +- Removed `authenticationRecord`, `disableAutomaticAuthentication` and `authenticate()` from the credential `UsernamePasswordCredential`. While MSAL does support this, allowing `authenticationRecord` arguably could result in users authenticating through an account other than the one they're specifying with the username and the password. + ## 2.0.0-beta.2 (2021-04-06) - Breaking change: Renamed errors `CredentialUnavailable` to `CredentialUnavailableError`, and `AuthenticationRequired` to `AuthenticationRequiredError`, to align with the naming convention used for error classes in the Azure SDKs in JavaScript.