Skip to content

Commit

Permalink
Fixed bug where dev-provided certificate was not being attached to cl…
Browse files Browse the repository at this point in the history
…ient assertion (#7088)

Fixes [acquireTokenByClientCredential broken for clientCertificate
#7082](#7082)

Applied boy-scout-rule to `ConfidentialClientApplication.spec.ts`
(contains unit tests). I've been waiting for a good opportunity to do
this. The ConfidentialClientApplication tests are now in line with the
other test files: All Managed Identity sources, ClientCredentialClient,
OnBehalfOfClient and UsernamePasswordClient.

Co-authored-by: @ericcan
  • Loading branch information
Robbie-Microsoft authored May 13, 2024
1 parent 63664d9 commit cacfcba
Show file tree
Hide file tree
Showing 5 changed files with 352 additions and 254 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fixed bug where dev-provided certificate was not being attached to client assertion #7088",
"packageName": "@azure/msal-node",
"email": "[email protected]",
"dependentChangeType": "patch"
}
40 changes: 22 additions & 18 deletions lib/msal-node/src/client/ClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -457,9 +457,9 @@ export abstract class ClientApplication {
serverTelemetryManager: serverTelemetryManager,
clientCredentials: {
clientSecret: this.clientSecret,
clientAssertion: this.developerProvidedClientAssertion
? await this.getClientAssertion(discoveredAuthority)
: undefined,
clientAssertion: await this.getClientAssertion(
discoveredAuthority
),
},
libraryInfo: {
sku: NodeConstants.MSAL_SKU,
Expand All @@ -478,22 +478,26 @@ export abstract class ClientApplication {
private async getClientAssertion(
authority: Authority
): Promise<ClientAssertionType> {
this.clientAssertion = ClientAssertion.fromAssertion(
await getClientAssertion(
this.developerProvidedClientAssertion,
this.config.auth.clientId,
authority.tokenEndpoint
)
);
if (this.developerProvidedClientAssertion) {
this.clientAssertion = ClientAssertion.fromAssertion(
await getClientAssertion(
this.developerProvidedClientAssertion,
this.config.auth.clientId,
authority.tokenEndpoint
)
);
}

return {
assertion: this.clientAssertion.getJwt(
this.cryptoProvider,
this.config.auth.clientId,
authority.tokenEndpoint
),
assertionType: NodeConstants.JWT_BEARER_ASSERTION_TYPE,
};
return (
this.clientAssertion && {
assertion: this.clientAssertion.getJwt(
this.cryptoProvider,
this.config.auth.clientId,
authority.tokenEndpoint
),
assertionType: NodeConstants.JWT_BEARER_ASSERTION_TYPE,
}
);
}

/**
Expand Down
55 changes: 55 additions & 0 deletions lib/msal-node/test/client/ClientTestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ import {
TEST_POP_VALUES,
TEST_TOKENS,
} from "../test_kit/StringConstants";
import { Configuration } from "../../src/config/Configuration";
import { TEST_CONSTANTS } from "../utils/TestConstants";
import { CryptoKeys } from "../utils/CryptoKeys";

const ACCOUNT_KEYS = "ACCOUNT_KEYS";
const TOKEN_KEYS = "TOKEN_KEYS";
Expand Down Expand Up @@ -357,6 +360,58 @@ export class ClientTestUtils {

return clientConfig;
}

static async createTestConfidentialClientConfiguration(
clientCapabilities?: Array<string>,
mockNetworkClient?: INetworkModule
): Promise<Configuration> {
const mockHttpClient = mockNetworkClient || {
sendGetRequestAsync<T>(): T {
return {} as T;
},
sendPostRequestAsync<T>(): T {
return {} as T;
},
};

const loggerOptions = {
loggerCallback: (): void => {},
piiLoggingEnabled: true,
logLevel: LogLevel.Verbose,
};

const cryptoKeys: CryptoKeys = new CryptoKeys();

const confidentialClientConfig: Configuration = {
auth: {
clientId: TEST_CONSTANTS.CLIENT_ID,
authority: TEST_CONSTANTS.AUTHORITY,
// clientSecret, clientAssertion
clientCertificate: {
thumbprint: cryptoKeys.thumbprint,
privateKey: cryptoKeys.privateKey,
},
knownAuthorities: [TEST_CONSTANTS.AUTHORITY],
cloudDiscoveryMetadata: "",
authorityMetadata: "",
clientCapabilities,
protocolMode: ProtocolMode.AAD,
},
// broker, cache
system: {
loggerOptions,
networkClient: mockHttpClient,
},
telemetry: {
application: {
appName: TEST_CONFIG.applicationName,
appVersion: TEST_CONFIG.applicationVersion,
},
},
};

return confidentialClientConfig;
}
}

interface checks {
Expand Down
Loading

0 comments on commit cacfcba

Please sign in to comment.