This repository has been archived by the owner on Oct 26, 2021. It is now read-only.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…DAP, if not found in LDAP
…esult and we don't know what to do
gdestuynder
suggested changes
Nov 14, 2016
@@ -26,7 +26,8 @@ def list_all_users(): | |||
should_return = False | |||
return_list = [] | |||
fetch_url = "%s/users" % auth0_config['auth0_api_url'] | |||
payload = {'connection': "%s" % auth0_config['auth0_connection'], 'fields': 'user_id,email,blocked', 'include_fields': 'true'} | |||
# payload = {'connection': "%s" % auth0_config['auth0_connection'], 'fields': 'user_id,email,blocked', 'include_fields': 'true'} |
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.
no commented out code =)
@@ -132,12 +137,14 @@ def main(prog_args=None): | |||
print "Cannot get id attribute for user" |
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.
wont the code fail here, if id is not defined/defined to last value from previous loop?
(also just noticed: id is a reserved python var, should use another name)
gdestuynder
approved these changes
Dec 21, 2016
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.
while there's still some outstanding issues, script works reasonably well - we can move these to issue requests (error reporting, unit testing, style)
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please review and check my logic on this. Before, the script would look at any auth0 accounts that came from the LDAP connection and then check whether they still exist in LDAP and block if not.
The new logic is instead to look at any accounts where the e-mail address is in our list of ldap-managed e-mail domains and if found, then first check if there's a canonical account that matches, if not, check for any aliases that match, finally if not found, then block the account in auth0.
This doesn't handle non-employee LDAP accounts at all, but I added the identities to the api call, so that can be parsed and added as a separate function. I want to get a review on what I have so far and make sure the logic is sound before proceeding.