Feat/signer geolocation backend 6960 - #8120
Conversation
Introduce the signer_geolocation policy with disabled, optional, and required modes plus requester override delegation, following existing PolicySpec patterns used by collect_metadata and identification_documents. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Freeze signer_geolocation policy on signature requests via FilePolicyApplier and resolve effective per-signer requirements from snapshots with dedicated policy and metadata validation services. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Persist frozen geolocationRequirement per signer at request creation, accept device geolocation on sign endpoints, validate submissions server-side, and store coordinates independently of collect_metadata. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Cover policy normalization, file snapshot applier, effective requirement resolution, metadata validation, and registry registration. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Decode JSON-string signer_geolocation payloads from OCS policy APIs and document geolocation fields in sign/request OpenAPI annotations so clients can submit and read per-signer geolocation requirements reliably. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Add unit, API DB, Behat, and Playwright scenarios for policy modes, requester overrides, collect_metadata independence, frozen requirements, and server-side enforcement when geolocation is required. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Align SignFileController geolocation parameter annotations with Psalm, fix Behat assertions to match sign/list API shapes, and document the devcontainer Behat workflow (ArrayCache + www-data). Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Restore LibresignSignerGeolocation docblocks for documentation while using @psalm-param for Psalm, then regenerate OpenAPI/TS artifacts so the sign geolocation parameter matches composer openapi output. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Add signer_geolocation to ValidatePolicySnapshot, regenerate OpenAPI/TS artifacts, and update FilePolicyApplier and RequestSignatureService unit tests for the new policy provider dependency. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Drop devcontainer Behat notes and the Playwright E2E spec from this branch. Backend coverage for LibreSign#6960 remains in PHPUnit and Behat; the Playwright file duplicated that work and included browser UI flows explicitly out of scope for the issue. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Reload sign request metadata from storage before writing the frozen geolocation requirement so mail notification counters are not wiped. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
| $return[] = $lastSignRequest; | ||
| } | ||
|
|
||
| if ($lastSignRequest instanceof SignRequestEntity) { | ||
| $requesterRequiresGeolocation = filter_var( | ||
| $signer['geolocationRequired'] ?? false, | ||
| FILTER_VALIDATE_BOOLEAN, | ||
| FILTER_NULL_ON_FAILURE, | ||
| ) ?? false; | ||
| $this->signerGeolocationPolicyService->persistEffectiveRequirementToStorage( | ||
| $lastSignRequest, | ||
| $file, | ||
| $requesterRequiresGeolocation, | ||
| $requester, | ||
| ); |
There was a problem hiding this comment.
It looks like we only persist the geolocation requirement to the last SignRequest created for the signer.
A signer can have multiple identifyMethods, and each iteration can create/update a SignRequest. Shouldn't we persist the effective requirement for every created sign request?
Otherwise, a sign request without geolocationRequirement will fall back to disabled during validation, which could bypass a required geolocation policy.
It would also be good to add a test with multiple identify methods for the same signer to validate if will work fine.
| if (is_array($signerMetadata) && $signerMetadata !== []) { | ||
| $data['metadata'] = $signerMetadata; | ||
| } | ||
|
|
There was a problem hiding this comment.
Could we avoid exposing the complete signer metadata here and only expose the geolocation metadata needed by this feature?
I think this should be based on the data stored in the SignRequest, not on the current policy. For example, a request may have required geolocation, the signer may already have signed and stored it, and the admin may disable the policy later. We should still return the historical data for that signer.
So, when present, I think we only need to expose geolocationRequirement and geolocation here. This also avoids unintentionally exposing unrelated current or future signer metadata with empty data through the file list API.
Could we also add a regression test covering this behavior to validate when this data is present and when not?
Persist the effective geolocation requirement inside the identifyMethods loop so signers with multiple identify methods do not bypass validation. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Return geolocationRequirement and geolocation from SignRequest storage instead of leaking unrelated signer metadata through the file list API. Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Resolves: #6960
📝 Summary
Adds backend support for the
signer_geolocationpolicy, allowing administrators to control whether signer device geolocation is disabled, optional, or required when signing documents.The policy follows existing Policy Workbench patterns (
collect_metadata,identification_documents) with three modes (disabled,optional,required) and optional requester override delegation. At request creation, the effective requirement is frozen per signer inmetadata.geolocationRequirement. Sign endpoints accept ageolocationpayload, validate it server-side, and persist coordinates independently ofcollect_metadata.OpenAPI specs and generated TypeScript types were updated to document the new policy and sign/request parameters.
🧪 How to test
GET /api/v1/file/list?details=1) and confirm the signer hasmetadata.geolocationRequirement: "required".geolocationand confirm the API returns422with a geolocation-required error.{ "method": "clickToSign", "geolocation": { "status": "collected", "latitude": -23.5505, "longitude": -46.6333, "accuracy": 25, "timestamp": 1700000000000 } }geolocationcoordinates.mode: "optional"andallowRequesterOverride: true, creating a request withgeolocationRequired: trueon selected signers, and verify only those signers are enforced.composer test:unit -- --filter SignerGeolocation vendor/bin/behat features/sign/signer_geolocation.feature -v vendor/bin/behat features/policies/signer_geolocation_policy.feature -v npx playwright test playwright/e2e/signer-geolocation-backend.spec.ts⚙️ API / Back‑end changes
signer_geolocationpolicy provider with modesdisabled,optional, andrequired, plusallowRequesterOverridedelegationgeolocationRequirementon signature request creation viaSignerGeolocationFilePolicyAppliergeolocationon sign endpoints; reject missing/invalid submissions when requiredcollect_metadatacomposer openapi✅ Checklist
collect_metadata🤖 AI (if applicable)