adobe-apiplatform / user-sync.py

Application for synchronizing Adobe customer directories via the User Management API
https://adobe-apiplatform.github.io/user-sync.py/en/user-manual/
MIT License
87 stars 67 forks source link

Can't get disabled users to deprovision #9

Closed adorton-adobe closed 7 years ago

adorton-adobe commented 7 years ago

I'm testing the sync tool with a couple of users that I've synced from my Active Directory sandbox. I was able to create and provision them successfully.

test users

When I mark those users as disabled in my sandbox AD, the sync tool doesn't do anything to deprovision them.

Here's the log I get when I run it after disabling the users in AD:

log.txt

What I expect to happen, since I'm not running with the "remove users" option, is for the disabled accounts to have their product entitlements removed, essentially disabling those accounts in the admin console.

For reference, here is my product config. I've ensured that those users only belong to product config groups that are mapped in the config file.

    - directory_group: ADOBE-ALL-APPS
      dashboard_groups:
        - All Apps
    - directory_group: ADOBE-PHOTOSHOP
      dashboard_groups:
        - Photoshop
    - directory_group: ADOBE-VIDEO
      dashboard_groups:
        - Default Custom plan - Video - 6 GB configuration
jamesg123 commented 7 years ago

Hi,

As far as I am aware, the code does not remove access to products for disabled AD users. If you were to remove the user from the groups as well as disabling you should get the desired result.

Thanks, Jim

adobeDan commented 7 years ago

@adorton-adobe, what happens when you run with the remove-users option? Does the expected de-provisioning happen in that case?

adorton-adobe commented 7 years ago

@jamesg123 - The scenario I describe in the issue (de-provisioning users no longer found in AD queries) is a core use case of the initial version of the sync tool. If that functionality isn't supported in this version, then it needs to be added ASAP. I can't provide the tool to customers without that functionality, and I've already committed to providing it to two customers, who expect it to be deployed this week.

adorton-adobe commented 7 years ago

@adobeDan - When I add the --remove-nonexistent-users flag, the sync tool attempts to delete all non-AdobeID users on my org (which right now belong to another domain that I've claimed). The two users I'm testing with are not removed (or attempted - I'm running in test mode).

This could be related to the fact that I have to enable the --users all flag to get those accounts to sync in the first place. If I run without that flag (still deleting users), the sync script won't do anything with those accounts.

I'm still running the RC. I don't believe 1.0 has any fixes to address the issues I'm seeing, but I'll test with it anyway.

adobeDan commented 7 years ago

@adorton-adobe - How do you define "disabled" - is that a specific field maintained by AD? Are you suggesting that we should read this field in the AD connector settings and, if it's set, we should remove the user from all mapped groups?

phil-levy commented 7 years ago

If you run with --remove-nonexistent-users, the account should be removed which includes removing from all groups and PLCs. That is the equivalent functionality. You do have to run with --users all or --users file f (where f lists all users) in order for this to work. Otherwise, sync can't determine if a user is out of the directory.

From: adorton-adobe [mailto:notifications@github.com] Sent: Monday, February 6, 2017 6:14 AM To: adobe-apiplatform/user-sync.py user-sync.py@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [adobe-apiplatform/user-sync.py] Can't get disabled users to deprovision (#9)

@jamesg123https://github.com/jamesg123 - The scenario I describe in the issue (de-provisioning users no longer found in AD queries) is a core use case of the initial version of the sync tool. If that functionality isn't supported in this version, then it needs to be added ASAP. I can't provide the tool to customers without that functionality, and I've already committed to providing it to two customers, who expect it to be deployed this week.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/adobe-apiplatform/user-sync.py/issues/9#issuecomment-277692785, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AH3oWr-72pVZ6Gk6CwGjRGoDEONVEl4wks5rZyqfgaJpZM4L0pSa.

adorton-adobe commented 7 years ago

@adobeDan - I define disabled as users marked as disabled in AD. More broadly, it means any user that is found in the Admin Console, but not AD.

@phil-levy - We don't always want to remove accounts. That would mean that a user's assets and libraries will be deleted along with the account. The default behavior should be for disabled users to be removed from PLCs, but not from the org (unless the "remove users" flag is enabled).

Why can't the sync tool determine if a user is no longer in the directory?

phil-levy commented 7 years ago

Not sure what you mean by asking why the sync tools can't determine if a user is no longer in the directory. That is exactly what it does.

In any case, we still need an answer of what it means to "mark a user as disabled". Is this an AD-specific thing? Is there some attribute value that indicates this? I expect we would be able to support this behavior in the next release at the end of the month when we have support for additional attributes.

thanks phil

adobeDan commented 7 years ago

@adorton-adobe What Phil said. You keep saying the phrase "users marked as disabled in AD" but we don't understand what that means. We are accessing AD as an LDAP source, which means we just see attributes and their value for each user, as well as what groups a user is in. Is there an attribute or group that is conventionally used to mark a user as "disabled"? Or is this some customer-specific thing?

metasemi commented 7 years ago

The disabled status is available as a bit in the userAccountControl attribute mask. The tool already exploits this. Per the example connector-ldap.yml, the default query, intended for AD, retrieves only users who are not disabled. This is the (!(userAccountControl ... )) clause, which can be removed to include disabled users.

So that's the answer to @phil-levy / @adobeDan 's Q from the client point of view.

What that implies for what we do is above my User Sync / LDAP pay grade. By not including the disabled users, does the default query remove them for consideration by --remove-users? If so, the query can be changed to include them as described above, but that's not the whole answer - the behavior of remove would also have to be sensitive to the disabled status. I'm not clear whether userAccountControl is just another attribute that's returned in the query (probably). If so, it would be straightforward to determine the entire and non-disabled sets. (At a bare painful minimum, if necessary the tool could query twice to get this information.) We would still need to think about exactly what to do with this information.

metasemi commented 7 years ago

D'oh! 'Metasemi' == Mike Orr

metasemi commented 7 years ago

A quick JXplorer experiment seems to indicate that userAccountControl is indeed 'just another attribute', so presumably available in the tool's query result.

adorton-adobe commented 7 years ago

I've detailed the disable/de-provisioning scenario below using sync V1. Let me know if the current version supports this. Basically, AD users are compared to Console users, and users found in the Console but not AD get their PLCs removed. That's the idea in a nutshell. If the current version doesn't support this, then we need to get this functionality added ASAP.

Let me know if you have any questions.

  1. create new users in AD, assign to mapped group(s)

new users

  1. run sync to create and provision new users
C:\Users\adorton\projects\adobe-sync-tool [adorton-v1 ≡ +4 ~2 -0 !]
λ  python .\dist\aedc.pex --config=config\config.yml
...
2017-02-08 12:48:43,740 INFO    ADD/REMOVE GROUPS -- SUCCESS - testuser2@ccestestdomain.com
2017-02-08 12:48:43,743 INFO    ADDED: ['All Apps']
2017-02-08 12:48:43,743 INFO    ADD/REMOVE GROUPS -- SUCCESS - testuser1@ccestestdomain.com
2017-02-08 12:48:43,743 INFO    ADDED: ['All Apps']
2017-02-08 12:48:43,744 DEBUG   Starting new HTTPS connection (1): usermanagement-stage.adobe.io
2017-02-08 12:48:45,453 DEBUG   https://usermanagement-stage.adobe.io:443 "POST /v2/usermanagement/action/5190B7C05746859A7F000101@AdobeOrg HTTP/1.1" 200 77
2017-02-08 12:48:45,461 DEBUG   ACTION SUCCESS -- 2 completed
2017-02-08 12:48:45,464 INFO    Processed users for domain ccestestdomain.com
2017-02-08 12:48:45,467 INFO    Finished processing

status of users in Console:

enabled users

  1. disable users in AD

disable users

  1. run sync tool to de-provision users
C:\Users\adorton\projects\adobe-sync-tool [adorton-v1 ≡ +4 ~2 -0 !]
λ  python .\dist\aedc.pex --config=config\config.yml
...
2017-02-08 12:53:15,783 INFO    DISABLE USER --  testuser2@ccestestdomain.com
2017-02-08 12:53:15,788 INFO    REMOVED -- [u'All Apps']
2017-02-08 12:53:15,789 INFO    DISABLE USER --  testuser1@ccestestdomain.com
2017-02-08 12:53:15,792 INFO    REMOVED -- [u'All Apps']
2017-02-08 12:53:15,793 DEBUG   Starting new HTTPS connection (1): usermanagement-stage.adobe.io
2017-02-08 12:53:17,246 DEBUG   https://usermanagement-stage.adobe.io:443 "POST /v2/usermanagement/action/5190B7C05746859A7F000101@AdobeOrg HTTP/1.1" 200 77
2017-02-08 12:53:17,255 DEBUG   ACTION SUCCESS -- 2 completed
2017-02-08 12:53:17,257 INFO    Processed users for domain ccestestdomain.com
2017-02-08 12:53:17,259 INFO    Finished processing

status of users in Console:

disabled users

metasemi commented 7 years ago

Just to be sure @adorton-adobe, in your failing v2 tests, are you leaving the all_users_filter at the default? Not uncommenting anything or defining your own in connector-ldap.yml, and not defining --source-filters on the command line?

adorton-adobe commented 7 years ago

I need to explicitly set --users all. Even though "all" is the default behavior, without that param, nothing syncs.

Here are my ldap filter settings. They are the AD examples in the connector config:

group_filter_format: "(&(objectCategory=group)(cn={group}))"
all_users_filter: "(&(objectClass=user)(objectCategory=person)(!(userAccountControl:1.2.840.113556.1.4.803:=2)))"

I don't define source filters. Here's the command I'm using:

python .\user-sync.pex --process-groups --users all
metasemi commented 7 years ago

Thanks. (Just wanted to make sure you didn't have a custom filter that would cause the disabled users to be loaded, which would prevent them from being seen as orphaned. Looks like you do not.)

metasemi commented 7 years ago

@adorton-adobe @phil-levy @adobeDan @jamesg123 I'm not the most knowledgeable person about this and shouldn't be trusted until someone who knows what they're talking about confirms, but I think the orphaned user recognition only applies to federated users (now), which is not the case in @adorton-adobe's dashboard snap above. Could this be what's going on?

adobeDan commented 7 years ago

@Metasemi thanks for tracking down the details of what "disabled" means in AD.

@adorton-adobe I believe that --users all is not the default behavior (see below for why).

What's going on here is the following:

  1. Andrew's older code always assumed that all users were created by syncing, so any user on the Adobe side that wasn't found on the customer side must have been "deleted" on the customer side. The new code only assumes this if you use the --users all setting.

  2. Andrew's older code didn't ever delete users from the Adobe side, it just removed them from all groups. In order to match this behavior, the new code would need a --disable-deleted-users setting (analogous to --remove-deleted-users).

If we added a --disable-deleted-users setting, then if you specified a custom source filter that limited the source to only users in mapped groups, and if you specified --users all --disable-deleted-users as your args, I believe the new code will do what Andrew's old code used to do. But of course that custom source filter would be a pain in the rear to write.

@phil-levy I am quite concerned that we have broken compatibility with the version that current customers are using, and even more concerned that we have no combination of flags that gets the new code to be compatible with the old code. I don't think we can wait another month before fixing this. I think we have two options:

  1. Add a --users mapped value that means "only sync in mapped users and also remove any mapped groups from Adobe-side users that aren't mapped" (and thus includes the function of --process-groups). This isn't exactly what the older code did (because it allows for groups that exist only on the Adobe side) but it's compatible with the older code as the customers were using it, and not quite as draconian if you create unmapped groups on the Adobe side.

  2. Add a --disable-deleted-users option and force customers to do the fancy source-filter thing.

Personally, I'm for option (1). I am willing to add handling of --users mapped over the next couple of days (I still have my old implementation) if someone (ahem, @jamesg123 @Metasemi) is willing to review/QE the change early next week.

jamesg123 commented 7 years ago

@adobeDan 1 - sounds like a good approach to me and as you have your old implementation it makes sense for you to add the change. I am happy to review once it is complete

adorton-adobe commented 7 years ago

I would also prefer option 1. It seems better to have a simple "backward compatible" mode than add complex functionality to accomplish the same thing. I'm also happy to help test if needed. Just let me know what branch I need to pull for testing.

adobeDan commented 7 years ago

@phil-levy Let's you and I discuss today and make a call.

metasemi commented 7 years ago

@adorton-adobe @phil-levy @adobeDan @jamesg123 Additionally to all this good stuff (which I agree with and of course will help review and test!) can anyone comment on the question in my most recent post? I'm under the impression that users can be recognized as orphaned only in the federated case. Is that correct? (The @adorton-adobe run shown above is not federated.)

phil-levy commented 7 years ago

Sorry for being unavailable for some of this discussion, but I do not understand what the problem is. As spec'd, user sync should remove users who have been disable or otherwise removed from the directory if --users all and --remove-nonexistent-users is specified. Is this not happening? If not, it sounds like a bug to be fixed. I'm unclear why we think we need more options and these seem very confusing with their interactions. Dan and I will discuss further, but can someone confirm if there is a bug?

thanks phil

From: adobeDan [mailto:notifications@github.com] Sent: Wednesday, February 8, 2017 11:58 PM To: adobe-apiplatform/user-sync.py user-sync.py@noreply.github.com Cc: Philip Levy plevy@adobe.com; Mention mention@noreply.github.com Subject: Re: [adobe-apiplatform/user-sync.py] Can't get disabled users to deprovision (#9)

@Metasemihttps://github.com/Metasemi thanks for tracking down the details of what "disabled" means in AD.

@adorton-adobehttps://github.com/adorton-adobe I believe that --users all is not the default behavior (see below for why).

What's going on here is the following:

  1. Andrew's older code always assumed that all users were created by syncing, so any user on the Adobe side that wasn't found on the customer side must have been "deleted" on the customer side. The new code only assumes this if you use the --users all setting.

  2. Andrew's older code didn't ever delete users from the adobe side, it just removed them from all groups. In order to match this behavior, the new code would need a --disable-deleted-users setting (analogous to --remove-deleted-users).

If we added a --disable-deleted-users setting, then if you specified a custom source filter that limited the source to only users in mapped groups, and if you specified --users all --disable-deleted-users as your args, I believe the new code will do what Andrew's old code used to do. But of course that custom source filter would be a pain in the rear to write.

I am quite concerned that we have broken compatibility with the version that current customers are using, and even more concerned that we have no combination of flags that gets the new code to be compatible with the old code. I don't think we can wait another month before fixing this. I think we have two options:

  1. Add a --users mapped value that means "only sync in mapped users and also remove any mapped groups from Adobe-side users that aren't mapped" (and thus includes the function of --process-groups). This isn't exactly what the older code did (because it allows for groups that exist only on the Adobe side) but it's compatible with the older code as the customers were using it, and not quite as draconian if you create unmapped groups on the Adobe side.

  2. Add a --disable-deleted-users option and force customers to do the fancy source-filter thing.

Personally, I'm for option (1). I am willing to add handling of --users mapped over the next couple of days (I still have my old implementation) if someone is willing to review/QE the change early next week.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/adobe-apiplatform/user-sync.py/issues/9#issuecomment-278571901, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AH3oWmhK7YlQd1OV1UDZ4kF3I21Ni8mNks5rascRgaJpZM4L0pSa.

adobeDan commented 7 years ago

@phil-levy There is no bug. Andrew is requesting an enhancement to be fast-tracked so that existing customers who are on the older version of the sync tool can get the functionality they are used to out of the newer version of the sync tool. Without this enhancement they cannot upgrade, because the functionality currently provided by the tool will break their existing workflow (which depends on users losing product access but remaining in the organization when "disabled" in the customer side directory).

@Metasemi There should be no difference in tool functionality between the federated and enterprise use cases. So if you are seeing differences that is a bad bug and should be reported.

metasemi commented 7 years ago

It was an inference from a non-exhaustive look at the code (why I described it as an "impression"). Glad to hear it's incorrect, but I'll test to confirm.

adorton-adobe commented 7 years ago

@adobeDan - how would --users mapped treat non-mapped PLCs and user groups that a user may be a part of?

V1 doesn't allow a user to be added to PLCs and groups that aren't mapped in config. Non-mapped groups are removed from a user when the sync process runs, because the directory is considered to be the source of authority. When users are de-provisioned, all PLCs and groups are removed from a user, including non-mapped.

adobeDan commented 7 years ago

@adorton-adobe Yes I know what the older tool did, but I have always found that to be very limiting from a customer perspective. Consider the very common situation of a customer with two domains, each run out of a separate directory on the customer's side, but both part of the Adobe organization. The customer has one set of mappings he uses with one domain (e.g., for his creatives) and another set of mappings that he uses with the other domain (e.g. for his acrobat/sign users). He runs separate sync jobs -- one for each domain -- because they are being run out of separate directories. Because the code runs against all the users in the org (not just one domain or another), the old code would have constantly been disabling the users from one domain when run for the other.

The point of the proposed --users mapped behavior is to allow for multiple sync jobs to be running against the same Adobe org, either from the same or separate customer-side directory. The assumption is that each sync job targets a different set of groups (both on the customer and Adobe sides), and is completely responsible for the membership of those groups (as opposed those users). Thus any user on the Adobe side but not the customer side who is in one of the groups mapped by the sync job would have that group removed. But if that user was also targeted by other sync jobs (or manually by an admin in the console) then the groups controlled by those jobs/admins would be left undisturbed.

I think this is actually completely backward compatible with the use of your older code by your current customers, because for a customer who really does use a single sync job as their source of authority, only that sync job's groups will be present on any user, so when that user gets disabled on the customer side the Adobe side user will lose all his groups. So I think you can safely have your customers upgrade to using this feature.

Make sense?

jamesg123 commented 7 years ago

@Metasemi @adobeDan Mike - your initial understanding of the code is correct:

The remove-non-existent-users flag (that triggers the orphaned user functionality) is only currently for federated users. Some code below to back this up:

app.py:

parser.add_argument('--remove-nonexistent-users',
                    help='Causes the user sync tool to remove Federated users that exist on the Adobe side if they are not in the customer side AD. This has the effect of deleting the user account if that account is owned by the organization under which the sync operation is being run.',
                    action='store_true', dest='remove_nonexistent_users')

rules.py

process_orphaned_dashboard_users

    if (remove_list_output_path != None):
        self.logger.info('Writing remove list to: %s', remove_list_output_path)
        self.write_remove_list(remove_list_output_path, orphaned_federated_dashboard_users)
    elif (remove_nonexistent_users):
        for dashboard_user in orphaned_federated_dashboard_users:
            user_key = self.get_dashboard_user_key(dashboard_user)
            remove_user_key_list.add(user_key)

I wasn't involved in the project when this code was added, but it seemed quite intentional so I went back to the spec (UBI-UserSyncSpecification-170916-1843-8.pdf page 2) which also seems to indicate this functionality should only be for federated users:

  1. If the option --remove-nonexistent-users is set, then each Federated (type 3) user on the Adobe side is examined to see if it exists on the customer side. If not, the user is removed from the Adobe side. Note that presence of the Adobe user in the "selected set" implies the user exists in the customer side and will not be removed, but absence from this set requires a lookup on the customer side to determine if the user exists there. If it cannot be determined whether or not the user exists on the customer side, then the user is not removed from the Adobe side.

This would be a fairly quick change, if you can confirm that remove-non-existent-users option should apply to both federated and enterprise users then I will modify the code.

adorton-adobe commented 7 years ago

@adobeDan - Thanks, that does make sense. I agree that the old policy of removing users from non-mapped PLCs/groups has some issues. Let's go ahead with what you've proposed.

adorton-adobe commented 7 years ago

@Metasemi @jamesg123 - this explains why my test enterprise users aren't removed when I run --remove-nonexistent-users and why they aren't dumped to the remove list when I generate one. I've created #16 to track this issue separately.

phil-levy commented 7 years ago

does anyone know why we don't remove enterprise users? That sounds like a bug to me.

phil

From: adorton-adobe [mailto:notifications@github.com] Sent: Friday, February 10, 2017 7:42 AM To: adobe-apiplatform/user-sync.py user-sync.py@noreply.github.com Cc: Philip Levy plevy@adobe.com; Mention mention@noreply.github.com Subject: Re: [adobe-apiplatform/user-sync.py] Can't get disabled users to deprovision (#9)

@Metasemihttps://github.com/Metasemi @jamesg123https://github.com/jamesg123 - this explains why my test enterprise users aren't removed when I run --remove-nonexistent-users and why they aren't dumped to the remove list when I generate one. I've created #16https://github.com/adobe-apiplatform/user-sync.py/issues/16 to track this issue separately.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/adobe-apiplatform/user-sync.py/issues/9#issuecomment-278978124, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AH3oWpz5kPs453XCCHcfjxCNGK8Rm0QHks5rbIUugaJpZM4L0pSa.

phil-levy commented 7 years ago

So I think this is now in flight with the following changes:

Closing the issue. Features should appear in a few weeks.