cis: Update CIS publisher rules to match production #537
Conversation
There was a problem hiding this comment.
I'm leaving a weak r+ here.
The LDAP changes make sense: ldap OR mozilliansorg can create ssh_public_keys and pgp_public_keys, but only LDAP can update them later. That makes it so CIS could make a dummy initial value, but LDAP is the clear source of truth and thus can overwrite later.
As to identities, allowing LDAP to assert ongoing ownership over subentries it owns, makes sense. The reluctance I have is, I can't assert that the list of subentries under identities is complete. But, under the guise of "if it's wrong, it'll probably just fail to update and someone will notice," I'm r+'ing it. I think it's actually complete, so, r+
Added to (a new) WIP list. |
Thanks for the link. Agreed, added r+. @dividehex Do you see any issues with these changes that might not be obvious to either of us? This is low likelihood, but there's not many folks with domain knowledge, so I thought I'd ask. |
This set of changes realigns the permissions granted to LDAP and DinoPark with the capabilities present and used in production today. (For example, DinoPark has no UI to modify SSH or PGP keys.) These were prepared in #523 to allow LDAP to update LDAP keys that can't be set by any other CIS integration, and this commit contains that work.
|
Note that the tests appear to be failing due to some sort of mismatch between the expectations of the tests, and the reality of our permissions, so that's either a good sign (these changes are to a meaningful file) and/or a bad sign (these changes require code spelunking and test review). |
We found in IAM-866 that the CIS publisher rules don't seem to line up with what we're expecting CIS to do. #523 offers up a set of fixes that we should check against the work done with the LDAP-to-CIS scripts during IAM work week, so this PR starts from that point and opens up with review requests to get the ball rolling.
(CI is temporarily broken; #529, #530)
TODO