cli-microsoft365
cli-microsoft365 copied to clipboard
pnp
Bug report: Impossible to create team when using application permissions
Priority
(Low) Something is a little off
Description
It is currently impossible to create a team when using application permissions. This is due to the application permissions request expecting a members property in the request body, and we currently do not demand this.
More information can be found on this URL: https://learn.microsoft.com/en-us/graph/api/team-post?view=graph-rest-1.0&tabs=http#examples
Steps to reproduce
- Login using application permissions
- Execute
m365 teams team add --name 'TEST' --description 'TEST'
Expected results
Team being created or a prompt for members
Actual results
Error: Request failed with status code 400
Diagnostics
Request error:
{
"url": "https://graph.microsoft.com/v1.0/teams",
"status": 400,
"statusText": "Bad Request",
"headers": {
"transfer-encoding": "chunked",
"content-type": "application/json",
"vary": "Accept-Encoding",
"strict-transport-security": "max-age=31536000",
"request-id": "51e35905-5bb0-4c3b-8616-df164c8099e3",
"client-request-id": "51e35905-5bb0-4c3b-8616-df164c8099e3",
"x-ms-ags-diagnostic": "{\"ServerInfo\":{\"DataCenter\":\"North Europe\",\"Slice\":\"E\",\"Ring\":\"4\",\"ScaleUnit\":\"003\",\"RoleInstance\":\"DB1PEPF0002D0AB\"}}",
"date": "Tue, 23 May 2023 09:52:51 GMT",
"connection": "close"
}
}
Thanks, @MathijsVerbeeck for rising this 👍. We will try to recheck this one ASAP
Makes sense I guess. I think the only fix here would be to introduce 2 new options
-
--ownerUserId [ownerUserId -
--ownerUserName [ownerUserName]
Both can be used with delegated or app only auth, but they will be required when using app only auth.
Makes sense I guess. I think the only fix here would be to introduce 2 new options
--ownerUserId [ownerUserId--ownerUserName [ownerUserName]Both can be used with delegated or app only auth, but they will be required when using app only auth.
I also thought something like that. Makes sense too.
Yeah, a team needs at least an owner indeed. We could also add a --memberUserNames option and allow a comma separated string of UPNs... (just like the chat message send command)
Yeah, a team needs at least an owner indeed. We could also add a
--memberUserNamesoption and allow a comma separated string of UPNs... (just like the chat message send command)
I'd say we start with just specifying a single owner, to at least get this command to work, and we can extend this command in general in the future I believe.
Yeah, a team needs at least an owner indeed. We could also add a
--memberUserNamesoption and allow a comma separated string of UPNs... (just like the chat message send command)I'd say we start with just specifying a single owner, to at least get this command to work, and we can extend this command in general in the future I believe.
I suggest that we decide now if we'd like to support a single- or multiple users, so that we can avoid breaking changes later on. If I for example look at spo site add and aad o365group add, we support --owners [owners]. I suggest we use the same convention here for consistency.
So
- in delegated mode, when you don't specify
--owners, it will add the signed in user. - in delegated mode, when you specify
--owners, it will add the owners, but not the signed in user if he isn't part of the owners string. - in app only mode,
--ownersis required.
?
What are owners? UPNs? IDs? Usually when we refer to a user we provide an option that accepts UPNs and an option that accepts IDs.
Yes, thinking about that though: why do we even do that? If it's a GUID or an UPN, why don't we verify what kind of value is being used in the CLI? Should be easy to do, and it would clean up a lot of commands with these superfluous options.
I don't know. 😊
Consistency could be an argument. For some commands this won't be that simple, e.g. for aad user get, how do you call an option that accepts both the ID and UPN? Also, some commands accept ID, UPN, and Email, in that case, we cannot tell whether an Email is UPN or a mail.
Should we add 2 options? ownerUserNames and ownerIds? Which can be used in both delegated and application auth?
If we add this, I feel like we should introduce the same options to add members.
Let's do that. And
ownerEmails?
Could include that one as well, so do we agree to add the options below?
-
ownerUserNames -
ownerIds -
ownerEmails -
memberUserNames -
memberIds -
memberEmails
Perfect. Thank you @martinlingstuyl, @milanholemans for the awesome brainstorm. @MathijsVerbeeck I think we area ready to open this up. Would you like to take the lead?
Perfect. Thank you @martinlingstuyl, @milanholemans for the awesome brainstorm. @MathijsVerbeeck I think we area ready to open this up. Would you like to take the lead?
Sure thing.!
Awesome 🤩. Please do remember Hacktoberfest starts on 26.09 so if you participate it's best to open the PR after that date 👍
Hi guys
Just some feedback - I came across the following issue:
Using just a single user works just fine. So it seems that either we have to change the new specs a little bit to allow only one user, or we have to update the team immediately, but then we have to enforce the --wait parameter.
That's a bummer! I think we should stick to the spec and update the team members afterward.
That's a bummer! I think we should stick to the spec and update the team members afterward.
Sure thing. So I create the team using the first user passed in either one of the owner options and then afterwards patch the team?
One (hopefully final) further remark I have is that we will have to only include this when using --wait, otherwise the team will most likely not yet be provisioned and it will throw an error.
good point, I don't think there is any better approach at the moment than enforcing --wait when you want to add multiple owners. Let's make sure that we have good error handling for this scenario and that we add a remark for it as well.
Hey @MathijsVerbeeck, just checking in. What is the latest regarding this issue?