Conversation
Introduce signer/observer participant roles at the database layer so request-signature flows can distinguish signing participants from view-only observers. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Allow administrators to enable observer participants through the policy workbench before requesters can assign view-only roles. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Persist participant roles, block observers from signing, and keep sequential signing logic scoped to actual signers only. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Replace the single add-signer action with an add dropdown, separate signers from observers in the participant list, and hide multi-signer controls when only one signer is present. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Run occ as www-data when setup runs as root and add a reusable script to reconfigure LibreSign after environment restarts. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Remove the experimental column that was not mapped by SignRequest and broke participant creation in local development databases. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Observers must not affect partial or fully signed file status, envelope progress, or pending signer checks used for signing order. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Send observer emails and in-app links to the validation page instead of the signing flow, and redirect legacy sign URLs to validation. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Show the plus icon and Add label on the participant dropdown action. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Group signers and observers in validation UI, recognize OBSERVING status, and ignore observers when checking partial or full signature completion. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Register observer UI strings in en_GB and translate the role as testemunha in pt_BR and pt_PT while keeping Observer in English. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Force-refresh validation data when opening the page right after signing so the first load does not fail before the signed document is ready. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Register Add and Add participant strings used by the request signature tab so the dropdown button is localized in English and Portuguese. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Switch SignerSelect copy and NcSelect input label based on participant role so adding a witness shows observer-specific search text. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Block observer-only signature requests in the UI and API, showing an error toast instead of treating the operation as successful. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Preserve observers when reordering signers, exclude them from DocMDP signer limits, map OBSERVING status in summaries, regenerate OpenAPI types, and add focused regression tests for policy and signing rules. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Keep lint-php-cs CI green after observer workflow changes. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
This reverts commit bd2f828. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
130da68 to
cfa42bc
Compare
Add a shared helper for the renamed Add participant control and update E2E specs that previously clicked the removed Add signer button. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Centralize account/email signer picking on the combobox used by the add-participant dialog so specs stop relying on removed placeholders. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Update request-signature E2E specs to select account and email signers through the shared combobox helpers. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
When an Everyone rule already exists, edit it instead of waiting for a missing Everyone scope option, and make save paths dirty the form first. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
…tate Reset stale certificates/signatures when needed, keep signature stamp in a graphic-capable mode, and click Sign document with a viewport-safe helper. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
…links Address PR review feedback for OBSERVING status in the contract, observer email wording, and authenticated deep-links for Account observers. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Keep observer SignerModel fields and add visibleElements from main.
|
Sorry for the many back-and-forth review rounds on this PR. The change touches many parts of LibreSign, so it ended up being a more difficult review than expected. Thank you for your patience and for fixing the issues as we found them. I think we are close now. Before the final review, there are two points left:
After that, I will review the PR again as a whole. |
Don't worry, it's the right thing to do. It's a long PR too |
Keep RequireSignRequestUuid signer-only so observer UUIDs cannot hit write endpoints, and authorize PDF viewing via RequireParticipantUuid. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Avoid PHPUnit coverage risky failures when the trait stub also executes entity and page-aware controller code under beStrictAboutCoverageMetadata. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
| // TRANSLATORS Toggle description explaining that document owners can add view-only participants. | ||
| const description = t('libresign', 'When enabled, document owners can add observers who track progress without signing.') |
There was a problem hiding this comment.
There is one policy interaction that is still not clear to the administrator.
An Email observer uses the public validation page, but when make_validation_url_private is enabled that page requires authentication. An external observer without a Nextcloud account will receive the email but will not be able to open the document.
I think both policies can still be enabled together, but we should warn about this consequence.
Could we show a warning when enabling Observer while validation access is authenticated-only, and also when making validation private while Observer is enabled?
Please also add regression coverage for this combination, including the Email observer flow.
Show a workbench warning when both policies are enabled, and cover the email observer flow that cannot open authenticated-only validation. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Expose sibling effective flags via resolvedStateMeta so the Workbench warning is backed by policy resolution instead of frontend-only store reads. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Deduplicate the repeated observer/signer subject, heading, and action cascades in notifySignDataUpdated and notifyUnsignedUser. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Unify RequireSignRequestUuid and RequireParticipantUuid handling through authorizeRequiredUuid, and rename loadNextcloudFileFromSignRequestUuid to the role-neutral loadNextcloudFileFromUuid. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Centralize policy_snapshot effectiveValue reads in ObserverProfilePolicyValue so the service and file applier stay in sync. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Add groupCount, userCount, and everyoneCount so Policy Workbench editor specs type-check against the OpenAPI EffectivePolicyState shape. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
PolicySourceTest still constructed ValidationAccessPolicy with no args after the sibling-meta reader dependency was added. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Use the active resolve context when reading sibling effective booleans so group/user workbench warnings do not follow the admin personal posture. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Nextcloud cannot autowire IPolicySource, which broke policy resolution across PHPUnit, Behat, and Playwright. Also apply php-cs on the new test. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
The only remaining failure was list.feature with cURL error 52 while 195 other scenarios passed on the same run. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
SQLite Behat CI keeps failing randomly with cURL error 52 empty reply from the PHP built-in server while other DB matrices stay green. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
| /** | ||
| * Retry transient PHP built-in server disconnects that show up in CI as | ||
| * cURL error 52 (Empty reply from server) during long Behat runs. | ||
| * | ||
| * @param TableNode|\Behat\Gherkin\Node\PyStringNode|array|null $body | ||
| */ | ||
| public function sendRequest(string $verb, string $url, $body = null, array $headers = [], array $options = []): void { | ||
| $attempts = 0; | ||
| $maxAttempts = 3; | ||
| while (true) { | ||
| try { | ||
| parent::sendRequest($verb, $url, $body, $headers, $options); | ||
| return; | ||
| } catch (\GuzzleHttp\Exception\ConnectException $exception) { | ||
| $attempts++; | ||
| if ($attempts >= $maxAttempts) { | ||
| throw $exception; | ||
| } | ||
| usleep(250000 * $attempts); | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This global retry looks unrelated to the Observer feature.
It was added after the SQLite Behat run failed with cURL error 52 / Empty reply from server, but this error only tells us that the test server did not return a response. It does not explain why the server stopped replying.
Overriding sendRequest() changes every Behat scenario and can retry POST, PATCH and DELETE requests too. If a write was already processed before the connection was lost, the retry can execute it twice. It can also hide a real PHP server or application failure.
Could we revert this change from this PR and investigate the SQLite/PHP built-in server failure separately?
If this is a general CI problem, it should have its own fix and regression coverage instead of changing the request behavior for the whole Behat suite.
There was a problem hiding this comment.
@vitormattos Can i open a issue for the Behat problem?
| return true; | ||
| } | ||
| if ($snapshotValue === false) { | ||
| return $this->isLivePolicyEnabled(); |
There was a problem hiding this comment.
Could we add a scoped-policy regression test for this fallback before changing the implementation?
For an existing request with enable_observer_profile = false in its snapshot, the code can check the live policy when an observer is added later.
I want to confirm that this uses the policy context of the target request, and not only the policy effective for the user doing the update.
A useful test would have different values for the manager/admin and for the target user or group.
Please cover this in PHPUnit and, if possible, also in Behat with group/user policy scopes.
We should test both directions:
- manager policy disabled, target policy enabled;
- manager policy enabled, target policy disabled.
If both cases already follow the target context, no fix is needed. If not, please keep the tests as regression coverage and use the correct target context when checking and upgrading the snapshot.
The retry was added for a SQLite empty-reply flake and changed request behavior for the whole suite; investigate that CI failure separately. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
When a disabled observer snapshot is upgraded or re-checked, use the file owner's policy context instead of the acting manager/current user. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Nextcloud master removed OCP\IServerContainer; InitialStateService now takes Psr\Container\ContainerInterface. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
3 participants
Resolves: #6957
📝 Summary
Adds an observer participant role (UI label: Observer / Testemunha) to signature requests. Observers can follow document progress and open a read-only view of the request (including PDF and signature positions), but cannot sign or perform write/signing operations. The feature is gated by the policy
enable_observer_profile(default: disabled).Backend is the source of truth: role validation, signing denial, status/progress calculations, DocMDP signer limits, sequential signing, e-mail/notification routing, UUID authorization, and API contracts all treat observers separately from signers. The request UI exposes separate Signers and Observers lists when the policy is enabled.
Authorization is split explicitly:
RequireSignRequestUuid— signer-only (signing UI, id-docs, signature elements, and other write paths reject observers)RequireParticipantUuid— participant read-only (PDF/getPdfFileallows both signers and observers)Also includes: policy snapshots for existing requests/envelopes, preserve observers when reordering signers, require at least one signer before requesting signatures, hide signing-order numbers for observers, OBSERVING status
4in API summaries, observer mail copy (“ready to view”), Account observer deep-links to the authenticated read-only filelist, and focused regression coverage (PHPUnit, Vitest, Behat, Playwright).🧪 How to test
filelist/sign?uuid=…), not the public validation/sign flow.GET /apps/libresign/pdf/<OBSERVER_UUID>→ 200POST /api/v1/id-docsandPOST /api/v1/signature/elements→ 422 (“Observers cannot sign this document”)🎨 UI / Front‑end changes
canSignignores observers🚧 Tasks
l10n/)⚙️ API / Back‑end changes
participant_rolecolumn on sign requests (signer|observer) with migrationenable_observer_profile(defaultfalse, system/group/user scopes) with per-request/envelope policy snapshotOBSERVINGas status4in signer summaries; reject invalidParticipantRolevalues; freeze role after signedRequireParticipantUuid(read-only PDF) vsRequireSignRequestUuid(signer-only writes)composer openapiif necessary🚧 Tasks
participant_profile_id(column was never shipped in official migrations)✅ Checklist
composer openapi,npm run typescript:generate)🤖 AI (if applicable)