Skip to content

Commit

Permalink
[identity] Migrate UsernamePasswordCredential to MSALClient (#29656)
Browse files Browse the repository at this point in the history
### Packages impacted by this PR

@azure/identity

### Issues associated with this PR

Resolves #29409 

### Describe the problem that is addressed by this PR

Migrates UsernamePasswordCredential to the MSALClient flow, simplifying
the implementation.
  • Loading branch information
maorleger committed May 13, 2024
1 parent 3158783 commit fa4fd3a
Show file tree
Hide file tree
Showing 9 changed files with 198 additions and 93 deletions.
2 changes: 2 additions & 0 deletions sdk/identity/identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
### Other Changes

- `DeviceCodeCredential` migrated to use MSALClient internally instead of MSALNode flow. This is an internal refactoring and should not result in any behavioral changes. [#29405](https://github.com/Azure/azure-sdk-for-js/pull/29405)
- `UsernamePasswordCredential` migrated to use MSALClient internally instead of MSALNode flow. This is an internal refactoring and should not result in any behavioral changes. [#29656](https://github.com/Azure/azure-sdk-for-js/pull/29656)


## 4.2.0 (2024-04-30)

Expand Down
2 changes: 1 addition & 1 deletion sdk/identity/identity/assets.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
"AssetsRepo": "Azure/azure-sdk-assets",
"AssetsRepoPrefixPath": "js",
"TagPrefix": "js/identity/identity",
"Tag": "js/identity/identity_72abb85c88"
"Tag": "js/identity/identity_a7eb8b7286"
}
26 changes: 15 additions & 11 deletions sdk/identity/identity/src/credentials/usernamePasswordCredential.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,11 @@ import {
processMultiTenantRequest,
resolveAdditionallyAllowedTenantIds,
} from "../util/tenantIdUtils";
import { MsalFlow } from "../msal/flows";
import { MsalUsernamePassword } from "../msal/nodeFlows/msalUsernamePassword";
import { UsernamePasswordCredentialOptions } from "./usernamePasswordCredentialOptions";
import { credentialLogger } from "../util/logging";
import { ensureScopes } from "../util/scopeUtils";
import { tracingClient } from "../util/tracing";
import { MsalClient, createMsalClient } from "../msal/nodeFlows/msalClient";

const logger = credentialLogger("UsernamePasswordCredential");

Expand All @@ -24,7 +23,9 @@ const logger = credentialLogger("UsernamePasswordCredential");
export class UsernamePasswordCredential implements TokenCredential {
private tenantId: string;
private additionallyAllowedTenantIds: string[];
private msalFlow: MsalFlow;
private msalClient: MsalClient;
private username: string;
private password: string;

/**
* Creates an instance of the UsernamePasswordCredential with the details
Expand Down Expand Up @@ -55,14 +56,12 @@ export class UsernamePasswordCredential implements TokenCredential {
options?.additionallyAllowedTenants,
);

this.msalFlow = new MsalUsernamePassword({
this.username = username;
this.password = password;

this.msalClient = createMsalClient(clientId, this.tenantId, {
...options,
logger,
clientId,
tenantId,
username,
password,
tokenCredentialOptions: options || {},
tokenCredentialOptions: options ?? {},
});
}

Expand Down Expand Up @@ -91,7 +90,12 @@ export class UsernamePasswordCredential implements TokenCredential {
);

const arrayScopes = ensureScopes(scopes);
return this.msalFlow.getToken(arrayScopes, newOptions);
return this.msalClient.getTokenByUsernamePassword(
arrayScopes,
this.username,
this.password,
newOptions,
);
},
);
}
Expand Down
57 changes: 52 additions & 5 deletions sdk/identity/identity/src/msal/nodeFlows/msalClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,35 +46,58 @@ export interface GetTokenWithSilentAuthOptions extends GetTokenOptions {
* Represents a client for interacting with the Microsoft Authentication Library (MSAL).
*/
export interface MsalClient {
/**
* Retrieves an access token by using a user's username and password.
*
* @param scopes - The scopes for which the access token is requested. These represent the resources that the application wants to access.
* @param username - The username provided by the developer.
* @param password - The user's password provided by the developer.
* @param options - Additional options that may be provided to the method.
* @returns An access token.
*/
getTokenByUsernamePassword(
scopes: string[],
username: string,
password: string,
options?: GetTokenOptions,
): Promise<AccessToken>;
/**
* Retrieves an access token by prompting the user to authenticate using a device code.
*
* @param scopes - The scopes for which the access token is requested. These represent the resources that the application wants to access.
* @param userPromptCallback - The callback function that allows developers to customize the prompt message.
* @param options - Additional options that may be provided to the method.
* @returns An access token.
*/
getTokenByDeviceCode(
arrayScopes: string[],
scopes: string[],
userPromptCallback: DeviceCodePromptCallback,
options?: GetTokenWithSilentAuthOptions,
): Promise<AccessToken>;
/**
* Retrieves an access token by using a client certificate.
*
* @param arrayScopes - The scopes for which the access token is requested. These represent the resources that the application wants to access.
* @param scopes - The scopes for which the access token is requested. These represent the resources that the application wants to access.
* @param certificate - The client certificate used for authentication.
* @param options - Additional options that may be provided to the method.
* @returns An access token.
*/
getTokenByClientCertificate(
arrayScopes: string[],
scopes: string[],
certificate: CertificateParts,
options?: GetTokenOptions,
): Promise<AccessToken>;

/**
* Retrieves an access token by using a client assertion.
*
* @param arrayScopes - The scopes for which the access token is requested. These represent the resources that the application wants to access.
* @param scopes - The scopes for which the access token is requested. These represent the resources that the application wants to access.
* @param clientAssertion - The client assertion used for authentication.
* @param options - Additional options that may be provided to the method.
* @returns An access token.
*/
getTokenByClientAssertion(
arrayScopes: string[],
scopes: string[],
clientAssertion: string,
options?: GetTokenOptions,
): Promise<AccessToken>;
Expand Down Expand Up @@ -495,6 +518,29 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
});
}

async function getTokenByUsernamePassword(
scopes: string[],
username: string,
password: string,
options: GetTokenOptions = {},
): Promise<AccessToken> {
msalLogger.getToken.info(`Attempting to acquire token using username and password`);

const msalApp = await getPublicApp(options);

return withSilentAuthentication(msalApp, scopes, options, () => {
const requestOptions: msal.UsernamePasswordRequest = {
scopes,
username,
password,
authority: state.msalConfig.auth.authority,
claims: options?.claims,
};

return msalApp.acquireTokenByUsernamePassword(requestOptions);
});
}

function getActiveAccount(): AuthenticationRecord | undefined {
if (!state.cachedAccount) {
return undefined;
Expand All @@ -508,5 +554,6 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov
getTokenByClientAssertion,
getTokenByClientCertificate,
getTokenByDeviceCode,
getTokenByUsernamePassword,
};
}
15 changes: 15 additions & 0 deletions sdk/identity/identity/test/internal/node/msalClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { msalPlugins } from "../../../src/msal/nodeFlows/msalPlugins";
import sinon from "sinon";
import { DeveloperSignOnClientId } from "../../../src/constants";
import { Context } from "mocha";
import { getUsernamePasswordStaticResources } from "../../msalTestUtils";

describe("MsalClient", function () {
describe("recorded tests", function () {
Expand Down Expand Up @@ -73,6 +74,20 @@ describe("MsalClient", function () {
assert.isNotEmpty(accessToken.token);
assert.isNotNaN(accessToken.expiresOnTimestamp);
});

it("supports getTokenByUsernamePassword", async function () {
const scopes = ["https://vault.azure.net/.default"];
const { username, password, clientId, tenantId } = getUsernamePasswordStaticResources();

const clientOptions = recorder.configureClientOptions({});
const client = msalClient.createMsalClient(clientId, tenantId, {
tokenCredentialOptions: { additionalPolicies: clientOptions.additionalPolicies },
});

const accessToken = await client.getTokenByUsernamePassword(scopes, username, password);
assert.isNotEmpty(accessToken.token);
assert.isNotNaN(accessToken.expiresOnTimestamp);
});
});

describe("#createMsalClient", function () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@

import { AzureLogger, setLogLevel } from "@azure/logger";
import { MsalTestCleanup, msalNodeTestSetup } from "../../node/msalNodeTestSetup";
import { Recorder, env, isLiveMode, isPlaybackMode } from "@azure-tools/test-recorder";
import { Recorder, isPlaybackMode } from "@azure-tools/test-recorder";
import { Context } from "mocha";
import { DeveloperSignOnClientId } from "../../../src/constants";
import { GetTokenOptions } from "@azure/core-auth";
import { MsalNode } from "../../../src/msal/nodeFlows/msalNodeCommon";
import { PublicClientApplication } from "@azure/msal-node";
import Sinon from "sinon";
import { UsernamePasswordCredential } from "../../../src";
import { assert } from "chai";
import { getUsernamePasswordStaticResources } from "../../msalTestUtils";

describe("UsernamePasswordCredential (internal)", function () {
let cleanup: MsalTestCleanup;
Expand All @@ -26,9 +24,10 @@ describe("UsernamePasswordCredential (internal)", function () {
cleanup = setup.cleanup;
recorder = setup.recorder;

getTokenSilentSpy = setup.sandbox.spy(MsalNode.prototype, "getTokenSilent");
// MsalClient calls to this method underneath when silent authentication can be attempted.
getTokenSilentSpy = setup.sandbox.spy(PublicClientApplication.prototype, "acquireTokenSilent");

// MsalClientSecret calls to this method underneath.
// MsalClient calls to this method underneath for interactive auth.
doGetTokenSpy = setup.sandbox.spy(
PublicClientApplication.prototype,
"acquireTokenByUsernamePassword",
Expand All @@ -46,38 +45,38 @@ describe("UsernamePasswordCredential (internal)", function () {
try {
new UsernamePasswordCredential(
undefined as any,
env.AZURE_CLIENT_ID!,
env.AZURE_USERNAME!,
env.AZURE_PASSWORD!,
"azure_client_id",
"azure_username",
"azure_password",
);
} catch (e: any) {
errors.push(e);
}
try {
new UsernamePasswordCredential(
env.AZURE_TENANT_ID!,
"azure_tenant_id",
undefined as any,
env.AZURE_USERNAME!,
env.AZURE_PASSWORD!,
"azure_username",
"azure_password",
);
} catch (e: any) {
errors.push(e);
}
try {
new UsernamePasswordCredential(
env.AZURE_TENANT_ID!,
env.AZURE_CLIENT_ID!,
"azure_tenant_id",
"azure_client_id",
undefined as any,
env.AZURE_PASSWORD!,
"azure_password",
);
} catch (e: any) {
errors.push(e);
}
try {
new UsernamePasswordCredential(
env.AZURE_TENANT_ID!,
env.AZURE_CLIENT_ID!,
env.AZURE_USERNAME!,
"azure_tenant_id",
"azure_client_id",
"azure_username",
undefined as any,
);
} catch (e: any) {
Expand All @@ -103,65 +102,55 @@ describe("UsernamePasswordCredential (internal)", function () {
});
});

// This is not the way to test persistence with acquireTokenByClientCredential,
// since acquireTokenByClientCredential caches at the method level, and not with the same cache used for acquireTokenSilent.
// I'm leaving this here so I can remember about this in the future.
it.skip("Authenticates silently after the initial request", async function (this: Context) {
// These tests should not run live because this credential requires user interaction.
if (isLiveMode()) {
this.skip();
}
it("Authenticates silently after the initial request", async function (this: Context) {
const { clientId, password, tenantId, username } = getUsernamePasswordStaticResources();
const credential = new UsernamePasswordCredential(
env.AZURE_TENANT_ID!,
env.AZURE_CLIENT_ID!,
env.AZURE_USERNAME!,
env.AZURE_PASSWORD!,
tenantId,
clientId,
username,
password,
recorder.configureClientOptions({}),
);

await credential.getToken(scope);
assert.equal(getTokenSilentSpy.callCount, 1);
assert.equal(doGetTokenSpy.callCount, 1);

await credential.getToken(scope);
assert.equal(getTokenSilentSpy.callCount, 2);
assert.equal(doGetTokenSpy.callCount, 1);
assert.equal(
getTokenSilentSpy.callCount,
1,
"getTokenSilentSpy.callCount should have been 1 (Silent authentication after the initial request).",
);
assert.equal(
doGetTokenSpy.callCount,
1,
"Expected no additional calls to doGetTokenSpy after the initial request.",
);
});

it("Authenticates with tenantId on getToken", async function (this: Context) {
// The live environment isn't ready for this test
if (isLiveMode()) {
this.skip();
}
const { clientId, password, tenantId, username } = getUsernamePasswordStaticResources();
const credential = new UsernamePasswordCredential(
env.AZURE_IDENTITY_TEST_TENANTID || env.AZURE_TENANT_ID!,
env.AZURE_IDENTITY_TEST_CLIENTID || env.AZURE_CLIENT_ID!,
env.AZURE_IDENTITY_TEST_USERNAME || env.AZURE_USERNAME!,
env.AZURE_IDENTITY_TEST_PASSWORD || env.AZURE_PASSWORD!,
tenantId,
clientId,
username,
password,
recorder.configureClientOptions({}),
);

await credential.getToken(scope, { tenantId: env.AZURE_TENANT_ID } as GetTokenOptions);
assert.equal(getTokenSilentSpy.callCount, 1);
await credential.getToken(scope);
assert.equal(doGetTokenSpy.callCount, 1);
});

it("authenticates (with allowLoggingAccountIdentifiers set to true)", async function (this: Context) {
const { clientId, password, tenantId, username } = getUsernamePasswordStaticResources();
if (isPlaybackMode()) {
// The recorder clears the access tokens.
this.skip();
}
const tenantId = env.AZURE_IDENTITY_TEST_TENANTID || env.AZURE_TENANT_ID!;
const clientId = isLiveMode() ? DeveloperSignOnClientId : env.AZURE_CLIENT_ID!;

const credential = new UsernamePasswordCredential(
tenantId,
clientId,
env.AZURE_IDENTITY_TEST_USERNAME || env.AZURE_USERNAME!,
env.AZURE_IDENTITY_TEST_PASSWORD || env.AZURE_PASSWORD!,
recorder.configureClientOptions({
loggingOptions: { allowLoggingAccountIdentifiers: true },
}),
);
const credential = new UsernamePasswordCredential(tenantId, clientId, username, password, {
loggingOptions: { allowLoggingAccountIdentifiers: true },
});
setLogLevel("info");
const spy = Sinon.spy(process.stderr, "write");

Expand Down

0 comments on commit fa4fd3a

Please sign in to comment.