Get-GSGroupMembers doesn't take Group ID as Identity

[Ephesoft-Stitus]

Get-GSGroupMembers Identity parameter help states:

The email or GroupID of the target group

However, passing the group ID to the command results in a 404 because the key isn't found. It only works with passing the email of the group as the identity.

This does not work

$Group = Get-GSGroup 'mygroupid'
$Group.id | Get-GSGroupMembers

Result:

Get-GSGroupMember : Exception calling "Execute" with "0" argument(s): "Google.Apis.Requests.RequestError Resource Not Found: groupKey [404]

This does work:

$Group = Get-GSGroup 'mygroupid'
$Group.email | Get-GSGroupMembers

[scrthq]

Thanks for pointing this out, @Ephesoft-Stitus !! Checking to see if there's a different property on the request object that needs to be filled instead if the ID is passed, as the documentation on Google's side explicitly states that either the email or unique ID should be acceptable: https://developers.google.com/admin-sdk/directory/v1/reference/members/list

[scrthq]

Found the issue, working on a fix now! Same fix will be applied to other Group functions as well where applicable. Thanks for raising this, @Ephesoft-Stitus !

[scrthq]

hey @Ephesoft-Stitus - sorry on the delay here, this has become a bigger fix than anticipated due to Group IDs also being strings, so the same method of testing if the value passed is an email or an ID as we do with Users isn't applicable (we do that currently by testing if the value is able to be cast as a long integer).

Issue source: Email name parts (e.g. mike instead of mike@domain.com) are accepted for convenience via the function automatically appending the Domain to the value if it is not recognized as an ID and does not have an @ in the string. This is an incredibly convenient feature of PSGSuite that I'd like to not break. Since Group IDs aren't easily identified, we run into this issue.

I initially tried approaching this using a new Id parameter and parameter sets (e.g. Email vs Id), but some of the group functions have multiple parameter sets already, making that approach not too feasible.

I tried nesting try/catch statements, but I feel it not only adds to code complexity unnecessarily, but also elongates the overall completion time of the function since you'd be literally retrying the same API call underneath the hood if the first attempt with Domain appended failed to confirm if it was actually an ID that was passed. For example, if you do actually pass a non-existent email/ID to the function, it would need to try and fail twice before returning.

---

My current thought is to add a switch parameter to the Group functions to block the appending of the domain and try with whatever was passed as-is. This would effectively allow you to state that you know you are either passing full email addresses or IDs to the Identity / Email parameter and to not attempt to resolve the address if @ is not found in the value. Current suggested parameter name is DoNotResolveGroupIdentity with a shorter alias of DoNotResolve.

Thoughts?

[Ephesoft-Stitus]

Hi Nate, No worries for the delay, I know how busy life can be.

It sounds like we need a way to classify a string as a group ID with some certainty and only attempt lookup using the group ID (without postfixing the email domain) if the string is classified as a group ID. This way we reduce the probability of the string being an email address and of hitting the API multiple times for a single lookup (or lookup failure).

I have a few hundred groups and all of them have these attributes in the group ID:

• 15 characters in length
• Alphanumeric
• All alpha characters are lowercase
• First two characters are numeric

I can match all of my groups with this regex: \d{2}([a-z]|\d)*

I reached out to my GSuite account executive to see if I can get validation about these assumptions and perhaps some more information on the group ID scheme. I'll let you know what I find out.

Thanks,

--Shaun

On Sat, Dec 28, 2019 at 1:26 PM Nate Ferrell notifications@github.com wrote:

hey @Ephesoft-Stitus https://github.com/Ephesoft-Stitus - sorry on the delay here, this has become a bigger fix than anticipated due to Group IDs also being strings, so the same method of testing if the value passed is an email or an ID as we do with Users isn't applicable (we do that currently by testing if the value is able to be cast as a long integer).

Issue source: Email name parts (e.g. mike instead of mike@domain.com) are accepted for convenience via the function automatically appending the Domain to the value if it is not recognized as an ID and does not have an @ in the string. This is an incredibly convenient feature of PSGSuite that I'd like to not break. Since Group IDs aren't easily identified, we run into this issue.

I initially tried approaching this using a new Id parameter and parameter sets (e.g. Email vs Id), but some of the group functions have multiple parameter sets already, making that approach not too feasible.

I tried nesting try/catch statements, but I feel it not only adds to code complexity unnecessarily, but also elongates the overall completion time of the function since you'd be literally retrying the same API call underneath the hood if the first attempt with Domain appended failed to confirm if it was actually an ID that was passed. For example, if you do actually pass a non-existent email/ID to the function, it would need to try and fail twice before returning.

My current thought is to add a switch parameter to the Group functions to block the appending of the domain and try with whatever was passed as-is. This would effectively allow you to state that you know you are either passing full email addresses or IDs to the Identity / Email parameter and to not attempt to resolve the address if @ is not found in the value. Current suggested parameter name is DoNotResolve.

Thoughts?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/scrthq/PSGSuite/issues/249?email_source=notifications&email_token=AMEFYQY3CTGTMCYML63AJDLQ26Y6XA5CNFSM4JL2FQW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHYRRKY#issuecomment-569448619, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMEFYQ6NVBBBVIZEXVVCAA3Q26Y6XANCNFSM4JL2FQWQ .

--

Shaun Titus | EPHESOFT Operations Architect (949) 335-5335 Web https://ephesoft.com/ | LinkedIn https://www.linkedin.com/company/ephesoft-inc/ | Twitter https://twitter.com/Ephesoft

[scrthq]

It might work out if we're able to tighten up that RegEx a bit, I'll do some testing as well

[WJurecki]

@scrthq,

Just as a reference to tighten the RegEx I found that \d{2}([a-z]|\d){13} is (only) slightly tighter and passed all of our domain's group IDs.

[scrthq]

The question is does that match any email name parts for groups? on mobile, but something like...

$null -eq (Get-GSGroup | Where-Object {$_.Email.Split('@')[0] -cmatch '\d{2}([a-z]|\d){13}'})

[WJurecki]

In my use-case, I have no conflicts, so your test returns True.

Will this apply to everyone all the time? ... there is that rule of absolutes.

[scrthq]

Mine returns False, so this would break my own domain basing that on RegEx. The flipside to this is that it's a non-issue if someone passes a full email, since that wouldn't even attempt to resolve and append a domain name. As someone where this would be impacting, the benefit outweighs the gain IMHO since I'm typically passing the full email anyway. Total impact for me is 3 groups out of 2313 total, so 0.13% of my groups run that risk, which is tiny. I tested the inverse as well and confirmed that the pattern matches every single group ID in my domain.

@WJurecki - This would apply to all -GSGroup functions, but only in the instance where someone passes just the email name part as the group ID. For what it's worth, we've run the same risk on the user side for a while now, but the is a smaller likelihood that someone would give a user an email address that's a bigint and could be confused with a User ID vs someone setting a group email to something that is 2 numbers followed by 13 a-z0-9 chars exactly.

@Ephesoft-Stitus @WJurecki - Thoughts on going the RegEx route given that it will always work as intended if the full email is passed? Or worth avoiding considering we've already proven that the RegEx route would still match false positives of actual emails?

[WJurecki]

@scrthq,

I think the RegEx route would be a very acceptable solution because as you point out, passing a full email address always works. (Honestly I have always passed full email addresses and have never depended on the domain being added to what I passed.)

[scrthq]

Tightened the RegEx up a little bit more and have narrowed it down to 1 unavoidable match against my 2313 groups now, still matching 100% of Ids as well: ^\d{2}[a-z\d]{13}$

[scrthq]

Full pattern to avoid appending @domain.com to: ^([\w._%+-]+@[\w.-]+\.[a-zA-Z]{2,}|\d{2}[a-z\d]{13})$

[WJurecki]

@scrthq,

That looks like it will work.

There are a couple minor things about the RegEx.

In the capture before the @ you use [\w._%+-]+, but the \w already includes the _ and therefore could be simplified to [\w.%+-]+.

In the domain name itself you are using [\w.-]+ which allows _ in the domain name, where as best I understand, is not allowed and therefore could be corrected to [a-zA-Z\d.-]+

For a final RegEx of ^([\w.%+-]+@[a-zA-Z\d.-]+\.[a-zA-Z]{2,}|\d{2}[a-z\d]{13})$

Obviously there are other cases where this won't detect invalid domain names (starting or ending with a -, having multiple consecutive ., length limits, currently allowed non-English characters, etc. But where do you stop with the small use cases?)

[scrthq]

fair call outs! RegEx for email address verification can vary a bit, but definitely valid points. I've updated the code on my end with the final RegEx you provided =]

Side note - testing #216 as well right now, hoping to have all of these rolled out by tonight at some point 🙂

[scrthq]

Alright @Ephesoft-Stitus - Updates have been pushed out in v2.35.0 that should allow this to work as intended! Please let me know if you hit any issues.

[Ephesoft-Stitus]

Thanks @scrthq - I missed the updated in late December as I was travelling. Just checking back on this now as I realized I hadn't seen an update in a long time. This is really, really, great and should significantly improve performance for my use case.

[scrthq]

@Ephesoft-Stitus No problem!