fix: skip users without email to prevent sync failures#2
Closed
kesor wants to merge 30 commits into
Closed
Conversation
lauraseidler
approved these changes
Jan 20, 2026
…ressions (awslabs#303) ssue #, if available: awslabs#299 Description of changes: Refines the regex for rate: rate\((?:(?:\d{2,}+|[2-9]) (?:minutes|hours|days))|(?:1 (?:minute|hour|day))\) Allows for: rate(9 hours) rate(17 hours) rate(57 hours) rate(123 days) rate(1 hour) but rejects: rate(2 hour) rate(20 hour) rate(1 hours) By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.40.0 to 0.45.0. - [Commits](golang/crypto@v0.40.0...v0.45.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-version: 0.45.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
* Feat: Delete users if inactive only * Update ScheduleExpression regex --------- Co-authored-by: Luis Moreira <luismendesmoreira@hotmail.com>
Bumps [google.golang.org/grpc](https://github.com/grpc/grpc-go) from 1.74.2 to 1.79.3. - [Release notes](https://github.com/grpc/grpc-go/releases) - [Commits](grpc/grpc-go@v1.74.2...v1.79.3) --- updated-dependencies: - dependency-name: google.golang.org/grpc dependency-version: 1.79.3 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [go.opentelemetry.io/otel](https://github.com/open-telemetry/opentelemetry-go) from 1.39.0 to 1.41.0. - [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases) - [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md) - [Commits](open-telemetry/opentelemetry-go@v1.39.0...v1.41.0) --- updated-dependencies: - dependency-name: go.opentelemetry.io/otel dependency-version: 1.41.0 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com>
* Edit the regex for the CloudFormation template for the PRECACHE_ORG_UNITS, to allow for a single OU path other than '/' * Updated handling of this environment variable so, if specified but and empty string or not set it disables precaching. * Updated handling of other optional comma separated string env_variables to have the same way.
* Edit the regex for the CloudFormation template for the PRECACHE_ORG_UNITS, to allow for a single OU path other than '/' * Updated handling of this environment variable so, if specified but and empty string or not set it disables precaching. * Updated handling of other optional comma separated string env_variables to have the same way.
Included in this change: Parameter PrecacheOrgUnits : When left empty will disable the pre-caching of both Groups and Users Parameter LogRetention : Added, config CloudWatch Log Group retention period, previously it defaulted to Indefinitely this default has been retain however, it is strongly recommended a more frugal option is selected. Parameter ScheduleExpression : When left empty will disable scheduling of the SSOSync lambda function. The default for this is unchanged Rate(15 minutes), where the lambda is being triggered by an external event or as part of CICD pipeline (such ac CodePipeline), this prevent concurrency limits being encountered. The Parameters page has also been re-grouped to be more intuitive. Quick Start templates: Added a simple template that can launched directly from CloudFormation, this can simply be launched from the repo. Currently, template creates a deployment in a single account with two nested stacks one for the secrets and the other for the lambda function. To update the deployment, download the latest version of the template and update the stack. * implement disabled schedule * Apply disable PreCache to Groups * Improve logging for precache activity * Remove Explicit RoleName * Added Log Retention Setting. * Tidy Up the Parameters page * Add QuickStart * Add job to update version strings in quickstart on release
This reverts commit 561a1c6.
Adjust SemanticVersion update sed regex Update node actions to latest versions
- Add validation to skip users with empty PrimaryEmail in SyncUsers - Add validation in getGoogleGroupsAndUsers for user matches - Add validation for group members without email - Log warnings for skipped users with their names for tracking Fixes fatal error when syncing users without email addresses
kesor
force-pushed
the
fix/skip-users-without-email
branch
from
f187e84 to
9553912
Compare
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent sync failures caused by Google Workspace users (and group members) that have an empty PrimaryEmail, by skipping those entries and logging warnings. In addition, it updates the SAR/SAM deployment templates and CI workflows to support new deployment options (e.g., log retention, schedule rule refactor, runtime update) and introduces a “quick-start” CloudFormation template.
Changes:
- Add guards in sync logic to skip users/group members without an email and avoid syncing when match queries are empty.
- Refactor Google group/user precaching behavior and add new configuration handling for nil/empty env/flag inputs.
- Update deployment/CI assets: SAR template updates (runtime, schedule rule, log retention), new quick-start template, and workflow/pipeline changes.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
internal/sync.go |
Adds “skip no-email” logic and refactors group/user fetching/precaching and sync execution flow. |
cmd/root.go |
Adjusts env/flag parsing defaults (notably nil vs empty slices) and fixes SYNC_METHOD default source. |
internal/config/config.go |
Removes unused default constant and fixes formatting. |
template.yaml |
SAR template updates: new parameters (log retention), schedule rule refactor, runtime bump, and env var behavior changes. |
quick-start/single-account.yaml |
Adds a new quick-start template that deploys Secrets + App as nested SAR applications. |
README.md |
Updates feature wording and adds quick-start documentation entry. |
cicd/cloudformation/release.yaml |
Updates CodePipeline resource to PipelineType: V2. |
.github/workflows/release.yml |
Updates checkout action version used in release workflow. |
.github/workflows/main.yml |
Updates checkout action version used in main workflow. |
.github/workflows/quickstarts.yml |
Adds workflow to auto-update quick-start templates’ SemanticVersion on releases. |
go.mod |
Updates dependencies and adjusts the Go version directive format. |
go.sum |
Locks updated dependency checksums. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+857
to
+863
| if m == nil { | ||
| log.WithFields(log.Fields{ | ||
| "func": funcName, | ||
| "group.Id": g.Id, | ||
| "member.Id": m.Id, | ||
| }).Error("nil user") | ||
| continue |
Comment on lines
1191
to
+1194
| func (s *syncGSuite) includeGroup(name string) bool { | ||
| if s.cfg.IncludeGroups == nil { | ||
| return false | ||
| } |
| }).Info("filter groups by ignoreList") | ||
|
|
||
| for _, g := range gGroups { | ||
| // Fetch Users |
Comment on lines
+98
to
+119
| var newDeletedUsers = make([]*admin.User, 0) | ||
| for _, deleteUser := range deletedUsers { | ||
| isDeletedUserActive := false | ||
| log.WithFields(log.Fields{ | ||
| "email": deleteUser.PrimaryEmail, | ||
| }).Debug("Inspecting deleted user email") | ||
| for _, activeGoogleUser := range activeGoogleUsers { | ||
| if deleteUser.PrimaryEmail == activeGoogleUser.PrimaryEmail { | ||
| isDeletedUserActive = true | ||
| log.WithFields(log.Fields{ | ||
| "email": deleteUser.PrimaryEmail, | ||
| }).Debug("User is active again! Breaking loop...") | ||
| break | ||
| } | ||
| } | ||
| if !isDeletedUserActive { | ||
| log.WithFields(log.Fields{ | ||
| "email": deleteUser.PrimaryEmail, | ||
| }).Debug("Inactive user email") | ||
| newDeletedUsers = append(newDeletedUsers, deleteUser) | ||
| } | ||
| } |
| To reduce the volume of apis, ssosync precaches the users and groups from the Google directory. If you directory is large >10,000 users, you may wish to limit the organizational units that are precached. The default is the whole directory '/'. The parameter accpets a comma separate list of OrgUnitPaths, /OU1,/OU2/OU3 in this example all users within the tree of /OU1 and /OU2/OU3 would be precached but not from /OU2 itself. Precaching can be disable by leaving the field empty. | ||
| Default: "/" | ||
| AllowedPattern: '(?!.*\s)|(^\/$)|(\/[a-zA-Z0-9_\/\- ]{1,50})(?:(,\/[a-zA-Z0-9_\/\- ]{1,50}))*' | ||
| AllowedPattern: '(?!.*\s)|(^\/$)|(\/[a-zA-Z0-9_\/\- ]{0,50})|(\/[a-zA-Z0-9_\/\- ]{0,50})(?:(,\/[a-zA-Z0-9_\/\- ]{1,50}))' |
| Description: | | ||
| To reduce the volume of apis, ssosync precaches the users and groups from the Google directory. If you directory is large >10,000 users, you may wish to limit the organizational units that are precached. The default is the whole directory '/'. The parameter accpets a comma separate list of OrgUnitPaths, /OU1,/OU2/OU3 in this example all users within the tree of /OU1 and /OU2/OU3 would be precached but not from /OU2 itself. Precaching can be disable by leaving the field empty. | ||
| Default: "/" | ||
| AllowedPattern: '(?!.*\s)|(^\/$)|(\/[a-zA-Z0-9_\/\- ]{0,50})|(\/[a-zA-Z0-9_\/\- ]{0,50})(?:(,\/[a-zA-Z0-9_\/\- ]{1,50}))' |
| - **📈 Scalable**: Supports large directories with user caching and pagination | ||
|
|
||
| ## 🚀 Quick Start | ||
| Use one of the [quick start](/quick-start/) templates to simplify the your deployment. Use these templates, with the **Sync from Git** option, to make updates to your deployment far simpler. |
Comment on lines
+691
to
+692
| Name: AWSSyncSchedule | ||
| ScheduleExpression: !Ref ScheduleExpression |
Comment on lines
+697
to
+702
| LogGroup: | ||
| Type: AWS::Logs::LogGroup | ||
| Condition: SetLogRetention | ||
| Properties: | ||
| LogGroupName: !Sub /aws/lambda/${SSOSyncFunction} | ||
| RetentionInDays: !Ref LogRetention |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes fatal error when syncing users without email addresses