WIPACrepo / keycloak-rest-services

Services surrounding KeyCloak, that use the REST API to read/update state
MIT License
1 stars 1 forks source link

add deprovision_mailing_lists action #79

Closed vbrik closed 1 year ago

vbrik commented 1 year ago

@dsschult I appreciate the feedback!

vbrik commented 1 year ago

This seems to be a fairly expensive operation. Would another way to do this be to iterate on all existing members in ml_group_members and only remove them if they have been institutionless for over the grace period? I'd assume that all members being removed must be institutionless.

I was assuming it's possible for users to be members of multiple institutions. E.g. if somebody is a member of CTA and IceCube, and then is removed from IceCube, they will not be institutionless (still member of CTA) but need to be removed from IceCube mailing lists.

dsschult commented 1 year ago

This seems to be a fairly expensive operation. Would another way to do this be to iterate on all existing members in ml_group_members and only remove them if they have been institutionless for over the grace period? I'd assume that all members being removed must be institutionless.

I was assuming it's possible for users to be members of multiple institutions. E.g. if somebody is a member of CTA and IceCube, and then is removed from IceCube, they will not be institutionless (still member of CTA) but need to be removed from IceCube mailing lists.

Hmm, good point. I don't think we cover that corner case very well. Not sure if we need multiple "institutionless" attrs, one for each experiment?

vbrik commented 1 year ago

One thing I could do is this:

I think this is neater than multiple "institutionless" attributes and will be close enough to what we are trying to accomplish.

dsschult commented 1 year ago

One thing I could do is this:

* add user attribute `institutions` with a list of user's institutions.

* periodically compare contents of `institutions` to actual institution membership (yet another action on keycloak-prov)

* if the two don't match, set attribute `last_institutions_change` (and update `institutions`)

* don't remove from mailing lists users whose `last_institutions_change` is recent

I think this is neater than multiple "institutionless" attributes and will be close enough to what we are trying to accomplish.

Sounds good to me.

vbrik commented 1 year ago

I messed up my branch and will need to re-create