-
Notifications
You must be signed in to change notification settings - Fork 153
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
replace inline checks with decorator #94
base: master
Are you sure you want to change the base?
Conversation
Implemented #57. This allows batch outside collaborator repo adding.
- This will change how DUO environment variables are sent in. Please see the updated README. - This also adds support for hidden commands where help text is not specified. This is useful for commands that you have retired. This is so if users type in the retired command, they still get some output that you can control -- indicating to them that the command is retired and that another command should be used instead -- while not showing up in the help text.
Added multiple-domain DUO support
Fixed bug #81.
Added more threads to duo error messages
Small change to include a non-zero exit status if the docker build command failed for any reason.
Update for proper exit status
Addresses #83
Properly removes all macOS "Smart Quotes".
Wow! Lots of good stuff in there. I will need a days to look it over. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome PR!
Please address the few items and I'll merge it in and make a new release.
raise NoSecretsProvidedError("Must provide secrets to enable authentication.") | ||
for variable, secret in secrets.items(): | ||
if "DUO_" in variable: | ||
domain, host, ikey, skey = secret.split(",") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems incorrect. All 3 of the variables must be provided for DUO to work.
I would suggest keeping the old code here.
"user_data_required": True, | ||
"help": "Sets team permissions to a specific repository in a specific GitHub organization.", | ||
"permitted_permissions": ["push", "pull"], # To grant admin, add this to the config for | ||
"enabled": True # this command in the config.py. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make disabled by default.
"DUO-HOST": "xxxxxxxx.duosecurity.com", | ||
"DUO-IKEY": "The IKEY for Duo", | ||
"DUO-SKEY": "The SKEY for Duo" | ||
"DUO_YOUR_DOMAIN": "your-domain-here.com,xxxxxxxx.duosecurity.com,THEDUOIKEY,THEDUOSKEY" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would prefer to keep this as is -- it won't break users who upgrade.
Unless you can convince me that this way is better.
No description provided.