Bug report: Cannot add an owner to a site if I don't have access to the site already #4600

martinlingstuyl · 2023-03-01T11:59:08Z

Description

Some time ago we refactored the spo site classic set into the spo site set command and combined group site and regular site setting operations.

This has resulted in an issue. We can no longer set owners for a site if we are currently not an owner or site member of that site.

In other words, if I am a SharePoint administrator, I sometimes want to give myself access to site collections using scripts. But now I cannot.

This command is affected:
https://pnp.github.io/cli-microsoft365/cmd/spo/site/site-set/

The reason for this situation is that the CLI first tries to retrieve if the site is a Group or not, right here

It does that using the site specific api, which it cannot access because the logged in user has no access yet.

I'm not sure how we should fix this yet. We had some thoughts on specific CRUD actions for Sitecollection admins, but these are currently nowhere near completion: #4173

What we could do is start using the tenant api to get the site details, (just like spo site list)

Steps to reproduce

Try to add your own account as an owner to a site you have no access to:

m365 spo site set --url "https://contoso.sharepoint.com/sites/some-site" --owners [email protected]

Implementation

We've decided to switch from the /sites/somesite/_api/site to use an admin API. We'll want to use the following API call to fetch the right information:

POST https://tenant-admin.sharepoint.com/_api/SPO.Tenant/RenderAdminListData

data:

{
    "parameters": {
        "ViewXml": "<View><Query><Where><Contains><FieldRef Name='SiteUrl'/><Value Type='Text'>https://tenant.sharepoint.com/sites/yoursite</Value></Contains></Where></Query><ViewFields><FieldRef Name=\"GroupId\"/><FieldRef Name=\"SiteId\"/><FieldRef Name=\"SiteUrl\"/></ViewFields></View>"
    }
}

We'll want to place this functionality in a util function, so we can reuse it in other places. I'd suggest we add the following function to the spo.ts util function:

/**
 * Retrieves a Custom Actions from a SharePoint site by Id.
 * @param adminUrl URL of the SharePoint admin site
 * @param camlQuery An optional viewQuery to add to the CAML query between the <Query> tags.
 * @param viewFields An optional array of internal names of fields to include in the response.
 */
getTenantSites(adminUrl: string, camlQuery?: string, viewFields?: string[])

The text was updated successfully, but these errors were encountered:

milanholemans · 2023-03-01T13:01:13Z

I think it should be possible for both owners of the site and admin users. Perhaps we have to add an --asAdmin flag like we do for pp commands.

martinlingstuyl · 2023-03-01T13:26:44Z

We would still need to check if the site is a group or not though...

By the way I think the command seems to almost always need admin privileges, so an extra flag should not be necessary.

nicodecleyre · 2023-03-01T13:56:15Z

Can it be an option to use the graph batch api to add an owner to a site where you don't have access to?

POST https://graph.microsoft.com/v1.0/$batch

{
    "requests": [
        {
            "id": "AddMember_0",
            "method": "POST",
            "url": "/groups/[GROUP_ID]/owners/$ref",
            "body": {
                "@odata.id": "https://graph.microsoft.com/v1.0/users/[USER_ID]"
            },
            "headers": {
                "content-type": "application/json"
            }
        },
        {
            "id": "AddMember_1",
            "method": "POST",
            "url": "/groups/[GROUP_ID]/owners/$ref",
            "body": {
                "@odata.id": "https://graph.microsoft.com/v1.0/users/[USER_ID]"
            },
            "headers": {
                "content-type": "application/json"
            }
        }
    ]
}

martinlingstuyl · 2023-03-01T14:33:07Z

The point is that we dont have the groupId yet. As soon as we would have it, the current implementation works fine, I believe.

nicodecleyre · 2023-03-01T15:14:48Z

The point is that we dont have the groupId yet. As soon as we would have it, the current implementation works fine, I believe.

Got it, then we can use this api:

POST https://tenant-admin.sharepoint.com/_api/SPO.Tenant/RenderAdminListData

data:

{
    "parameters": {
        "ViewXml": "<View><Query><Where><Contains><FieldRef Name='SiteUrl'/><Value Type='Text'>https://tenant.sharepoint.com/sites/yoursite</Value></Contains></Where></Query><ViewFields><FieldRef Name=\"GroupId\"/><FieldRef Name=\"SiteId\"/><FieldRef Name=\"SiteUrl\"/></ViewFields></View>"
    }
}

But it would need an accesstoken for the admin site resource though

martinlingstuyl · 2023-03-01T15:50:00Z

We could also reuse ${spoAdminUrl}/_vti_bin/client.svc/ProcessQuery as we do in spo site list, the access token is no problem I think..

martinlingstuyl · 2023-03-01T15:57:12Z

Well, you need to add the xml envelope specific to this endpoint.... (check out site-list.ts)

nicodecleyre · 2023-03-01T21:17:41Z

Well, you need to add the xml envelope specific to this endpoint.... (check out site-list.ts)

I tried to use the xml envelope in combination with the ${spoAdminUrl}/_vti_bin/client.svc/ProcessQuery. But i'm getting an 'invalid request' response. Am I doing something wrong here?

the data:

<soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/">
    <soapenv:Body>
        <GetListItems xmlns="http://schemas.microsoft.com/sharepoint/soap/">
            <listName>DO_NOT_DELETE_SPLIST_TENANTADMIN_AGGREGATED_SITECOLLECTIONS</listName>
            <ViewFields>
                <FieldRef Name="GroupId"/>
                <FieldRef Name="SiteId"/>
                <FieldRef Name="SiteUrl"/>
            </ViewFields>
            <View>
                <Query>
                    <Where>
                        <Contains>
                            <FieldRef Name="SiteUrl"/>
                            <Value Type="Text">https://tenant.sharepoint.com/sites/yoursite</Value>
                        </Contains>
                    </Where>
                </Query>
            </View>
        </GetListItems>
    </soapenv:Body>
</soapenv:Envelope>

nicodecleyre · 2023-03-01T21:30:02Z

Also noticed we can do a get request to get the group id:

GET https://tenant-admin.sharepoint.com/_api/web/lists/getbytitle('DO_NOT_DELETE_SPLIST_TENANTADMIN_AGGREGATED_SITECOLLECTIONS')/items?$select=Title,SiteId,SiteUrl,GroupId&$filter=SiteUrl eq 'https://tenant.sharepoint.com/sites/yoursite'

nicodecleyre · 2023-03-01T21:51:23Z

Well, you need to add the xml envelope specific to this endpoint.... (check out site-list.ts)

I tried to use the xml envelope in combination with the ${spoAdminUrl}/_vti_bin/client.svc/ProcessQuery. But i'm getting an 'invalid request' response. Am I doing something wrong here?

the data:

<soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/">
    <soapenv:Body>
        <GetListItems xmlns="http://schemas.microsoft.com/sharepoint/soap/">
            <listName>DO_NOT_DELETE_SPLIST_TENANTADMIN_AGGREGATED_SITECOLLECTIONS</listName>
            <ViewFields>
                <FieldRef Name="GroupId"/>
                <FieldRef Name="SiteId"/>
                <FieldRef Name="SiteUrl"/>
            </ViewFields>
            <View>
                <Query>
                    <Where>
                        <Contains>
                            <FieldRef Name="SiteUrl"/>
                            <Value Type="Text">https://tenant.sharepoint.com/sites/yoursite</Value>
                        </Contains>
                    </Where>
                </Query>
            </View>
        </GetListItems>
    </soapenv:Body>
</soapenv:Envelope>

This seems to work regarding the ${spoAdminUrl}/_vti_bin/client.svc/ProcessQuery api.

<Request AddExpandoFieldTypeSuffix="true" SchemaVersion="15.0.0.0" LibraryVersion="16.0.0.0" ApplicationName="CLI for Microsoft 365 v6.4.0" xmlns="http://schemas.microsoft.com/sharepoint/clientquery/2009">
    <Actions>
        <ObjectPath Id="2" ObjectPathId="1"/>
        <ObjectPath Id="4" ObjectPathId="3"/>
        <Query Id="5" ObjectPathId="3">
            <Query>
                <Properties/>
            </Query>
            <ChildItemQuery>
                <Properties>
                    <Property Name="GroupId"/>
                </Properties>
            </ChildItemQuery>
        </Query>
    </Actions>
    <ObjectPaths>
        <Constructor Id="1" TypeId="{268004ae-ef6b-4e9b-8425-127220d84719}"/>
        <Method Id="3" ParentId="1" Name="GetSitePropertiesFromSharePointByFilters">
            <Parameters>
                <Parameter TypeId="{b92aeee2-c92c-4b67-abcc-024e471bc140}">
                    <Property Name="Filter" Type="String">Url -eq 'https://tenant.sharepoint.com/sites/yoursite'</Property>
                    <Property Name="IncludeDetail" Type="Boolean">false</Property>
                    <Property Name="IncludePersonalSite" Type="Enum">0</Property>
                    <Property Name="StartIndex" Type="String">0</Property>
                    <Property Name="Template" Type="String"></Property>
                </Parameter>
            </Parameters>
        </Method>
    </ObjectPaths>
</Request>

But it doesn't seem to recognize the SiteId property (tried to show all the properties but it doesn't show a site id either.. 😞)

martinlingstuyl · 2023-03-01T22:19:28Z

Maybe just an id? It should have one, although it might be named different..

nicodecleyre · 2023-03-01T22:25:01Z

Maybe just an id? It should have one, although it might be named different..

doesn't seem so.. The same goes for spo site list, it doesn't display an id

nicodecleyre · 2023-03-02T15:48:21Z

So tested this out. We can fix this by using the get api https://tenant-admin.sharepoint.com/_api/web/lists/getbytitle('DO_NOT_DELETE_SPLIST_TENANTADMIN_AGGREGATED_SITECOLLECTIONS')/items?$select=Title,SiteId,SiteUrl,GroupId&$filter=SiteUrl eq 'https://tenant.sharepoint.com/sites/yoursite' to retrieve the groupId and siteId for sites where we don't have access to. So the loadSiteIds function would look like this:

The situation as is regarding a site where you don't have access to:

The situation with the get api regarding a site where you don't have access to (siteId & groupId temporary logged to show that they get retrieved):

martinlingstuyl · 2023-03-03T07:53:58Z

Hi @nicodecleyre, the only thing that I'm slightly concerned about with this way of doing things is the 5000-item 'limit' on SharePoint.

With the tenant endpoint, SP will take care of loading and filtering. If we do it ourselves using this list, we might run into these annoying errors with large tenants.

nicodecleyre · 2023-03-03T08:19:50Z

Let's stick then with the first option and use RenderAdminListData, it's the official api that the ui uses in the new SharePoint admin portal and it's a wrapper over the RenderListDataAsStream, which can handle the 5000 threshold limit..

martinlingstuyl · 2023-03-03T20:07:52Z

Does it contain a SiteId?

nicodecleyre · 2023-03-03T20:10:16Z

Does it contain a SiteId?

Yes

martinlingstuyl · 2023-03-04T19:51:00Z

Ok, interesting, does it allow to $select properties to optimize the request? If so, we could use it.

Maybe we could check how the output relates to the current spo site list output. We might move this into a util function and use it in both places.

For example: getTenantSites(adminUrl:string, filter:string, select:string)

nicodecleyre · 2023-03-05T09:43:52Z

Ok, interesting, does it allow to $select properties to optimize the request? If so, we could use it.

No, it ignores the $select query parameter. The selection of the properties happen in the ViewFields element like in the example above

Maybe we could check how the output relates to the current spo site list output. We might move this into a util function and use it in both places.

For example: getTenantSites(adminUrl:string, filter:string, select:string)

Also no, the properties the 2 api's return don't match 100%. The /_api/site (which is currently used by the spo site list command) returns properties that the RenderAdminListData doesn't return and visa versa. However, we could use the RenderAdminListData api to refactor the spo hubsite list where currently the /_api/web/lists/GetByTitle('DO_NOT_DELETE_SPLIST_TENANTADMIN_AGGREGATED_SITECOLLECTIONS')/RenderListDataAsStream api is used. But it won't change a thing regarding getting or not getting the site when you don't have access to it, it will just make the code look nicer

martinlingstuyl · 2023-03-06T09:16:19Z

the RenderListDataAsStream, which can handle the 5000 threshold limit..

By the way, out of curiosity, are you sure RenderListDataAsStream can handle the threshold limit? It's basically an endpoint to send CAML queries to. CAML queries can also run into threshold limit errors. Do you have documentation to support this?

Aside from that, I agree with you that we should use the wrapper endpoint, as it's used by the SP user interface as well and as it contains the correct properties.

Also no, the properties the 2 api's return don't match 100%. The /_api/site (which is currently used by the spo site list command) returns properties that the RenderAdminListData doesn't return and visa versa.

In that case: let's leave the spo site list command, as it's working right now and there's no need to refactor it, certainly not when this would incur a breaking change.

However, we could use the RenderAdminListData api to refactor the spo hubsite list where currently the /_api/web/lists/GetByTitle('DO_NOT_DELETE_SPLIST_TENANTADMIN_AGGREGATED_SITECOLLECTIONS')/RenderListDataAsStream api is used.

I like this idea. Also, because that will remove the need to use executeCommandWithOutput in that command. Let's create a util function while we're fixing this bug. I'll rework the specs.

martinlingstuyl · 2023-03-06T09:26:08Z

@pnp/cli-for-microsoft-365-maintainers any comments on the renewed specs, based on the discussion?

martinlingstuyl · 2023-03-06T09:26:58Z

Thanks for the research @nicodecleyre!

nicodecleyre · 2023-03-06T09:39:16Z

By the way, out of curiosity, are you sure RenderListDataAsStream can handle the threshold limit? It's basically an endpoint to send CAML queries to. CAML queries can also run into threshold limit errors. Do you have documentation to support this?

No official documentation found regarding that. Only that it's the recommended approach when retrieving info from large lists and some blog posts that say it can handle the threshold limit

Adam-it · 2023-03-16T21:14:05Z

@pnp/cli-for-microsoft-365-maintainers any comments on the renewed specs, based on the discussion?

first of all sorry for the late response 🙏
I checked the discussion. Seems like all what I could think of was already checked 😅.
seems like a solid plan 🤔. +1

martinlingstuyl · 2023-05-10T11:46:01Z

let's get this shipped!

nicodecleyre · 2023-05-10T14:39:50Z

Something I can work on?

martinlingstuyl · 2023-05-10T15:57:54Z

Sure @nicodecleyre!

martinlingstuyl added bug needs discussion labels Mar 1, 2023

martinlingstuyl changed the title ~~Bug report: Cannot set owner of a site if I am not a site member~~ Bug report: Cannot add an owner to a site if I don't have access to the site already Mar 1, 2023

martinlingstuyl added needs research and removed needs discussion labels Mar 2, 2023

martinlingstuyl added needs peer review Needs second pair of eyes to review the spec or PR and removed needs research labels Mar 6, 2023

martinlingstuyl added help wanted and removed needs peer review Needs second pair of eyes to review the spec or PR labels May 10, 2023

martinlingstuyl added work in progress and removed help wanted labels May 10, 2023

martinlingstuyl assigned nicodecleyre May 10, 2023

nicodecleyre linked a pull request May 11, 2023 that will close this issue

Enhances spo site set with ability to adding owner without having permissions, Closes #4600 #4861

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug report: Cannot add an owner to a site if I don't have access to the site already #4600

Bug report: Cannot add an owner to a site if I don't have access to the site already #4600

martinlingstuyl commented Mar 1, 2023 •

edited

Loading

milanholemans commented Mar 1, 2023

martinlingstuyl commented Mar 1, 2023

nicodecleyre commented Mar 1, 2023

martinlingstuyl commented Mar 1, 2023

nicodecleyre commented Mar 1, 2023 •

edited

Loading

martinlingstuyl commented Mar 1, 2023

martinlingstuyl commented Mar 1, 2023

nicodecleyre commented Mar 1, 2023

nicodecleyre commented Mar 1, 2023 •

edited

Loading

nicodecleyre commented Mar 1, 2023

martinlingstuyl commented Mar 1, 2023

nicodecleyre commented Mar 1, 2023

nicodecleyre commented Mar 2, 2023

martinlingstuyl commented Mar 3, 2023

nicodecleyre commented Mar 3, 2023

martinlingstuyl commented Mar 3, 2023

nicodecleyre commented Mar 3, 2023

martinlingstuyl commented Mar 4, 2023

nicodecleyre commented Mar 5, 2023

martinlingstuyl commented Mar 6, 2023 •

edited

Loading

martinlingstuyl commented Mar 6, 2023

martinlingstuyl commented Mar 6, 2023

nicodecleyre commented Mar 6, 2023

Adam-it commented Mar 16, 2023

martinlingstuyl commented May 10, 2023

nicodecleyre commented May 10, 2023

martinlingstuyl commented May 10, 2023

Bug report: Cannot add an owner to a site if I don't have access to the site already #4600

Bug report: Cannot add an owner to a site if I don't have access to the site already #4600

Comments

martinlingstuyl commented Mar 1, 2023 • edited Loading

Description

Steps to reproduce

Implementation

milanholemans commented Mar 1, 2023

martinlingstuyl commented Mar 1, 2023

nicodecleyre commented Mar 1, 2023

martinlingstuyl commented Mar 1, 2023

nicodecleyre commented Mar 1, 2023 • edited Loading

martinlingstuyl commented Mar 1, 2023

martinlingstuyl commented Mar 1, 2023

nicodecleyre commented Mar 1, 2023

nicodecleyre commented Mar 1, 2023 • edited Loading

nicodecleyre commented Mar 1, 2023

martinlingstuyl commented Mar 1, 2023

nicodecleyre commented Mar 1, 2023

nicodecleyre commented Mar 2, 2023

martinlingstuyl commented Mar 3, 2023

nicodecleyre commented Mar 3, 2023

martinlingstuyl commented Mar 3, 2023

nicodecleyre commented Mar 3, 2023

martinlingstuyl commented Mar 4, 2023

nicodecleyre commented Mar 5, 2023

martinlingstuyl commented Mar 6, 2023 • edited Loading

martinlingstuyl commented Mar 6, 2023

martinlingstuyl commented Mar 6, 2023

nicodecleyre commented Mar 6, 2023

Adam-it commented Mar 16, 2023

martinlingstuyl commented May 10, 2023

nicodecleyre commented May 10, 2023

martinlingstuyl commented May 10, 2023

martinlingstuyl commented Mar 1, 2023 •

edited

Loading

nicodecleyre commented Mar 1, 2023 •

edited

Loading

nicodecleyre commented Mar 1, 2023 •

edited

Loading

martinlingstuyl commented Mar 6, 2023 •

edited

Loading