[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 on the profile (password_last_used and access_key_last_used). Last Login is the newest of those two timestamps, so it reflects the latest password sign-in or access-key use. Per-key detail stays 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 the most recent of password-based AWS sign-in (PasswordLastUsed, including Management Console) and access-key last used. Profile carries password_last_used and access_key_last_used so reviewers can still tell the signals apart. If neither timestamp is present, Last Login 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 already synced; they now report as disabled rather than carrying no status.
• 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. It is now max(PasswordLastUsed, newest access-key last used). The two timestamps stay on the profile so console vs programmatic activity is still distinguishable.
• 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: 3 | Threads Resolved: 0
Criteria: Criteria status: loaded .claude/skills/ci-review.md from trusted base ba13ed074d1d.
Review mode: full
View review run

Review Summary

Full-diff review of the IAM last-login / access-key work at 7127c6d0. Prior feedback is addressed: getAccessKeyLastUsed now returns an (accessKeyUsage, error) pair so a failed lookup is distinguishable from a never-used key, the secret owner reference is keyed by the IAM user ARN, the redundant iam:ListAccessKeys was dropped from the IAMListPermissions example, and 7127c6d0 answers @luisina-santos by making Last Login the newest of PasswordLastUsed and access-key use, with both timestamps kept separately on the profile. The full PR diff was scanned for security and correctness — no blocking issues found; the three suggestions below all follow from that final semantics change and from the new access-key status mapping.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

• pkg/connector/iam_user.go:115 — now that Last Login is derived from key activity, a transient ListAccessKeys / GetAccessKeyLastUsed throttle or denial silently regresses Last Login to the older PasswordLastUsed (or unsets it entirely for a key-only user); the password-only design was immune to this.
• pkg/connector/iam_user.go:113 — the PR title and description still say Last Login is PasswordLastUsed only, which 7127c6d0 reversed. The README and docs/connector.mdx are correct; the PR body is not, and both approvals predate that commit.
• pkg/connector/secrets.go:121 — any status other than StatusTypeActive maps to RESOURCE_STATUS_DISABLED, so an unknown or empty status would report an active key as disabled rather than unspecified.

Prompt for AI agents

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

Suggestions:

In `pkg/connector/iam_user.go`:
- Around line 115: Last Login is now activity.mostRecent(), the newest of PasswordLastUsed
  and access-key use. When getLoginActivity cannot reach IAM (ListAccessKeys denied or
  throttled, or every per-key GetAccessKeyLastUsed failing) it returns a nil accessKeyLastUsed
  after only a Debug log, so a user whose most recent activity was key use publishes an older
  Last Login than the previous sync, and a key-only user publishes none at all. Decide how a
  partial failure should behave: either propagate it as a rate-limit / error annotation so the
  SDK can retry instead of writing a regressed value, or track that key data was unavailable
  and skip emitting WithLastLogin rather than emitting the stale password timestamp. Update
  TestGetLoginActivity_KeepsConsoleLoginWhenKeysCannotBeListed, which currently asserts the
  regressed value is the intended outcome.
- Around line 113: The PR title and description still describe the pre-7127c6d0 behavior
  (Last Login is PasswordLastUsed only). The code and the docs now say Last Login is the most
  recent of password sign-in and access-key use. Update the PR title and body so the
  description, the approvals, and any release notes derived from them match what merges.

In `pkg/connector/secrets.go`:
- Around line 121: keyStatus defaults to v2.Status_RESOURCE_STATUS_DISABLED and is only raised
  to ENABLED for iamTypes.StatusTypeActive, so an unrecognized or empty key.Status reports an
  active credential as disabled. Switch on the status explicitly: Active to ENABLED, Inactive
  to DISABLED, anything else to v2.Status_RESOURCE_STATUS_UNSPECIFIED with a Debug log naming
  the unknown value, so an unknown variant fails safe instead of looking revoked.

[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.

[luisina-santos]

looks good to me. I'm only worried about the console/password sign-in only showed as last login, should we make it configurable for customers who wants to see any newest activity (including access key last used value) as last login date? I know it is better than the old behavior, and I think that's what the customer is asking for, last login should reflect last login on any kind of login, just use the latest one IMO

[mateoHernandez123]

@luisina-santos Agreed — Last Login should reflect the newest activity of either kind.

In 7127c6d0, Last Login is now the later of PasswordLastUsed and the newest access-key last-used time. password_last_used and access_key_last_used stay on the profile so console vs programmatic activity is still distinguishable. No extra flag.

[github-actions]

No blocking issues found.