From 305ed296f45504d7751fcf6c98aff0dcb0fd1755 Mon Sep 17 00:00:00 2001 From: Yen Su <49794855+yens1@users.noreply.github.com> Date: Wed, 5 Aug 2026 01:30:35 -0700 Subject: [PATCH] fix(SharePoint Node): Point delegated Sites.Selected users at URL or ID mode (#35369) Co-authored-by: Claude Sonnet 5 --- .../Microsoft/SharePoint/test/v2/site.test.ts | 50 +++++++++++++++---- .../Microsoft/SharePoint/v2/site/index.ts | 30 ++++++----- 2 files changed, 58 insertions(+), 22 deletions(-) diff --git a/packages/nodes-base/nodes/Microsoft/SharePoint/test/v2/site.test.ts b/packages/nodes-base/nodes/Microsoft/SharePoint/test/v2/site.test.ts index 5e1fe452a80..99aa74f2cf5 100644 --- a/packages/nodes-base/nodes/Microsoft/SharePoint/test/v2/site.test.ts +++ b/packages/nodes-base/nodes/Microsoft/SharePoint/test/v2/site.test.ts @@ -120,23 +120,53 @@ describe('Microsoft SharePoint v2 — site selection', () => { expect(thrown?.description).toContain('Sites.Read.All application permission'); }); - it('keeps the permission-naming message for delegated refusals', async () => { + it('points delegated sign-ins without search rights at URL or ID mode', async () => { ctx.getNodeParameter.mockReturnValue('microsoftOAuth2Api'); - // Mirrors the transport's delegated 403 shape: the permission-naming - // message is set as the error option, as delegatedApiError does + // The old permission-naming message must be replaced, not merely unread — + // mock it present so a leak would show up in the assertions below apiRequest.mockRejectedValue( new NodeApiError( mock(), - { message: 'refused' }, - { - httpCode: '403', - message: 'the credential may be missing the Sites.Read.All permission', - }, + {}, + { httpCode: '403', message: 'the credential may be missing the Sites.Read.All permission' }, ), ); - // URL paste would hit the same refusal — the accurate message must survive - await expect(getSites.call(ctx)).rejects.toThrow(/Sites\.Read\.All/); + let thrown: NodeApiError | undefined; + try { + await getSites.call(ctx); + } catch (error) { + thrown = error as NodeApiError; + } + + expect(thrown).toBeInstanceOf(NodeApiError); + expect(thrown?.httpCode).toBe('403'); + expect(thrown?.message).toBe('This credential cannot search sites'); + const description = thrown?.description ?? ''; + expect(description).toContain('URL or ID mode'); + expect(description).toContain('Sites.Selected'); + // Sites.Read.All is the optional, secondary path — not the primary instruction + expect(description.indexOf('URL or ID mode')).toBeLessThan( + description.indexOf('Sites.Read.All'), + ); + // The transport's original wording must not survive into the new message + expect(thrown?.message).not.toContain('missing'); + expect(description).not.toContain('missing'); + }); + + it('passes through a non-403 delegated error unchanged', async () => { + ctx.getNodeParameter.mockReturnValue('microsoftOAuth2Api'); + const original = new NodeApiError(mock(), { message: 'boom' }, { httpCode: '500' }); + apiRequest.mockRejectedValue(original); + + await expect(getSites.call(ctx)).rejects.toBe(original); + }); + + it('passes through a non-NodeApiError delegated rejection unchanged', async () => { + ctx.getNodeParameter.mockReturnValue('microsoftOAuth2Api'); + apiRequest.mockRejectedValue(new Error('boom')); + + await expect(getSites.call(ctx)).rejects.toThrow('boom'); }); it('offers search first, with URL and ID modes alongside', () => { diff --git a/packages/nodes-base/nodes/Microsoft/SharePoint/v2/site/index.ts b/packages/nodes-base/nodes/Microsoft/SharePoint/v2/site/index.ts index 10397601943..cd9cd766d92 100644 --- a/packages/nodes-base/nodes/Microsoft/SharePoint/v2/site/index.ts +++ b/packages/nodes-base/nodes/Microsoft/SharePoint/v2/site/index.ts @@ -4,6 +4,7 @@ import type { INodeListSearchResult, INodeParameterResourceLocator, INodeProperties, + JsonObject, } from 'n8n-workflow'; import { NodeApiError, NodeOperationError } from 'n8n-workflow'; @@ -93,18 +94,23 @@ export async function getSites( }, )) as SiteSearchReply); } catch (error) { - // An app with only per-site permissions can't list what it can't see — - // point at the URL mode. Delegated refusals keep the transport's message, - // which names the missing permission (URL paste would hit the same 403). - if ( - error instanceof NodeApiError && - error.httpCode === '403' && - getSharePointCredentialType.call(this) === SERVICE_PRINCIPAL_AUTH - ) { - throw new NodeOperationError(this.getNode(), 'This app registration cannot search sites', { - description: - "An app registration with only per-site permissions can't list sites. Choose the site by pasting its URL instead — that still works — or grant the app the Sites.Read.All application permission to enable search.", - }); + if (error instanceof NodeApiError && error.httpCode === '403') { + if (getSharePointCredentialType.call(this) === SERVICE_PRINCIPAL_AUTH) { + throw new NodeOperationError(this.getNode(), 'This app registration cannot search sites', { + description: + "An app registration with only per-site permissions can't list sites. Choose the site by pasting its URL instead — that still works — or grant the app the Sites.Read.All application permission to enable search.", + }); + } + throw new NodeApiError( + this.getNode(), + { message: 'This credential cannot search sites' } as JsonObject, + { + message: 'This credential cannot search sites', + description: + "Switch the Site field to URL or ID mode — Microsoft Graph doesn't support site search under Sites.Selected, and neither mode needs that permission. If URL or ID mode also fails, the credential likely lacks a different permission or consent, not just Sites.Read.All.", + httpCode: '403', + }, + ); } throw error; }