Skip to content

Commit

Permalink
Skip "login_hint" opaque claim if domainHint param is set (#7008)
Browse files Browse the repository at this point in the history 
Skip "login_hint" opaque claim if domainHint param is set.
  • Loading branch information
@konstantin-msft
konstantin-msft committed Apr 10, 2024
1 parent cd48cb7 commit e190037
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 2 deletions.
7 changes: 7 additions & 0 deletions change/@azure-msal-common-cf220b04-9008-4c32-a000-78ae1cfa07ef.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Skip login_hint opaque claim if domainHint param is set #7008",
"packageName": "@azure/msal-common",
"email": "kshabelko@microsoft.com",
"dependentChangeType": "patch"
}
3 changes: 2 additions & 1 deletion lib/msal-browser/docs/accounts.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,11 @@ async function getTokenPopup(request, homeAccountId) {

As of `@azure/msal-browser@3.2.0`, all login hint values can be used to search for and filter accounts. In order to filter by login hint, MSAL will compare the `loginHint` value in the `AccountFilter` object against the following account attributes (in order of precedence) to search for matches:

- `login_hint` ID token claim
- `login_hint` ID token claim. Ignored in favor of the account attributes listed below when `domainHint` param is set as both opaque `login_hint` and `domain_hint` can't be used together
- `username` account property
- `upn` ID token claim


> Note: All attributes above can be passed into the account filter as the `loginHint` property. The account filter will also accept the `username` attribute as `username`, and will yield a more performant search.
#### Using `login_hint` claim
Expand Down
10 changes: 9 additions & 1 deletion lib/msal-common/src/client/AuthorizationCodeClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -557,9 +557,17 @@ export class AuthorizationCodeClient extends BaseClient {
parameterBuilder.addSid(request.sid);
} else if (request.account) {
const accountSid = this.extractAccountSid(request.account);
const accountLoginHintClaim = this.extractLoginHint(
let accountLoginHintClaim = this.extractLoginHint(
request.account
);

if (accountLoginHintClaim && request.domainHint) {
this.logger.warning(
`AuthorizationCodeClient.createAuthCodeUrlQueryString: "domainHint" param is set, skipping opaque "login_hint" claim. Please consider not passing domainHint`
);
accountLoginHintClaim = null;
}

// If login_hint claim is present, use it over sid/username
if (accountLoginHintClaim) {
this.logger.verbose(
Expand Down
160 changes: 160 additions & 0 deletions lib/msal-common/test/client/AuthorizationCodeClient.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,166 @@ describe("AuthorizationCodeClient unit tests", () => {
).toBe(true);
});

it("skips login_hint claim if domainHint param is set", async () => {
sinon
.stub(
Authority.prototype,
<any>"getEndpointMetadataFromNetwork"
)
.resolves(ALTERNATE_OPENID_CONFIG_RESPONSE.body);

const config: ClientConfiguration =
await ClientTestUtils.createTestClientConfiguration();
const client = new AuthorizationCodeClient(config);
const testAccount = TEST_ACCOUNT_INFO;
// @ts-ignore
const testTokenClaims: Required<
Omit<
TokenClaims,
| "home_oid"
| "upn"
| "cloud_instance_host_name"
| "cnf"
| "emails"
>
> = {
ver: "2.0",
iss: `${TEST_URIS.DEFAULT_INSTANCE}9188040d-6c67-4c5b-b112-36a304b66dad/v2.0`,
sub: "AAAAAAAAAAAAAAAAAAAAAIkzqFVrSaSaFHy782bbtaQ",
exp: 1536361411,
name: "Abe Lincoln",
preferred_username: "AbeLi@microsoft.com",
oid: "00000000-0000-0000-66f3-3332eca7ea81",
tid: "3338040d-6c67-4c5b-b112-36a304b66dad",
nonce: "123523",
sid: "testSid",
login_hint: "opaque-login-hint-claim",
};

const authCodeUrlRequest: CommonAuthorizationUrlRequest = {
redirectUri: TEST_URIS.TEST_REDIRECT_URI_LOCALHOST,
scopes: [
...TEST_CONFIG.DEFAULT_GRAPH_SCOPE,
...TEST_CONFIG.DEFAULT_SCOPES,
],
account: {
...testAccount,
idTokenClaims: testTokenClaims,
},
correlationId: RANDOM_TEST_GUID,
authenticationScheme: AuthenticationScheme.BEARER,
authority: TEST_CONFIG.validAuthority,
responseMode: ResponseMode.FRAGMENT,
domainHint: TEST_CONFIG.DOMAIN_HINT,
};
const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest);
expect(
loginUrl.includes(
`${AADServerParamKeys.SID}=${encodeURIComponent(
testTokenClaims.sid
)}`
)
).toBe(false);
expect(
loginUrl.includes(
`${AADServerParamKeys.LOGIN_HINT}=${encodeURIComponent(
testAccount.username
)}`
)
).toBe(true);
expect(
loginUrl.includes(
`${AADServerParamKeys.DOMAIN_HINT}=${encodeURIComponent(
TEST_CONFIG.DOMAIN_HINT
)}`
)
).toBe(true);
expect(
loginUrl.includes(
`${HeaderNames.CCS_HEADER}=${encodeURIComponent(
`Oid:${TEST_DATA_CLIENT_INFO.TEST_UID}@${TEST_DATA_CLIENT_INFO.TEST_UTID}`
)}`
)
).toBe(true);
});

it("picks up both loginHint and domainHint params", async () => {
sinon
.stub(
Authority.prototype,
<any>"getEndpointMetadataFromNetwork"
)
.resolves(ALTERNATE_OPENID_CONFIG_RESPONSE.body);

const config: ClientConfiguration =
await ClientTestUtils.createTestClientConfiguration();
const client = new AuthorizationCodeClient(config);
const testAccount = TEST_ACCOUNT_INFO;
// @ts-ignore
const testTokenClaims: Required<
Omit<
TokenClaims,
| "home_oid"
| "upn"
| "cloud_instance_host_name"
| "cnf"
| "emails"
>
> = {
ver: "2.0",
iss: `${TEST_URIS.DEFAULT_INSTANCE}9188040d-6c67-4c5b-b112-36a304b66dad/v2.0`,
sub: "AAAAAAAAAAAAAAAAAAAAAIkzqFVrSaSaFHy782bbtaQ",
exp: 1536361411,
name: "Abe Lincoln",
preferred_username: "AbeLi@microsoft.com",
oid: "00000000-0000-0000-66f3-3332eca7ea81",
tid: "3338040d-6c67-4c5b-b112-36a304b66dad",
nonce: "123523",
sid: "testSid",
login_hint: "opaque-login-hint-claim",
};

const authCodeUrlRequest: CommonAuthorizationUrlRequest = {
redirectUri: TEST_URIS.TEST_REDIRECT_URI_LOCALHOST,
scopes: [
...TEST_CONFIG.DEFAULT_GRAPH_SCOPE,
...TEST_CONFIG.DEFAULT_SCOPES,
],
account: {
...testAccount,
idTokenClaims: testTokenClaims,
},
correlationId: RANDOM_TEST_GUID,
authenticationScheme: AuthenticationScheme.BEARER,
authority: TEST_CONFIG.validAuthority,
responseMode: ResponseMode.FRAGMENT,
domainHint: TEST_CONFIG.DOMAIN_HINT,
loginHint: TEST_CONFIG.LOGIN_HINT,
};
const loginUrl = await client.getAuthCodeUrl(authCodeUrlRequest);
expect(
loginUrl.includes(
`${AADServerParamKeys.SID}=${encodeURIComponent(
testTokenClaims.sid
)}`
)
).toBe(false);
expect(
loginUrl.includes(
`${AADServerParamKeys.LOGIN_HINT}=${encodeURIComponent(
TEST_CONFIG.LOGIN_HINT
)}`
)
).toBe(true);
expect(
loginUrl.includes(
`${AADServerParamKeys.DOMAIN_HINT}=${encodeURIComponent(
TEST_CONFIG.DOMAIN_HINT
)}`
)
).toBe(true);
});

it("Prefers sid over loginHint if both provided and prompt=None", async () => {
// Override with alternate authority openid_config
sinon
Expand Down

0 comments on commit e190037

Please sign in to comment.