Conversation
agustin-conductor
requested review from
a team,
FeliLucero1,
JavierCarnelli-ConductorOne,
aldevv,
btipling,
johnallers,
luisina-santos,
mateovespConductor and
sergiocorral-conductorone
WalkthroughAdds an opt-in CloudTrail-backed AWS SSO last-login event feed: new boolean config to enable it, conditional CloudTrail client initialization during setup, and a paginated event feed that queries CloudTrail LookupEvents and emits SSO user last-login events. Changes
Sequence Diagram(s)sequenceDiagram
actor Admin
participant Config as Configuration
participant Connector as AWS Connector
participant Setup as SetupClients
participant CloudTrail as CloudTrail Client
participant EventFeed as SSO Login EventFeed
participant API as CloudTrail API
Admin->>Config: Enable SyncSSOUserLastLogin
Config->>Connector: New(ctx, config)
Connector->>Connector: set syncSSOUserLastLogin
Connector->>Setup: SetupClients(ctx)
alt SyncSSOUserLastLogin enabled
Setup->>CloudTrail: cloudtrail.NewFromConfig()
CloudTrail-->>Setup: client
Setup->>Connector: store client
end
Admin->>Connector: EventFeeds(ctx)
alt flag enabled && client exists
Connector->>EventFeed: newSSOLoginEventFeed(client, region)
EventFeed-->>Connector: feed
Connector-->>Admin: [EventFeed]
else
Connector-->>Admin: nil
end
Admin->>EventFeed: ListEvents(ctx, startAt, pageToken)
EventFeed->>EventFeed: decode token & compute window
EventFeed->>API: LookupEvents(input)
API-->>EventFeed: events + nextToken
EventFeed->>EventFeed: parse JSON, filter Federate/Authenticate, extract identity
EventFeed-->>Admin: []*v2.Event, StreamState
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
| SyncSSOUserLastLogin = field.BoolField( | ||
| "sync-sso-user-last-login", | ||
| field.WithDisplayName("Sync SSO User Last Login"), | ||
| field.WithDescription("Enable fetching last login time for SSO users from CloudTrail (requires cloudtrail:LookupEvents permission)"), |
There was a problem hiding this comment.
Shouldn't we add a default value for this new config?
There was a problem hiding this comment.
if i'm not wrong it takes false as default, but will make it explicit.
agustin-conductor
force-pushed
the
feature/sso_last_login
branch
from
c7fa1ce to
07caa9a
Compare
agustin-conductor
requested review from
FeliLucero1 and
JavierCarnelli-ConductorOne
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@go.mod`:
- Around line 5-25: The go.mod pins google.golang.org/grpc v1.71.0 which
requires Go 1.22.7+; update the project Go version to at least 1.22.7
(add/update the "go" directive in go.mod and adjust CI/toolchains) or downgrade
google.golang.org/grpc to a release compatible with the current minimum Go
version, and verify aws-sdk-go-v2 v1.41.1 behavior doesn't change credential
resolution in your environment; specifically check entries referencing
google.golang.org/grpc and github.com/aws/aws-sdk-go-v2 and ensure CI images,
build Dockerfiles, and developer toolchains reflect the chosen Go version.
In `@pkg/connector/events_last_login.go`:
- Around line 26-55: Add an EndAt timestamp to the pagination token and carry it
through unmarshalling and page-generation so CloudTrail NextToken is always used
with the original StartTime/EndTime: update the eventFeedPageToken struct to
include EndAt, modify unmarshalEventFeedPageToken to populate EndAt (defaulting
to time.Now() when a fresh token is created) and set LatestEventSeen/StartAt as
before, ensure the code that builds/encodes the next-page token (where
NextPageToken/NextPageToken is produced) persists EndAt, and update the
LookupEvents caller to use the token's EndAt instead of calling time.Now();
clear EndAt when pagination completes (NextPageToken is empty).
- Around line 171-177: The code extracts identityStoreID from
ctEvent.UserIdentity.OnBehalfOf.IdentityStoreArn but doesn't guard against
missing/malformed ARNs, which can produce an invalid user ARN; update the logic
around identityStoreArn/identityStoreID (the variables identityStoreArn and
identityStoreID and the usage that builds the user ARN) to first check that
ctEvent.UserIdentity.OnBehalfOf.IdentityStoreArn is non-empty and that the split
produced a non-empty identityStoreID, and if not, skip adding the event (or log
and continue) so you never construct or append a malformed ARN (ensure the check
occurs before the code that builds/uses the user ARN).
🧹 Nitpick comments (3)
pkg/config/config.go (1)
101-106: Consider validating the last‑login flag against SSO enablement.If this flag is enabled while SSO is disabled, the feature silently no‑ops. A small validation helps catch misconfiguration early.
✅ Suggested validation in
ValidateConfigfunc ValidateConfig(ctx context.Context, awsc *Aws) error { + if awsc.GetBool(SyncSSOUserLastLogin.FieldName) && !awsc.GetBool(GlobalAwsSsoEnabledField.FieldName) { + return fmt.Errorf("sync-sso-user-last-login requires global-aws-sso-enabled") + } if awsc.GetBool(UseAssumeField.FieldName) { err := ValidateExternalId(awsc.GetString(ExternalIdField.FieldName)) if err != nil { return err }pkg/connector/events_last_login.go (1)
224-229: Consider de‑duping events with identical timestamps.
StartAtis advanced toLatestEventSeen, which can re‑emit events that share the same timestamp. If downstream doesn’t de‑dupe, consider tracking the lastEventID(or advancing by a safe delta) to avoid duplicates.pkg/connector/connector.go (1)
403-412: Add a warning when the flag is enabled but no CloudTrail client exists.This helps surface misconfiguration (e.g., SSO disabled or client init skipped) instead of silently returning no feeds.
🔎 Suggested log
func (c *AWS) EventFeeds(ctx context.Context) []connectorbuilder.EventFeed { l := ctxzap.Extract(ctx) + if c.syncSSOUserLastLogin && c.cloudTrailClient == nil { + l.Warn("syncSSOUserLastLogin enabled but cloudTrailClient is nil; check SSO/CloudTrail setup") + } if !c.syncSSOUserLastLogin || c.cloudTrailClient == nil { return nil } l.Debug("syncSSOUserLastLogin enabled. adding ssoLoginEventFeed")
| require ( | ||
| github.com/aws/aws-sdk-go-v2 v1.36.3 | ||
| github.com/aws/aws-sdk-go-v2 v1.41.1 | ||
| github.com/aws/aws-sdk-go-v2/config v1.29.2 | ||
| github.com/aws/aws-sdk-go-v2/credentials v1.17.55 | ||
| github.com/aws/aws-sdk-go-v2/service/cloudtrail v1.55.5 | ||
| github.com/aws/aws-sdk-go-v2/service/iam v1.35.0 | ||
| github.com/aws/aws-sdk-go-v2/service/identitystore v1.25.4 | ||
| github.com/aws/aws-sdk-go-v2/service/organizations v1.31.0 | ||
| github.com/aws/aws-sdk-go-v2/service/ssoadmin v1.27.5 | ||
| github.com/aws/aws-sdk-go-v2/service/sts v1.33.10 | ||
| github.com/aws/smithy-go v1.22.2 | ||
| github.com/aws/smithy-go v1.24.0 | ||
| github.com/conductorone/baton-sdk v0.7.9 | ||
| github.com/ennyjfrick/ruleguard-logfatal v0.0.2 | ||
| github.com/grpc-ecosystem/go-grpc-middleware v1.4.0 | ||
| github.com/quasilyte/go-ruleguard/dsl v0.3.22 | ||
| github.com/spf13/viper v1.19.0 | ||
| github.com/stretchr/testify v1.11.1 | ||
| go.uber.org/zap v1.27.0 | ||
| google.golang.org/grpc v1.71.0 | ||
| google.golang.org/protobuf v1.36.5 | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
aws-sdk-go-v2 v1.41.1 release notes github
💡 Result:
github.com/aws/aws-sdk-go-v2v1.41.1 exists as a Git tag (v1.41.1) and is shown on pkg.go.dev as published Jan 9, 2026. [1]- The repo’s “release notes” for the SDK are maintained in
CHANGELOG.md(linked from the module page). [1] - Note: the GitHub Releases page for
aws-sdk-go-v2is primarily date-based (e.g.,release-YYYY-MM-DD), not one GitHub Release per semver tag likev1.41.1. [2]
Sources: [1] [2]
🌐 Web query:
google.golang.org/grpc v1.71.0 release notes minimum go version
💡 Result:
google.golang.org/grpc v1.71.0 bumps the minimum supported Go version to Go 1.22.7 (listed under “Dependencies” in the v1.71.0 release notes). [1]
🌐 Web query:
aws-sdk-go-v2 service cloudtrail v1.55.5 release notes
💡 Result:
I couldn’t find any official AWS SDK for Go v2 release notes specifically for github.com/aws/aws-sdk-go-v2/service/cloudtrail v1.55.5 (it doesn’t appear in the top-level aws/aws-sdk-go-v2 GitHub release notes). (github.com)
What I can confirm from AWS’s published v2 release notes is that, on 2025-11-20, the CloudTrail service module was released/updated to v1.55.0 with the feature: “CloudTrail aggregated events to simplify monitoring of data events at scale.” (github.com)
Also note: v1.55.5 does exist as a release number for the deprecated v1 SDK repo (aws/aws-sdk-go), but that’s a different SDK line than aws-sdk-go-v2. (github.com)
Citations:
- 1: https://github.com/aws/aws-sdk-go-v2/releases?utm_source=openai
- 2: https://github.com/aws/aws-sdk-go-v2/releases?utm_source=openai
- 3: https://github.com/aws/aws-sdk-go/releases?utm_source=openai
🌐 Web query:
aws-sdk-go-v2 CHANGELOG.md v1.41 breaking changes
💡 Result:
For the AWS SDK for Go v2, there are no breaking changes called out for “v1.41” in the repo’s published release notes—updates in that range are minor-version bumps (v1.x → v1.(x+1)), which are not where the project ships intentional breaking API changes under SemVer.
In the repo release notes where a module hits v1.41.0 (example: github.com/aws/aws-sdk-go-v2/service/sts: v1.41.0), the change is listed as a Feature addition (STS GetWebIdentityToken support), not a breaking change. [1]
Source
- AWS SDK for Go v2 GitHub Releases (shows
service/sts: v1.41.0as a Feature, no breaking-change note) [1]
🌐 Web query:
grpc-go v1.71.0 v1.70.0 breaking changes migration guide
💡 Result:
grpc-go v1.70.0 → v1.71.0: breaking / migration notes
1) Custom balancers that record metrics
Breaking change: balancer.BuildOptions.MetricsRecorder was removed.
Migration: get the recorder from the balancer.ClientConn instead:
- Before: use
opts.MetricsRecorder - After: use
cc.MetricsRecorder()(new method onbalancer.ClientConn) [1][2]
2) Types implementing balancer.ClientConn (usually in tests or intercepting/wrapping ClientConn)
Breaking change: implementations must now embed/delegate another balancer.ClientConn implementation (to allow grpc-go to add methods without breaking you).
Migration: change your wrapper/test double to embed the passed-in balancer.ClientConn and forward calls, instead of implementing the interface “from scratch”. Also ensure the embedded delegate is never nil (or you may panic). [1][2][3]
3) balancer/endpointsharding constructor signature changed
Breaking change: constructor now takes the child balancer builder and an options struct.
Migration: update your call sites to pass (childBuilder, esOpts) as required by the new NewBalancer signature. [1][4]
(Related) v1.70.0 behavior change you may hit while upgrading
If you have service configs with an invalid retryPolicy, grpc-go now rejects them (potential breaking change if you previously relied on permissive parsing). Fix the service config to match the gRFC requirements. [5]
Sources: [1] [2] [3] [4] [5]
🌐 Web query:
aws-sdk-go-v2 v1.40 v1.41 compatibility changes
💡 Result:
What changed from aws-sdk-go-v2 v1.40.x → v1.41.x (compatibility-impacting)
1) Default credential chain behavior change (v1.40.0)
v1.40.0 added AWS Login credentials (credentials/logincreds) to the default credential chain. This is the main “compatibility” concern because it can change which credentials get picked up automatically (e.g., if your environment/shared config enables AWS Login, it may now be selected where it previously wasn’t). [1]
2) No documented breaking API change for core module in v1.41.0 / v1.41.1
v1.41.0 (Dec 8, 2025) and v1.41.1 (Jan 9, 2026) exist as new core module versions, but AWS did not call out a core-module breaking change in the high-level release notes (they mostly list service-module updates). [2]
Version timeline (core module)
github.com/aws/aws-sdk-go-v2v1.40.0 — Nov 19, 2025 [2]github.com/aws/aws-sdk-go-v2v1.41.0 — Dec 8, 2025 [2]github.com/aws/aws-sdk-go-v2v1.41.1 — Jan 9, 2026 [2]
Sources:
[1] https://github.com/aws/aws-sdk-go-v2/releases (release “2025-11-19.2” highlights github.com/aws/aws-sdk-go-v2: v1.40.0 and the AWS Login creds addition)
[2] https://pkg.go.dev/github.com/aws/aws-sdk-go-v2?tab=versions (version list + dates)
Verify gRPC minimum Go version requirement.
google.golang.org/grpc v1.71.0 requires Go 1.22.7 or later; confirm this aligns with the project's minimum supported Go version. While gRPC v1.71.0 includes breaking changes for custom balancer implementations, they do not affect typical SDK usage. aws-sdk-go-v2 v1.41.1 has no documented breaking changes; note that v1.40.0 added AWS Login credentials to the default credential chain, which may alter credential resolution behavior.
🤖 Prompt for AI Agents
In `@go.mod` around lines 5 - 25, The go.mod pins google.golang.org/grpc v1.71.0
which requires Go 1.22.7+; update the project Go version to at least 1.22.7
(add/update the "go" directive in go.mod and adjust CI/toolchains) or downgrade
google.golang.org/grpc to a release compatible with the current minimum Go
version, and verify aws-sdk-go-v2 v1.41.1 behavior doesn't change credential
resolution in your environment; specifically check entries referencing
google.golang.org/grpc and github.com/aws/aws-sdk-go-v2 and ensure CI images,
build Dockerfiles, and developer toolchains reflect the chosen Go version.
5 participants
Adds event feed to get the Identity Center Users (sso_user) last login information.
This requires:
NEW PERMISSION
{ "Sid": "EventFeedPermissions", "Effect": "Allow", "Action": [ "cloudtrail:LookupEvents" ], "Resource": "*" },Pre-requisite:
AWS account with CloudTrail setup to log events.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.