diff --git a/src/app/core/interceptors/error.interceptor.spec.ts b/src/app/core/interceptors/error.interceptor.spec.ts index 951e22394..68b73a2b9 100644 --- a/src/app/core/interceptors/error.interceptor.spec.ts +++ b/src/app/core/interceptors/error.interceptor.spec.ts @@ -295,6 +295,34 @@ describe('errorInterceptor', () => { expect(toastServiceMock.showError).not.toHaveBeenCalled(); }); + it('should navigate to request-access for 403 node URL with trailing slash and query', async () => { + setup('browser', false, '/projects/abc'); + const request = createRequest('/api/v2/nodes/abc/?related_counts=forks'); + const error = new HttpErrorResponse({ + status: 403, + error: {}, + url: '/api/v2/nodes/abc/?related_counts=forks', + }); + + const caught = await runInterceptor(request, error); + + expect(caught?.status).toBe(403); + expect(router.navigate).toHaveBeenCalledWith(['/request-access', 'abc']); + }); + + it('should not redirect on 403 for node relationship URL', async () => { + setup('browser', false, '/projects/abc'); + const request = createRequest('/api/v2/nodes/abc/preprints/'); + const error = new HttpErrorResponse({ status: 403, error: {}, url: '/api/v2/nodes/abc/preprints/' }); + + const caught = await runInterceptor(request, error); + + expect(caught?.status).toBe(403); + expect(router.navigate).not.toHaveBeenCalled(); + expect(loaderServiceMock.hide).toHaveBeenCalled(); + expect(toastServiceMock.showError).not.toHaveBeenCalled(); + }); + it('should navigate to forbidden for 403 without a node URL', async () => { setup(); const request = createRequest('/api/v2/users/me/'); @@ -308,6 +336,18 @@ describe('errorInterceptor', () => { expect(toastServiceMock.showError).not.toHaveBeenCalled(); }); + it('should not set SSR response status for node relationship 403', async () => { + setup('server'); + const request = createRequest('/api/v2/nodes/abc/preprints/'); + const error = new HttpErrorResponse({ status: 403, error: {}, url: '/api/v2/nodes/abc/preprints/' }); + + const caught = await runInterceptor(request, error); + + expect(caught?.status).toBe(403); + expect(responseInit.status).toBeUndefined(); + expect(router.navigate).not.toHaveBeenCalled(); + }); + it('should not redirect on 403 when current route is a draft registry', async () => { setup('browser', false, '/registries/drafts/abc'); const request = createRequest('/api/v2/nodes/abc'); diff --git a/src/app/core/interceptors/error.interceptor.ts b/src/app/core/interceptors/error.interceptor.ts index 1709438d0..952ee58e2 100644 --- a/src/app/core/interceptors/error.interceptor.ts +++ b/src/app/core/interceptors/error.interceptor.ts @@ -26,7 +26,8 @@ import { BYPASS_ERROR_INTERCEPTOR } from './error-interceptor.tokens'; const NO_AUTH_REDIRECT_HEADER = 'X-No-Auth-Redirect'; const REQUEST_ACCESS_URL_REGEX = /\/v2\/nodes\/[^/]+\/requests\/?$/i; -const NODE_ID_URL_REGEX = /\/v2\/nodes\/([^/]+)/; +const NODE_RESOURCE_URL_REGEX = /\/v2\/nodes\/([^/?]+)\/?(?:\?|$)/i; +const NODE_RELATED_URL_REGEX = /\/v2\/nodes\/[^/?]+\/[^?]/i; const DRAFT_REGISTRY_ROUTE_REGEX = /\/registries\/drafts\/.+/i; const isSilentForbidden = (req: HttpRequest, error: HttpErrorResponse): boolean => @@ -35,6 +36,11 @@ const isSilentForbidden = (req: HttpRequest, error: HttpErrorResponse): const isPendingModerationForbidden = (error: HttpErrorResponse): boolean => error.error?.errors?.[0]?.detail === PREPRINT_PENDING_MODERATION_DETAIL; +const getNodeResourceId = (url: string | null | undefined): string | undefined => + url?.match(NODE_RESOURCE_URL_REGEX)?.[1]; + +const isNodeRelatedUrl = (url: string | null | undefined): boolean => !!url && NODE_RELATED_URL_REGEX.test(url); + const getErrorMessage = (error: HttpErrorResponse): string => { if (error.error instanceof ErrorEvent) { return error.error.message; @@ -76,20 +82,24 @@ export const errorInterceptor: HttpInterceptorFn = (req, next) => { }; const handleForbidden = (url: string | null) => { - const nodeId = url?.match(NODE_ID_URL_REGEX)?.[1]; + const nodeId = getNodeResourceId(url); - if (!nodeId) { - router.navigate(['/forbidden']); - } else if (!DRAFT_REGISTRY_ROUTE_REGEX.test(router.url)) { - router.navigate(['/request-access', nodeId]); + if (nodeId) { + if (!DRAFT_REGISTRY_ROUTE_REGEX.test(router.url)) { + router.navigate(['/request-access', nodeId]); + } + return; } + + router.navigate(['/forbidden']); }; const applySsrStatus = (error: HttpErrorResponse) => { if ( req.context.get(BYPASS_ERROR_INTERCEPTOR) || !shouldPropagateHttpStatusToSsr(error.status) || - isSilentForbidden(req, error) + isSilentForbidden(req, error) || + (error.status === 403 && isNodeRelatedUrl(error.url)) ) { return; } @@ -122,7 +132,7 @@ export const errorInterceptor: HttpInterceptorFn = (req, next) => { handleUnauthorized(); break; case 403: - if (!isSilentForbidden(req, error) && !isPendingModerationForbidden(error)) { + if (!isSilentForbidden(req, error) && !isPendingModerationForbidden(error) && !isNodeRelatedUrl(error.url)) { handleForbidden(error.url); } break; diff --git a/src/app/features/project/overview/components/project-overview-metadata/project-overview-metadata.component.spec.ts b/src/app/features/project/overview/components/project-overview-metadata/project-overview-metadata.component.spec.ts index a8931935f..6276c6f72 100644 --- a/src/app/features/project/overview/components/project-overview-metadata/project-overview-metadata.component.spec.ts +++ b/src/app/features/project/overview/components/project-overview-metadata/project-overview-metadata.component.spec.ts @@ -143,7 +143,7 @@ describe('ProjectOverviewMetadataComponent', () => { expect(dispatchMock).toHaveBeenCalledWith(new GetBibliographicContributors('project-1', ResourceType.Project)); expect(dispatchMock).toHaveBeenCalledWith(new GetProjectInstitutions('project-1')); expect(dispatchMock).toHaveBeenCalledWith(new GetProjectIdentifiers('project-1')); - expect(dispatchMock).toHaveBeenCalledWith(new GetProjectPreprints('project-1')); + expect(dispatchMock).toHaveBeenCalledWith(new GetProjectPreprints(MOCK_PROJECT_OVERVIEW.preprintsUrl)); expect(dispatchMock).toHaveBeenCalledWith(new FetchSelectedSubjects('project-1', ResourceType.Project)); expect(dispatchMock).toHaveBeenCalledWith(new GetProjectSubmissions('project-1')); expect(dispatchMock).toHaveBeenCalledWith(new GetCustomItemMetadata('project-1')); diff --git a/src/app/features/project/overview/components/project-overview-metadata/project-overview-metadata.component.ts b/src/app/features/project/overview/components/project-overview-metadata/project-overview-metadata.component.ts index 4ab4f60a5..010e1e7e8 100644 --- a/src/app/features/project/overview/components/project-overview-metadata/project-overview-metadata.component.ts +++ b/src/app/features/project/overview/components/project-overview-metadata/project-overview-metadata.component.ts @@ -132,7 +132,7 @@ export class ProjectOverviewMetadataComponent { this.actions.getBibliographicContributors(project.id, ResourceType.Project); this.actions.getInstitutions(project.id); this.actions.getIdentifiers(project.id); - this.actions.getPreprints(project.id); + this.actions.getPreprints(project.preprintsUrl); this.actions.getSubjects(project.id, ResourceType.Project); this.actions.getProjectSubmissions(project.id); this.actions.getLicense(project.licenseId); diff --git a/src/app/features/project/overview/mappers/project-overview.mapper.ts b/src/app/features/project/overview/mappers/project-overview.mapper.ts index 186b39b44..67d7b0614 100644 --- a/src/app/features/project/overview/mappers/project-overview.mapper.ts +++ b/src/app/features/project/overview/mappers/project-overview.mapper.ts @@ -19,6 +19,7 @@ export class ProjectOverviewMapper { ? (relationships?.view_only_links?.links?.related?.meta['count'] as number) : 0, licenseId: relationships?.license?.data?.id, + preprintsUrl: relationships?.preprints?.links?.related?.href ?? null, }; } } diff --git a/src/app/features/project/overview/models/project-overview.model.ts b/src/app/features/project/overview/models/project-overview.model.ts index f1bd65c38..7c1d95c06 100644 --- a/src/app/features/project/overview/models/project-overview.model.ts +++ b/src/app/features/project/overview/models/project-overview.model.ts @@ -7,5 +7,6 @@ export interface ProjectOverviewModel extends BaseNodeModel { parentId?: string; rootParentId?: string; licenseId?: string; + preprintsUrl?: string | null; contributors?: ContributorModel[]; } diff --git a/src/app/features/project/overview/services/project-overview.service.spec.ts b/src/app/features/project/overview/services/project-overview.service.spec.ts index 314248cf7..edf896f75 100644 --- a/src/app/features/project/overview/services/project-overview.service.spec.ts +++ b/src/app/features/project/overview/services/project-overview.service.spec.ts @@ -152,13 +152,14 @@ describe('ProjectOverviewService', () => { it('getProjectPreprints should request preprints and map response', async () => { setup(); + const preprintsUrl = `${MOCK_API_URL}/nodes/${projectId}/preprints/`; const apiResponse = { data: [] } as unknown as NodePreprintsResponseJsonApi; const mapperSpy = vi.spyOn(NodePreprintMapper, 'getNodePreprints').mockReturnValue(MOCK_NODE_PREPRINTS); jsonApiService.get.mockReturnValue(of(apiResponse)); - const response = await firstValueFrom(service.getProjectPreprints(projectId)); + const response = await firstValueFrom(service.getProjectPreprints(preprintsUrl)); - expect(jsonApiService.get).toHaveBeenCalledWith(`${MOCK_API_URL}/nodes/${projectId}/preprints/`); + expect(jsonApiService.get).toHaveBeenCalledWith(preprintsUrl); expect(mapperSpy).toHaveBeenCalledWith(apiResponse.data); expect(response).toEqual(MOCK_NODE_PREPRINTS); }); diff --git a/src/app/features/project/overview/services/project-overview.service.ts b/src/app/features/project/overview/services/project-overview.service.ts index 9b93a910c..1b2716972 100644 --- a/src/app/features/project/overview/services/project-overview.service.ts +++ b/src/app/features/project/overview/services/project-overview.service.ts @@ -80,9 +80,9 @@ export class ProjectOverviewService { .pipe(map((response) => NodeStorageMapper.getNodeStorage(response.data))); } - getProjectPreprints(projectId: string): Observable { + getProjectPreprints(preprintsUrl: string): Observable { return this.jsonApiService - .get(`${this.apiUrl}/nodes/${projectId}/preprints/`) + .get(preprintsUrl) .pipe(map((response) => NodePreprintMapper.getNodePreprints(response.data))); } diff --git a/src/app/features/project/overview/store/project-overview.actions.ts b/src/app/features/project/overview/store/project-overview.actions.ts index 16c79931a..56a7458c5 100644 --- a/src/app/features/project/overview/store/project-overview.actions.ts +++ b/src/app/features/project/overview/store/project-overview.actions.ts @@ -35,7 +35,7 @@ export class GetProjectStorage { export class GetProjectPreprints { static readonly type = '[Project Overview] Get Project Preprints'; - constructor(public projectId: string) {} + constructor(public preprintsUrl: string | null | undefined) {} } export class UpdateProjectPublicStatus { diff --git a/src/app/features/project/overview/store/project-overview.state.ts b/src/app/features/project/overview/store/project-overview.state.ts index 8cd50508f..80b2c8159 100644 --- a/src/app/features/project/overview/store/project-overview.state.ts +++ b/src/app/features/project/overview/store/project-overview.state.ts @@ -187,6 +187,10 @@ export class ProjectOverviewState { @Action(GetProjectPreprints) getProjectPreprints(ctx: StateContext, action: GetProjectPreprints) { + if (!action.preprintsUrl) { + return; + } + const state = ctx.getState(); ctx.patchState({ preprints: { @@ -195,7 +199,7 @@ export class ProjectOverviewState { }, }); - return this.projectOverviewService.getProjectPreprints(action.projectId).pipe( + return this.projectOverviewService.getProjectPreprints(action.preprintsUrl).pipe( tap((preprints) => { ctx.patchState({ preprints: { diff --git a/src/app/shared/models/nodes/base-node-relationships-json-api.model.ts b/src/app/shared/models/nodes/base-node-relationships-json-api.model.ts index 9c007cc4e..10efcc471 100644 --- a/src/app/shared/models/nodes/base-node-relationships-json-api.model.ts +++ b/src/app/shared/models/nodes/base-node-relationships-json-api.model.ts @@ -22,7 +22,7 @@ export interface BaseNodeRelationships { logs?: ToManyRel<'logs'>; node_links?: ToManyRel<'node-links'>; parent?: ToOneRel<'nodes'>; - preprints?: ToManyRel<'preprints'>; + preprints?: ToManyRel<'preprints'> | null; region?: ToOneRel<'regions'>; registrations?: ToManyRel<'registrations'>; root?: ToOneRel<'nodes'>; diff --git a/src/testing/mocks/project-overview.mock.ts b/src/testing/mocks/project-overview.mock.ts index 3a6e21045..af352c322 100644 --- a/src/testing/mocks/project-overview.mock.ts +++ b/src/testing/mocks/project-overview.mock.ts @@ -57,4 +57,5 @@ export const MOCK_PROJECT_OVERVIEW: ProjectOverviewModel = { contributors: [], forksCount: 0, viewOnlyLinksCount: 0, + preprintsUrl: 'https://api.osf.io/v2/nodes/project-1/preprints/', };