Fixed processing of role assignment deletions.

Change-Id: Ib791702e2a09e7f907e664b1a262544cd9484735
Signed-off-by: Pino de Candia <giuseppe.decandia@gmail.com>

tatu/db/models.py

@@ -18,11 +18,13 @@ from sqlalchemy.exc import IntegrityError
 from sqlalchemy.ext.declarative import declarative_base
 import sshpubkeys
 
+from oslo_log import log as logging
 from tatu.castellano import get_secret, store_secret
 from tatu.ks_utils import getProjectRoleNamesForUser
 from tatu.utils import canonical_uuid_string, generateCert, revokedKeysBase64, random_uuid
 
 Base = declarative_base()
+LOG = logging.getLogger(__name__)
 
 
 class Authority(Base):
@@ -167,7 +169,24 @@ def getRevokedKeysBase64(session, auth_id):
 def revokeUserCert(session, cert):
     cert.revoked = True
     session.add(cert)
-    session.add(db.RevokedKey(cert.auth_id, serial=cert.serial))
+    session.add(RevokedKey(auth_id=cert.auth_id, serial=cert.serial))
+    session.commit()
+
+
+def revokeUserCertsForRoleChange(session, user_id, proj_id, new_roles):
+    new_roles = set(new_roles)
+    for cert in session.query(UserCert).filter(UserCert.user_id == user_id).filter(UserCert.auth_id == proj_id):
+        if cert.revoked: continue
+        # A certificate is too permissive if it allows roles that were removed in Keystone.
+        old_roles = cert.principals.split(",")
+        removed_roles = set(old_roles) - new_roles
+        if len(removed_roles) > 0:
+            LOG.info("Revoking certificate with serial {} for user {}"
+                     " because roles/principals {} were removed."
+                .format(cert.serial, cert.user_name, removed_roles))
+            cert.revoked = True
+            session.add(cert)
+            session.add(RevokedKey(auth_id=cert.auth_id, serial=cert.serial))
     session.commit()
 
 
@@ -176,7 +195,7 @@ def revokeUserCerts(session, user_id):
     for u in session.query(UserCert).filter(UserCert.user_id == user_id):
         u.revoked = True
         session.add(u)
-        session.add(RevokedKey(u.auth_id, serial=u.serial))
+        session.add(RevokedKey(auth_id=u.auth_id, serial=u.serial))
     session.commit()
 
 
@@ -185,7 +204,7 @@ def revokeUserCertsInProject(session, user_id, project_id):
     for u in session.query(UserCert).filter(UserCert.user_id == user_id).filter(UserCert.auth_id == project_id):
         u.revoked = True
         session.add(u)
-        session.add(RevokedKey(u.auth_id, serial=u.serial))
+        session.add(RevokedKey(auth_id=u.auth_id, serial=u.serial))
     session.commit()
 
 

tatu/notifications.py

@@ -69,13 +69,16 @@ class NotificationEndpoint(object):
             # TODO: look for domain if project isn't available
             proj_id = payload['project']
             for user_id in users:
+                roles = ks_utils.getProjectRoleNamesForUser(proj_id, user_id)
                 try:
-                    db.revokeUserCertsInProject(se, user_id, proj_id)
+                    se = self.Session()
+                    db.revokeUserCertsForRoleChange(se, user_id, proj_id, roles)
                 except Exception as e:
                     LOG.error(
                         "Failed to revoke user {} certificates in project {} "
-                        "after role assignment change, due to exception {}"
+                        "after role {} was removed, due to exception {}"
-                            .format(user_id, proj_id, e))
+                            .format(user_id, proj_id, payload['role'], e))
+                    import traceback; traceback.print_exc()
                     se.rollback()
                     self.Session.remove()
         elif event_type ==  'identity.user.deleted':
@@ -92,7 +95,7 @@ class NotificationEndpoint(object):
                 se.rollback()
                 self.Session.remove()
         elif event_type == 'compute.instance.delete.end':
-            instance_id = payload.get('instance_id')
+            instance_id = canonical_uuid_string(payload.get('instance_id'))
             host = db.getHost(se, instance_id)
             if host is not None:
                 _deleteHost(self.Session, host)
@@ -175,24 +178,39 @@ def sync(engine):
 
     LOG.info("Revoke user certificates if user was deleted or lost a role.")
     for cert in db.getUserCerts(session_factory()):
-        # Invalidate the cert if the user was removed from Keystone
+        if cert.revoked: continue
-        if cert.user_id not in ks_user_ids:
+        se = session_factory()
-            db.revokeUserCert(cert)
-            continue
 
-        # Invalidate the cert if it has any principals that aren't current
+        try:
-        p = ks_utils.getProjectRoleNamesForUser(cert.auth_id, cert.user_id)
+            # Invalidate the cert if the user was removed from Keystone
-        cert_p = cert.principals.split(",")
+            if cert.user_id not in ks_user_ids:
-        if len(set(cert_p) - set(p)) > 0:
+                db.revokeUserCert(se, cert)
-            se = session_factory()
+                continue
-            db.revokeUserCert(cert)
+
+            # Invalidate the cert if it has any principals that aren't current
+            roles = ks_utils.getProjectRoleNamesForUser(cert.auth_id,
+                                                        cert.user_id)
+            old_roles = cert.principals.split(",")
+            removed_roles = set(old_roles) - set(roles)
+            if len(removed_roles) > 0:
+                LOG.info("Revoking certificate with serial {} for user {}"
+                         " because roles/principals {} were removed."
+                    .format(cert.serial, cert.user_name, removed_roles))
+                db.revokeUserCert(se, cert)
+
+        except:
+            LOG.error(
+            "Failed to delete certificate with serial {} for user {}"
+                .format(cert.serial, cert.user_id))
+            se.rollback()
+            session_factory.remove()
 
     # Iterate through all the instance IDs in Tatu. Clean up DNS and PAT for
     # any that no longer exist in Nova.
     LOG.info("Delete DNS and PAT resources of any server that was deleted.")
     instance_ids = set()
     for instance in nova.servers.list(search_opts={'all_tenants': True}):
-        instance_ids.add(instance.id)
+        instance_ids.add(canonical_uuid_string(instance.id))
     for host in db.getHosts(session_factory()):
         if host.id not in instance_ids:
             _deleteHost(session_factory, host)
@@ -200,7 +218,7 @@ def sync(engine):
 
 def main():
     transport = oslo_messaging.get_notification_transport(CONF)
-    targets = [oslo_messaging.Target(topic='notifications')]
+    targets = [oslo_messaging.Target(topic='tatu_notifications')]
     storage_engine = create_engine(CONF.tatu.sqlalchemy_engine)
 
     endpoints = [NotificationEndpoint(storage_engine)]

tatu/pat.py

@@ -103,7 +103,7 @@ def deletePatEntries(ip_port_tuples):
     pat_entries = DRAGONFLOW.get_all(PATEntry)
     tuples = set(ip_port_tuples)
     for entry in pat_entries:
-        if (entry.pat.id, entry.pat_l4_port) in tuples:
+        if (entry.pat.id, str(entry.pat_l4_port)) in tuples:
             DRAGONFLOW.delete(entry)