Skip to content

feat(geolocation): finalize signer geolocation backend with IP-based location - #8414

Merged
vitormattos merged 21 commits into
LibreSign:mainfrom
lfals:feat/geolocation-ip-based
Sep 26, 2026
Merged

vitormattos merged 21 commits into
LibreSign:mainfrom
lfals:feat/geolocation-ip-based

Conversation

@lfals

@lfals lfals commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Resolves: #8086

📝 Summary

Finalizes the signer geolocation backend for #8086:

  • Split into two independent policies: signer_device_geolocation and signer_ip_geolocation
  • Final device contract names (deviceGeolocation*) and nested stored metadata under geolocation.{device,ip}
  • Write-once policy snapshots (absent ⇒ disabled; no live-policy fallback)
  • Local MaxMind City GeoIP via scoped LibreSign/3rdparty (geoip2/geoip2)
  • Admin-only GET/POST /api/v1/admin/geoip
  • IP lookup from Nextcloud IRequest::getRemoteAddress(), collected before the sync/async signing split; failures never block signing

Minimal frontend renames keep existing device flows green. Full GeoIP / Policy Workbench / audit UI remains #8087.

🧪 How to test

  1. Configure a MaxMind City .mmdb path (e.g. tests/php/fixtures/geoip/GeoIP2-City-Test.mmdb).
  2. As admin:
    • GET /ocs/v2.php/apps/libresign/api/v1/admin/geoip
    • POST /ocs/v2.php/apps/libresign/api/v1/admin/geoip with {"path":"..."}
      Confirm status and that no signer data is returned.
  3. Enable:
    • signer_device_geolocation → required
    • signer_ip_geolocation → enabled
  4. Create a signature request and confirm both policies are frozen in policy_snapshot.
  5. Sign without deviceGeolocation → 422.
  6. Sign with deviceGeolocation → 200; metadata includes geolocation.device and geolocation.ip (at least status + sourceIp).
  7. Confirm later policy enable/disable does not change existing requests; only new requests pick up the new effective policy.
  8. Confirm GeoIP failures / missing DB never block signing.
  9. Focused checks (optional):
    • composer test:unit -- --filter GeoIp
    • composer test:unit -- --filter SignerIpGeolocation
    • Behat: tests/integration/features/sign/signer_geolocation.feature
    • Behat: tests/integration/features/policies/signer_geolocation_policy.feature
How to see this running using GitHub Codespaces

1. Open the Codespace

  • Authenticate to GitHub
  • Go to the branch: feat/geolocation-ip-based
  • Click the Code button and select the Codespaces tab.
  • Click "Create codespace on feat/geolocation-ip-based"

2. Wait for the environment to start

  • Wait until you see:
    ✍️ LibreSign is up!

3. Access LibreSign in the browser

  • Open the Ports tab and open port 80 via the globe icon.

4. Login credentials

  • Username: admin
  • Password: admin

🎨 UI / Front‑end changes

  • Rename device geolocation fields to the final contract (deviceGeolocation, deviceGeolocationRequired, deviceGeolocationRequirement)
  • Read nested geolocation.device in validation/sign helpers
  • Update Policy Workbench key to signer_device_geolocation (IP policy UI deferred to Finalize signer geolocation frontend with IP-based location #8087)
  • Vitest coverage updated for the renames
  • Screenshots before/after — N/A (contract rename only; no GeoIP UI here)

🚧 Tasks

⚙️ API / Back‑end changes

  • Independent policies + write-once snapshot freeze
  • Nested geolocation.device / geolocation.ip; independent from collect_metadata
  • GeoIP services (config, status, lookup, normalizer) + admin GeoIP API
  • Lookup before sync/async split; non-blocking on failure
  • Ship geoip2/geoip2 through scoped LibreSign/3rdparty
  • OpenAPI + generated TypeScript types regenerated
  • Unit + Behat coverage for device rename / GeoIP services / IP policy appliers
  • REUSE: add LICENSES/Apache-2.0.txt for MaxMind test fixture attribution
  • Capabilities — N/A
  • Docs follow-up in LibreSign/documentation if needed

🚧 Tasks

✅ Checklist

🤖 AI (if applicable)

  • The content of this PR was partially or fully generated using AI

@codecov-commenter

codecov-commenter commented Sep 17, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/components/Request/IdentifySigner.vue 77.77% 0 Missing and 2 partials ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Files with missing lines Coverage Δ
src/components/validation/EnvelopeValidation.vue 75.92% <100.00%> (+6.51%) ⬆️
src/components/validation/SignerDetails.vue 52.79% <100.00%> (+12.91%) ⬆️
src/helpers/signerGeolocation.ts 87.03% <100.00%> (ø)
src/services/signSubmit.ts 76.54% <ø> (ø)
src/store/files.js 75.97% <100.00%> (+1.11%) ⬆️
...ttings/PolicyWorkbench/settings/realDefinitions.ts 100.00% <ø> (ø)
...ench/settings/signer-geolocation/realDefinition.ts 73.33% <ø> (ø)
src/views/SignPDF/_partials/Sign.vue 59.93% <100.00%> (+8.14%) ⬆️
src/components/Request/IdentifySigner.vue 76.54% <77.77%> (-6.80%) ⬇️

... and 224 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lfals
lfals marked this pull request as ready for review September 17, 2026 02:32
@lfals
lfals requested a review from a team as a code owner September 17, 2026 02:32
@lfals
lfals force-pushed the feat/geolocation-ip-based branch from 31f7c03 to fd0841f Compare September 17, 2026 20:04
@YvesCesar
YvesCesar self-requested a review September 17, 2026 21:36
@lfals
lfals force-pushed the feat/geolocation-ip-based branch from fd0841f to 521efbf Compare September 18, 2026 12:50

@YvesCesar YvesCesar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for your contribution, @lfals :)

I added a few points for review.

A question:

About the 3rdparty submodule bump in this PR: it points to 2e04147, which is only reachable from the fork branch, right? LibreSign/3rdparty#101 was closed without being merged, so geoip2/geoip2 currently has no home in LibreSign/3rdparty. The commit itself is fine (the scoped package is there under composer/geoip2/geoip2/), it just needs to land on a LibreSign/3rdparty branch and the SHA retargeted before this can be merged.

Comment thread src/components/Request/IdentifySigner.vue Outdated
Comment thread lib/Service/File/FileListService.php Outdated
Comment thread lib/Service/GeoIp/GeoIpLookupService.php Outdated
Comment thread lib/Service/GeoIp/GeoIpLookupResultNormalizer.php Outdated
Comment thread lib/Service/SignerIpGeolocation/SignerIpGeolocationPolicyService.php Outdated
Comment thread lib/Service/RequestSignatureService.php
@github-project-automation github-project-automation Bot moved this from 0. Backlog to 1. to do in LibreSign Roadmap Sep 18, 2026
@YvesCesar
YvesCesar self-requested a review September 21, 2026 13:37
@lfals

lfals commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

@YvesCesar , 3rdparty submodule updated!

@lfals

lfals commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

@vitormattos behat failing again, now we have logs. I'll take a look, but feel free to do too

@YvesCesar YvesCesar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lfals

I brought up a few more points. Could you take a look?

@lfals
lfals requested a review from YvesCesar September 23, 2026 14:13
YvesCesar
YvesCesar previously approved these changes Sep 23, 2026
@YvesCesar

Copy link
Copy Markdown
Member

Thanks, @lfals

@vitormattos this PR looks good to me.

@cursor
cursor Bot force-pushed the feat/geolocation-ip-based branch from c3266e2 to 0730106 Compare September 25, 2026 03:21
@lfals
lfals force-pushed the feat/geolocation-ip-based branch from 0730106 to 3c22341 Compare September 25, 2026 03:30
Pin the submodule to the local commit that ships the scoped MaxMind
GeoIP2 reader for City database lookups.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Introduce GeoIP configuration, database validation, lookup/normalization
services, and admin GET/POST /api/v1/admin/geoip for LibreSign#8086.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Rename the device policy/API fields, freeze write-once snapshots,
nest device metadata under geolocation.device, and collect IP evidence
before the sync/async signing split for LibreSign#8086.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Refresh OpenAPI specs and generated TypeScript types for the final
signer geolocation and admin GeoIP APIs.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Rename device geolocation fields/policy keys and read nested
geolocation.device so existing flows stay green ahead of LibreSign#8087.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Validate IP status and unavailable reason enums when exposing
stored geolocation metadata through the file list API.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
REUSE requires LICENSES/Apache-2.0.txt for the MaxMind test
database attribution used by the GeoIP lookup tests.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Envelope snapshots may only contain enable_observer_profile, so treat
device geolocation as frozen only when its snapshot entry is present.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Expose geolocation.ip.sourceIp only to the signer themselves or the
request owner, matching rejection metadata visibility rules.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Derive database readiness from the cached reader instead of calling
getStatus() first, and drop the unused AddressNotFound helper.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Extract envelope/child snapshot lookup into one trait so device, IP,
and rejection policies keep the same oldest-child rule.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
The geolocation freeze closure no longer reads the requester user.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Satisfy prefer-object-has-own eslint rule in IdentifySigner.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
findSnapshot is typed as array<string, mixed>, so returning it from
getPolicyValue violated the declared policy shape. Route through the
typed normalizer and mark trait implementations with Override.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
findSnapshot was array<string, mixed>, so getPolicyValue could not
honestly return the policy shape. Move envelope/child lookup into
FrozenFilePolicySnapshotResolver<T> and drop the trait abstracts that
only passed under Override attributes.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
Point the submodule at the merged LibreSign/3rdparty#101 tip so the
geoip2 package is reachable from the official repo before merge.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
An envelope never carries the geolocation snapshots itself, they are
frozen on its child files. sync() therefore resolved the current policy
and wrote it on the envelope, and the snapshot resolver then preferred
that value for every child, replacing the frozen one after a policy
change. Skip sync on envelopes, like the rejection applier does.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
v15.0.0 to v15.0.3 stored the device geolocation policy as
signer_geolocation, the signer requirement as geolocationRequirement
and the collected payload flat under geolocation. Rename them in app
config, user preferences, group permission sets, file policy snapshots
and sign request metadata so upgraded instances keep enforcing the
configured policy and in-flight requirements.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
MySQL JSON does not preserve object key order the same way as SQLite,
so assertSame on nested policy_snapshot entries failed even when the
rename itself succeeded. Sort keys recursively on both sides.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
After rebasing onto main, retarget the submodule to the current
LibreSign/3rdparty tip already used on upstream main.

Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
@lfals
lfals force-pushed the feat/geolocation-ip-based branch from 3c22341 to b0062d4 Compare September 25, 2026 03:33
Comment thread lib/Migration/Version18005Date20260922000000.php
Signed-off-by: Luis Amorim <luisfelipeamorim@hotmail.com>
@lfals
lfals requested a review from vitormattos September 26, 2026 17:15

@vitormattos vitormattos left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Luiz.

I reviewed the latest push. The migration now processes metadata in bounded batches, so the memory concern from the previous review is addressed. The current CI is also fully green, including the Playwright suite.

For the Playwright environment itself, we created separate follow-up work to align LibreSign with the current Nextcloud E2E/Playwright pattern and later consume the shared LibreCode workflow:

These are follow-up improvements to the test environment and are not blockers for this PR.

From my side, this is ready to approve.

@vitormattos
vitormattos merged commit d4f7e42 into LibreSign:main Sep 26, 2026
67 checks passed
@lfals
lfals deleted the feat/geolocation-ip-based branch September 26, 2026 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Finalize signer geolocation backend with IP-based location

4 participants