[CXH-205] - Report IAM console last login separately from access key activity - AWS Connector

[mateoHernandez123]

Description

• Bug fix
• New feature

CXH-205 is the Clario request to tell console sign-in apart from access-key activity, and to review IAM access keys (active vs inactive, last used). Geoff's #123 already added console_access_enabled via iam:GetLoginProfile. This PR does the remaining work: IAM Last Login was mixing password sign-in with access-key use and keeping the oldest timestamp, so a console login could disappear and an automated key could keep a user looking active.

I split the two signals. Last Login is PasswordLastUsed only. Key activity stays on access_key_last_used on the IAM user, and on each access-key secret when Sync secrets is on (status, last used, service, region, owner keyed by the user's ARN). Nikki noted in March that Clario signs in through SSO; I asked her to confirm whether the misleading Last Login they see is on an IAM user or an Identity Center user. This change is on the IAM path. Identity Center last login is a separate CloudTrail feed (--sync-sso-user-last-login) and is not this bug.

Sync:

• IAM Users (iam_user) — Last Login is password-based AWS sign-in (PasswordLastUsed), including Management Console. Profile now carries password_last_used and access_key_last_used. Never-signed-in stays unset. console_access_enabled is unchanged from feat: surface IAM user console access (LoginProfile) status #123 and still gated by --sync-iam-user-console-access.
• SSO Users (sso_user) — unchanged. They cannot hold access keys. Last login still comes from the CloudTrail event feed when Organizations + Identity Center + --sync-sso-user-last-login are all on.
• Access keys (access-key) — when --sync-secrets is on: Active/Inactive status, last-used timestamp, service, region. Unused keys omit the N/A placeholders. Secret owner/identity now use the IAM user ARN so C1 can resolve the key to its user (UserId previously matched nothing). Inactive keys are synced as disabled, not dropped.
• Groups / Roles / Accounts — unchanged surface.

Provisioning:

• Unchanged. No Grant/Revoke or account-lifecycle changes in this PR.

Auth:

Unchanged (access key / assume-role). No new required flags.

--sync-secrets and --sync-iam-user-console-access remain off by default. Last Login itself does not depend on them. Per-key Inventory and console_access_enabled still need those settings on the tenant. --sync-sso-user-last-login is also off by default and needs Organizations + Identity Center + cloudtrail:LookupEvents.

Architecture highlights:

• C1 has a single UserTrait.LastLogin. Folding key use into it makes dormant-console review impossible, so the field is password-only by design.
• access_key_last_used on the user does not require Sync secrets; ListAccessKeys / GetAccessKeyLastUsed already run on the IAM user path.
• Secret CreatedById / IdentityId must be the IAM user ARN, because iam_user resources are keyed by ARN.
• Docs: IAM vs Identity Center last login, optional Cloud-hosted/self-hosted/Terraform flags, iam:GetLoginProfile on existing policy Sids (Sids not renamed), optional cloudtrail:LookupEvents Sid for SSO last login.

Useful links:

• Linear ticket CXH-205 — console login visibility and access key review
• PR #123 — IAM user console access (LoginProfile)
• AWS IAM User PasswordLastUsed
• AWS GetAccessKeyLastUsed
• AWS GetLoginProfile

[linear-code]

CXH-205

[github-actions]

Connector PR Review: [CXH-205] - Report IAM console last login separately from access key activity - AWS Connector

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base ba13ed074d1d.
Review mode: incremental since b9f517ce
View review run

Review Summary

The new commit reworks getAccessKeyLastUsed to return an (accessKeyUsage, error) pair so both callers can tell a failed lookup apart from a key IAM has never reported usage for, adds iam_user and secrets tests for the failure path and for N/A placeholders alongside a real last-used date, and corrects the "inactive keys were dropped" wording in the README, docs, and code comments. That addresses the prior feedback on the indistinguishable zero-value return and on the inaccurate inactive-key claim — though the distinction is currently drawn only in logs, so a failed lookup and a never-used key still produce byte-identical synced output. The full PR diff was scanned for security and correctness issues and none were found; the one new suggestion concerns the log level of the new error path.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• pkg/connector/secrets.go:103 — the GetAccessKeyLastUsed error path was demoted from Warn to Debug, so a missing iam:GetAccessKeyLastUsed permission now produces no default-visible log signal.

Prompt for AI agents

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

## Suggestions

In `pkg/connector/secrets.go`:
- Around line 103: The GetAccessKeyLastUsed error path was changed from
  logger.Warn to logger.Debug in this commit. Combined with the Debug-level
  logs at pkg/connector/iam_user.go:306 and :317, a connector role missing
  iam:ListAccessKeys or iam:GetAccessKeyLastUsed now silently syncs access
  keys with no last-used data and emits nothing at the default info log
  level. Per the repo log-level rules, an upstream 4xx (AccessDenied) that
  triggers skip-and-continue belongs at Warn (L1); Debug is reserved for
  missing-but-expected values (L3). Change this call back to logger.Warn
  (keeping the newly added access_key_id field), and if per-key log volume
  is the concern, apply logarithmic sampling (1, 10, 100, every 1000) with a
  total_occurrences field rather than lowering the level. Consider the same
  for the matching Debug call in pkg/connector/iam_user.go around line 317.

Note: this review run could not write the machine-readable review-state marker (the CI sandbox blocked every route to emitting it), so the next review will run in full mode rather than incremental.

[github-actions]

No blocking issues found.

[github-actions]

No blocking issues found.

[FeliLucero1]

Ran a deep code review (8 finders + SDK-aware verification) over this diff — came back clean, nothing to block on. 🎉

A few things worth calling out though, not bugs, just fyi:

• The old getLastLogin had a real latent bug (it tracked the oldest timestamp instead of the newest, so a fresh key use could clobber a legit console login with a stale date). Nice catch splitting that out, and the new table-driven tests in iam_user_last_login_test.go nail down exactly that old failure mode so it can't regress.
• The secret owner/identity switch from UserId → ARN — checked this against the "never change resource ID derivation" rule since it touches identifiers. It's fine: iam_user resources are keyed by ARN everywhere else in this repo, so the old UserId reference was already pointing at nothing. This is a fix, not a break.
• One finder flagged the logger.Warn in getAccessKeyLastUsed as a convention violation, but that got refuted on verification — this repo's own error-handling patterns doc says skip-and-continue paths should log at Warn, not Debug. So that's correct as-is, false alarm.

One non-code thing from the ticket thread: the PR description flags that whether Clario's misleading Last Login is on an IAM user vs an Identity Center/SSO user is still unconfirmed with the customer (per Mateo's comment). Worth getting that answer landed before merge/release messaging, since this PR is scoped to the IAM path only.

[sergiocorral-conductorone]

Automated review

[github-actions]

No blocking issues found.

[FeliLucero1]

Dug into fe1130f4 a bit more. Good fix overall — cleanly separates "lookup failed" from "key was never used" — but it also quietly downgraded the GetAccessKeyLastUsed failure log from Warn to Debug in two spots:

• pkg/connector/secrets.go:103
• pkg/connector/iam_user.go:317
• the logger.Warn(...) that used to live in getAccessKeyLastUsed (old secrets.go:187, pre-fe1130f4) got deleted entirely

Went and checked whether that's actually justified, both against our own docs and against real AWS behavior:

AWS's own docs say this failure mode is common at this connector's scale, not rare:

• GetAccessKeyLastUsed API ref → Common Error Types: AccessDeniedException, ThrottlingException, ServiceUnavailable are all documented, expected failure modes for this call, not edge cases.
• AWS's own security blog on querying IAM at scale explicitly warns that IAM/STS throttling is a global, account-level token bucket that gets hit hard exactly by this connector's shape of workload — cross-account AssumeRole fan-out across an Organization, one GetAccessKeyLastUsed call per key per user.

So this isn't "Debug is wrong" in the abstract — Warn-per-item at org scale really would be noisy, that part's a fair concern. But we already have the right tool for that (the sampling pattern) and this commit reached for plain Debug instead, which means if the connector role loses iam:GetAccessKeyLastUsed permission tomorrow, it fails on every single call, forever, with zero visible signal anywhere — no Warn, no aggregate count, nothing an operator would notice.

Side note on the two docs claims this whole feature rests on, for the record (both check out against AWS's actual API docs):

• AccessKeyLastUsed data type confirms Region/ServiceName really do return literal "N/A" for a never-used key — the notApplicable filtering logic is correct.
• User object docs confirm PasswordLastUsed tracks password-based sign-ins only (console/marketplace/forums) — independent of access-key API activity by construction, matching the PR's core premise.

Fun fact for context: Rule 4 + the sampling pattern both landed in baton-admin#371 (OPS-502), merged 2026-03-17, and got synced into this repo the same day via 34235a4d. The commit message says those 6 rules were distilled from 10+ real PRs across baton-aws/jira/databricks/incident-io/c1/sdk — so this specific Warn-vs-Debug tradeoff is exactly the kind of thing that doc was written to prevent, not a theoretical nitpick.