From 6f640ca6110d2bf9995c3a3b3c68d49d0ba5b62c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Germ=C3=A1n=20Saracca?= Date: Tue, 3 Dec 2024 15:15:34 -0300 Subject: [PATCH] feat: sanitize future installation terms of use --- src/info/domain/models/TermsOfUse.ts | 2 +- .../repositories/DataverseInfoRepository.ts | 2 +- src/info/domain/useCases/getTermsOfUse.ts | 4 ++-- .../mappers/JSTermsOfUseMapper.ts | 18 ++++++++++++++++++ .../DataverseInfoJSDataverseRepository.ts | 5 +++-- src/sections/sign-up/SignUp.tsx | 1 - .../FormFields.module.scss | 11 +++++++++++ .../FormFields.tsx | 9 +++++---- .../ValidTokenNotLinkedAccountForm.tsx | 5 +++-- ...GetTermsOfUse.ts => useGetApiTermsOfUse.ts} | 4 ++-- .../DataverseInfoMockLoadingkRepository.ts | 2 +- .../info/DataverseInfoMockRepository.ts | 2 +- .../component/info/models/TermsOfUseMother.ts | 4 +--- .../component/sections/sign-up/SignUp.spec.tsx | 2 +- .../ValidTokenNotLinkedAccountForm.spec.tsx | 14 +++++++++----- .../shared/hooks/useGetTermsOfUse.spec.ts | 16 ++++++++-------- 16 files changed, 67 insertions(+), 34 deletions(-) create mode 100644 src/info/infrastructure/mappers/JSTermsOfUseMapper.ts rename src/shared/hooks/{useGetTermsOfUse.ts => useGetApiTermsOfUse.ts} (90%) diff --git a/src/info/domain/models/TermsOfUse.ts b/src/info/domain/models/TermsOfUse.ts index c69bad018..96d933bfd 100644 --- a/src/info/domain/models/TermsOfUse.ts +++ b/src/info/domain/models/TermsOfUse.ts @@ -1 +1 @@ -export type TermsOfUse = string | null +export type TermsOfUse = string diff --git a/src/info/domain/repositories/DataverseInfoRepository.ts b/src/info/domain/repositories/DataverseInfoRepository.ts index 9c8cd4f3d..25f924edf 100644 --- a/src/info/domain/repositories/DataverseInfoRepository.ts +++ b/src/info/domain/repositories/DataverseInfoRepository.ts @@ -3,5 +3,5 @@ import { TermsOfUse } from '../models/TermsOfUse' export interface DataverseInfoRepository { getVersion(): Promise - getTermsOfUse: () => Promise + getApiTermsOfUse: () => Promise } diff --git a/src/info/domain/useCases/getTermsOfUse.ts b/src/info/domain/useCases/getTermsOfUse.ts index 53eaa2324..bf6f31791 100644 --- a/src/info/domain/useCases/getTermsOfUse.ts +++ b/src/info/domain/useCases/getTermsOfUse.ts @@ -1,10 +1,10 @@ import { type TermsOfUse } from '../../../info/domain/models/TermsOfUse' import { DataverseInfoRepository } from '../repositories/DataverseInfoRepository' -export function getTermsOfUse( +export function getApiTermsOfUse( dataverseInfoRepository: DataverseInfoRepository ): Promise { - return dataverseInfoRepository.getTermsOfUse().catch((error) => { + return dataverseInfoRepository.getApiTermsOfUse().catch((error) => { throw error }) } diff --git a/src/info/infrastructure/mappers/JSTermsOfUseMapper.ts b/src/info/infrastructure/mappers/JSTermsOfUseMapper.ts new file mode 100644 index 000000000..af6d859fe --- /dev/null +++ b/src/info/infrastructure/mappers/JSTermsOfUseMapper.ts @@ -0,0 +1,18 @@ +import DOMPurify from 'dompurify' +import { TermsOfUse } from '@/info/domain/models/TermsOfUse' + +export class JSTermsOfUseMapper { + static toSanitizedTermsOfUse(jsTermsOfUse: TermsOfUse): TermsOfUse { + DOMPurify.addHook('afterSanitizeAttributes', function (node) { + // set all elements owning target to target=_blank and rel=noopener for security reasons. See https://developer.chrome.com/docs/lighthouse/best-practices/external-anchors-use-rel-noopener + if ('target' in node) { + node.setAttribute('target', '_blank') + node.setAttribute('rel', 'noopener') + } + }) + // DOMPurify docs 👉 https://github.com/cure53/DOMPurify + const cleanedHTML = DOMPurify.sanitize(jsTermsOfUse, { USE_PROFILES: { html: true } }) + + return cleanedHTML + } +} diff --git a/src/info/infrastructure/repositories/DataverseInfoJSDataverseRepository.ts b/src/info/infrastructure/repositories/DataverseInfoJSDataverseRepository.ts index a7049c46a..f1627fd4e 100644 --- a/src/info/infrastructure/repositories/DataverseInfoJSDataverseRepository.ts +++ b/src/info/infrastructure/repositories/DataverseInfoJSDataverseRepository.ts @@ -3,6 +3,7 @@ import { axiosInstance } from '@/axiosInstance' import { DataverseInfoRepository } from '@/info/domain/repositories/DataverseInfoRepository' import { DataverseVersion } from '@/info/domain/models/DataverseVersion' import { TermsOfUse } from '@/info/domain/models/TermsOfUse' +import { JSTermsOfUseMapper } from '../mappers/JSTermsOfUseMapper' interface JSDataverseDataverseVersion { number: string @@ -29,12 +30,12 @@ export class DataverseInfoJSDataverseRepository implements DataverseInfoReposito }) } - async getTermsOfUse() { + async getApiTermsOfUse() { //TODO - implement using js-dataverse const response = await axiosInstance.get<{ data: { message: TermsOfUse } }>( '/api/v1/info/apiTermsOfUse', { excludeToken: true } ) - return response.data.data.message + return JSTermsOfUseMapper.toSanitizedTermsOfUse(response.data.data.message) } } diff --git a/src/sections/sign-up/SignUp.tsx b/src/sections/sign-up/SignUp.tsx index 16992f88f..49574ad36 100644 --- a/src/sections/sign-up/SignUp.tsx +++ b/src/sections/sign-up/SignUp.tsx @@ -8,7 +8,6 @@ import { ValidTokenNotLinkedAccountForm } from './valid-token-not-linked-account import styles from './SignUp.module.scss' // TODO:ME - All use cases will return same error message so this is blocking us for making requests to other public use cases like get root collection, should work removing access token from localstorage but we need it for future call -// TODO:ME - Ask about the format of the terms of use, html string? just text string? what is shown in the box if there is just a url string ? interface SignUpProps { userRepository: UserRepository diff --git a/src/sections/sign-up/valid-token-not-linked-account-form/FormFields.module.scss b/src/sections/sign-up/valid-token-not-linked-account-form/FormFields.module.scss index 72df568e9..54cb7a61b 100644 --- a/src/sections/sign-up/valid-token-not-linked-account-form/FormFields.module.scss +++ b/src/sections/sign-up/valid-token-not-linked-account-form/FormFields.module.scss @@ -1,3 +1,5 @@ +@import 'node_modules/@iqss/dataverse-design-system/src/lib/assets/styles/design-tokens/colors.module'; + .form-container { scroll-margin-top: 62px; } @@ -15,3 +17,12 @@ text-align: left; } } + +.terms-of-use-wrapper { + max-height: 200px; + padding: 12px; + overflow-y: auto; + background-color: #f5f5f5; + border: solid 1px $dv-border-color; + border-radius: 6px; +} diff --git a/src/sections/sign-up/valid-token-not-linked-account-form/FormFields.tsx b/src/sections/sign-up/valid-token-not-linked-account-form/FormFields.tsx index d19e0a00b..74e4c4795 100644 --- a/src/sections/sign-up/valid-token-not-linked-account-form/FormFields.tsx +++ b/src/sections/sign-up/valid-token-not-linked-account-form/FormFields.tsx @@ -271,10 +271,11 @@ export const FormFields = ({ userRepository, formDefaultValues, termsOfUse }: Fo render={({ field: { onChange, ref, value }, fieldState: { invalid, error } }) => ( - { const { tokenData } = useContext(AuthContext) - const { termsOfUse, isLoading: isLoadingTermsOfUse } = useGetTermsOfUse(dataverseInfoRepository) + const { termsOfUse, isLoading: isLoadingTermsOfUse } = + useGetApiTermsOfUse(dataverseInfoRepository) const defaultUserName = ValidTokenNotLinkedAccountFormHelper.getTokenDataValue( diff --git a/src/shared/hooks/useGetTermsOfUse.ts b/src/shared/hooks/useGetApiTermsOfUse.ts similarity index 90% rename from src/shared/hooks/useGetTermsOfUse.ts rename to src/shared/hooks/useGetApiTermsOfUse.ts index a12908ec8..5aeac761b 100644 --- a/src/shared/hooks/useGetTermsOfUse.ts +++ b/src/shared/hooks/useGetApiTermsOfUse.ts @@ -7,7 +7,7 @@ interface UseGetTermsOfUseReturnType { isLoading: boolean } -export const useGetTermsOfUse = ( +export const useGetApiTermsOfUse = ( dataverseInfoRepository: DataverseInfoRepository ): UseGetTermsOfUseReturnType => { const [termsOfUse, setTermsOfUse] = useState(null) @@ -18,7 +18,7 @@ export const useGetTermsOfUse = ( const handleGetUseOfTerms = async () => { setIsLoading(true) try { - const termsOfUse = await dataverseInfoRepository.getTermsOfUse() + const termsOfUse = await dataverseInfoRepository.getApiTermsOfUse() setTermsOfUse(termsOfUse) } catch (err) { diff --git a/src/stories/shared-mock-repositories/info/DataverseInfoMockLoadingkRepository.ts b/src/stories/shared-mock-repositories/info/DataverseInfoMockLoadingkRepository.ts index b6916f67a..f6dce5f88 100644 --- a/src/stories/shared-mock-repositories/info/DataverseInfoMockLoadingkRepository.ts +++ b/src/stories/shared-mock-repositories/info/DataverseInfoMockLoadingkRepository.ts @@ -7,7 +7,7 @@ export class DataverseInfoMockLoadingRepository implements DataverseInfoMockRepo return new Promise(() => {}) } - getTermsOfUse(): Promise { + getApiTermsOfUse(): Promise { return new Promise(() => {}) } } diff --git a/src/stories/shared-mock-repositories/info/DataverseInfoMockRepository.ts b/src/stories/shared-mock-repositories/info/DataverseInfoMockRepository.ts index 7e0671c69..cbce0e9eb 100644 --- a/src/stories/shared-mock-repositories/info/DataverseInfoMockRepository.ts +++ b/src/stories/shared-mock-repositories/info/DataverseInfoMockRepository.ts @@ -13,7 +13,7 @@ export class DataverseInfoMockRepository implements DataverseInfoRepository { }) } - getTermsOfUse(): Promise { + getApiTermsOfUse(): Promise { return new Promise((resolve) => { setTimeout(() => { resolve(TermsOfUseMother.create()) diff --git a/tests/component/info/models/TermsOfUseMother.ts b/tests/component/info/models/TermsOfUseMother.ts index 64933502d..9db4628ee 100644 --- a/tests/component/info/models/TermsOfUseMother.ts +++ b/tests/component/info/models/TermsOfUseMother.ts @@ -1,10 +1,8 @@ -import { faker } from '@faker-js/faker' -import isChromatic from 'chromatic/isChromatic' import { TermsOfUse } from '@/info/domain/models/TermsOfUse' export class TermsOfUseMother { static create(): TermsOfUse { - return isChromatic() ? 'https://some-terms-of-use-url.com' : faker.lorem.paragraphs(8) + return '

Terms of Use SPA dev

Please see our full terms of use

Thanks for reading!

' } static createEmpty(): TermsOfUse { diff --git a/tests/component/sections/sign-up/SignUp.spec.tsx b/tests/component/sections/sign-up/SignUp.spec.tsx index f359d17f8..e773d5b17 100644 --- a/tests/component/sections/sign-up/SignUp.spec.tsx +++ b/tests/component/sections/sign-up/SignUp.spec.tsx @@ -9,7 +9,7 @@ const userRepository: UserRepository = {} as UserRepository describe('SignUp', () => { beforeEach(() => { - dataverseInfoRepository.getTermsOfUse = cy.stub().resolves('Terms of use') + dataverseInfoRepository.getApiTermsOfUse = cy.stub().resolves('Terms of use') }) it('renders the valid token not linked account form and correct alerts when hasValidTokenButNotLinkedAccount prop is true', () => { diff --git a/tests/component/sections/sign-up/valid-token-not-linked-account-form/ValidTokenNotLinkedAccountForm.spec.tsx b/tests/component/sections/sign-up/valid-token-not-linked-account-form/ValidTokenNotLinkedAccountForm.spec.tsx index 369027d91..54a8c5501 100644 --- a/tests/component/sections/sign-up/valid-token-not-linked-account-form/ValidTokenNotLinkedAccountForm.spec.tsx +++ b/tests/component/sections/sign-up/valid-token-not-linked-account-form/ValidTokenNotLinkedAccountForm.spec.tsx @@ -4,11 +4,15 @@ import { UserRepository } from '@/users/domain/repositories/UserRepository' import { ValidTokenNotLinkedAccountForm } from '@/sections/sign-up/valid-token-not-linked-account-form/ValidTokenNotLinkedAccountForm' import { AuthContextMother } from '@tests/component/auth/AuthContextMother' import { UserDTO } from '@/users/domain/useCases/DTOs/UserDTO' +import { TermsOfUseMother } from '@tests/component/info/models/TermsOfUseMother' +import { JSTermsOfUseMapper } from '@/info/infrastructure/mappers/JSTermsOfUseMapper' const dataverseInfoRepository: DataverseInfoRepository = {} as DataverseInfoRepository const userRepository: UserRepository = {} as UserRepository -const termsOfUseMock = 'Terms of use' +const termsOfUseMock = TermsOfUseMother.create() +const sanitizedTermsOfUseMock = JSTermsOfUseMapper.toSanitizedTermsOfUse(termsOfUseMock) + const mockUserName = 'mockUserName' const mockFirstName = 'mockFirstName' const mockLastName = 'mockLastName' @@ -16,7 +20,7 @@ const mockEmail = 'mockEmail@email.com' describe('ValidTokenNotLinkedAccountForm', () => { beforeEach(() => { - dataverseInfoRepository.getTermsOfUse = cy.stub().resolves(termsOfUseMock) + dataverseInfoRepository.getApiTermsOfUse = cy.stub().resolves(sanitizedTermsOfUseMock) userRepository.register = cy.stub().as('registerUser').resolves() }) @@ -51,7 +55,7 @@ describe('ValidTokenNotLinkedAccountForm', () => { cy.findByLabelText('Given Name').should('have.value', mockFirstName) cy.findByLabelText('Family Name').should('have.value', mockLastName) cy.findByLabelText('Email').should('have.value', mockEmail) - cy.findByText(termsOfUseMock).should('exist') + cy.findByText('Terms of Use SPA dev').should('exist') }) it('renders the form fields with the correct default values when tokenData does not have preferred username, given name, family name and email', () => { @@ -79,7 +83,7 @@ describe('ValidTokenNotLinkedAccountForm', () => { cy.findByLabelText('Given Name').should('have.value', '') cy.findByLabelText('Family Name').should('have.value', '') cy.findByLabelText('Email').should('have.value', '') - cy.findByText(termsOfUseMock).should('exist') + cy.findByText('Terms of Use SPA dev').should('exist') }) }) @@ -216,7 +220,7 @@ describe('ValidTokenNotLinkedAccountForm', () => { }) it('shows no terms message when there are no terms of use', () => { - dataverseInfoRepository.getTermsOfUse = cy.stub().resolves(null) + dataverseInfoRepository.getApiTermsOfUse = cy.stub().resolves(null) cy.customMount( { +describe('useGetApiTermsOfUse', () => { it('should return terms of use correctly', async () => { - dataverseInfoRepository.getTermsOfUse = cy.stub().resolves(termsOfUseMock) + dataverseInfoRepository.getApiTermsOfUse = cy.stub().resolves(termsOfUseMock) - const { result } = renderHook(() => useGetTermsOfUse(dataverseInfoRepository)) + const { result } = renderHook(() => useGetApiTermsOfUse(dataverseInfoRepository)) await act(() => { expect(result.current.isLoading).to.deep.equal(true) @@ -26,9 +26,9 @@ describe('useGetTermsOfUse', () => { describe('Error handling', () => { it('should return correct error message when there is an error type catched', async () => { - dataverseInfoRepository.getTermsOfUse = cy.stub().rejects(new Error('Error message')) + dataverseInfoRepository.getApiTermsOfUse = cy.stub().rejects(new Error('Error message')) - const { result } = renderHook(() => useGetTermsOfUse(dataverseInfoRepository)) + const { result } = renderHook(() => useGetApiTermsOfUse(dataverseInfoRepository)) await act(() => { expect(result.current.isLoading).to.deep.equal(true) @@ -42,9 +42,9 @@ describe('useGetTermsOfUse', () => { }) it('should return correct error message when there is not an error type catched', async () => { - dataverseInfoRepository.getTermsOfUse = cy.stub().rejects('Error message') + dataverseInfoRepository.getApiTermsOfUse = cy.stub().rejects('Error message') - const { result } = renderHook(() => useGetTermsOfUse(dataverseInfoRepository)) + const { result } = renderHook(() => useGetApiTermsOfUse(dataverseInfoRepository)) await act(() => { expect(result.current.isLoading).to.deep.equal(true)