Replacing ADAL.js with MSAL.js

[vman]

Just wanted to get some thoughts around this. After recently investigating granting access to specific scopes for the Office 365 CLI (blog here), I started looking into implementing something similar out of the box for the CLI.

But soon released that the CLI currently uses the adal-node library which doesn't allow granting consent to specific scopes as its based on AAD v1. The yammer consent mechanism is more of a manual approach if I am not wrong.

Since MSAL is based on AAD v2 and hence allows granting consent to specific scopes and is also going to be the auth library of choice moving forward, should we look at using it for the CLI?

This would enable the consent or login commands to have an additional scope option which would only request the required permissions on the tenant. Just as an example:

Either

login --scope Group.ReadWrite.All

OR

consent --scope Group.ReadWrite.All

The default option could be to request all permissions just like now, with the option to request specific scopes for the more security conscious tenants.

[rabwill]

@vman also noticed the adal-node library is going on a maintenance mode. It has some deprecated dependencies as well and the alternative is msal, see this discussion AzureAD/azure-activedirectory-library-for-nodejs#229 , they are also working on msal-node

[vman]

That's interesting, so looks like msal-node is in initial planning stages right now and would be a while until ready for GA. In the meantime, could we update to msal-js as recommended in the thread? I am happy to take a first stab at this.

[waldekmastykarz]

The problem is that MSAL doesn't support device code auth which we need, so until that changes, we can't move.

As for the consent command, it uses the same URL for OAuth v2.0 consent flow as you would get when using MSAL, so from that point of view, it would make little difference. For the consent command, we chose to refer to a service rather to a specific scope to simplify things. Otherwise, for each command, we'd need to track which set of permissions it requires. If there is a need for it, we could extend the consent command to support both services and scopes.

[garrytrinder]

MSAL also doesn't support Client Certificate currently and it doesn't appear on the roadmap which we would also need, so that's another reason we can't move.

[vman]

Looks like we will have to track progress of msal-node for getting the device code flow: AzureAD/microsoft-authentication-library-for-js#1281

[waldekmastykarz]

Shall we close this issue until the GA of MSAL with the features we need and revisit the topic then?

[vman]

Sure. Makes sense to me.

On Wed, 11 Mar 2020 at 6:18 pm, Waldek Mastykarz notifications@github.com
wrote:

Shall we close this issue until the GA of MSAL with the features we need
and revisit the topic then?

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#1399 (comment),
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AABKNYVBRIO6J4W3XX73QSTRG7IXJANCNFSM4LD5UIJQ
.

[garrytrinder]

Just noticed that Device Code auth was added to msal-node on Apr 10, the library is currently in alpha however good to know that there is now a path forward for us to take.

AzureAD/microsoft-authentication-library-for-js#1409

[waldekmastykarz]

Nice! Have you heard/seen anything regarding the planned release date?

[garrytrinder]

I haven't but can see if I can find the owner of the PG responsible and ask. I know this is alpha but we could consider creating a new branch to test out the new module, if we find any issues then we can feed this back into the development making for a smoother transition.

[waldekmastykarz]

Good idea (both of them actually ;) )

[garrytrinder]

A change to the logging in msal-node will cause us some issues, currently we can change the logging levels when commands are executed using the LogLevel.setLoggingOptions() method and direct the logging output to stdout.

In msal-node, a logging callback function is defined in the initialization of the PublicClientApplication class, which replaces AuthenticationContext, and cannot be changed after initialization.

[waldekmastykarz]

Could we point it to Utils.logOutput which we use for processing all vorpal output before printing to stdout?

[garrytrinder]

This is what I have for initializing the PublicClientApplication in the constructor where the loggerCallback function and logLevel is defined.

this.pca = new PublicClientApplication({
      auth: {
        clientId: config.cliAadAppId,
        authority: `https://login.microsoftonline.com/${config.tenant}`
      }, system: {
        loggerOptions: {
          loggerCallback: (loglevel: LogLevel, message: string, containsPii: boolean) => {
            Utils.logOutput(message);
          },
          piiLoggingEnabled: false,
          logLevel: LogLevel.Verbose
        }
      }
    });

We can use Utils.logOuput here, however as logLevel is defined in the constructor, which we currently change on the fly in adal-node based on the presence of the debug option when the command is executed, we would just flood the output with logging messages.

I think that we would need to implement something similar to Set-PnPTraceLog in PnP PowerShell to turn enable or disable logging output to the console globally and remove the --debug option from commands.

[waldekmastykarz]

I wonder if that's the case though. The issue you mentioned could be the case with CLI using immersive mode, where verbosity is set separately for each command. With v3 removing the immersive mode, CLI lifecycle doesn't span beyond the command execution, so theoretically we should be fine because there is no way to run two commands with running CLI just once.