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

SetSiteCollectionAdminsAsync doesn't set a single site collection administrator #1165

Closed
1 task done
lucmoco opened this issue Apr 24, 2023 · 10 comments
Closed
1 task done
Assignees
Labels
area:admin 📜 Admin library related help wanted Extra attention is needed

Comments

@lucmoco
Copy link

lucmoco commented Apr 24, 2023

Category

  • Bug

Describe the bug

I'm using the ISiteCollectionManager.SetSiteCollectionAdminsAsync method to set the site collection administrators of a communication site.

When I call this method with a single user in sharePointAdminLoginNames, the method sets the user as primary site collection administrator (which is correct) but does not remove other site collection administrators.

Steps to reproduce

  1. Create a SharePoint communication site
  2. In the central administration, set two site collection administrators:
  • UserA: primary site admin
  • UserB: site admin
  1. Use the PnP Core SDK method ISiteCollectionManager.SetSiteCollectionAdminsAsync to set UserB as single site admin (and thus primary site admin):
ISiteCollectionManager siteCollectionManager = ...; // get your site collection manager here
await siteCollectionManager.SetSiteCollectionAdminsAsync(new Uri("https://tenant.sharepoint.com/sites/the-site-url"), new List<string> { "i:0#.f|membership|userb@tenant.onmicrosoft.com" });

Expected behavior

I expect the following list of site administrators:

  • UserB: primary site admin

Instead, I get the following list of site administrators:

  • UserA: site admin
  • UserB: primary site admin

Environment details (development & target environment)

  • SDK version: PnP.Core.Admin 1.9.0, PnP.Core.Auth 1.9.0
  • OS: Windows 10
  • SDK used in: ASP.Net Web API
  • Framework: .NET 7.0
  • Browser(s): Chrome 112.0.5615.138 (Official Build) (64-bit)
  • Tooling: Visual Studio 2022 Version 17.5.4
  • Additional details: The call is done with the client credentials flow, with role "Sites.FullControl.All"
@lucmoco lucmoco changed the title SetSiteCollectionAdminsAsync doens't set a single site collection administrator SetSiteCollectionAdminsAsync doesn't set a single site collection administrator Apr 24, 2023
@jansenbe jansenbe added question Further information is requested area:admin 📜 Admin library related labels Apr 26, 2023
@jansenbe
Copy link
Contributor

@lucmoco : this method indeed does not remove site collection admins, I'll clarify that in the docs. I think it's better to also include a remove method that allows for removal of a secondary administrator or a owner from the Microsoft 365 group when the site is group connected.

@jansenbe jansenbe added help wanted Extra attention is needed and removed question Further information is requested labels Apr 26, 2023
@lucmoco
Copy link
Author

lucmoco commented May 2, 2023

@jansenbe, thank you for your explanation. But in that case, is there a possibility to set a single administrator, or do I have to wait for the secondary administrator or owner removal method ? In my case the site is not group-connected, it's a communication site, but of course it would be good to support both scenarios.

@jansenbe
Copy link
Contributor

jansenbe commented May 2, 2023

@lucmoco : correct, we'll first have to add a removal method allowing you to remove the secondary admin. Depending on how time sensitive your ask is you can code this yourselves via our custom API support (https://pnp.github.io/pnpcore/using-the-sdk/basics-customapirequests.html).

@mloitzl
Copy link
Contributor

mloitzl commented May 2, 2023

@jansenbe @lucmoco So something like
RemoveSiteCollectionAdmin( ISiteCollectionAdmin adminToRemove )... with a ISiteCollectionAdmin instance previously retrieved using GetSiteCollectionAdmins?

@lucmoco
Copy link
Author

lucmoco commented May 2, 2023

@mloitzl, yes, that could be a possibility. Having such a method would certainly be useful, but in my case I would have to diff the current admins with the wished ones and compute the list of admins to remove.

Certainly not impossible but I would prefer having a flag stating if we want to add only administrators (the current behaviour) or set the exact list of administrators. This could be for example an additional parameter in the ISiteCollectionManager.SetSiteCollectionAdminsAsync method, maybe a parameter updateOptions of an enum type like this one:

public enum CollectionUpdateOptions
{
    /// <summary>
    /// Add additional items to the collection, but do not remove items.
    /// </summary>
    AddOnly,

    /// <summary>
    /// Update the collection so it matches the provided items, adding and removing items as needed.
    /// </summary>
    SetExact,
}

The default value would be CollectionUpdateOptions.AddOnly so the method remains backward-compatible.

@jansenbe
Copy link
Contributor

jansenbe commented May 3, 2023

Thanks for thinking along for a solution @mloitzl and @lucmoco :-) I've been checking again on how things currently work in tenant admin and I'm proposing the following. Let's stick with the existing SetSiteCollectionAdmin methods but add the CollectionUpdateOptions enum as optional parameter having a value of AddOnly to ensure backwards compatability. When the option SetExact is chosen the implementation can then use the POST _api/SPO.Tenant/SetSiteAdministrators API call versus the current POST _api/Microsoft.Online.SharePoint.TenantAdministration.Tenant/SetSiteSecondaryAdministrators (they payload is almost identical).

image

image

Next to that it would be great to have a method SetAsPrimarySiteCollectionAdministrator on ISiteCollectionAdmin that allows for making one of the existing site collection admins the primary one. This can be done using a PATCH _api/SPO.Tenant/sites('side id')

image

image

The returned value is a SpoOperation object which indicates setting the primary admin is an async operation. Using the WaitForSpoOperationCompleteAsync method you can 'wait' for the operation to complete.

image

@mloitzl : would you be so kind to work on this?

@mloitzl
Copy link
Contributor

mloitzl commented May 4, 2023

@jansenbe Thanks for your clarifications.
I will work on this during the upcoming weekends.

@lucmoco
Copy link
Author

lucmoco commented May 4, 2023

@jansenbe, this looks good thank you. Thanks to you and @mloitzl for your involvement !

@mloitzl
Copy link
Contributor

mloitzl commented May 20, 2023

@jansenbe @lucmoco
Here we go:
#1184

Have a nice weekend!

@jansenbe
Copy link
Contributor

@lucmoco : the change from @mloitzl has been merged, closing this issue now.

@mloitzl : thanks a lot for helping out 🚀👍💖

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:admin 📜 Admin library related help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants