diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md index d6806ed2036e..ab0e8176534b 100644 --- a/sdk/identity/identity/CHANGELOG.md +++ b/sdk/identity/identity/CHANGELOG.md @@ -14,7 +14,7 @@ - `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) - +- `AuthorizationCodeCredential` migrated to use MSALClient internally instead of MSALNode flow. This is an internal refactoring and should not result in any behavioral changes. [#29831](https://github.com/Azure/azure-sdk-for-js/pull/29831) ## 4.2.0 (2024-04-30) diff --git a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts index 576857ca0a51..82b2d353ec86 100644 --- a/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts +++ b/sdk/identity/identity/src/credentials/authorizationCodeCredential.ts @@ -7,12 +7,11 @@ import { resolveAdditionallyAllowedTenantIds, } from "../util/tenantIdUtils"; import { AuthorizationCodeCredentialOptions } from "./authorizationCodeCredentialOptions"; -import { MsalAuthorizationCode } from "../msal/nodeFlows/msalAuthorizationCode"; -import { MsalFlow } from "../msal/flows"; import { checkTenantId } from "../util/tenantIdUtils"; 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("AuthorizationCodeCredential"); @@ -24,12 +23,13 @@ const logger = credentialLogger("AuthorizationCodeCredential"); * https://learn.microsoft.com/entra/identity-platform/v2-oauth2-auth-code-flow */ export class AuthorizationCodeCredential implements TokenCredential { - private msalFlow: MsalFlow; + private msalClient: MsalClient; private disableAutomaticAuthentication?: boolean; private authorizationCode: string; private redirectUri: string; private tenantId?: string; private additionallyAllowedTenantIds: string[]; + private clientSecret?: string; /** * Creates an instance of AuthorizationCodeCredential with the details needed @@ -102,7 +102,7 @@ export class AuthorizationCodeCredential implements TokenCredential { options?: AuthorizationCodeCredentialOptions, ) { checkTenantId(logger, tenantId); - let clientSecret: string | undefined = clientSecretOrAuthorizationCode; + this.clientSecret = clientSecretOrAuthorizationCode; if (typeof redirectUriOrOptions === "string") { // the clientId+clientSecret constructor @@ -113,7 +113,7 @@ export class AuthorizationCodeCredential implements TokenCredential { // clientId only this.authorizationCode = clientSecretOrAuthorizationCode; this.redirectUri = authorizationCodeOrRedirectUri as string; - clientSecret = undefined; + this.clientSecret = undefined; options = redirectUriOrOptions as AuthorizationCodeCredentialOptions; } @@ -123,15 +123,10 @@ export class AuthorizationCodeCredential implements TokenCredential { options?.additionallyAllowedTenants, ); - this.msalFlow = new MsalAuthorizationCode({ + this.msalClient = createMsalClient(clientId, tenantId, { ...options, - clientSecret, - clientId, - tenantId, - tokenCredentialOptions: options || {}, logger, - redirectUri: this.redirectUri, - authorizationCode: this.authorizationCode, + tokenCredentialOptions: options ?? {}, }); } @@ -156,10 +151,16 @@ export class AuthorizationCodeCredential implements TokenCredential { newOptions.tenantId = tenantId; const arrayScopes = ensureScopes(scopes); - return this.msalFlow.getToken(arrayScopes, { - ...newOptions, - disableAutomaticAuthentication: this.disableAutomaticAuthentication, - }); + return this.msalClient.getTokenByAuthorizationCode( + arrayScopes, + this.redirectUri, + this.authorizationCode, + this.clientSecret, + { + ...newOptions, + disableAutomaticAuthentication: this.disableAutomaticAuthentication, + }, + ); }, ); } diff --git a/sdk/identity/identity/src/credentials/deviceCodeCredential.ts b/sdk/identity/identity/src/credentials/deviceCodeCredential.ts index 7f9b0fa92b3c..a1f68f0e6b8c 100644 --- a/sdk/identity/identity/src/credentials/deviceCodeCredential.ts +++ b/sdk/identity/identity/src/credentials/deviceCodeCredential.ts @@ -70,6 +70,7 @@ export class DeviceCodeCredential implements TokenCredential { this.userPromptCallback = options?.userPromptCallback ?? defaultDeviceCodePromptCallback; this.msalClient = createMsalClient(clientId, tenantId, { ...options, + logger, tokenCredentialOptions: options || {}, }); this.disableAutomaticAuthentication = options?.disableAutomaticAuthentication; diff --git a/sdk/identity/identity/src/msal/nodeFlows/msalClient.ts b/sdk/identity/identity/src/msal/nodeFlows/msalClient.ts index 4d6a14de28f4..64803cee0644 100644 --- a/sdk/identity/identity/src/msal/nodeFlows/msalClient.ts +++ b/sdk/identity/identity/src/msal/nodeFlows/msalClient.ts @@ -116,6 +116,26 @@ export interface MsalClient { options?: GetTokenOptions, ): Promise; + /** + * Retrieves an access token by using an authorization code flow. + * + * @param scopes - The scopes for which the access token is requested. These represent the resources that the application wants to access. + * @param authorizationCode - An authorization code that was received from following the + authorization code flow. This authorization code must not + have already been used to obtain an access token. + * @param redirectUri - The redirect URI that was used to request the authorization code. + Must be the same URI that is configured for the App Registration. + * @param clientSecret - An optional client secret that was generated for the App Registration. + * @param options - Additional options that may be provided to the method. + */ + getTokenByAuthorizationCode( + scopes: string[], + redirectUri: string, + authorizationCode: string, + clientSecret?: string, + options?: GetTokenWithSilentAuthOptions, + ): Promise; + /** * Retrieves the last authenticated account. This method expects an authentication record to have been previously loaded. * @@ -548,6 +568,36 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov return msalToPublic(clientId, state.cachedAccount); } + async function getTokenByAuthorizationCode( + scopes: string[], + redirectUri: string, + authorizationCode: string, + clientSecret?: string, + options: GetTokenWithSilentAuthOptions = {}, + ): Promise { + msalLogger.getToken.info(`Attempting to acquire token using authorization code`); + + let msalApp: msal.ConfidentialClientApplication | msal.PublicClientApplication; + if (clientSecret) { + // If a client secret is provided, we need to use a confidential client application + // See https://learn.microsoft.com/entra/identity-platform/v2-oauth2-auth-code-flow#request-an-access-token-with-a-client_secret + state.msalConfig.auth.clientSecret = clientSecret; + msalApp = await getConfidentialApp(options); + } else { + msalApp = await getPublicApp(options); + } + + return withSilentAuthentication(msalApp, scopes, options, () => { + return msalApp.acquireTokenByCode({ + scopes, + redirectUri, + code: authorizationCode, + authority: state.msalConfig.auth.authority, + claims: options?.claims, + }); + }); + } + return { getActiveAccount, getTokenByClientSecret, @@ -555,5 +605,6 @@ To work with multiple accounts for the same Client ID and Tenant ID, please prov getTokenByClientCertificate, getTokenByDeviceCode, getTokenByUsernamePassword, + getTokenByAuthorizationCode, }; } diff --git a/sdk/identity/identity/test/internal/node/msalClient.spec.ts b/sdk/identity/identity/test/internal/node/msalClient.spec.ts index e49d4acfa4bd..bb0773936fcf 100644 --- a/sdk/identity/identity/test/internal/node/msalClient.spec.ts +++ b/sdk/identity/identity/test/internal/node/msalClient.spec.ts @@ -164,18 +164,13 @@ describe("MsalClient", function () { }); describe("CAE support", function () { - let sandbox: sinon.SinonSandbox; let subject: msalClient.MsalClient; const clientId = "client-id"; const tenantId = "tenant-id"; afterEach(async function () { - sandbox.restore(); - }); - - beforeEach(async function () { - sandbox = sinon.createSandbox(); + sinon.restore(); }); describe("when CAE is enabled", function () { @@ -191,7 +186,7 @@ describe("MsalClient", function () { beforeCacheAccess: sinon.stub(), }; - sandbox.stub(msalPlugins, "generatePluginConfiguration").returns({ + sinon.stub(msalPlugins, "generatePluginConfiguration").returns({ broker: { isEnabled: false, enableMsaPassthrough: false, @@ -235,7 +230,7 @@ describe("MsalClient", function () { beforeCacheAccess: sinon.stub(), }; - sandbox.stub(msalPlugins, "generatePluginConfiguration").returns({ + sinon.stub(msalPlugins, "generatePluginConfiguration").returns({ broker: { isEnabled: false, enableMsaPassthrough: false, @@ -268,9 +263,71 @@ describe("MsalClient", function () { }); }); - describe("#getTokenByDeviceCode", function () { - let sandbox: sinon.SinonSandbox; + describe("#getTokenByAuthorizationCode", function () { + const clientId = "client-id"; + const tenantId = "tenant-id"; + const fakeTokenResponse = { + accessToken: "token", + expiresOn: new Date(Date.now() + 3600 * 1000), + account: { + environment: "environment", + homeAccountId: "homeAccountId", + localAccountId: "localAccountId", + tenantId: "tenantId", + username: "username", + }, + }; + const scopes = ["https://vault.azure.net/.default"]; + + afterEach(async function () { + sinon.restore(); + }); + + describe("with clientSecret", function () { + it("uses a confidentialClientApplication", async function () { + const client = msalClient.createMsalClient(clientId, tenantId); + + const publicClientStub = sinon.stub( + PublicClientApplication.prototype, + "acquireTokenByCode", + ); + const confidentialClientStub = sinon + .stub(ConfidentialClientApplication.prototype, "acquireTokenByCode") + .resolves(fakeTokenResponse as AuthenticationResult); + + await client.getTokenByAuthorizationCode(scopes, "code", "redirectUri", "clientSecret"); + + assert.equal(publicClientStub.callCount, 0); + assert.equal(confidentialClientStub.callCount, 1); + }); + }); + + describe("without clientSecret", function () { + it("uses a publicClientApplication", async function () { + const client = msalClient.createMsalClient(clientId, tenantId); + + const publicClientStub = sinon + .stub(PublicClientApplication.prototype, "acquireTokenByCode") + .resolves(fakeTokenResponse as AuthenticationResult); + const confidentialClientStub = sinon.stub( + ConfidentialClientApplication.prototype, + "acquireTokenByCode", + ); + + await client.getTokenByAuthorizationCode( + scopes, + "code", + "redirectUri", + undefined /* clientSecret */, + ); + assert.equal(publicClientStub.callCount, 1); + assert.equal(confidentialClientStub.callCount, 0); + }); + }); + }); + + describe("#getTokenByDeviceCode", function () { const clientId = "client-id"; const tenantId = "tenant-id"; const deviceCodeCallback: () => void = () => { @@ -278,11 +335,7 @@ describe("MsalClient", function () { }; afterEach(async function () { - sandbox.restore(); - }); - - beforeEach(async function () { - sandbox = sinon.createSandbox(); + sinon.restore(); }); describe("with silent authentication", function () { @@ -299,7 +352,7 @@ describe("MsalClient", function () { authenticationRecord, }); - const silentAuthSpy = sandbox + const silentAuthSpy = sinon .stub(ClientApplication.prototype, "acquireTokenSilent") .resolves({ accessToken: "token", @@ -319,14 +372,14 @@ describe("MsalClient", function () { }); it("attempts silent authentication without AuthenticationRecord", async function () { - const silentAuthStub = sandbox + const silentAuthStub = sinon .stub(ClientApplication.prototype, "acquireTokenSilent") .resolves({ accessToken: "token", expiresOn: new Date(), } as AuthenticationResult); - const clientCredentialAuthStub = sandbox + const clientCredentialAuthStub = sinon .stub(PublicClientApplication.prototype, "acquireTokenByDeviceCode") .resolves({ accessToken: "token", @@ -371,7 +424,7 @@ describe("MsalClient", function () { }, }); - sandbox + sinon .stub(ClientApplication.prototype, "acquireTokenSilent") .rejects(new AbortError("operation has been aborted")); // AbortErrors should get re-thrown @@ -385,7 +438,7 @@ describe("MsalClient", function () { it("throws when silentAuthentication fails and disableAutomaticAuthentication is true", async function () { const scopes = ["https://vault.azure.net/.default"]; - sandbox + sinon .stub(ClientApplication.prototype, "acquireTokenSilent") .rejects(new AuthenticationRequiredError({ scopes }));