Fix user names in tokenless authentications script #53
Conversation
In this script, user names were read from LDAP, where underscores and periods are allowed characters. However, these characters aren’t allowed in GitHub user names, which is why they are replaced with dashes. Because of this, user names containing underscores and periods weren’t linked correctly in the tokenless authentications list. This commit fixes this by performing the substitution directly within the affected script.
updater/scripts/tokenless-auth.sh
Outdated
| @@ -10,4 +10,5 @@ zcat -f /var/log/github/gitauth.log.1* | | |||
| sort | | |||
| uniq -c | | |||
| sort -rn | | |||
| awk '{printf("%s\t%s\t%s\n",$2,$3,$1)}' | |||
| awk '{printf("%s\t%s\t%s\n",$2,$3,$1)}' | | |||
| awk '{gsub(/[_.]/, "-", $1)}1' | |||
There was a problem hiding this comment.
Wouldn't it be easier/more clear to make this in Python? I think it makes sense to use bash etc. to reduce the amount of data (to avoid transporting lots of data over SSH). But massaging the data could be done in Python: https://github.com/Autodesk/hubble/blob/master/updater/reports/ReportTokenlessAuth.py
What do you think?
There was a problem hiding this comment.
Actually, I’d prefer doing the heavy lifting in the scripts and queries and to use Python mostly for controlling. That’s because the scripts might also be useful on their own, and I think they should have the same output as Hubble.
There was a problem hiding this comment.
Hmm. as a compromise, can't we roll the gsub call into the first awk statement?
There was a problem hiding this comment.
This works:
awk '{gsub(/[_.]/, "-", $1); printf("%s\t%s\t%s\n",$2,$3,$1)}'
There was a problem hiding this comment.
Cool, just keep in mind that the $1 in gsub should be a $2, because the order of the two commands is inverted now.
In this script, user names were read from LDAP, where underscores and periods are allowed characters. However, these characters aren’t allowed in GitHub user names, which is why they are replaced with dashes.
Because of this, user names containing underscores and periods weren’t linked correctly in the tokenless authentications list. This pull request fixes this by performing the substitution directly within the affected script.