feat(admin/logs): capture request user agent on log entries#241
Conversation
Add a nullable user_agent column to admin_logs and capture the request User-Agent header in LogService at write time (the request object is recycled before the deferred insert, so it must be read synchronously). Stored raw and capped at 512 chars; the admin logs read side parses it into a device label. Non-HTTP actions and absent headers store null.
📝 WalkthroughWalkthroughCaptures HTTP User-Agent at log time (truncated to 512 chars) and persists it. Adds DB column and entity field, selects the value in log queries, introduces GeoLocationService (cached, batch resolve), parses device labels from stored User-Agent, and updates tests and Swagger to surface ChangesUser-Agent header capture & geo enrichment
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
GET /admin/logs now returns two derived fields per entry: - device: the stored user agent parsed into a 'Browser Major . OS Version' label by a dependency-free parser (Chrome, Firefox, Safari, Edge, Opera across Windows, macOS, iOS, Android, Linux). - location: a 'Region, CC' label resolved from ip_address via freeipapi.com, a keyless HTTPS endpoint, over the built-in fetch. Cached per IP and degrades to null on any failure so the feed never breaks. The offline geo-IP database that would avoid the network call is a banned dependency. Both fields are null when they cannot be resolved (no IP, no user agent, private/loopback address, or provider unreachable).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/common/services/tests/log.service.spec.ts`:
- Around line 65-87: Add a test that verifies LogService.log truncates
user_agent to 512 characters: create a request via makeRequest(...) with a
'user-agent' header longer than 512 chars, call service.log (e.g.,
service.log('user-1', AdminLogActionType.LOGIN, 'x', req,
AdminLogStatus.SUCCESS)), await flushImmediates(), and assert
mockAdminLogRepository.create was called with expect.objectContaining({
user_agent: longUserAgent.slice(0, 512) }) to confirm the truncation boundary is
enforced.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 953f5239-5a08-4286-a6cd-73b6292de2ba
📒 Files selected for processing (4)
src/common/services/log.service.tssrc/common/services/tests/log.service.spec.tssrc/database/migrations/1781308800000-AddUserAgentToAdminLogs.tssrc/modules/admin/logs/entities/admin-log.entity.ts
…ion-device feat(admin/logs): expose location and device on admin logs feed
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/admin/logs/services/geo-location.service.ts`:
- Around line 101-111: The isNonRoutable(ip: string) guard misses IPv4-mapped
IPv6 and IPv6 link-local ranges; update isNonRoutable to
normalize/case-insensitively check for "::ffff:" mapped addresses (e.g. strings
starting with "::ffff:" and then an IPv4 pattern) and treat them as non-routable
when the mapped IPv4 is private/loopback, and also add a check for IPv6
link-local prefix "fe80::/10" (e.g. ip.toLowerCase().startsWith('fe80:') or a
/^fe8[0-9]:/i pattern). Keep existing checks (127.0.0.1, ::1, 10., 192.168.,
169.254., 172.16-31, fc/ fd) and apply case-insensitive comparisons; implement
these checks inside the same isNonRoutable function.
In `@src/modules/admin/logs/tests/geo-location.service.spec.ts`:
- Line 22: Test suite is leaving mocked global.fetch assigned (e.g.,
global.fetch = jest.fn()), which jest.restoreAllMocks() doesn't revert; capture
the original fetch reference (const originalFetch = global.fetch) before tests
run (in a top-level beforeAll or beforeEach) and then restore it in the existing
afterEach (or add an afterEach) by setting global.fetch = originalFetch or
deleting global.fetch so the mock doesn't leak to other suites; reference the
existing afterEach(() => jest.restoreAllMocks()) and any places assigning
global.fetch = jest.fn() to implement the save/restore.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8aedb12a-6893-4ed5-91f0-95ecc0666bf3
📒 Files selected for processing (11)
src/modules/admin/logs/actions/admin-logs-list.action.tssrc/modules/admin/logs/admin-logs.module.tssrc/modules/admin/logs/admin-logs.service.tssrc/modules/admin/logs/docs/admin-logs-swagger.doc.tssrc/modules/admin/logs/interfaces/admin-logs.interfaces.tssrc/modules/admin/logs/services/geo-location.service.tssrc/modules/admin/logs/tests/admin-logs.controller.spec.tssrc/modules/admin/logs/tests/admin-logs.service.spec.tssrc/modules/admin/logs/tests/geo-location.service.spec.tssrc/modules/admin/logs/tests/parse-user-agent.util.spec.tssrc/modules/admin/logs/utils/parse-user-agent.util.ts
Description
First of two stacked PRs adding location and device to the admin activity log. This PR is the write side only: it captures the request User-Agent so the data starts accumulating before the read side (PR 2) surfaces it. No API response changes here.
user_agentcolumn toadmin_logs(varchar 512, mirrors the existingip_addresscolumn).LogServicenow readsreq.headers['user-agent']and stores it, capped at 512 chars. It is read synchronously because the request object is recycled before the deferred insert runs. Non-HTTP actions and absent headers storenull.Browser Major . OS Versiondevice label on read, so the parser can improve later without a backfill.Related Issue
N/A. Requested directly by the frontend (admin activity log design needs location and device columns).
Type of Change
How Has This Been Tested?
pnpm exec jest src/common/services/tests/log.service.spec.tspasses 14/14, including the newcaptures the User-Agent header when presentandstores a null user_agent when the header is absentcases. Verified end to end against a local Postgres that the column is created and populated.Test Evidence
Screenshot: 14/14 log.service spec run attached.
Additional Notes
PR 2 (expose location + device on
GET /admin/logs) is stacked on this branch and will be retargeted todevonce this merges.Summary by CodeRabbit
New Features
Documentation
Tests