Add option to import suspended users

[balciiberk]

This commit fixes #21

• When the flag --import-suspended-users is passed it also imports the users that are suspended on VOMS-Admin
• When it's not passed the behavior is the same as before
• When creating a user on IAM, instead of directly making it not suspended, it checks if the user is suspended on VOMS-Admin

I tested it on our wlcg-auth-dev instance and it looks like it's working as expected but it would be great if you double-check.

[federicaagostini]

Hi @balciiberk, thank you for your PR! We are going to test it as well, just from the code we see that the disabled user information is propagated only for a newly created user in IAM (i.e. a patch operation, such as AUP signature update, is not present for the account status). Is this what we want?

[giacomini]

The changes look ok to me. My only comment is what Federica already wrote: if a suspended user is imported, their status cannot be changed later from the importer itself, but only through IAM. I don't think it's an issue, just something to be aware of.

[chaen]

As discussed already with @balciiberk I think the status in IAM should be the same as the one in VOMS

[balciiberk]

• I edited the --import-suspended-users flag to import also suspended certificates (e.g. when the AUP isn't signed on VOMS Admin, it suspends both the user and the certificates. So it doesn't make sense to import the suspended user and not the suspended certificate)
• I added an option --synchronize-activation-status which synchronizes the active/suspended status of existing users from VOMS Admin to IAM.

[giacomini]

Nitpicking: for uniformity, can you please use the same spelling for synchroniSation as in the existing code? if you prefer, you can change the existing code, which would make it indeed more uniform since we have authZ.

[federicaagostini]

Hi Berk, trying to import a user which is suspended in VOMS Admin into IAM, with

• --import-suspended-users option: for an already existing IAM account, I see from the importer logs that the suspended certificate is imported, but from IAM interface the last modification time is not "now". Moreover, the user status is not updated. While for a not-existing IAM account, the suspended user is created in IAM with disabled status and the suspended certificate is imported
• --synchronize-activation-status option: the syncronization is skipped for a suspended user in both cases of existing and not-existing IAM accounts
• --import-suspended-users --synchronize-activation-status options: the only difference from running the first option alone is that the status is updated even for an already existing IAM account.

In no-one of the above cases the End time is updated. Do you agree about all the tests outcome?

As a conclusion, I would suggest to have only one option that behaves as the two together, or in case experiments have different needs, I would consider a default having the behavior of the two, and the second option used only to skip the import of suspended users. @giacomini what do you think?

We still did not consider the End time of a newly created user here, but I guess it will happen in another issue/PR.

[giacomini]

In no-one of the above cases the End time is updated. Do you agree about all the tests outcome?

As a conclusion, I would suggest to have only one option that behaves as the two together, or in case experiments have different needs, I would consider a default having the behavior of the two, and the second option used only to skip the import of suspended users. @giacomini what do you think?

I agree that the best would be to have the most useful behaviour as the default, but it would not be backwards compatible.

We can indeed join the two options.

We still did not consider the End time of a newly created user here, but I guess it will happen in another issue/PR.

Yes, we should set the end time at creation, but also sync it during an update.

F.

[balciiberk]

Hi Federica,

Thanks a lot for testing the behaviors. The first and third bullets are exactly what we expected. But for the second bullet, I actually don't want it to be skipped for suspended users. I just realized it's because I was just checking --import-suspended-users flag to skip or not skip the user, but I should have checked the other 2 flags, too. Now, as you told I merged those 2 flags and --synchronize-end-time flag that I just created. And now it checks that merged flag to skip or not skip the suspended user, so I think now it should behave as how we want.

I didn't want to change the default behavior because I'm not sure if voms-importer is used anywhere other than CERN, and if it's used, I didn't want to break their config.

Cheers,
Berk

[federicaagostini]

Hi Berk,

just to recap: the --synchronize-end-time option is the only one at the time of writing added with this PR. And what it does when used, is to import user status (active true/false) for both already existing and newly created IAM users. Moreover, when used, it also sync the end time.

I think the end time should be always synchronized (even without using this option, that at this stage can be renamed as something like --import-user-status), but let's see what @giacomini thinks about it.

By the way, as far as I know CERN is the only one using the voms-importer I think.

[giacomini]

Let's do what is needed at CERN right now, without too many options.