Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception is thrown in acquireTokenByClientCredential if tenantId is missing #5805

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Exception is thrown in acquireTokenByClientCredential if tenantId is missing #5805",
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
Robbie-Microsoft marked this conversation as resolved.
Show resolved Hide resolved
"comment": "Exception is thrown in acquireTokenByClientCredential if tenantId is missing #5805",
"packageName": "@azure/msal-node",
"email": "[email protected]",
"dependentChangeType": "patch"
}
11 changes: 11 additions & 0 deletions lib/msal-common/src/error/ClientAuthError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,10 @@ export const ClientAuthErrorMessage = {
userCanceledError: {
code: "user_canceled",
desc: "User canceled the flow."
},
missingTenantIdError: {
code: "missing_tenant_id_error",
desc: "A tenant id - not common or organizations - must be specified when using the client_credentials flow."
}
};

Expand Down Expand Up @@ -576,4 +580,11 @@ export class ClientAuthError extends AuthError {
static createUserCanceledError(): ClientAuthError {
return new ClientAuthError(ClientAuthErrorMessage.userCanceledError.code, ClientAuthErrorMessage.userCanceledError.desc);
}

/**
* Creates an error for during acquireTokenByClientCredential when TenantId is set to "common" or "organizations"
*/
static createMissingTenantIdError(): ClientAuthError {
return new AuthError(ClientAuthErrorMessage.missingTenantIdError.code, ClientAuthErrorMessage.missingTenantIdError.desc);
}
}
13 changes: 12 additions & 1 deletion lib/msal-node/src/client/ConfidentialClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import {
AuthError,
Constants,
IAppTokenProvider,
OIDC_DEFAULT_SCOPES
OIDC_DEFAULT_SCOPES,
UrlString
} from "@azure/msal-common";
import { IConfidentialClientApplication } from "./IConfidentialClientApplication";
import { OnBehalfOfRequest } from "../request/OnBehalfOfRequest";
Expand Down Expand Up @@ -97,6 +98,16 @@ export class ConfidentialClientApplication extends ClientApplication implements
clientAssertion
};

/*
* valid request should not have "common" or "organizations" in lieu of the tenant_id in the authority in the auth configuration
* example authority: "https://login.microsoftonline.com/TenantId",
*/
const authority = new UrlString(validRequest.authority);
const tenantId = authority.getUrlComponents().PathSegments[0];
if ((tenantId === "common") || (tenantId === "organizations")) {
throw ClientAuthError.createMissingTenantIdError();
}

const azureRegionConfiguration: AzureRegionConfiguration = {
azureRegion: validRequest.azureRegion,
environmentRegion: process.env[REGION_ENVIRONMENT_VARIABLE]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { ClientConfiguration, AuthorizationCodeClient, RefreshTokenClient, Authe
import { TEST_CONSTANTS } from '../utils/TestConstants';
import { Configuration } from "../../src/config/Configuration";
import { AuthorizationCodeRequest } from "../../src/request/AuthorizationCodeRequest";
import { UsernamePasswordRequest } from '../../src';
import { ClientAuthError, UsernamePasswordRequest } from '../../src';
import { RefreshTokenRequest } from "../../src/request/RefreshTokenRequest";
import { fakeAuthority, setupAuthorityFactory_createDiscoveredInstance_mock } from './test-fixtures';

Expand Down Expand Up @@ -193,6 +193,39 @@ describe('ConfidentialClientApplication', () => {
);
});

test('acquireTokenByClientCredential throws missingTenantIdError if \"common\" or "\"organization\" were provided as the tenant id', async () => {
const testProvider: msalCommon.IAppTokenProvider = () => {
return new Promise<msalCommon.AppTokenProviderResult>(
(resolve) => resolve({
accessToken: "accessToken",
expiresInSeconds: 3601,
refreshInSeconds: 1801,
}))};

const configWithExtensibility: Configuration = {
auth: {
clientId: TEST_CONSTANTS.CLIENT_ID,
authority: TEST_CONSTANTS.DEFAULT_AUTHORITY,
clientAssertion: "testAssertion"
},
}

const request: ClientCredentialRequest = {
scopes: TEST_CONSTANTS.DEFAULT_GRAPH_SCOPE,
skipCache: false
};

setupAuthorityFactory_createDiscoveredInstance_mock();
const MockClientCredentialClient = getMsalCommonAutoMock().ClientCredentialClient;

jest.spyOn(msalCommon, 'ClientCredentialClient').mockImplementation((conf) => new MockClientCredentialClient(conf));

const authApp = new ConfidentialClientApplication(configWithExtensibility);
authApp.SetAppTokenProvider(testProvider);

await expect(authApp.acquireTokenByClientCredential(request)).rejects.toMatchObject(ClientAuthError.createMissingTenantIdError());
});

test('acquireTokenByClientCredential handles AuthErrors as expected', async () => {
const request: ClientCredentialRequest = {
scopes: TEST_CONSTANTS.DEFAULT_GRAPH_SCOPE,
Expand Down