Skip to content

Commit

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

@azure/identity

### Issues associated with this PR

Resolves #29406

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

Following the existing patterns, this PR migrates
AuthorizationCodeCredential to
the MSALClient, moving away from MSALNodeCommon
  • Loading branch information
maorleger authored Jun 7, 2024
1 parent 5165940 commit 0d9a747
Show file tree
Hide file tree
Showing 5 changed files with 143 additions and 37 deletions.
2 changes: 1 addition & 1 deletion sdk/identity/identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

Expand All @@ -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 ?? {},
});
}

Expand All @@ -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,
},
);
},
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
51 changes: 51 additions & 0 deletions sdk/identity/identity/src/msal/nodeFlows/msalClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,26 @@ export interface MsalClient {
options?: GetTokenOptions,
): Promise<AccessToken>;

/**
* 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<AccessToken>;

/**
* Retrieves the last authenticated account. This method expects an authentication record to have been previously loaded.
*
Expand Down Expand Up @@ -548,12 +568,43 @@ 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<AccessToken> {
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,
getTokenByClientAssertion,
getTokenByClientCertificate,
getTokenByDeviceCode,
getTokenByUsernamePassword,
getTokenByAuthorizationCode,
};
}
93 changes: 73 additions & 20 deletions sdk/identity/identity/test/internal/node/msalClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -268,21 +263,79 @@ 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 = () => {
// no-op
};

afterEach(async function () {
sandbox.restore();
});

beforeEach(async function () {
sandbox = sinon.createSandbox();
sinon.restore();
});

describe("with silent authentication", function () {
Expand All @@ -299,7 +352,7 @@ describe("MsalClient", function () {
authenticationRecord,
});

const silentAuthSpy = sandbox
const silentAuthSpy = sinon
.stub(ClientApplication.prototype, "acquireTokenSilent")
.resolves({
accessToken: "token",
Expand All @@ -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",
Expand Down Expand Up @@ -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

Expand All @@ -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 }));

Expand Down

0 comments on commit 0d9a747

Please sign in to comment.