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

Rebase dev onto v3 #5843

Merged
merged 31 commits into from
Apr 4, 2023
Merged

Rebase dev onto v3 #5843

merged 31 commits into from
Apr 4, 2023

Conversation

konstantin-msft
Copy link
Collaborator

@konstantin-msft konstantin-msft commented Apr 3, 2023

  • Reconcile changes after rebasing.
  • Update package-lock.json files.

tnorling and others added 25 commits April 3, 2023 08:47
Exposes nativeBrokerPlugin to enable token acquisition through the
system broker via msal-node-extensions and msal-runtime

---------

Co-authored-by: Thomas Norling <thnorlin@microsoft.com>
Co-authored-by: Doğan Erişen <v-derisen@microsoft.com>
This PR replaces the default value of the typ header in SHRs from "JWT"
to "pop" to match the SHR specification.
We've been receiving reports of users receiving the error:

`Error: TypeError
Error Description: The "listener" argument must be of type function.
Received an instance of Object - []`

I wasn't able to reproduce the issue from any user reports, however, a
fellow Microsoft employee who works on the VSCode team received the same
error in a VSCode extension she was building. She walked me through the
build steps for her VSCode extension and reproduction steps, and then I
was finally able to reproduce the issue.

As it relates to msal-node's HttpClient: I'm not sure why this error was
being thrown, because the nodejs code where the error occurs adheres to
the documentation on NodeJS's website. Specifically, I'm talking about
[https.request](https://nodejs.org/docs/latest-v16.x/api/http.html#httprequesturl-options-callback).

The relevant code in msal-node's code base is [located
here](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-node/src/network/HttpClient.ts#L249).
According to the docs, we can make a https request this way:
 ```https.request(url, options)```.

However, I needed to change that linked code to the following to resolve
the error:
```
const customOptions: https.RequestOptions = {
        hostname: url.hostname, // <-- new code
        path: url.pathname, // <-- new code
        method: httpMethod,
        headers: options?.headers || emptyHeaders,
    };
...
https.request(customOptions)
```

Update: I have been looking into this for a while, and the only
conclusion I can draw is that maybe the NodeJS docs are incorrect, or
this is a NodeJS bug.

---------

Co-authored-by: Robert Ginsburg <rgins16@gmail.com>
- New Sample with Hybrid + Native bridge enabled.
- Small API changes related to handling new "spa_accountid" parameter
being returned from server in hybrid flow.

---------

Co-authored-by: Sameera Gajjarapu <sameera.gajjarapu@microsoft.com>
Co-authored-by: Lalima Sharda <lalimasharda@microsoft.com>
This PR:
* Breaks down `handleNativeResponse` into various blocks
* Cache logic separated

Will be followed up with silent cache lookup updates in the new helper
functions

---------

Co-authored-by: Hector Morales <hemoral@microsoft.com>
)

As the title says... added bash script to access key vault and write the
necessary info to the dotenv file
Trigger e2e tests only for PRs that are ready for review.
Release PR : This PR contains the changelogs and version bumps for the
March 2023 releases.

The following modules have had their versions bumped:
 | Module | Old Version | New Version |
| ---    | ---         | ---         |
| msal-angular | 2.5.3 |  2.5.4 |
| msal-browser | 2.33.0 |  2.33.1 |
| msal-common | 10.0.0 |  10.0.1 |
| msal-node | 1.15.0 |  1.16.0 |
| msal-react | 1.5.3 |  1.5.4 |
| node-token-validation | ??? |  ??? |
| msal-node-extensions | 1.0.0-alpha.30 |  1.0.0-alpha.31 |

Co-authored-by: konstantin-msft <konstantin-msft@users.noreply.github.com>
This PR contains the updated package-lock.json files for all npm
packages published in the March release.
Updates msal-browser-e2e workflow to use multiple jobs for running
browser e2e tests. This gives better visibility to failed tests, ability
to retry only the failed ones, and slightly improves run time.
links to sample READMEs were broken because of file name casing, this
fixes the issue.

p.s. silent-flow sample did not have a readme -I added one for
consistency based on the other auth code sample, though its less than
ideal
Removing myself, @jmprieur and @jennyf19 from some components
Bug fix:
- Added missing `tenant_region_scope` and `tenant_region_sub_scope`
claims from Token Claims

---------

Co-authored-by: Lalima Sharda <lalimasharda@microsoft.com>
Documentation updates for changes made in #4536.

Note: updates to
[Authority](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-common/docs/authority.md)
and
[Performance](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-common/docs/performance.md)
already cover behavior changes made in the PR above, this PR is to fill
in the gaps.
* Updates the caching doc for more clarity and links to new distributed
cache samples
* Adds a performance doc to provide suggestions for improvements,
monitoring etc.

---------

Co-authored-by: Bogdan Gavril <bogavril@microsoft.com>
Co-authored-by: Sameera Gajjarapu <sameera.gajjarapu@microsoft.com>
- Fix missing telemetry queue information
Today on every lookup of an account in the cache we iterate through
every entry in the cache (including cache entries not belonging to
MSAL), attempt to parse it into JSON and then validate that it matches
our account shape. This is incredibly inefficient and becomes more
inefficient with each additional thing stored in browser storage.

This PR adds a new cache entry (with a static key) which contains an
array of keys to all of the accounts known to MSAL. Now when the
accounts need to be enumerated the worst case number of cache entries
touched and parsed is 1 + number of accounts.

A new configuration option is added which determines whether or not this
new cache entry should be created on initialization. This is to help
migrate apps using localStorage from versions before this change.

Follow up PR will address the same issue for id, access and refresh
tokens.

---------

Co-authored-by: Thomas Norling <thnorlin@microsoft.com>
Co-authored-by: Doğan Erişen <v-derisen@microsoft.com>
I've added React to the [Electron system browser
Sample](https://github.com/AzureAD/microsoft-authentication-library-for-js/tree/dev/samples/msal-node-samples/ElectronSystemBrowserTestApp)
and Retired the [React Electron
sample](https://github.com/AzureAD/microsoft-authentication-library-for-js/tree/dev/samples/msal-node-samples/ElectronReactTestApp).
I've also added a warning message to the
[ElectronTestApp](https://github.com/AzureAD/microsoft-authentication-library-for-js/tree/dev/samples/msal-node-samples/ElectronTestApp)
sample and removed the e2e tests due to the sample not following the
[authentication best practices for native
apps](https://www.rfc-editor.org/rfc/rfc8252).

---------

Co-authored-by: Doğan Erişen <v-derisen@microsoft.com>
…#5760)

Adds a sample demonstrating the distributed token caching pattern for
client credentials flow. Includes collecting basic performance metrics.
Adds pipeline to build/test 1P library against 3P changes
Follow up to #5792, addressing the same problem but for token lookups.

Today on every lookup of tokens in the cache we iterate through every
entry in the cache (including cache entries not belonging to MSAL),
attempt to parse it into JSON and then validate that it matches our
credential shape. This is incredibly inefficient and becomes more
inefficient with each additional thing stored in browser storage.

This PR adds a new cache entry which contains an object of keys to all
of the tokens known to MSAL for a given clientId. Now when the tokens
need to be enumerated the worst case number of cache entries touched and
parsed is 1 + number of tokens of the given type.

Example:

```js
key: msal.token.keys.<clientId>
value: {
  idToken: ["idToken-key-1", "idToken-key-2"],
  accessToken: ["accessToken-key-1", "accessToken-key-2"],
  refreshToken: ["refreshToken-key-1"]
}
```

---------

Co-authored-by: Thomas Norling <thnorlin@microsoft.com>
The current distributed caching sample RedisTestApp is rather convoluted
with framework and wrapper details that take the attention away from
caching aspects. The replacement
[auth-code-distributed-cache](#5772)
is leaner and structurally alike to other distributed cache samples
([on-behalf-of-distributed-cache](#5766),
[client-credentials-distributed-cache](#5760))
to aid understanding.

The PR removes the RedisTestApp and all references to it.
- Update `package-lock.json` files.
- Add `@azure/node-token-validation` to lerna bootstrapping list.
@github-actions github-actions bot added documentation Related to documentation. extensions Related to extensions for the base libraries msal-angular Related to @azure/msal-angular package msal-browser Related to msal-browser package msal-common Related to msal-common package labels Apr 3, 2023
@codecov-commenter
Copy link

codecov-commenter commented Apr 3, 2023

Codecov Report

❗ No coverage uploaded for pull request base (msal-browser-v3@8a82b7c). Click here to learn what that means.
The diff coverage is n/a.

Flag Coverage Δ
msal-angular 96.61% <0.00%> (?)
msal-browser 84.12% <0.00%> (?)
msal-common 84.13% <0.00%> (?)
msal-node 78.40% <0.00%> (?)
msal-node-extensions 45.81% <0.00%> (?)
msal-react 94.82% <0.00%> (?)
node-token-validation 86.68% <0.00%> (?)

@tnorling
Copy link
Collaborator

tnorling commented Apr 3, 2023

  • Add @azure/node-token-validation to lerna bootstrapping list.

IIRC we were planning to remove this from our tooling since we're not working on it anymore. cc. @jo-arroyo @peterzenz

@konstantin-msft
Copy link
Collaborator Author

  • Add @azure/node-token-validation to lerna bootstrapping list.

IIRC we were planning to remove this from our tooling since we're not working on it anymore. cc. @jo-arroyo @peterzenz

Even though we are not working on it anymore we keep updating its' package.lock every release.

@tnorling
Copy link
Collaborator

tnorling commented Apr 3, 2023

Even though we are not working on it anymore we keep updating its' package.lock every release.

Right, I'm saying we would like to stop doing that.

@konstantin-msft
Copy link
Collaborator Author

Even though we are not working on it anymore we keep updating its' package.lock every release.

Right, I'm saying we would like to stop doing that.

@peterzenz @jo-arroyo In case we decide to not work on node-token-validtion module anymore I can create a separate PR to lock msal-common dep version for it.

Copy link
Contributor

@peterzenz peterzenz left a comment

Choose a reason for hiding this comment

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

Automation for the rebase, not providing a detailed review. There are no callouts on areas that need additional conflict resolution or additional review.

@@ -0,0 +1,7 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

I don't think these change files should be included. They were deleted before the last v2 release went out as they were integrated into changelogs.

Copy link
Member

Choose a reason for hiding this comment

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

I realize this was the state of dev before the release, we may need to pull from dev again into msal-browser-v3 after merging this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They will be deleted automatically during next rebase.

@konstantin-msft konstantin-msft merged commit 0e7234b into msal-browser-v3 Apr 4, 2023
35 of 36 checks passed
@konstantin-msft konstantin-msft deleted the rebase_dev_onto_v3 branch April 4, 2023 20:59
hectormmg pushed a commit that referenced this pull request Apr 4, 2023
- Reconcile changes after rebasing.
- Update `package-lock.json` files.
@ghost
Copy link

ghost commented May 3, 2023

🎉@azure/msal-react@v2.0.0-alpha.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented May 3, 2023

🎉@azure/msal-angular@v3.0.0-alpha.0 has been released which incorporates this pull request.:tada:

Handy links:

@sp90
Copy link

sp90 commented Jun 8, 2023

The bot link to "offical npm release" does not include -alpha.0 only links to 3.0.0

FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Related to documentation. extensions Related to extensions for the base libraries msal-angular Related to @azure/msal-angular package msal-browser Related to msal-browser package msal-common Related to msal-common package msal-node Related to msal-node package msal-react Related to @azure/msal-react node-token-validation samples Related to the samples apps for the library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet