Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Escape hyphens in user/group character classes #163

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

tim-hutchinson
Copy link
Contributor

Issue #, if available: #162

Description of changes:
Added an escape to the character class usage of the hyphen character in the CloudFormation template.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@tim-hutchinson
Copy link
Contributor Author

I'm actually not sure I have the escaping right. It matches the single \ used for escaping the * in the same pattern, but the CF docs indicate that it should need a \\ in order to escape the JSON parsing of the regex itself.

@ChrisPates
Copy link
Contributor

Not to worry, I'll be putting together, a set of tests in the CICD pipeline to go with this.

Do you have some example query strings (please generalise to avoid 'over sharing') that I can use as test cases?

@tim-hutchinson
Copy link
Contributor Author

tim-hutchinson commented Jan 5, 2024

The only ones we're using in our environment are emails with hyphens, so
email:aws-sso-*
email=aws-sso-example-role@company.com
name:Admin* email:aws-* -- this is from the ssosync readme/helptext

It looks like some other examples were captured in a comment at https://github.com/awslabs/ssosync/blob/master/internal/google/client.go#L126

Some tangentially related ones would be spaces in group names and the memberKey search queries. I can try to test against the Google API to figure out what exactly the behavior for spaces-in-names is, but it's not immediately clear from their documentation. It might be easier to only filter on the field names/operators, rather than trying to sort out the value restrictions. There'd definitely be a possibility for the CF stack accepting an ultimately invalid value, but you'd never prevent a valid one and would catch common mistakes like typos in the fields (e.g. emial)

# Not sure if the CloudFormation regex processing is case-insensitive by default, but adding the Java flag syntax for it here in case
# This also doesn't quite check for the space-separated repetition
(?i)(name[:=]|email[:=]|memberKey=).+

@ChrisPates
Copy link
Contributor

So yes, the escaping has been done correctly. Thank you for you contribution.

Copy link
Contributor

@ChrisPates ChrisPates left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, will merge and add to the next release.

@ChrisPates ChrisPates merged commit 0edb2bf into awslabs:master Jan 12, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants