feat: add last login to user sync

[edg4r0]

What

Adds each user's last login to the user sync as a WithLastLogin user trait.

Dropbox exposes no last-login field on the member endpoints (members/list_v2, members/get_info_v2), so it is derived from the team audit log: POST /2/team_log/get_events filtered to the member's account_id and the login_success event type.

Why it works this way

• Oldest-first ordering. Dropbox returns events oldest-first with no descending-sort option, so GetLastLogin pages through all of a member's login_success events and keeps the max timestamp. In practice a member's logins fit in one page (limit 1000), so this is usually a single API call.
• Graceful degradation. Requires the events.read scope (Team Auditing). A missing scope or a fetch error logs a warning and syncs the user without the last-login trait rather than failing the whole sync. Users with no login within the audit retention window (plan-dependent) likewise sync without the trait.

Changes

• pkg/connector/dropbox/payload_models.go — request/response models for get_events (GetEventsBody, GetEventsPayload, EventEntry, EventTypeArg)
• pkg/connector/dropbox/user_endpoints.go — GetLastLogin, paginating login_success events and returning the max timestamp
• pkg/connector/dropbox/urls.go — documented GetEventsURL const + required scope
• pkg/connector/users.go — thread *time.Time into userResource; fetch last login per member during List

Testing

Verified end-to-end against a live Dropbox Business tenant:

• Users with logins report the correct timestamp — cross-checked against an independent per-user max over the audit log (all matched).
• A user with multiple login events correctly returns the most recent (this specifically validates the oldest-first pagination fix; a naive first-result approach returned the oldest).
• Users without any login event sync cleanly with no trait and no error.
• go build, go vet, and go test ./... pass. golangci-lint run introduces no new issues.

Cost note for reviewers

This is one get_events call per user (occasionally more if a very active user's login_success count exceeds 1000 within the retention window). On a large, active tenant that is meaningful extra API traffic. If preferred, this could be reworked as a single bulk scan of the logins event category, joining by account_id client-side — happy to switch if reviewers want that trade-off.

[github-actions]

🟡 Suggestion: On a tenant missing the events.read scope, GetLastLogin returns a 403 for every member, so this Warn fires once per user (potentially thousands per sync). Consider detecting the scope/permission failure once and suppressing the repeat, or logarithmic sampling with a total_occurrences field, to avoid alert noise. (confidence: medium)

[github-actions]

Connector PR Review: feat: add last login to user sync

Blocking Issues: 0 | Suggestions: 3 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base 7f8d8ceab7b9.
Review mode: full
View review run

Review Summary

Scanned the full PR diff for security and correctness. The change derives each member's last login from the team audit log (/2/team_log/get_events filtered to login_success) and threads it into the user trait. Graceful degradation, timestamp parsing, context propagation, and rate-limit handling are all correct — no blocking issues found. Three non-blocking suggestions relate to the newly-required events.read scope and per-sync API cost.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• docs/connector.mdx:85-88 — sync permissions list is missing the events.read (Team Auditing) scope now required for last-login; operators following the docs will silently get users without the trait.
• pkg/connector/users.go:106-116 — on tenants lacking events.read, the per-user Warn fires once for every member (potentially thousands/sync); consider detecting the failure once or logarithmic sampling.
• pkg/connector/dropbox/user_endpoints.go:195 / users.go:106 — one get_events call per user (N+1) is meaningful traffic on large active tenants; the PR already acknowledges the bulk-scan alternative. Also GetEventsURL (urls.go:57) is defined but unused while the code uses inline path strings, and /2/team_log/get_events/continue has no const.

Prompt for AI agents

Verify each finding against the current code and only fix it if needed.

## Suggestions

In `docs/connector.mdx`:
- Around line 85-88: The "For syncing (read-only) operations" permissions list does not
  include the `events.read` (Team Auditing) scope, which the new last-login feature
  requires when calling /2/team_log/get_events during user sync. Add `events.read` to
  the read-only permissions list so operators grant it and last-login populates.

In `pkg/connector/users.go`:
- Around line 106-116: When a tenant is missing the events.read scope, GetLastLogin
  returns a 403 for every member and logger.Warn fires once per user (potentially
  thousands per sync). Detect the missing-scope/permission failure once and stop
  repeating the warning for the rest of the sync, or apply logarithmic sampling
  (1, 10, 100, every 1000) with a total_occurrences field.

In `pkg/connector/dropbox/user_endpoints.go` and `pkg/connector/dropbox/urls.go`:
- Around user_endpoints.go line 195-246 and users.go line 106: GetLastLogin makes one
  get_events call per user (N+1). On large active tenants this is significant extra
  API traffic; consider the bulk single-scan of the logins event category joined by
  account_id client-side (already noted in the PR description). Also GetEventsURL
  (urls.go line 57) is defined but never used because the code passes inline path
  strings to c.url(); use the constant and add a constant for the
  /2/team_log/get_events/continue path for consistency.

[github-actions]

No blocking issues found.

[luisina-santos]

we probably want to implement it using usage event. I was just checking if it is a good fit for this case!