Closed waldekmastykarz closed 4 months ago
Nice suggestion @waldekmastykarz some thoughts from me.
m365 account set --identity <identity>
Did you mean m365 identity set
?
Where identity is a human-readable identifier of the signed in identities to choose from
Could we extend the login
command with an optional --identityName
property, so the user can define a name that is relevant to them?
change the login command so that it doesn't log out previously signed in user. Also, after signing in, it sets this identity as active.
This might be assumed but the login
command should also add the identity to a pool of signed in identities.
Did you mean m365 identity set?
Yes! Updated
Could we extend the login command with an optional --identityName property, so the user can define a name that is relevant to them?
Possibly. I'd suggest that we first look at what's available to us from MSAL and if that's sufficient or if we need something on top. I wonder if something like user name or app name would be sufficient, or maybe even we could make the identity set
command interactive so that you can select the identity from the list and don't need to memorize anything.
This might be assumed but the login command should also add the identity to a pool of signed in identities.
Yes, I've updated the spec for completeness.
I like it @waldekmastykarz.
So when you have logged in to four accounts and you log out of the active identity, what will m365 status
return?
Good question @martinlingstuyl! We can either pick one from the list (last or first for example) or introduce another status and prompt user to select an identity. Which one do you think would be more intuitive?
I think it would be best to not auto-select another identity. This could be rather dangerous 😀
Good point. I'll update the spec.
Also: how about interactively selecting an identity from a list using the arrow and enter keys?
Also: how about interactively selecting an identity from a list using the arrow and enter keys?
That's exactly what I meant. Let's see if we can do this using inquirer which we already use for prompts.
Sorry for the late response.
I like the idea a lot 👍. Seems like it would give a lot of flexibility.
I wonder if we could introduce a new option which would apply to all commands (like --debug) which would allow to change the identity along the way executing the command, like --identity
. I wonder if this would not give even more flexibility if user could just (optionally of course) specify some identity in one command and a different one in a second command, and CLI would do all the switching and executing the command to the proper place 🤔.
... But ok, this I would leave for the future 😉.
Interesting idea @Adam-it. What would be the use case for it?
I like your idea @Adam-it, if we were to take a use case of sending an email using different identities, you could do this with three commands.
m365 outlook mail send ... --identity <identity>
m365 outlook mail send ... --identity <identity>
m365 outlook mail send ... --identity <identity>
As apposed to using the proposed identity set
command.
m365 outlook mail send ...
m365 identity set --identity <identity>
m365 outlook mail send ...
m365 identity set --identity <identity>
m365 outlook mail send ...
m365 identity set --identity <identity>
I think there is room for both options here.
@waldekmastykarz sorry for the not sufficient response. I am currently on holiday mainly over a phone 😋. I will try to catch up over the weekend when I will be two days out of the woods and next to some laptop 😅. .... anyway. What @garrytrinder described is exactly what I had in mind. Good work @garrytrinder reading my thoughts ...magic powers I guess 🧙♂️😉. @waldekmastykarz there is no really good use case except that it may be more convenient for the user to do all in single line instead of two lines 😅. So it is really something nice to have we could introduce later on 👍. I had something in mind like we have a lot of commands we do in context of identity A but in the middle of this all we want to execute a command with identity B 😉. Like :
m365 spo file add ....
m365 spo folder rename...
m365 spo listitem remove....
// TODO: send mail using different identity. and then move on with other stuff. I would need to do 3 lines identity set -> send mail -> identity set back to previous one. Alternatively I may pass identity as option to any command which will run this one command only with different identity (the passed one). In background CLI actually would use the identity set command to manage this but the user may just use one liner 😉
m365 outlook mail send ... --identity <identity>
m365 spo list list....
etc...
I hope it's more understandable now.. I did my very best 🤩.... better I may do on weekend 😋
m365 outlook mail send ...
m365 identity set --identity <identity>
m365 outlook mail send ...
m365 identity set --identity <identity>
m365 outlook mail send ...
m365 identity set --identity <identity>
...might not even work reliably, because it sets identity system-wide, so if you were to run something else in parallel, you might get mixed identities.
I'd say that this is a separate use case that we investigate once we have the basics implemented.
Also, when we think about introducing a global option, we need to consider it being available on all commands, like login
, status
, spfx *
, cli completion *
etc. so we need to give it a proper thought if that's something that we really want.
I'd say that this is a separate use case that we investigate once we have the basics implemented.
I'm happy with this.
Love the idea about multiple identities ❤. Super handy for the consultants who handles gazillion of accounts and does IT Pro operations for them.
As an extension to the command, I was thinking whether we can show which identity was used for executing command. As a user who manages multiple tenants, and when I execute, I would love to see and confirm whether the command is executed against the right tenant. Even now the same issue is there, but when we give the support of concurrent account sign-in, probability of execution against wrong tenant would be on a higher end.
My idea would be something like this
m365 teams team add --name "Architecture" --description "Architecture Discussion"
Executing using the identity megan@contoso.com
@odata.context : https://graph.microsoft.com/v1.0/$metadata#teams('7f0d5bfc-fae5-43c0-8bdf-030916daab70')/operations/$entity Value : {"apps":[],"channels":[],"WorkflowId":"northcentralus.ff8de907-cd7e-4505-a165-b38ad8373d18"} attemptsCount : 1 createdDateTime : 2022-08-19T11:22:38.166614Z error : null id : f9374ac4-3c1a-427e-85bf-3e4c56a3423d lastActionDateTime : 2022-08-19T11:22:38.166614Z operationType : createTeam status : notStarted targetResourceId : 7f0d5bfc-fae5-43c0-8bdf-030916daab70 targetResourceLocation: /teams('7f0d5bfc-fae5-43c0-8bdf-030916daab70')
Used Identity Prompt could be made as a configuration so that it could be disabled via settings. This may not be relevant for the scheduled script usages, but would be handy for IT pros who does execution against multiple tenants.
As an extension to the command, I was thinking whether we can show which identity was used for executing command.
Great idea! We should include it in the debug output. I'll add it to the spec.
As a user who manages multiple tenants, and when I execute, I would love to see and confirm whether the command is executed against the right tenant.
How would that work in a script, where each command would prompt you to confirm the identity that you want to use if there are multiple signed in?
Great idea! We should include it in the debug output. I'll add it to the spec.
That will be wonderful ❤
How would that work in a script, where each command would prompt you to confirm the identity that you want to use if there are multiple signed in?
Here my thought process is to show this output always when the output type is text.
Let me try to explain my thoughts here.
So my understanding in the whole process is that, when you execute m365 identity set --identity <identity>
, all the commands which follows after that will use that identity as default.
So that being the case, my thought process was to log the message Executing as <identity>
to the console and then show the output of the command. This would be the case only when the output is text. For JSON, its flow will remain as usual.
Even for that, we can give a Configuration Settings something like alwaysShowExecutingIdentity
. Message Executing as <identity>
will be shown on the console only when the flag is enabled.
Am I making sense?
If I have understood the process wrong and If the process is that - Each execution will prompt for identity, then above scenarios will not stand and does not make sense.
So that being the case, my thought process was to log the message Executing as
to the console and then show the output of the command. This would be the case only when the output is text. For JSON, its flow will remain as usual.
Right, but that's just a log message, not a prompt, right? We could print it even in JSON output. As long as we send it to stderr it won't be mixed up with the command's output. We'd need to check though how it would work in PowerShell that has its own notion of output streams.
Would we show this message always, or only if there are multiple identities signed in? Also, would we show it always or only in verbose and debug output?
Right, but that's just a log message, not a prompt, right?
Correct. It is just a message which shows before the execution of the command and gets console log.
Would we show this message always, or only if there are multiple identities signed in?
My vote would be always irrespective of multiple identities to make it consistent irrespective of how many identities are available.
Also, would we show it always or only in verbose and debug output?
My vote would be to show it always in the verbose and debug. And in normal mode, it will show only if the config setting alwaysShowExecutingIdentity
is true. Otherwise it will not be shown
Right, but that's just a log message, not a prompt, right?
Correct. It is just a message which shows before the execution of the command and gets console log.
Would we show this message always, or only if there are multiple identities signed in?
My vote would be always irrespective of multiple identities to make it consistent irrespective of how many identities are available.
Also, would we show it always or only in verbose and debug output?
My vote would be to show it always in the verbose and debug. And in normal mode, it will show only if the config setting
alwaysShowExecutingIdentity
is true. Otherwise it will not be shown
Hey @pnp/cli-for-microsoft-365-maintainers, what do you think about Arjun's proposal to always include identity information?
I think it might give a lot of clutter in the logstream, on the other hand I agree that this can be life-saving information.
Two thoughts:
@arjunumenon 's proposal: if the information is only available on verbose
and debug
, we'd still risk missing it. I'm usually only setting verbose
or debug
in specific scenarios where I want to see what happens. Might it be better to always show the warning line in the logstream, unless you've set a hide... config key? I still wouldn't like the clutter though, so:
another proposal would be to show a warning line when executing the first m365
command in a given shell session, or the first command after using identity/account/login commands. This would reduce the clutter but still alert the user. I'm not sure if it's possible though. @waldekmastykarz, do you think that can be done?
+1 to always add in verbose and debug and to normal output when some config setting set to true which by default is false.
@martinlingstuyl very interesting idea with showing the warning message when first command executed 🤔. Should it only be shown when we have more than one identity logged in?. What if along the way we add or logout from some accounts. Should this warning be shown every time something changes?. TBH I still believe we may have scenarios that the user may not see this warning as it got lost along the way of work 🤔
another proposal would be to show a warning line when executing the first m365 command in a given shell session, or the first command after using identity/account/login commands. This would reduce the clutter but still alert the user. I'm not sure if it's possible though. @waldekmastykarz, do you think that can be done?
Unfortunately, each CLI command is run standalone, and shells have no notion of a session, so I don't think we can do this.
Another thing we need to check is how that line would be handled in PowerShell. If it turns out that it collides with the primary output, or complicates processing it, then we should reconsider showing it always, because it would make CLI harder to use.
which would allow to change the identity along the way executing the command, like --identity. I wonder if this would not give even more flexibility
Yesterday I ran into the need for this: I'm used to running a single script in multiple posh windows to update a lot of sites as fast as possible. This is a possibility with PnP PowerShell because the sign in session is restricted to the PowerShell terminal session. I can sign into multiple posh terminals using multiple identities and update the sites, not having to worry about throttling.
For the CLI this is currently not possible, as we don't support multiple identities and as the sign in session is system wide.
I'd like to be able though, to sign into multiple identities and being able to use an --identity flag on every command so I can use the CLI for this as well as PnP Powershell.
I'd like to be able though, to sign into multiple identities and being able to use an --identity flag on every command so I can use the CLI for this as well as PnP Powershell.
I still have my doubts about this approach. I think it would be preferable to set the identity on a session and we should look for ways to do that.
I think it would be preferable to set the identity on a session and we should look for ways to do that.
One way I just realized that would be possible is to assign the identity to the PID of the current shell process which we can get from process.ppid
. That way we could imitate a "session" and store all kind of information applicable only to the current terminal process.
One way I just realized that would be possible is to assign the identity to the PID of the current shell process which we can get from
process.ppid
Freakin' awesome 😁 (And persisted in the GitHub issue/process)
Thinking some more about the logout command, I wonder if it wouldn't be more in line if:
Here's some recent findings:
research if switching accounts is possible between app-only and delegated or only delegated
That's not possible using MSAL. MSAL bases its multi-account support off of the client, and public and confidential clients are two different clients, so if we want to support not only multiple accounts but also multiple accounts, we'd need to handle it ourselves. With MSAL, we'd only have the ability to store multiple user accounts for the same client and the moment we use a different client (AAD app), we'd need to destroy the previously stored client and account information.
Since you can use any AAD app with the CLI, I suggest that, if we want to proceed with this functionality, we handle support for multiple accounts ourselves. We'd store all information about the different apps used with the login command, and then let users select which identity they want to use.
Thoughts @pnp/cli-for-microsoft-365-maintainers?
Ah, awesome that you dug into this. So basically you're saying that if you want to use multiple apps to sign in, this would not be supported by the MSAL setup for multiple accounts?
Using multiple apps is a pretty often used scenario on my machine, where im signing into m365 using the default app as well as custom apps with certificates.
I'd like it to be as flexible as possible here. So creating our own setup would be the way forward as far as im concerned.
So, do we need to cache refresh tokens for this to work?
If possible, I'd go for full flexibility as well. Both delegated and app authentication together would be awesome.
Using multiple apps is a pretty often used scenario on my machine, where im signing into m365 using the default app as well as custom apps with certificates.
I'd like it to be as flexible as possible here. So creating our own setup would be the way forward as far as im concerned.
So, do we need to cache refresh tokens for this to work?
Thanks for bringing this up. This provides some extra important information for implementing this feature. We've been caching refresh tokens already, so this wouldn't be anything new for us. When implementing this feature, we'd build an extra management layer on top of MSAL, so that we can properly store multiple identities and after selecting one, pass it to MSAL for auth. For listing and selecting identities, we should then not only show the user name but also app- and tenant ID, auth type, and perhaps extend the login command to let users specify a user-friendly name to select the identity by.
Hi guys,
I've been experimenting with this feature as I for one could really use it. (And all people who work in similar IT Services)
I'm currently as far as being able to sign in with multiple accounts. 🚀
I'm a bit late to the party here but that SC looks awesome @martinlingstuyl! That's a feature I would love to see implemented. A question I've here. Why are we listing the other accounts in an array within the original object? Wouldn't it make more sense to return a list that lists all accounts and add a boolean property for which one is selected?
That would be one of the things we'd need to discuss indeed @Jwaegebaert.
For one, it would be a breaking change to give it back like that.
Also: in the code I'm trying to change as little as possible about the auth.service
object, because that's used like everywhere :-) Working with an array would be annoying all over the place. We would need to change the code everywhere.
Also we're saving the service to a json file as a cache. That's an object as well. It would also be a breaking change if we switched to saving an array.
So I just chose the easy route for now:
I've added an otherAccounts
property on the auth.service
object to work with it.
I fear the refactor would be huge otherwise.
Of course the code structure would not need to define the response output 1 on 1, but then again: it's a breaking change there as well.
Would it make more sense if it read availableIdentities
?
Extra argument for the response output here: If you're passing the status into a variable, it's easier and more logical for the user to work with an object:
$status = m365 status | ConvertFrom-Json
write-host $status.connectedAs
Instead of having to write $status[0].connectedAs
or $status | where-object { $_.active -eq $true } | select "connectedAs"
When signed into multiple accounts, switch to another account
m365 identity set [options]
Option | Description |
---|---|
-i, --id [id] |
The Id (GUID) of the identity to switch to. Can be found by running m365 status . Specify either id or name but not both. |
-n, --name [name] |
The name of the identity to switch to. Can be found by running m365 status . Specify either id or name but not both. |
The Id used here is the localAccountId
as MSAL returns it. That way we can find the account to logout from. My proposition is to add that localAccountId
as an extra property identityId
on the m365 status output. This id should also be saved in our cache, so we we know what MSAL account to switch to and logout from.
The name
property is the value as visible in the connectedAs
property when running m365 status. It can be a UPN or the application name.
When not specifying an option, the user will be prompted to select an identity using our inquirer prompts. For this to happen, the prompt config key will need to be enabled, otherwise a validation error will be thrown.
Note: We currently have two caches: 1) The MSAL token cache which is saved to a file
.cli-m365-msal.json
and b) Our own cache which saves the auth.service object and is saved to.cli-m365-tokens.json
. Because we are now able to log into multiple accounts, we should save the localAccountId, as a link between both caches. If we do it like this, the caching can just remain the same.
Switch to a user account by name:
m365 identity set --name martin@contoso.com
Switch to an application account by name:
m365 identity set --name 'My application'
Switch to an account by id:
m365 identity set --id '0bb7cb89-7fae-4775-a01a-c372cc167371'
m365 logout [options]
Option | Description |
---|---|
-i, --identityId [identityId] |
The optional Id (GUID) of the identity to logout from. Can be found by running m365 status . Specify either identityId or identityName but not both. If not specified, all accounts will be logged out from. |
-n, --identityName [identityName] |
The optional name of the identity to switch to. Can be found by running m365 status . Specify either identityId or identityName but not both. If not specified, all accounts will be logged out from |
Log out from all accounts:
m365 logout
Log out from a single (user) account by name:
m365 logout --identityName martin@contoso.com
Log out from a single application account by name:
m365 logout --identityName 'My application'
Log out from a single account by id:
m365 logout --identityId '0bb7cb89-7fae-4775-a01a-c372cc167371'
Show the list of currently signed in identities
m365 identity list [options]
No options
Returns a list of currently signed in identities:
m365 identity list
Remarks on this @pnp/cli-for-microsoft-365-maintainers?
@martinlingstuyl why not in both places use the same option naming? I like identityId
and identityName
more then id
and name
. Seems more descriptive.
Also, why not name the command m365 identity switch
? It won't be actually updating so set
does not seem fit IMO 🤔.
As to the option names: that's because of our naming convention: not reusing the last noun in the option name.
As to switch
: I just copied over identity set
from the initial specs. I agree with you that switch sounds better, and it should be possible in such a case as this. What do you think @waldekmastykarz? (As you created the initial specs)
I'd avoid introducing additional verbs. set
is how we're defining updating/activating throughout the CLI and I think it's still a good fit for this purpose.
@martinlingstuyl MSAL has native support for multiple accounts. Are you using it or have you chosen a different path? I wonder how much can we use off of it, rather than building it ourselves?
Hi @waldekmastykarz, yes I'm absolutely using MSAL native multi-account support here.
It's basically the MSAL Token Store that you can access through functions like getAllAccounts()
.
As I've written somewhere in my comment: we store the MSAL token store in a json file (.cli-m365-msal.json
), and we also have our own extra json file (.cli-m365-tokens.json
) with account-information relating to the CLI, for example the login method.
In my work with this, I've kept everything largely the same. The only thing I had to change was adding an extra ID (localAccountId
) to our own json file store, to make sure we know how to get the correct account from MSAL to work with it.
And of course, i've updated our own .cli-m365-tokens.json
json token store with an array of the additional identities.
For one, it would be a breaking change to give it back like that.
That is a fair point, it's also quite a common command that is used in scripts. So on the breaking change
chain, it would be enormous.
Would it make more sense if it read availableIdentities?
That's a good suggestion. I'm more keen on this name.
Regarding the m365 identity set
specs, I've got nothing to add to them. They look good to go. Also more of a fan of using set
Any other feedback @pnp/cli-for-microsoft-365-maintainers?
For the status command, would it make sense to keep its output as-is and then introduce another identity list
command that lists available signed in identities so that the whole feature is implemented in a set of commands that are grouped together by name?
That's perfectly fine as well 👍 would be similar to how az works
Ok, I've updated the issue specs. Time for a last review @pnp/cli-for-microsoft-365-maintainers
Just to check: does MSAL also supports multiple apps signed on simultaneously using app-only auth?
One more thing that popped to my mind. Consider running a script that you want to ensure is running under a specific identity. Right now, you'd have to run the status command and compare the name of the currently active identity with the intended one, which adds complexity. Alternatively, you'd call identity set
specifying your identity and the name/id you specified is not available in MSAL cache, I suppose the command and hence the script would fail acting as a failsafe. I think this is something that we should document somewhere to help people build robust scripts and avoid side-effects.
I'd need to check that: haven't tried that yet!
Plus a good point on documenting!
Add support for signing in using multiple accounts. MSAL supports this capability natively so we can build on top of it.
login
command so that it doesn't log out previously signed in user. Also, after signing in, it adds the identity to the list of available connections (if it wasn't already present in the list) and sets this connection as activelogin
command to set the connection name when logging in:--connectionName
logout
command will sign out of all connectionsconnection list
command to show a list of all signed in users (m365 status
will keep returning the currently active identity)connection use
command to select which identity to use:m365 connection use --identity <identity>
, whereidentity
is a human-readable identifier of the signed in identities to choose from. The selected identity get set as active and will be used by CLI when running commands. If user selects an invalid identity, we keep the previously selected identity as active.connection set
command to update the connection nameconnection remove
command to remove/signout from a connection by name. If you remove the active connection account, leave the CLI in identity-less state. CLI will prompt you to login or select a connection (if available)m365 connection list specs
Show the list of available connections
Usage
Options
No options
Examples
Returns a list of available connections:
m365 connection use specs
When signed in with multiple identities, switch to another connection
Usage
Options
-n, --name <name>
m365 connection list
.Remarks
The Id used here is the
localAccountId
as MSAL returns it. That way we can find the account to logout from. My proposition is to add thatlocalAccountId
as an extra propertyidentityId
on the m365 status output. This id should also be saved in our cache, so we we know what MSAL account to switch to and logout from.The
name
property is the value as visible in thename
property when runningm365 connection list
. By default it is a combination of principal object Id and tenant Id. But it can be configured usingm365 connection set
.Failures: When the command fails, the user should be put into an identity-less state, otherwise side effects might occur in scripts where people expected an identity to be selected, while in fact the previously selected identity is used.
Examples
Switch to another connection by a default connection name:
Switch to another connection by a custom connection name:
m365 connection set specs
When signed in with multiple identities, update a specified connection
Usage
Options
-n, --name <name>
m365 connection list
.--newName <newName>
Examples
Update a connection with a new name
m365 connection remove specs
When signed in with multiple identities, remove a connection
Usage
Options
-n, --name <name>
m365 connection list
.Examples
Remove a connection by a default connection name:
Remove a connection by a custom connection name:
Discussed in https://github.com/pnp/cli-microsoft365/discussions/3453