awslabs / ssosync

Populate AWS SSO directly with your G Suite users and groups using either a CLI or AWS Lambda
Apache License 2.0
512 stars 175 forks source link

Escape hyphens in user/group character classes #163

Closed tim-hutchinson closed 7 months ago

tim-hutchinson commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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