Cli-microsoft365: Update command: m365 login, pass in --clientId and --tenantId, removing the need for environment variables

[YannickRe]

Usage

m365 login [options]

Description

Allow to pass in clientId and tenantId as options to the m365 login command when using the certificate authentication option.

Options

Would leverage the existing set of options and add :
| Option | Description |
| ----------------------- | ----------------------------------------- |
| —clientId [clientId] | Pass in the clientId of the custom application, only required when using certificate authentication |
| —tenantId [tenantId] | Pass in the tenantId where the custom application is registered, only required when using certificate authentication |

Additional Information

I believe passing in both values instead of setting them as environment variables would make it more straightforward to use, would be more in line with how all commands in CLI work (none of them require environment variables) and more in line with how PnP.PowerShell handles this.

[waldekmastykarz]

Thanks for the suggestion @YannickRe! There are quite some differences between how PnP PowerShell and CLI for Microsoft 365 handle authentication. That said, it's a valid suggestion. Let us have a look how we could properly handle it.

[waldekmastykarz]

Note for implementation, if we can support this in the login command, then we should also update the cli consent and cli reconsent commands to support this as well and offer a consistent experience

[garrytrinder]

Could we use the same mechanism as we discussed in #1945 to set the clientId and tenantId?

When passing them via login command we persist, effectively using the internal implementation of cli config set.

Maybe we should also clear out the settings if m365 logout is called.

[YannickRe]

@garrytrinder I'm not sure. I don't like the idea of having these two values persisted on disk, because it would mean they will be reused for every subsequent session (if you don't call m365 logout explicitly). I'd prefer to make this explicit every time I make a login command and if I don't supply it, it should fallback to the built-in id and not my previously persisted id.
Just my 2c.

[waldekmastykarz]

@YannickRe so here's a difference between PowerShell and bash/zsh/cmd: in PowerShell you have the notion of a session. In other shells you don't. So for your suggestion to work, we would have to persist the tenant and clientId along with token information. Each command execution in CLI for Microsoft 365 is each own process that doesn't know anything about what else happened in the current session. So disk is the only way for us to persist that so that we can retrieve access token in subsequent command executions.

@garrytrinder if we were to implement support for specifying clientId and tenantId in the login command, we could remove environment variables for these properties altogether as they would duplicate the functionality. Since clientId and tenantId belong with auth, the best would be to store them along with tokens rather than in a separate setting file.

[YannickRe]

Hah, learned something new. That's what you get for staying in the Windows world for so long 😃
I'll leave the rest of the discussion up to you, as I don't completely "get" it but I "feel" it is going in the right direction 🙂

[garrytrinder]

if we were to implement support for specifying clientId and tenantId in the login command, we could remove environment variables for these properties altogether as they would duplicate the functionality.

This would be a good move however wouldn't we need to keep this duplication so that we don't break existing usage?

Since clientId and tenantId belong with auth, the best would be to store them along with tokens rather than in a separate setting file.

Sounds like a plan 🚀

[waldekmastykarz]

This would be a good move however wouldn't we need to keep this duplication so that we don't break existing usage?

In v3 we'd absolutely keep everything as-is to preserve backwards compatibility. We could mark is as deprecated and remove in v4.