Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion lib/Service/IdentifyMethod/Email.php
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,19 @@ private function throwIfIsAuthenticatedWithDifferentAccount(): void {
}
throw new LibresignException(json_encode([
'action' => JSActions::ACTION_DO_NOTHING,
'errors' => [['message' => $this->identifyService->getL10n()->t('This document is not yours. Log out and use the sign link again.')]],
'page_state' => 'authentication_required',
'page_state_data' => [
// TRANSLATORS Title shown when a signing link is opened while a different authenticated Nextcloud session is active. This is an expected security check, not a system failure.
'title' => $this->identifyService->getL10n()->t('Authentication required'),
// TRANSLATORS Description shown when the current authenticated Nextcloud session cannot be used to sign a document because it does not match the authorized signer.
'description' => $this->identifyService->getL10n()->t('The current authenticated session cannot be used to sign this document.'),
'noteType' => 'info',
'icon' => 'info',
],
'errors' => [[
// TRANSLATORS Guidance shown when the current authenticated Nextcloud session cannot be used to sign a document because it does not match the authorized signer. Keep it generic and do not mention emails or delivery channels.
'message' => $this->identifyService->getL10n()->t('To continue, sign out from the current account and open the signing link again, or open the signing link in a browser session where this account is not active.'),
]],
]));
}

Expand Down
32 changes: 20 additions & 12 deletions playwright/e2e/sign-wrong-session.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,17 @@ import { createMailpitClient, waitForEmailTo, extractSignLink } from '../support

/**
* When an authenticated Nextcloud user visits a sign link that belongs to a
* different email address, LibreSign must block the attempt with a clear error
* message instead of silently failing or allowing the wrong user to sign.
* different email address, LibreSign must block the attempt with clear
* authentication guidance instead of silently failing or allowing the wrong
* user to sign.
*
* The admin is logged in as admin@email.tld but the email sign request is for
* signer01@libresign.coop — they do NOT match, so the backend throws:
* "This document is not yours. Log out and use the sign link again."
* signer01@libresign.coop — they do NOT match, so the backend must render the
* neutral authentication-required state and keep the signing UI unavailable.
*/
test('authenticated user sees error when accessing another signer\'s email link', async ({ page }) => {
test('authenticated user sees authentication guidance when accessing another signer\'s email link', async ({ page }) => {
test.setTimeout(90_000)

await login(
page.request,
process.env.NEXTCLOUD_ADMIN_USER ?? 'admin',
Expand Down Expand Up @@ -50,10 +53,9 @@ test('authenticated user sees error when accessing another signer\'s email link'
await page.getByRole('textbox', { name: 'URL of a PDF file' }).fill('https://raw.githubusercontent.com/LibreSign/libresign/main/tests/php/fixtures/pdfs/small_valid.pdf')
await page.getByRole('button', { name: 'Send' }).click()

// Email signer — only the email method is active so there are no tabs in the Add signer dialog
// Email signer — only the email method is active so there are no tabs in the Add signer dialog.
await page.getByRole('button', { name: 'Add signer' }).click()
await page.getByPlaceholder('Email').click()
await page.getByPlaceholder('Email').pressSequentially('signer01@libresign.coop', { delay: 50 })
await page.getByPlaceholder('Email').fill('signer01@libresign.coop')
await page.getByRole('option', { name: 'signer01@libresign.coop' }).click()
await page.getByRole('textbox', { name: 'Signer name' }).fill('Signer 01')
await page.getByRole('button', { name: 'Save' }).click()
Expand All @@ -63,16 +65,22 @@ test('authenticated user sees error when accessing another signer\'s email link'

// Retrieve the sign link from the notification email sent to the signer.
// The admin is intentionally NOT logged out — this simulates the wrong-session scenario.
const email = await waitForEmailTo(mailpit, 'signer01@libresign.coop', 'LibreSign: There is a file for you to sign')
const email = await waitForEmailTo(mailpit, 'signer01@libresign.coop', 'LibreSign: There is a file for you to sign', {
timeout: 60_000,
})
const signLink = extractSignLink(email.Text)
if (!signLink) throw new Error('Sign link not found in email')

// Admin is still logged in (admin@email.tld) but navigates to a link
// that belongs to signer01@libresign.coop — the emails do NOT match.
// The identity check runs on page load; the "Sign the document." button is
// never rendered — the error is shown directly in the signing status panel.
// never rendered — the authentication-required guidance is shown directly.
await page.goto(signLink)

// Backend must return the "wrong session" error via ACTION_DO_NOTHING.
await expect(page.getByText('This document is not yours. Log out and use the sign link again.')).toBeVisible()
// Backend must keep blocking the flow via ACTION_DO_NOTHING while explaining
// how to continue safely without exposing either account identity.
await expect(page.getByText('Authentication required')).toBeVisible()
await expect(page.getByText('The current authenticated session cannot be used to sign this document.')).toBeVisible()
await expect(page.getByText('To continue, sign out from the current account and open the signing link again, or open the signing link in a browser session where this account is not active.')).toBeVisible()
await expect(page.getByRole('button', { name: 'Sign the document.' })).toHaveCount(0)
})
37 changes: 34 additions & 3 deletions src/tests/views/DefaultPageError.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ vi.mock('@nextcloud/vue/components/NcIconSvgWrapper', () => ({
default: { name: 'NcIconSvgWrapper', template: '<span />', props: ['path', 'size'] },
}))
vi.mock('@nextcloud/vue/components/NcNoteCard', () => ({
default: { name: 'NcNoteCard', template: '<div class="nc-note-card"><slot /></div>', props: ['type'] },
default: { name: 'NcNoteCard', template: '<div class="nc-note-card" :data-type="type"><slot /></div>', props: ['type'] },
}))

import DefaultPageError from '../../views/DefaultPageError.vue'
Expand All @@ -38,7 +38,7 @@ describe('DefaultPageError', () => {
it('shows "An error occurred" title and error message when errors are provided via initial state', () => {
loadState.mockImplementation((app, key, defaultValue) => {
if (app === 'libresign' && key === 'errors') {
return [{ message: 'This document is not yours. Log out and use the sign link again.' }]
return [{ message: 'Something went wrong' }]
}
return defaultValue
})
Expand All @@ -47,7 +47,38 @@ describe('DefaultPageError', () => {

expect(wrapper.find('.title').text()).toBe('An error occurred')
expect(wrapper.find('.description').text()).toBe('')
expect(wrapper.find('.nc-note-card').text()).toContain('This document is not yours. Log out and use the sign link again.')
expect(wrapper.find('.nc-note-card').text()).toContain('Something went wrong')
expect(wrapper.find('.nc-note-card').attributes('data-type')).toBe('error')
})

it('shows authentication-required guidance with non-error styling for wrong authenticated session state', () => {
loadState.mockImplementation((app, key, defaultValue) => {
if (app !== 'libresign') {
return defaultValue
}

if (key === 'page_state_data') {
return {
title: 'Authentication required',
description: 'The current authenticated session cannot be used to sign this document.',
noteType: 'info',
icon: 'info',
}
}

if (key === 'errors') {
return [{ message: 'To continue, sign out from the current account and open the signing link again, or open the signing link in a browser session where this account is not active.' }]
}

return defaultValue
})

const wrapper = mount(DefaultPageError)

expect(wrapper.find('.title').text()).toBe('Authentication required')
expect(wrapper.find('.description').text()).toBe('The current authenticated session cannot be used to sign this document.')
expect(wrapper.find('.nc-note-card').text()).toContain('To continue, sign out from the current account and open the signing link again, or open the signing link in a browser session where this account is not active.')
expect(wrapper.find('.nc-note-card').attributes('data-type')).not.toBe('error')
})

it('shows "Page not found" title and description when no errors are present', () => {
Expand Down
38 changes: 30 additions & 8 deletions src/views/DefaultPageError.vue
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@
<NcEmptyContent :name="title"
:description="description">
<template #icon>
<NcIconSvgWrapper :path="mdiAlertCircleOutline" :size="80" class="alert-icon" />
<NcIconSvgWrapper :path="iconPath" :size="80" :class="iconClass" />
</template>
<template #action>
<div v-if="errors.length" class="error-messages">
<NcNoteCard v-for="(error, index) in errors"
:key="index"
type="error">
:type="noteCardType">
{{ error.message }}
</NcNoteCard>
</div>
Expand All @@ -37,6 +37,7 @@ import { computed } from 'vue'

import {
mdiAlertCircleOutline,
mdiInformationOutline,
} from '@mdi/js'
import { t } from '@nextcloud/l10n'

Expand All @@ -51,8 +52,17 @@ defineOptions({
})

type ErrorRow = { message: string }
type NoteCardType = 'error' | 'info'
type IconVariant = 'error' | 'info'
type PageStateData = {
title?: string
description?: string
noteType?: NoteCardType
icon?: IconVariant
}

const logoLibreSign = logoLibreSignAsset
const pageStateData = computed<PageStateData>(() => loadState<PageStateData>('libresign', 'page_state_data', {}))

const errors = computed<ErrorRow[]>(() => {
const loadedErrors = loadState<ErrorRow[]>('libresign', 'errors', [])
Expand All @@ -66,13 +76,21 @@ const errors = computed<ErrorRow[]>(() => {
return []
})

const title = computed(() => (errors.value.length
? t('libresign', 'An error occurred')
: t('libresign', 'Page not found')))
const title = computed(() => pageStateData.value.title ?? (errors.value.length
? t('libresign', 'An error occurred')
: t('libresign', 'Page not found')))

const description = computed(() => pageStateData.value.description ?? (errors.value.length
? ''
: t('libresign', 'Sorry but the page you are looking for does not exist, has been removed, moved or is temporarily unavailable.')))

const noteCardType = computed<NoteCardType>(() => pageStateData.value.noteType ?? 'error')

const iconVariant = computed<IconVariant>(() => pageStateData.value.icon ?? 'error')

const description = computed(() => (errors.value.length
? ''
: t('libresign', 'Sorry but the page you are looking for does not exist, has been removed, moved or is temporarily unavailable.')))
const iconPath = computed(() => (iconVariant.value === 'info' ? mdiInformationOutline : mdiAlertCircleOutline))

const iconClass = computed(() => (iconVariant.value === 'info' ? 'info-icon' : 'alert-icon'))
</script>

<style lang="scss" scoped>
Expand Down Expand Up @@ -128,5 +146,9 @@ const description = computed(() => (errors.value.length
.alert-icon {
color: #e53c3c;
}

.info-icon {
color: var(--color-primary-element);
}
}
</style>
33 changes: 30 additions & 3 deletions tests/php/Unit/Service/IdentifyMethod/EmailTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,14 +199,14 @@ public static function providerThrowIfIsAuthenticatedWithDifferentAccount(): arr
'signerEmail' => 'signer@example.com',
'code' => null,
'identified' => false,
'errorMessage' => 'This document is not yours',
'errorMessage' => 'open the signing link again',
],
'authenticated_wrong_email' => [
'userEmail' => 'admin@example.com',
'signerEmail' => 'signer@example.com',
'code' => null,
'identified' => false,
'errorMessage' => 'This document is not yours',
'errorMessage' => 'open the signing link again',
],
'authenticated_wrong_email_token_in_progress' => [
'userEmail' => 'admin@example.com',
Expand All @@ -220,11 +220,38 @@ public static function providerThrowIfIsAuthenticatedWithDifferentAccount(): arr
'signerEmail' => 'signer@example.com',
'code' => 'abc123',
'identified' => true,
'errorMessage' => 'This document is not yours',
'errorMessage' => 'open the signing link again',
],
];
}

public function testThrowIfIsAuthenticatedWithDifferentAccountProvidesAuthenticationRequiredPageState(): void {
$user = $this->createMock(IUser::class);
$user->method('getEMailAddress')->willReturn('admin@example.com');
$this->userSession->method('getUser')->willReturn($user);

$identifyMethod = $this->getClass();
$identifyMethod->getEntity()->setIdentifierValue('signer@example.com');

try {
self::invokePrivate($identifyMethod, 'throwIfIsAuthenticatedWithDifferentAccount');
self::fail('Expected LibresignException to be thrown');
} catch (LibresignException $exception) {
$payload = json_decode($exception->getMessage(), true, flags: JSON_THROW_ON_ERROR);

self::assertSame(2000, $payload['action']);
self::assertSame('authentication_required', $payload['page_state']);
self::assertSame('Authentication required', $payload['page_state_data']['title']);
self::assertSame('The current authenticated session cannot be used to sign this document.', $payload['page_state_data']['description']);
self::assertSame('info', $payload['page_state_data']['noteType']);
self::assertSame('info', $payload['page_state_data']['icon']);
self::assertSame(
'To continue, sign out from the current account and open the signing link again, or open the signing link in a browser session where this account is not active.',
$payload['errors'][0]['message']
);
}
}

public function testThrowIfFileNotFoundThrowsControlledErrorWhenNodeIdIsInvalid(): void {
[$identifyMethod] = $this->setupFileNotFoundContext(nodeId: null);

Expand Down
Loading