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

Add getAccount and enhance account filtering #6499

Merged
merged 20 commits into from
Sep 26, 2023
Merged

Add getAccount and enhance account filtering #6499

merged 20 commits into from
Sep 26, 2023

Conversation

hectormmg
Copy link
Member

@hectormmg hectormmg commented Sep 20, 2023

This PR:

  • Adds and accountFilter: AccountFilter param to getAllAccounts() in order to enable the narrowing-down of cached accounts returned
  • Adds getAccountByFilter API to replace now marked-for-deprecation getAccountBy APIs
  • Extends AccountFilter class to optionally include all the properties in AccountInfo except idToken and idTokenClaims so accounts are filterable by all of their searchable properties
  • Adds new AccountFilter properties to the set of matchers in CacheManager.getAccountsFilteredBy() as well as their respective private matching methods
  • Adds filtering by loginHint

@github-actions github-actions bot added msal-browser Related to msal-browser package msal-common Related to msal-common package labels Sep 20, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2023

Codecov Report

Merging #6499 (90e87ce) into dev (81d34b4) will decrease coverage by 1.24%.
Report is 329 commits behind head on dev.
The diff coverage is 61.03%.

Flag Coverage Δ
msal-angular 96.73% <ø> (+0.22%) ⬆️
msal-browser 85.43% <ø> (-1.04%) ⬇️
msal-common 84.73% <ø> (+0.19%) ⬆️
msal-core ?
msal-node 80.75% <ø> (-2.64%) ⬇️
msal-node-extensions 68.51% <60.77%> (-7.13%) ⬇️
msal-react 94.24% <ø> (-0.45%) ⬇️
node-token-validation ?
Files Coverage Δ
extensions/msal-node-extensions/src/Dpapi.ts 100.00% <100.00%> (ø)
.../msal-node-extensions/src/error/NativeAuthError.ts 100.00% <100.00%> (ø)
extensions/msal-node-extensions/src/index.ts 100.00% <100.00%> (ø)
...nsions/msal-node-extensions/src/packageMetadata.ts 100.00% <100.00%> (+100.00%) ⬆️
...-extensions/src/persistence/DataProtectionScope.ts 100.00% <100.00%> (ø)
...nsions/msal-node-extensions/src/utils/Constants.ts 100.00% <100.00%> (ø)
...sions/msal-node-extensions/src/utils/TypeGuards.ts 100.00% <100.00%> (ø)
lib/msal-angular/src/constants.ts 100.00% <ø> (ø)
lib/msal-angular/src/msal.broadcast.service.ts 100.00% <ø> (ø)
lib/msal-angular/src/msal.guard.ts 90.78% <ø> (+0.64%) ⬆️
... and 57 more

... and 184 files with indirect coverage changes

@github-actions github-actions bot added the documentation Related to documentation. label Sep 20, 2023
@hectormmg hectormmg changed the title Add Multi-tenant support to MSAL Browser Add account filtering to MSAL Browser Sep 22, 2023
@hectormmg hectormmg marked this pull request as ready for review September 22, 2023 19:01
@hectormmg hectormmg changed the title Add account filtering to MSAL Browser Add getAccount and enhance account filtering Sep 22, 2023
Copy link
Collaborator

@tnorling tnorling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple nits but otherwise looks great! Thanks for doing this!

Copy link
Member

@sameerag sameerag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments, approved. Much clear now, Thanks.

hectormmg and others added 9 commits September 25, 2023 10:24
Co-authored-by: Sameera Gajjarapu <sameera.gajjarapu@microsoft.com>
Co-authored-by: Thomas Norling <thomas.norling@microsoft.com>
Co-authored-by: Thomas Norling <thomas.norling@microsoft.com>
Co-authored-by: Thomas Norling <thomas.norling@microsoft.com>
Co-authored-by: Thomas Norling <thomas.norling@microsoft.com>
Co-authored-by: Sameera Gajjarapu <sameera.gajjarapu@microsoft.com>
@@ -86,10 +86,12 @@ export const TEST_TOKENS = {
POP_TOKEN:
"eyJ0eXAiOiJKV1QiLCJub25jZSI6InFMZmZKT255c2dnVnhhdGxSbVhvR0dnYkx3NDV5LTdsWkswaHJWSm9zeDQiLCJhbGciOiJSUzI1NiIsIng1dCI6InZhcF9pdmtIdHRMRmNubm9CWEF3SjVIWDBLNCIsImtpZCI6InZhcF9pdmtIdHRMRmNubm9CWEF3SjVIWDBLNCJ9.eyJhdWQiOiIwMDAwMDAwMy0wMDAwLTAwMDAtYzAwMC0wMDAwMDAwMDAwMDAiLCJpc3MiOiJodHRwczovL3N0cy53aW5kb3dzLXBwZS5uZXQvMTllZWEyZjgtZTE3YS00NzBmLTk1NGQtZDg5N2M0N2YzMTFjLyIsImlhdCI6MTYxODAwNzU2MCwibmJmIjoxNjE4MDA3NTYwLCJleHAiOjE2MTgwMTE0NjAsImFjY3QiOjAsImFjciI6IjEiLCJhY3JzIjpbInVybjp1c2VyOnJlZ2lzdGVyc2VjdXJpdHlpbmZvIiwidXJuOm1pY3Jvc29mdDpyZXExIiwidXJuOm1pY3Jvc29mdDpyZXEyIiwidXJuOm1pY3Jvc29mdDpyZXEzIiwiYzEiLCJjMiIsImMzIiwiYzQiLCJjNSIsImM2IiwiYzciLCJjOCIsImM5IiwiYzEwIiwiYzExIiwiYzEyIiwiYzEzIiwiYzE0IiwiYzE1IiwiYzE2IiwiYzE3IiwiYzE4IiwiYzE5IiwiYzIwIiwiYzIxIiwiYzIyIiwiYzIzIiwiYzI0IiwiYzI1Il0sImFpbyI6IkUyTmdZRGo3Y0dVWEczT1p4OXhXSjVNRWg5MXU2NVFTZnAzVXYycVpLSHByaHRtVkY2d0EiLCJhbXIiOlsicHdkIl0sImFwcF9kaXNwbGF5bmFtZSI6IlBLLU1TQUxUZXN0Mi4wIiwiYXBwaWQiOiIzZmJhNTU2ZS01ZDRhLTQ4ZTMtOGUxYS1mZDU3YzEyY2I4MmUiLCJhcHBpZGFjciI6IjAiLCJjbmYiOnsia2lkIjoiVjZOX0hNUGFnTnBZU193eE0xNFg3M3EzZVd6YlRyOVozMVJ5SGtJY04wWSIsInhtc19rc2wiOiJzdyJ9LCJmYW1pbHlfbmFtZSI6IkJhc2ljIFVzZXIiLCJnaXZlbl9uYW1lIjoiQ2xvdWQgSURMQUIiLCJpZHR5cCI6InVzZXIiLCJpcGFkZHIiOiIyNC4xNy4yNDYuMjA5IiwibmFtZSI6IkNsb3VkIElETEFCIEJhc2ljIFVzZXIiLCJvaWQiOiJiZTA2NGMzNy0yNjE3LTQ2OGMtYjYyNy0yNWI0ZTQ4MTdhZGYiLCJwbGF0ZiI6IjMiLCJwdWlkIjoiMTAwMzQwMDAwMDU0NzdCQSIsInJoIjoiMC5BQUFBLUtMdUdYcmhEMGVWVGRpWHhIOHhIRzVWdWo5S1hlTklqaHI5VjhFc3VDNEJBTmsuIiwic2NwIjoiRmlsZXMuUmVhZCBNYWlsLlJlYWQgb3BlbmlkIHByb2ZpbGUgVXNlci5SZWFkIGVtYWlsIiwic2lnbmluX3N0YXRlIjpbImttc2kiXSwic3ViIjoidExjaFl1bUczSXZZT1VrQlprU0EzbWhnOEVfYnNGZDhuU2licUlOX0UxVSIsInRpZCI6IjE5ZWVhMmY4LWUxN2EtNDcwZi05NTRkLWQ4OTdjNDdmMzExYyIsInVuaXF1ZV9uYW1lIjoiSURMQUJAbXNpZGxhYjAuY2NzY3RwLm5ldCIsInVwbiI6IklETEFCQG1zaWRsYWIwLmNjc2N0cC5uZXQiLCJ1dGkiOiJ5a2tPd3dyTkFVT1k5SUVXejRITEFBIiwidmVyIjoiMS4wIiwid2lkcyI6WyJiNzlmYmY0ZC0zZWY5LTQ2ODktODE0My03NmIxOTRlODU1MDkiXSwieG1zX3N0Ijp7InN1YiI6Imx2QnRkdmVkdDRkT1pyeGZvQjdjbV9UTkU3THFucG5lcGFHc3EtUmZkS2MifSwieG1zX3RjZHQiOjE1NDQ1NzQzNjN9.VPBqUrMDc-H1T4paZoSbGaec0lBoJqSiu13chxJmgee1lDxUFr2pM52tqzPPH6N_Yk-VQ0_AKTyvfnbAQw4mryhp3SJytZbU7FedrXX7oq2laLh9s0K_Hz1EZSj5xg3SSUxXmKEjdePN6d0_5MLlt1P-LcL2PAGgkEEBhUfDm6pAxyTMO8Mw1DUYbq7kr_IzyQ71V-kuoYHDjazghSIwOkidoWMCPP-HIENVbFEKUDKFGDiOzU76IagUBAYUQ4JD1bC9hHA-OO6AV8xLK7UoPyx9UH7fLbiImzhARBxMkmAQu9v2kwn5Hl9hoBEBhlu48YOYOr4O3GxwKisff87R9Q",
REFRESH_TOKEN: "thisIsARefreshT0ken",
// [SuppressMessage("Microsoft.Security", "CS002:SecretInNextLine", Justification="Fake credential used for testing purposes")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add this to all test tokens? We can take it as a new item in the backlog.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but cred scan only flags new credentials it finds in the diff

@hectormmg hectormmg merged commit 4865958 into dev Sep 26, 2023
43 of 44 checks passed
@hectormmg hectormmg deleted the multi-tenant branch September 26, 2023 18:07
@Nodios
Copy link

Nodios commented Feb 14, 2024

Hi,

Just a quick question regarding this change. Is there a reason why the interface IPublicClientApplication was not updated with the method definition changes? I don't know if I'm missing something

See example below:

Interface

Implementation

getAllAccounts(accountFilter?: AccountFilter): AccountInfo[] {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Related to documentation. msal-browser Related to msal-browser package msal-common Related to msal-common package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants