From aa8fabda3bb415b43de3b5014e4dbd22ce554bac Mon Sep 17 00:00:00 2001
From: Tom Mitchell <tcmitchell@users.noreply.github.com>
Date: Thu, 9 Feb 2017 12:20:48 -0500
Subject: [PATCH 01/11] Add set_swap_nonce to support IdP swapping

set_swap_nonce identifies a source account for the swap so that
later a destination account can be used for the swap.
---
 plugins/chapiv1rpc/chapi/MemberAuthority.py | 17 ++++++
 plugins/marm/MAv1Implementation.py          | 64 +++++++++++++++++++++
 tools/MA_constants.py                       |  5 ++
 tools/client.py                             |  7 +++
 4 files changed, 93 insertions(+)

diff --git a/plugins/chapiv1rpc/chapi/MemberAuthority.py b/plugins/chapiv1rpc/chapi/MemberAuthority.py
index f41a008..e27a69c 100644
--- a/plugins/chapiv1rpc/chapi/MemberAuthority.py
+++ b/plugins/chapiv1rpc/chapi/MemberAuthority.py
@@ -522,6 +522,19 @@ def remove_member_attribute(self,
                                                         mc._session, value)
         return mc._result
 
+    def set_swap_nonce(self, member_urn, nonce, credentials, options):
+        """Remove attribute to member"""
+        with MethodContext(self, MA_LOG_PREFIX,
+                           'set_swap_nonce',
+                           {'member_urn': member_urn, 'nonce': nonce},
+                           credentials, options, read_only=False) as mc:
+            if not mc._error:
+                mc._result = \
+                    self._delegate.set_swap_nonce(mc._client_cert, member_urn,
+                                                  nonce, credentials, options,
+                                                  mc._session)
+        return mc._result
+
 
 # Base class for implementations of MA API
 # Must be  implemented in a derived class, and that derived class
@@ -640,3 +653,7 @@ def add_member_attribute(self, client_cert, member_urn, att_name,
     def remove_member_attribute(self, client_cert, member_urn, att_name, \
                                     credentials, options, session, att_value=None):
         raise CHAPIv1NotImplementedError('')
+
+    def set_swap_nonce(self, client_cert, member_urn, nonce, credentials,
+                       options, session):
+        raise CHAPIv1NotImplementedError('')
diff --git a/plugins/marm/MAv1Implementation.py b/plugins/marm/MAv1Implementation.py
index d57d505..fcf065b 100644
--- a/plugins/marm/MAv1Implementation.py
+++ b/plugins/marm/MAv1Implementation.py
@@ -1698,3 +1698,67 @@ def _lookup_outside_cert_info(self, session, m_ids, fields, result):
                 val = getattr(row, MA.field_mapping[f])
                 result[row.member_id][f] = self.transform_for_result(val)
         return result
+
+    def set_swap_nonce(self, client_cert, member_urn, nonce, credentials,
+                       options, session):
+        """Add a swap nonce attribute to a member account. This account can
+        then be a source account for a swap operation.
+
+        For now this operation is limited to accounts from the GPO IdP.
+        """
+        # find the uid
+        uids = self.get_uids_for_attribute(session, MA.MEMBER_URN, member_urn)
+        if len(uids) == 0:
+            raise CHAPIv1ArgumentError('No member with URN ' + member_urn)
+        member_uid = uids[0]
+
+        # Determine if the eppn is valid for swapping (is from the GPO IdP)
+        rows = self.get_attr_for_uid(session, MA.MEMBER_EPPN, member_uid)
+        if not rows:
+            msg = 'No EPPN found for %s (%s)'
+            chapi_warn(MA_LOG_PREFIX, msg % (member_urn, eppn))
+            raise CHAPIv1ArgumentError('No EPPN for %s' % (member_urn))
+        eppn = rows[0]
+        eppn_regex = '.*@gpolab.bbn.com$'
+        if not re.match(eppn_regex, eppn):
+            msg = 'EPPN %s is invalid for swap operation, must be from GPO IdP'
+            chapi_warn(MA_LOG_PREFIX, msg % (eppn))
+            raise CHAPIv1ArgumentError('Account is invalid for swap')
+
+        # Everything checks out, add the swap nonce attribute
+        result = self.add_member_attribute(client_cert, member_urn,
+                                           MA.SWAP_NONCE, nonce, True,
+                                           credentials, options, session)
+        # If adding the attribute failed, return the error
+        if result['code']:
+            return result
+        nonce_ts = datetime.datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%S')
+        result2 = self.add_member_attribute(client_cert, member_urn,
+                                            MA.SWAP_NONCE_TS, nonce_ts, True,
+                                            credentials, options, session)
+        if result2['code']:
+            msg = 'Unable to set attribute %s on %s. Continuing without it.'
+            chapi_warn(MA_LOG_PREFIX, msg % (MA.SWAP_NONCE_TS, member_urn))
+            raise CHAPIv1ArgumentError('Account is invalid for swap')
+
+        # Return the NONCE result, not the NONCE_TS result.
+        return result
+
+    def swap_identities(self, client_cert, member_urn, nonce, credentials,
+                        options, session):
+        """Swaps two identities by swapping their ePPNs. Will
+        also swap the email addresses if they differ.
+
+        The source account is identified by a nonce which has previously
+        been established by a call to `set_swap_nonce`.
+
+        Also adds an attribute to the destination account indicating that it
+        has already been swapped. Accounts can only be swapped once. The
+        value of this attribute is a timestamp to indicate when the swap
+        occurred.
+
+        Destination URN must be an account based in the NCSA identity provider.
+        """
+
+        # Can we clear the NONCE and NONCE_TS from the source account?
+        pass
diff --git a/tools/MA_constants.py b/tools/MA_constants.py
index 5bb2b7b..d825d5a 100644
--- a/tools/MA_constants.py
+++ b/tools/MA_constants.py
@@ -194,3 +194,8 @@
 
 USER_CRED_LIFE_YEARS = 1 # See MA.get_user_credential
 # FIXME: username regex
+
+SWAP_NONCE = 'ncsa_nonce'
+SWAP_NONCE_TS = 'ncsa_nonce_ts'
+MEMBER_URN = 'MEMBER_URN'
+MEMBER_EPPN = '_GENI_MEMBER_EPPN'
diff --git a/tools/client.py b/tools/client.py
index d97888d..9bcbfbb 100644
--- a/tools/client.py
+++ b/tools/client.py
@@ -421,6 +421,13 @@ def main(args=sys.argv, do_print=True):
             _do_ssl(framework, suppress_errors, reason, fcn,
                     member_eppn, project_id, slice_id)
 
+    # MA Swap nonce method
+    elif opts.method in ['set_swap_nonce']:
+        options = {}
+        (result, msg) = \
+            _do_ssl(framework, suppress_errors, reason, fcn, opts.urn,
+                    opts.string_arg, opts.credentials, options)
+
     # Methods that take attributes and options
     elif client_attributes:
         (result, msg) = _do_ssl(framework, suppress_errors, reason, fcn,

From 75c6af0bfec737725d872f3c9b57e7ddc5a25ca3 Mon Sep 17 00:00:00 2001
From: Tom Mitchell <tcmitchell@users.noreply.github.com>
Date: Mon, 13 Feb 2017 09:41:52 -0500
Subject: [PATCH 02/11] Initial implementation of IdP swapping

Implemented set_swap_nonce and swap_identities in skeletal form.
Works to swap identities, needs error checking. Implemented several
helper routines to find, update, and delete member attributes.
---
 etc/member_authority_policy.json            | 50 ++++++++----
 plugins/chapiv1rpc/chapi/MemberAuthority.py | 25 +++++-
 plugins/marm/MAv1Implementation.py          | 86 ++++++++++++++++++---
 tools/MA_constants.py                       |  4 +-
 tools/client.py                             |  4 +-
 5 files changed, 140 insertions(+), 29 deletions(-)

diff --git a/etc/member_authority_policy.json b/etc/member_authority_policy.json
index 3c8c2da..dfb202e 100644
--- a/etc/member_authority_policy.json
+++ b/etc/member_authority_policy.json
@@ -28,11 +28,11 @@
      ],
      "policies" : [
        "ME.MAY_$METHOD<-ME.IS_AUTHORITY",
-       "ME.MAY_$METHOD<-ME.IS_OPERATOR", 
+       "ME.MAY_$METHOD<-ME.IS_OPERATOR",
        "ME.MAY_$METHOD<-ME.INVOKING_ON_$SELF",
        "ME.MAY_$METHOD_$MEMBER<-ME.SHARES_PROJECT_$MEMBER",
-       "ME.MAY_$METHOD<-ME.IS_PROJECT_LEAD_AND_SEARCHING_BY_EMAIL", 
-       "ME.MAY_$METHOD<-ME.IS_PROJECT_ADMIN_AND_SEARCHING_BY_EMAIL", 
+       "ME.MAY_$METHOD<-ME.IS_PROJECT_LEAD_AND_SEARCHING_BY_EMAIL",
+       "ME.MAY_$METHOD<-ME.IS_PROJECT_ADMIN_AND_SEARCHING_BY_EMAIL",
        "ME.MAY_$METHOD<-ME.IS_SEARCHING_FOR_PROJECT_LEAD_BY_UID",
        "ME.MAY_$METHOD<-ME.HAS_PENDING_REQUEST_TO_MEMBER",
        "ME.MAY_$METHOD<-ME.HAS_PENDING_REQUEST_FROM_MEMBER"
@@ -58,11 +58,11 @@
     ],
     "policies" : [
        "ME.MAY_$METHOD<-ME.IS_AUTHORITY",
-       "ME.MAY_$METHOD<-ME.IS_OPERATOR", 
+       "ME.MAY_$METHOD<-ME.IS_OPERATOR",
        "ME.MAY_$METHOD<-ME.INVOKING_ON_$SELF",
        "ME.MAY_$METHOD_$MEMBER<-ME.SHARES_PROJECT_$MEMBER",
-       "ME.MAY_$METHOD<-ME.IS_PROJECT_LEAD_AND_SEARCHING_BY_EMAIL", 
-       "ME.MAY_$METHOD<-ME.IS_PROJECT_ADMIN_AND_SEARCHING_BY_EMAIL", 
+       "ME.MAY_$METHOD<-ME.IS_PROJECT_LEAD_AND_SEARCHING_BY_EMAIL",
+       "ME.MAY_$METHOD<-ME.IS_PROJECT_ADMIN_AND_SEARCHING_BY_EMAIL",
        "ME.MAY_$METHOD<-ME.IS_SEARCHING_FOR_PROJECT_LEAD_BY_UID",
        "ME.MAY_$METHOD<-ME.HAS_PENDING_REQUEST_TO_MEMBER",
        "ME.MAY_$METHOD<-ME.HAS_PENDING_REQUEST_FROM_MEMBER"
@@ -75,8 +75,8 @@
        "ME.INVOKING_ON_$MEMBER<-CALLER"
      ],
     "policies" : [
-      "ME.MAY_$METHOD<-ME.IS_AUTHORITY", 
-      "ME.MAY_$METHOD<-ME.IS_OPERATOR", 
+      "ME.MAY_$METHOD<-ME.IS_AUTHORITY",
+      "ME.MAY_$METHOD<-ME.IS_OPERATOR",
       "ME.MAY_$METHOD<-ME.INVOKING_ON_$SELF"
     ]
   },
@@ -87,8 +87,8 @@
        "ME.INVOKING_ON_$MEMBER<-CALLER"
      ],
     "policies" : [
-      "ME.MAY_$METHOD<-ME.IS_AUTHORITY", 
-      "ME.MAY_$METHOD<-ME.IS_OPERATOR", 
+      "ME.MAY_$METHOD<-ME.IS_AUTHORITY",
+      "ME.MAY_$METHOD<-ME.IS_OPERATOR",
       "ME.MAY_$METHOD<-ME.INVOKING_ON_$SELF"
     ]
   },
@@ -99,8 +99,8 @@
        "ME.INVOKING_ON_$MEMBER<-CALLER"
      ],
     "policies" : [
-      "ME.MAY_$METHOD<-ME.IS_AUTHORITY", 
-      "ME.MAY_$METHOD<-ME.IS_OPERATOR", 
+      "ME.MAY_$METHOD<-ME.IS_AUTHORITY",
+      "ME.MAY_$METHOD<-ME.IS_OPERATOR",
       "ME.MAY_$METHOD<-ME.INVOKING_ON_$SELF"
     ]
   },
@@ -111,7 +111,7 @@
        "ME.INVOKING_ON_$MEMBER<-CALLER"
      ],
     "policies" : [
-      "ME.MAY_$METHOD<-ME.IS_OPERATOR", 
+      "ME.MAY_$METHOD<-ME.IS_OPERATOR",
       "ME.MAY_$METHOD<-ME.INVOKING_ON_$SELF"
     ]
   },
@@ -218,7 +218,7 @@
       "policies" : [
         "ME.MAY_$METHOD<-ME.IS_AUTHORITY",
         "ME.MAY_$METHOD<-ME.IS_OPERATOR"
-      ] 
+      ]
     },
 
     "revoke_member_privilege" : {
@@ -226,7 +226,7 @@
       "policies" : [
         "ME.MAY_$METHOD<-ME.IS_AUTHORITY",
         "ME.MAY_$METHOD<-ME.IS_OPERATOR"
-      ] 
+      ]
     },
 
     "add_member_attribute" : {
@@ -251,5 +251,25 @@
         "ME.MAY_$METHOD<-ME.IS_OPERATOR",
         "ME.MAY_$METHOD<-ME.INVOKING_ON_$SELF"
       ]
+    },
+
+    "set_swap_nonce" : {
+      "__DOC__" : "self",
+       "assertions" : [
+         "ME.INVOKING_ON_$MEMBER<-CALLER"
+       ],
+      "policies" : [
+        "ME.MAY_$METHOD<-ME.INVOKING_ON_$SELF"
+      ]
+    },
+
+    "swap_identities" : {
+      "__DOC__" : "self",
+       "assertions" : [
+         "ME.INVOKING_ON_$MEMBER<-CALLER"
+       ],
+      "policies" : [
+        "ME.MAY_$METHOD<-ME.INVOKING_ON_$SELF"
+      ]
     }
 }
diff --git a/plugins/chapiv1rpc/chapi/MemberAuthority.py b/plugins/chapiv1rpc/chapi/MemberAuthority.py
index e27a69c..379b509 100644
--- a/plugins/chapiv1rpc/chapi/MemberAuthority.py
+++ b/plugins/chapiv1rpc/chapi/MemberAuthority.py
@@ -1,5 +1,5 @@
 #----------------------------------------------------------------------
-# Copyright (c) 2011-2016 Raytheon BBN Technologies
+# Copyright (c) 2011-2017 Raytheon BBN Technologies
 #
 # Permission is hereby granted, free of charge, to any person obtaining
 # a copy of this software and/or hardware specification (the "Work") to
@@ -523,7 +523,9 @@ def remove_member_attribute(self,
         return mc._result
 
     def set_swap_nonce(self, member_urn, nonce, credentials, options):
-        """Remove attribute to member"""
+        """Set a swap nonce attribute on the member for later use
+        to swap identities.
+        """
         with MethodContext(self, MA_LOG_PREFIX,
                            'set_swap_nonce',
                            {'member_urn': member_urn, 'nonce': nonce},
@@ -535,6 +537,21 @@ def set_swap_nonce(self, member_urn, nonce, credentials, options):
                                                   mc._session)
         return mc._result
 
+    def swap_identities(self, member_urn, nonce, credentials, options):
+        """Swap identities by making this user point to the identity with
+        the matching nonce.
+        """
+        with MethodContext(self, MA_LOG_PREFIX,
+                           'swap_identities',
+                           {'member_urn': member_urn, 'nonce': nonce},
+                           credentials, options, read_only=False) as mc:
+            if not mc._error:
+                mc._result = \
+                    self._delegate.swap_identities(mc._client_cert, member_urn,
+                                                   nonce, credentials, options,
+                                                   mc._session)
+        return mc._result
+
 
 # Base class for implementations of MA API
 # Must be  implemented in a derived class, and that derived class
@@ -657,3 +674,7 @@ def remove_member_attribute(self, client_cert, member_urn, att_name, \
     def set_swap_nonce(self, client_cert, member_urn, nonce, credentials,
                        options, session):
         raise CHAPIv1NotImplementedError('')
+
+    def swap_identities(self, client_cert, member_urn, nonce, credentials,
+                        options, session):
+        raise CHAPIv1NotImplementedError('')
diff --git a/plugins/marm/MAv1Implementation.py b/plugins/marm/MAv1Implementation.py
index fcf065b..f305d46 100644
--- a/plugins/marm/MAv1Implementation.py
+++ b/plugins/marm/MAv1Implementation.py
@@ -1,5 +1,5 @@
 # ----------------------------------------------------------------------
-# Copyright (c) 2013-2016 Raytheon BBN Technologies
+# Copyright (c) 2013-2017 Raytheon BBN Technologies
 #
 # Permission is hereby granted, free of charge, to any person obtaining
 # a copy of this software and/or hardware specification (the "Work") to
@@ -1699,6 +1699,33 @@ def _lookup_outside_cert_info(self, session, m_ids, fields, result):
                 result[row.member_id][f] = self.transform_for_result(val)
         return result
 
+    def _map_attr(self, attr):
+        if attr in MA.field_mapping:
+            attr = MA.field_mapping[attr]
+        return attr
+
+    def _make_attr_query(self, session, uid=None, name=None, value=None):
+        q = session.query(MemberAttribute)
+        if uid is not None:
+            q = q.filter(MemberAttribute.member_id == uid)
+        if name is not None:
+            q = q.filter(MemberAttribute.name == self._map_attr(name))
+        if value is not None:
+            q = q.filter(MemberAttribute.value == value)
+        return q
+
+    def _get_first_attr(self, session, uid=None, name=None, value=None):
+        q = self._make_attr_query(session, uid, name, value)
+        return q.first()
+
+    def _get_all_attrs(self, session, uid=None, name=None, value=None):
+        q = self._make_attr_query(session, uid, name, value)
+        return q.all()
+
+    def _delete_attr(self, session, uid, name, value=None):
+        q = self._make_attr_query(session, uid, name, value)
+        return q.delete()
+
     def set_swap_nonce(self, client_cert, member_urn, nonce, credentials,
                        options, session):
         """Add a swap nonce attribute to a member account. This account can
@@ -1707,24 +1734,29 @@ def set_swap_nonce(self, client_cert, member_urn, nonce, credentials,
         For now this operation is limited to accounts from the GPO IdP.
         """
         # find the uid
-        uids = self.get_uids_for_attribute(session, MA.MEMBER_URN, member_urn)
-        if len(uids) == 0:
+        member = self._get_first_attr(session, None, MA.MEMBER_URN, member_urn)
+        if not member:
             raise CHAPIv1ArgumentError('No member with URN ' + member_urn)
-        member_uid = uids[0]
 
         # Determine if the eppn is valid for swapping (is from the GPO IdP)
-        rows = self.get_attr_for_uid(session, MA.MEMBER_EPPN, member_uid)
-        if not rows:
+        eppn = self._get_first_attr(session, member.member_id, MA.MEMBER_EPPN)
+        if not eppn:
             msg = 'No EPPN found for %s (%s)'
-            chapi_warn(MA_LOG_PREFIX, msg % (member_urn, eppn))
+            chapi_warn(MA_LOG_PREFIX, msg % (member_urn, member.member_id))
             raise CHAPIv1ArgumentError('No EPPN for %s' % (member_urn))
-        eppn = rows[0]
+        eppn = eppn.value
         eppn_regex = '.*@gpolab.bbn.com$'
         if not re.match(eppn_regex, eppn):
             msg = 'EPPN %s is invalid for swap operation, must be from GPO IdP'
             chapi_warn(MA_LOG_PREFIX, msg % (eppn))
             raise CHAPIv1ArgumentError('Account is invalid for swap')
 
+        # TODO: does this user already have a nonce?
+        # TODO: is this a duplicate nonce?
+
+        # TODO: Is there a better way to create the new nonce row?
+        # TODO:   YES! -- see below for an example
+
         # Everything checks out, add the swap nonce attribute
         result = self.add_member_attribute(client_cert, member_urn,
                                            MA.SWAP_NONCE, nonce, True,
@@ -1759,6 +1791,42 @@ def swap_identities(self, client_cert, member_urn, nonce, credentials,
 
         Destination URN must be an account based in the NCSA identity provider.
         """
+        # find the uid
+        dest = self._get_first_attr(session, None, MA.MEMBER_URN, member_urn)
+        if not dest:
+            raise CHAPIv1ArgumentError('No member with URN ' + member_urn)
+
+        source = self._get_first_attr(session, None, MA.SWAP_NONCE, nonce)
+        if not source:
+            raise CHAPIv1ArgumentError('Invalid swap tag ' + nonce)
+
+        dest_eppn = self._get_first_attr(session, dest.member_id,
+                                         MA.MEMBER_EPPN)
+        dest_email = self._get_first_attr(session, dest.member_id,
+                                          MA.MEMBER_EMAIL)
+        source_eppn = self._get_first_attr(session, source.member_id,
+                                           MA.MEMBER_EPPN)
+        source_email = self._get_first_attr(session, source.member_id,
+                                          MA.MEMBER_EMAIL)
+
+        # Swap the EPPNS
+        tmp = dest_eppn.value
+        dest_eppn.value = source_eppn.value
+        source_eppn.value = tmp
+
+        # Swap the email addresses if they differ
+        if dest_email.value != source_email.value:
+            tmp = dest_email.value
+            dest_email.value = source_email.value
+            source_email.value = tmp
+
+        # Add a timestamp for when the swap was made
+        ts = datetime.datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%S')
+        ts_attr = MemberAttribute(MA.SWAP_DONE_TS, ts, dest.member_id, False)
+        session.add(ts_attr)
 
         # Can we clear the NONCE and NONCE_TS from the source account?
-        pass
+        self._delete_attr(session, source.member_id, MA.SWAP_NONCE, nonce)
+        self._delete_attr(session, source.member_id, MA.SWAP_NONCE_TS)
+
+        return self._successReturn(True)
diff --git a/tools/MA_constants.py b/tools/MA_constants.py
index d825d5a..50d8861 100644
--- a/tools/MA_constants.py
+++ b/tools/MA_constants.py
@@ -1,5 +1,5 @@
 #----------------------------------------------------------------------
-# Copyright (c) 2011-2016 Raytheon BBN Technologies
+# Copyright (c) 2011-2017 Raytheon BBN Technologies
 #
 # Permission is hereby granted, free of charge, to any person obtaining
 # a copy of this software and/or hardware specification (the "Work") to
@@ -197,5 +197,7 @@
 
 SWAP_NONCE = 'ncsa_nonce'
 SWAP_NONCE_TS = 'ncsa_nonce_ts'
+SWAP_DONE_TS = 'ncsa_swapped'
 MEMBER_URN = 'MEMBER_URN'
 MEMBER_EPPN = '_GENI_MEMBER_EPPN'
+MEMBER_EMAIL = 'MEMBER_EMAIL'
diff --git a/tools/client.py b/tools/client.py
index 9bcbfbb..a1f6c89 100644
--- a/tools/client.py
+++ b/tools/client.py
@@ -1,6 +1,6 @@
 #!/usr/bin/env python
 # ----------------------------------------------------------------------
-# Copyright (c) 2013-2016 Raytheon BBN Technologies
+# Copyright (c) 2013-2017 Raytheon BBN Technologies
 #
 # Permission is hereby granted, free of charge, to any person obtaining
 # a copy of this software and/or hardware specification (the "Work") to
@@ -422,7 +422,7 @@ def main(args=sys.argv, do_print=True):
                     member_eppn, project_id, slice_id)
 
     # MA Swap nonce method
-    elif opts.method in ['set_swap_nonce']:
+    elif opts.method in ['set_swap_nonce', 'swap_identities']:
         options = {}
         (result, msg) = \
             _do_ssl(framework, suppress_errors, reason, fcn, opts.urn,

From 132b5f04db532c7b59b443c44f9c881dec7d64d9 Mon Sep 17 00:00:00 2001
From: Tom Mitchell <tcmitchell@users.noreply.github.com>
Date: Mon, 13 Feb 2017 16:57:55 -0500
Subject: [PATCH 03/11] Swap identities with CH generated id

Have the CH generate the swap id (formerly nonce) instead of
allowing the client to pass one in. This allows the CH to control
uniqueness more easily. The generated id is returned from the
call to create_swap_id(), and is later used with swap_identities()
to swap two identities.
---
 etc/member_authority_policy.json            |  2 +-
 plugins/chapiv1rpc/chapi/MemberAuthority.py | 14 ++---
 plugins/marm/MAv1Implementation.py          | 60 ++++++++++-----------
 tools/chapi_utils.py.in                     |  9 ++++
 4 files changed, 47 insertions(+), 38 deletions(-)

diff --git a/etc/member_authority_policy.json b/etc/member_authority_policy.json
index dfb202e..ab4d009 100644
--- a/etc/member_authority_policy.json
+++ b/etc/member_authority_policy.json
@@ -253,7 +253,7 @@
       ]
     },
 
-    "set_swap_nonce" : {
+    "create_swap_id" : {
       "__DOC__" : "self",
        "assertions" : [
          "ME.INVOKING_ON_$MEMBER<-CALLER"
diff --git a/plugins/chapiv1rpc/chapi/MemberAuthority.py b/plugins/chapiv1rpc/chapi/MemberAuthority.py
index 379b509..8ad7336 100644
--- a/plugins/chapiv1rpc/chapi/MemberAuthority.py
+++ b/plugins/chapiv1rpc/chapi/MemberAuthority.py
@@ -522,18 +522,18 @@ def remove_member_attribute(self,
                                                         mc._session, value)
         return mc._result
 
-    def set_swap_nonce(self, member_urn, nonce, credentials, options):
+    def create_swap_id(self, member_urn, credentials, options):
         """Set a swap nonce attribute on the member for later use
         to swap identities.
         """
         with MethodContext(self, MA_LOG_PREFIX,
-                           'set_swap_nonce',
-                           {'member_urn': member_urn, 'nonce': nonce},
+                           'create_swap_id',
+                           {'member_urn': member_urn},
                            credentials, options, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
-                    self._delegate.set_swap_nonce(mc._client_cert, member_urn,
-                                                  nonce, credentials, options,
+                    self._delegate.create_swap_id(mc._client_cert, member_urn,
+                                                  credentials, options,
                                                   mc._session)
         return mc._result
 
@@ -671,8 +671,8 @@ def remove_member_attribute(self, client_cert, member_urn, att_name, \
                                     credentials, options, session, att_value=None):
         raise CHAPIv1NotImplementedError('')
 
-    def set_swap_nonce(self, client_cert, member_urn, nonce, credentials,
-                       options, session):
+    def create_swap_id(self, client_cert, member_urn, credentials, options,
+                       session):
         raise CHAPIv1NotImplementedError('')
 
     def swap_identities(self, client_cert, member_urn, nonce, credentials,
diff --git a/plugins/marm/MAv1Implementation.py b/plugins/marm/MAv1Implementation.py
index f305d46..04f636e 100644
--- a/plugins/marm/MAv1Implementation.py
+++ b/plugins/marm/MAv1Implementation.py
@@ -1726,10 +1726,12 @@ def _delete_attr(self, session, uid, name, value=None):
         q = self._make_attr_query(session, uid, name, value)
         return q.delete()
 
-    def set_swap_nonce(self, client_cert, member_urn, nonce, credentials,
+    def create_swap_id(self, client_cert, member_urn, credentials,
                        options, session):
-        """Add a swap nonce attribute to a member account. This account can
-        then be a source account for a swap operation.
+        """Create a swap id that can be used later with swap_identities
+        to swap two accounts. The account that calls this method is the
+        source of the identity swap. The account that calls swap_identities
+        with this swap_id is the destination of the identity swap.
 
         For now this operation is limited to accounts from the GPO IdP.
         """
@@ -1751,38 +1753,35 @@ def set_swap_nonce(self, client_cert, member_urn, nonce, credentials,
             chapi_warn(MA_LOG_PREFIX, msg % (eppn))
             raise CHAPIv1ArgumentError('Account is invalid for swap')
 
-        # TODO: does this user already have a nonce?
-        # TODO: is this a duplicate nonce?
-
-        # TODO: Is there a better way to create the new nonce row?
-        # TODO:   YES! -- see below for an example
-
-        # Everything checks out, add the swap nonce attribute
-        result = self.add_member_attribute(client_cert, member_urn,
-                                           MA.SWAP_NONCE, nonce, True,
-                                           credentials, options, session)
-        # If adding the attribute failed, return the error
-        if result['code']:
-            return result
-        nonce_ts = datetime.datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%S')
-        result2 = self.add_member_attribute(client_cert, member_urn,
-                                            MA.SWAP_NONCE_TS, nonce_ts, True,
-                                            credentials, options, session)
-        if result2['code']:
-            msg = 'Unable to set attribute %s on %s. Continuing without it.'
-            chapi_warn(MA_LOG_PREFIX, msg % (MA.SWAP_NONCE_TS, member_urn))
-            raise CHAPIv1ArgumentError('Account is invalid for swap')
-
-        # Return the NONCE result, not the NONCE_TS result.
-        return result
+        # Does this user already have a nonce? If so, return it
+        nonce = self._get_first_attr(session, member.member_id, MA.SWAP_NONCE)
+        if nonce:
+            return self._successReturn(nonce.value)
+
+        # Avoid duplicate nonces. Get all nonces, then generate random id
+        # until it is not in the list of all nonces
+        nonces = [n.value for n in self._get_all_attrs(session, None,
+                                                       MA.SWAP_NONCE, None)]
+        nonce = random_id(8)
+        while nonce in nonces:
+            nonce = random_id(8)
+
+        nonce_attr = MemberAttribute(MA.SWAP_NONCE, nonce, member.member_id,
+                                     False)
+        session.add(nonce_attr)
+        ts = datetime.datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%S')
+        ts_attr = MemberAttribute(MA.SWAP_NONCE_TS, ts, member.member_id,
+                                  False)
+        session.add(ts_attr)
+        return self._successReturn(nonce)
 
     def swap_identities(self, client_cert, member_urn, nonce, credentials,
                         options, session):
         """Swaps two identities by swapping their ePPNs. Will
         also swap the email addresses if they differ.
 
-        The source account is identified by a nonce which has previously
-        been established by a call to `set_swap_nonce`.
+        The source account is identified by an id which has previously
+        been established by a call to `create_swap_id`.
 
         Also adds an attribute to the destination account indicating that it
         has already been swapped. Accounts can only be swapped once. The
@@ -1814,7 +1813,8 @@ def swap_identities(self, client_cert, member_urn, nonce, credentials,
         dest_eppn.value = source_eppn.value
         source_eppn.value = tmp
 
-        # Swap the email addresses if they differ
+        # Swap the email addresses if they differ so that the user
+        # gets the email address they most recently used, at NCSA.
         if dest_email.value != source_email.value:
             tmp = dest_email.value
             dest_email.value = source_email.value
diff --git a/tools/chapi_utils.py.in b/tools/chapi_utils.py.in
index a8105cd..ae98c54 100644
--- a/tools/chapi_utils.py.in
+++ b/tools/chapi_utils.py.in
@@ -28,6 +28,8 @@ from email.mime.text import MIMEText
 from email.header import Header
 import os.path
 import datetime
+import string
+import random
 from chapi_log import *
 
 
@@ -120,3 +122,10 @@ def get_implementation_info(log_prefix):
         "code_release_date": str(code_timestamp),
         "site_update_date": str(code_timestamp)
     }
+
+def random_id(length=6):
+    chars = string.digits + string.uppercase
+    result = '';
+    for i in range(length):
+        result += chars[random.randint(0, 35)]
+    return result

From 743966830b06bdb32ba3cabd4b8adbe41aa6dfaa Mon Sep 17 00:00:00 2001
From: Tom Mitchell <tcmitchell@users.noreply.github.com>
Date: Tue, 14 Feb 2017 15:51:39 -0500
Subject: [PATCH 04/11] Swap identity tweaks

Store standard dates in the database instead of formatted strings.
Reduce PEP8 errors. Rename attributes to indicate GPO IdP swapping
instead of NCSA (destination) IdP. Store the source URN for
bookkeeping.
---
 plugins/chapiv1rpc/chapi/MemberAuthority.py |  4 +--
 plugins/marm/MAv1Implementation.py          | 30 ++++++++++++---------
 test/travis-build                           |  4 +--
 tools/MA_constants.py                       | 12 ++++-----
 4 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/plugins/chapiv1rpc/chapi/MemberAuthority.py b/plugins/chapiv1rpc/chapi/MemberAuthority.py
index 8ad7336..2cebff5 100644
--- a/plugins/chapiv1rpc/chapi/MemberAuthority.py
+++ b/plugins/chapiv1rpc/chapi/MemberAuthority.py
@@ -1,4 +1,4 @@
-#----------------------------------------------------------------------
+# ----------------------------------------------------------------------
 # Copyright (c) 2011-2017 Raytheon BBN Technologies
 #
 # Permission is hereby granted, free of charge, to any person obtaining
@@ -19,7 +19,7 @@
 # WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE WORK OR THE USE OR OTHER DEALINGS
 # IN THE WORK.
-#----------------------------------------------------------------------
+# ----------------------------------------------------------------------
 
 import tools.pluginmanager as pm
 from DelegateBase import DelegateBase
diff --git a/plugins/marm/MAv1Implementation.py b/plugins/marm/MAv1Implementation.py
index 04f636e..d281231 100644
--- a/plugins/marm/MAv1Implementation.py
+++ b/plugins/marm/MAv1Implementation.py
@@ -1754,23 +1754,24 @@ def create_swap_id(self, client_cert, member_urn, credentials,
             raise CHAPIv1ArgumentError('Account is invalid for swap')
 
         # Does this user already have a nonce? If so, return it
-        nonce = self._get_first_attr(session, member.member_id, MA.SWAP_NONCE)
+        nonce = self._get_first_attr(session, member.member_id,
+                                     MA.SWAP_ID_ATTR)
         if nonce:
             return self._successReturn(nonce.value)
 
         # Avoid duplicate nonces. Get all nonces, then generate random id
         # until it is not in the list of all nonces
         nonces = [n.value for n in self._get_all_attrs(session, None,
-                                                       MA.SWAP_NONCE, None)]
+                                                       MA.SWAP_ID_ATTR, None)]
         nonce = random_id(8)
         while nonce in nonces:
             nonce = random_id(8)
 
-        nonce_attr = MemberAttribute(MA.SWAP_NONCE, nonce, member.member_id,
+        nonce_attr = MemberAttribute(MA.SWAP_ID_ATTR, nonce, member.member_id,
                                      False)
         session.add(nonce_attr)
-        ts = datetime.datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%S')
-        ts_attr = MemberAttribute(MA.SWAP_NONCE_TS, ts, member.member_id,
+        ts = datetime.datetime.utcnow().replace(microsecond=0)
+        ts_attr = MemberAttribute(MA.SWAP_TS_ATTR, ts, member.member_id,
                                   False)
         session.add(ts_attr)
         return self._successReturn(nonce)
@@ -1787,15 +1788,13 @@ def swap_identities(self, client_cert, member_urn, nonce, credentials,
         has already been swapped. Accounts can only be swapped once. The
         value of this attribute is a timestamp to indicate when the swap
         occurred.
-
-        Destination URN must be an account based in the NCSA identity provider.
         """
         # find the uid
         dest = self._get_first_attr(session, None, MA.MEMBER_URN, member_urn)
         if not dest:
             raise CHAPIv1ArgumentError('No member with URN ' + member_urn)
 
-        source = self._get_first_attr(session, None, MA.SWAP_NONCE, nonce)
+        source = self._get_first_attr(session, None, MA.SWAP_ID_ATTR, nonce)
         if not source:
             raise CHAPIv1ArgumentError('Invalid swap tag ' + nonce)
 
@@ -1806,7 +1805,9 @@ def swap_identities(self, client_cert, member_urn, nonce, credentials,
         source_eppn = self._get_first_attr(session, source.member_id,
                                            MA.MEMBER_EPPN)
         source_email = self._get_first_attr(session, source.member_id,
-                                          MA.MEMBER_EMAIL)
+                                            MA.MEMBER_EMAIL)
+        source_urn = self._get_first_attr(session, source.member_id,
+                                          MA.MEMBER_URN)
 
         # Swap the EPPNS
         tmp = dest_eppn.value
@@ -1821,12 +1822,15 @@ def swap_identities(self, client_cert, member_urn, nonce, credentials,
             source_email.value = tmp
 
         # Add a timestamp for when the swap was made
-        ts = datetime.datetime.utcnow().strftime('%Y-%m-%dT%H:%M:%S')
-        ts_attr = MemberAttribute(MA.SWAP_DONE_TS, ts, dest.member_id, False)
+        ts = datetime.datetime.utcnow().replace(microsecond=0)
+        ts_attr = MemberAttribute(MA.SWAP_TS_ATTR, ts, dest.member_id, False)
         session.add(ts_attr)
+        from_attr = MemberAttribute(MA.SWAP_FROM_ATTR, source_urn.value,
+                                    dest.member_id, False)
+        session.add(from_attr)
 
         # Can we clear the NONCE and NONCE_TS from the source account?
-        self._delete_attr(session, source.member_id, MA.SWAP_NONCE, nonce)
-        self._delete_attr(session, source.member_id, MA.SWAP_NONCE_TS)
+        self._delete_attr(session, source.member_id, MA.SWAP_ID_ATTR, nonce)
+        self._delete_attr(session, source.member_id, MA.SWAP_TS_ATTR)
 
         return self._successReturn(True)
diff --git a/test/travis-build b/test/travis-build
index eb70090..990aae5 100755
--- a/test/travis-build
+++ b/test/travis-build
@@ -1,7 +1,7 @@
 #!/bin/sh
 
-# Limits as of February 2, 2017
-ERROR_LIMIT=2847
+# Limits as of February 14, 2017
+ERROR_LIMIT=2844
 WARNING_LIMIT=46
 
 RESULT=0
diff --git a/tools/MA_constants.py b/tools/MA_constants.py
index 50d8861..10a0288 100644
--- a/tools/MA_constants.py
+++ b/tools/MA_constants.py
@@ -1,4 +1,4 @@
-#----------------------------------------------------------------------
+# ----------------------------------------------------------------------
 # Copyright (c) 2011-2017 Raytheon BBN Technologies
 #
 # Permission is hereby granted, free of charge, to any person obtaining
@@ -19,7 +19,7 @@
 # WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE WORK OR THE USE OR OTHER DEALINGS
 # IN THE WORK.
-#----------------------------------------------------------------------
+# ----------------------------------------------------------------------
 
 credential_types = [
     {"type" : "geni_sfa", "version" : "3"},
@@ -192,12 +192,12 @@
 ]
 updatable_key_fields = ["KEY_DESCRIPTION", "_GENI_KEY_FILENAME"]
 
-USER_CRED_LIFE_YEARS = 1 # See MA.get_user_credential
+USER_CRED_LIFE_YEARS = 1  # See MA.get_user_credential
 # FIXME: username regex
 
-SWAP_NONCE = 'ncsa_nonce'
-SWAP_NONCE_TS = 'ncsa_nonce_ts'
-SWAP_DONE_TS = 'ncsa_swapped'
+SWAP_ID_ATTR = 'gpo_swap_id'
+SWAP_TS_ATTR = 'gpo_swap_ts'
+SWAP_FROM_ATTR = 'gpo_swap_from'
 MEMBER_URN = 'MEMBER_URN'
 MEMBER_EPPN = '_GENI_MEMBER_EPPN'
 MEMBER_EMAIL = 'MEMBER_EMAIL'

From f50658a9a1a8e57a34c82424b4f9bb13efc339bc Mon Sep 17 00:00:00 2001
From: Tom Mitchell <tcmitchell@users.noreply.github.com>
Date: Tue, 14 Feb 2017 16:00:15 -0500
Subject: [PATCH 05/11] Testing PEP8 Travis build issue

---
 test/travis-build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/travis-build b/test/travis-build
index 990aae5..ad0603f 100755
--- a/test/travis-build
+++ b/test/travis-build
@@ -11,6 +11,7 @@ WARNING_COUNT=`python test/pep8.py --filename '*.py,*.py.in' --ignore E . | wc -
 if test $ERROR_COUNT -gt $ERROR_LIMIT
 then
         echo "Error count $ERROR_COUNT is greater than limit $ERROR_LIMIT"
+        python test/pep8.py --filename '*.py,*.py.in' --ignore W .
         RESULT=1
 fi
 

From 2366249763fb73d51eeba4666c9b7ba23a9d83de Mon Sep 17 00:00:00 2001
From: Tom Mitchell <tcmitchell@users.noreply.github.com>
Date: Tue, 14 Feb 2017 16:21:17 -0500
Subject: [PATCH 06/11] Fix 2 PEP8 errors

---
 tools/chapi_utils.py.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/chapi_utils.py.in b/tools/chapi_utils.py.in
index ae98c54..2a9a2e3 100644
--- a/tools/chapi_utils.py.in
+++ b/tools/chapi_utils.py.in
@@ -123,9 +123,10 @@ def get_implementation_info(log_prefix):
         "site_update_date": str(code_timestamp)
     }
 
+
 def random_id(length=6):
     chars = string.digits + string.uppercase
-    result = '';
+    result = ''
     for i in range(length):
         result += chars[random.randint(0, 35)]
     return result

From 6634d75a82fccfe3c143297c3f0665c920b26aed Mon Sep 17 00:00:00 2001
From: Tom Mitchell <tcmitchell@users.noreply.github.com>
Date: Tue, 14 Feb 2017 16:21:40 -0500
Subject: [PATCH 07/11] Remove travis build debugging

---
 test/travis-build | 1 -
 1 file changed, 1 deletion(-)

diff --git a/test/travis-build b/test/travis-build
index ad0603f..990aae5 100755
--- a/test/travis-build
+++ b/test/travis-build
@@ -11,7 +11,6 @@ WARNING_COUNT=`python test/pep8.py --filename '*.py,*.py.in' --ignore E . | wc -
 if test $ERROR_COUNT -gt $ERROR_LIMIT
 then
         echo "Error count $ERROR_COUNT is greater than limit $ERROR_LIMIT"
-        python test/pep8.py --filename '*.py,*.py.in' --ignore W .
         RESULT=1
 fi
 

From 264f233cc4a63cf5776d6de2a976d42f446d427e Mon Sep 17 00:00:00 2001
From: Tom Mitchell <tcmitchell@users.noreply.github.com>
Date: Wed, 22 Feb 2017 10:43:05 -0500
Subject: [PATCH 08/11] Make identity swap a single API call

Kill the notion of a nonce and instead allow authorities to invoke
swap_identities with the two identities as URNs.
---
 etc/member_authority_policy.json   |  2 +-
 plugins/marm/MAv1Implementation.py | 18 ++++++------------
 2 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/etc/member_authority_policy.json b/etc/member_authority_policy.json
index ab4d009..5e00252 100644
--- a/etc/member_authority_policy.json
+++ b/etc/member_authority_policy.json
@@ -269,7 +269,7 @@
          "ME.INVOKING_ON_$MEMBER<-CALLER"
        ],
       "policies" : [
-        "ME.MAY_$METHOD<-ME.INVOKING_ON_$SELF"
+        "ME.MAY_$METHOD<-ME.IS_AUTHORITY"
       ]
     }
 }
diff --git a/plugins/marm/MAv1Implementation.py b/plugins/marm/MAv1Implementation.py
index d281231..adb21d0 100644
--- a/plugins/marm/MAv1Implementation.py
+++ b/plugins/marm/MAv1Implementation.py
@@ -1776,7 +1776,7 @@ def create_swap_id(self, client_cert, member_urn, credentials,
         session.add(ts_attr)
         return self._successReturn(nonce)
 
-    def swap_identities(self, client_cert, member_urn, nonce, credentials,
+    def swap_identities(self, client_cert, source_urn, dest_urn, credentials,
                         options, session):
         """Swaps two identities by swapping their ePPNs. Will
         also swap the email addresses if they differ.
@@ -1790,13 +1790,13 @@ def swap_identities(self, client_cert, member_urn, nonce, credentials,
         occurred.
         """
         # find the uid
-        dest = self._get_first_attr(session, None, MA.MEMBER_URN, member_urn)
+        dest = self._get_first_attr(session, None, MA.MEMBER_URN, dest_urn)
         if not dest:
-            raise CHAPIv1ArgumentError('No member with URN ' + member_urn)
+            raise CHAPIv1ArgumentError('No member with URN ' + dest_urn)
 
-        source = self._get_first_attr(session, None, MA.SWAP_ID_ATTR, nonce)
+        source = self._get_first_attr(session, None, MA.MEMBER_URN, source_urn)
         if not source:
-            raise CHAPIv1ArgumentError('Invalid swap tag ' + nonce)
+            raise CHAPIv1ArgumentError('No member with URN ' + source_urn)
 
         dest_eppn = self._get_first_attr(session, dest.member_id,
                                          MA.MEMBER_EPPN)
@@ -1806,8 +1806,6 @@ def swap_identities(self, client_cert, member_urn, nonce, credentials,
                                            MA.MEMBER_EPPN)
         source_email = self._get_first_attr(session, source.member_id,
                                             MA.MEMBER_EMAIL)
-        source_urn = self._get_first_attr(session, source.member_id,
-                                          MA.MEMBER_URN)
 
         # Swap the EPPNS
         tmp = dest_eppn.value
@@ -1825,12 +1823,8 @@ def swap_identities(self, client_cert, member_urn, nonce, credentials,
         ts = datetime.datetime.utcnow().replace(microsecond=0)
         ts_attr = MemberAttribute(MA.SWAP_TS_ATTR, ts, dest.member_id, False)
         session.add(ts_attr)
-        from_attr = MemberAttribute(MA.SWAP_FROM_ATTR, source_urn.value,
+        from_attr = MemberAttribute(MA.SWAP_FROM_ATTR, source_urn,
                                     dest.member_id, False)
         session.add(from_attr)
 
-        # Can we clear the NONCE and NONCE_TS from the source account?
-        self._delete_attr(session, source.member_id, MA.SWAP_ID_ATTR, nonce)
-        self._delete_attr(session, source.member_id, MA.SWAP_TS_ATTR)
-
         return self._successReturn(True)

From 0d401c5c6d9853887b38cda1ab85db048239d109 Mon Sep 17 00:00:00 2001
From: Tom Mitchell <tcmitchell@users.noreply.github.com>
Date: Wed, 22 Feb 2017 10:44:56 -0500
Subject: [PATCH 09/11] Reduce PEP8 warnings in member authority

---
 plugins/chapiv1rpc/chapi/MemberAuthority.py | 142 ++++++++++----------
 test/travis-build                           |   4 +-
 2 files changed, 76 insertions(+), 70 deletions(-)

diff --git a/plugins/chapiv1rpc/chapi/MemberAuthority.py b/plugins/chapiv1rpc/chapi/MemberAuthority.py
index 2cebff5..3c75a36 100644
--- a/plugins/chapiv1rpc/chapi/MemberAuthority.py
+++ b/plugins/chapiv1rpc/chapi/MemberAuthority.py
@@ -31,6 +31,7 @@
 
 ma_logger = logging.getLogger('mav1')
 
+
 # RPC handler for Member Authority (MA) API calls
 class MAv1Handler(HandlerBase):
     def __init__(self):
@@ -50,42 +51,42 @@ def get_version(self, options={}):
     # Generic V2 service methods
     def create(self, type, credentials, options):
         if type == "MEMBER":
-            result = \
-                self._errorReturn(CHAPIv1ArgumentError("method create not supported for MEMBER"))
+            msg = "method create not supported for MEMBER"
+            result = self._errorReturn(CHAPIv1ArgumentError(msg))
         elif type == "KEY":
             result = self.create_key(credentials, options)
         else:
-            result = self._errorReturn(CHAPIv1ArgumentError("Invalid type: %s" % type))
+            msg = "Invalid type: %s" % (type)
+            result = self._errorReturn(CHAPIv1ArgumentError(msg))
         return result
 
     def update(self, type, urn, credentials, options):
         if type == "MEMBER":
-            result = \
-                self.update_member_info(urn, credentials, options)
+            result = self.update_member_info(urn, credentials, options)
         elif type == "KEY":
-            result = \
-                self.update_key(urn, credentials, options)
+            result = self.update_key(urn, credentials, options)
         else:
-            result = self._errorReturn(CHAPIv1ArgumentError("Invalid type: %s" % type))
+            msg = "Invalid type: %s" % (type)
+            result = self._errorReturn(CHAPIv1ArgumentError(msg))
         return result
 
     def delete(self, type, urn, credentials, options):
         if type == "MEMBER":
-            result = \
-                self._errorReturn(CHAPIv1ArgumentError("method delete not supported for MEMBER"))
+            msg = "method delete not supported for MEMBER"
+            result = self._errorReturn(CHAPIv1ArgumentError(msg))
         elif type == "KEY":
-            result = \
-                self.delete_key( urn, credentials, options)
+            result = self.delete_key(urn, credentials, options)
         else:
-             result = self._errorReturn(CHAPIv1ArgumentError("Invalid type: %s" % type))
+            msg = "Invalid type: %s" % (type)
+            result = self._errorReturn(CHAPIv1ArgumentError(msg))
         return result
 
     def lookup(self, type, credentials, options):
         if not isinstance(options, dict):
-            return self._errorReturn(CHAPIv1ArgumentError("Options argument must be dictionary"))
+            msg = "Options argument must be dictionary"
+            return self._errorReturn(CHAPIv1ArgumentError(msg))
         if type == "MEMBER":
-            result = \
-                self.lookup_allowed_member_info(credentials, options)
+            result = self.lookup_allowed_member_info(credentials, options)
         elif type == "KEY":
             # In v1 we return a dictionary (indexed by member URN)
             # of a list of dictionaries, one for each key of that user
@@ -98,31 +99,32 @@ def lookup(self, type, credentials, options):
                 self.lookup_keys(credentials, options)
             if result['code'] == NO_ERROR:
                 v2_result = {}
-#                chapi_info("LOOKUP", "RESULT = %s" % result)
+                # chapi_info("LOOKUP", "RESULT = %s" % result)
                 for member_urn, key_infos in result['value'].items():
                     for key_info in key_infos:
-#                        chapi_info("LOOKUP", "MURN = %s KEY_INFO = %s" % \
-#                                       (member_urn, key_info))
+                        # chapi_info("LOOKUP", "MURN = %s KEY_INFO = %s" % \
+                        #            (member_urn, key_info))
                         if 'KEY_MEMBER' not in key_info:
                             key_info['KEY_MEMBER'] = member_urn
                         key_id = key_info['KEY_ID']
                         v2_result[key_id] = key_info
                 result = self._successReturn(v2_result)
         else:
-            result = self._errorReturn(CHAPIv1ArgumentError("Invalid type: %s" % type))
+            msg = "Invalid type: %s" % (type)
+            result = self._errorReturn(CHAPIv1ArgumentError(msg))
         return result
 
-
     # MEMBER service methods
 
     def lookup_allowed_member_info(self, credentials, options):
         with MethodContext(self, MA_LOG_PREFIX, 'lookup_allowed_member_info',
                            {}, credentials, options, read_only=True) as mc:
             if not mc._error:
-                mc._result = self._delegate.lookup_allowed_member_info(mc._client_cert,
-                                                                       credentials,
-                                                                       options,
-                                                                       mc._session)
+                mc._result = \
+                    self._delegate.lookup_allowed_member_info(mc._client_cert,
+                                                              credentials,
+                                                              options,
+                                                              mc._session)
         return mc._result
 
     def lookup_public_member_info(self, credentials, options):
@@ -177,8 +179,8 @@ def lookup_identifying_member_info(self, credentials, options):
         return mc._result
 
     def lookup_public_identifying_member_info(self, credentials, options):
-        """Return both public and identifying information about members specified in options
-        filter and query fields
+        """Return both public and identifying information about members
+        specified in options filter and query fields
 
         This call is protected
         Authorized by client cert and credentials
@@ -189,9 +191,9 @@ def lookup_public_identifying_member_info(self, credentials, options):
             if not mc._error:
                 mc._result = \
                     self._delegate.lookup_public_identifying_member_info(mc._client_cert,
-                                                                  credentials,
-                                                                  options,
-                                                                  mc._session)
+                                                                         credentials,
+                                                                         options,
+                                                                         mc._session)
         return mc._result
 
     def lookup_login_info(self, credentials, options):
@@ -219,7 +221,7 @@ def get_credentials(self, member_urn, credentials, options):
         """
         with MethodContext(self, MA_LOG_PREFIX,
                            'get_credentials',
-                           {'member_urn' : member_urn},
+                           {'member_urn': member_urn},
                            credentials, options, read_only=True) as mc:
             if not mc._error:
                 mc._result = \
@@ -238,7 +240,7 @@ def update_member_info(self, member_urn, credentials, options):
         """
         with MethodContext(self, MA_LOG_PREFIX,
                            'update_member_info',
-                           {'member_urn' : member_urn},
+                           {'member_urn': member_urn},
                            credentials, options, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
@@ -250,16 +252,16 @@ def update_member_info(self, member_urn, credentials, options):
         return mc._result
 
     def create_member(self, attributes, credentials, options):
-        """Create a new member using the specified attributes.  Attribute email is
-        required.  Returns the attributes of the resulting member record, including
-        the uid and urn.
+        """Create a new member using the specified attributes.  Attribute
+        email is required.  Returns the attributes of the resulting member
+        record, including the uid and urn.
 
         This call is protected
         Authorized by client cert and credentials
         """
         with MethodContext(self, MA_LOG_PREFIX,
                            'create_member',
-                           {'attributes' : attributes},
+                           {'attributes': attributes},
                            credentials, options, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
@@ -276,7 +278,8 @@ def create_key(self, credentials, options):
         """Create a record for a key pair for given member
         Arguments:
             member_urn: URN of member for which to retrieve credentials
-            options: 'fields' containing the fields for the key pair being stored
+            options: 'fields' containing the fields for the key pair being
+                     stored
         Return:
             Dictionary of name/value pairs for created key record
             including the KEY_ID
@@ -297,7 +300,8 @@ def create_key(self, credentials, options):
     def delete_key(self, key_id, credentials, options):
         """Delete a specific key pair for given member
         Arguments:
-            key_id: KEY_ID (unique for member/key fingerprint) of key(s) to be deleted
+            key_id: KEY_ID (unique for member/key fingerprint) of key(s) to
+                    be deleted
         Return:
             True if succeeded
 
@@ -305,7 +309,7 @@ def delete_key(self, key_id, credentials, options):
         """
         with MethodContext(self, MA_LOG_PREFIX,
                            'delete_key',
-                           {'key_id' : key_id},
+                           {'key_id': key_id},
                            credentials, options, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
@@ -331,7 +335,7 @@ def update_key(self, key_id, credentials, options):
         """
         with MethodContext(self, MA_LOG_PREFIX,
                            'update_key',
-                           {'key_id' : key_id},
+                           {'key_id': key_id},
                            credentials, options, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
@@ -359,9 +363,9 @@ def lookup_keys(self, credentials, options):
             if not mc._error:
                 mc._result = \
                     self._delegate.lookup_keys(mc._client_cert,
-                                              credentials,
-                                              options,
-                                              mc._session)
+                                               credentials,
+                                               options,
+                                               mc._session)
         return mc._result
 
     def create_certificate(self, member_urn, credentials, options):
@@ -371,15 +375,15 @@ def create_certificate(self, member_urn, credentials, options):
         """
         with MethodContext(self, MA_LOG_PREFIX,
                            'create_certificate',
-                           {'member_urn' : member_urn},
+                           {'member_urn': member_urn},
                            credentials, options, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
                     self._delegate.create_certificate(mc._client_cert,
-                                              member_urn,
-                                              credentials,
-                                              options,
-                                              mc._session)
+                                                      member_urn,
+                                                      credentials,
+                                                      options,
+                                                      mc._session)
         return mc._result
 
     # ClientAuth API
@@ -399,7 +403,7 @@ def list_authorized_clients(self, member_id):
         """
         with MethodContext(self, MA_LOG_PREFIX,
                            'list_authorized_clients',
-                           {'member_id' : member_id}, [], {},
+                           {'member_id': member_id}, [], {},
                            read_only=True) as mc:
             if not mc._error:
                 mc._result = \
@@ -413,8 +417,9 @@ def authorize_client(self, member_id, client_urn, authorize_sense):
         """
         with MethodContext(self, MA_LOG_PREFIX,
                            'authorize_client',
-                           {'member_id' : member_id,'client_urn' : client_urn,
-                            'authorize_sense' : authorize_sense},
+                           {'member_id': member_id,
+                            'client_urn': client_urn,
+                            'authorize_sense': authorize_sense},
                            [], {}, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
@@ -425,7 +430,6 @@ def authorize_client(self, member_id, client_urn, authorize_sense):
                                                     mc._session)
         return mc._result
 
-
     # member disable API
     def enable_user(self, member_urn, enable_sense, credentials, options):
         """Enable or disable a user based on URN. If enable_sense is False, then user
@@ -433,8 +437,8 @@ def enable_user(self, member_urn, enable_sense, credentials, options):
         """
         with MethodContext(self, MA_LOG_PREFIX,
                            'enable_user',
-                           {'member_urn' : member_urn,
-                            'enable_sense' : enable_sense},
+                           {'member_urn': member_urn,
+                            'enable_sense': enable_sense},
                            credentials, options, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
@@ -447,13 +451,14 @@ def enable_user(self, member_urn, enable_sense, credentials, options):
         return mc._result
 
     # member privilege (private)
-    def add_member_privilege(self, member_uid, privilege, credentials, options):
+    def add_member_privilege(self, member_uid, privilege, credentials,
+                             options):
         """Add a privilege to a member.
         privilege is either OPERATOR or PROJECT_LEAD
         """
         with MethodContext(self, MA_LOG_PREFIX,
                            'add_member_privilege',
-                           {'member_uid' : member_uid,'privilege' : privilege},
+                           {'member_uid': member_uid, 'privilege': privilege},
                            credentials, options, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
@@ -465,11 +470,12 @@ def add_member_privilege(self, member_uid, privilege, credentials, options):
                                                         mc._session)
         return mc._result
 
-    def revoke_member_privilege(self, member_uid, privilege, credentials, options):
+    def revoke_member_privilege(self, member_uid, privilege, credentials,
+                                options):
         """Revoke a privilege for a member."""
         with MethodContext(self, MA_LOG_PREFIX,
                            'revoke_member_privilege',
-                           {'member_uid' : member_uid,'privilege' : privilege},
+                           {'member_uid': member_uid, 'privilege': privilege},
                            credentials, options, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
@@ -487,9 +493,9 @@ def add_member_attribute(self,
         """Add an attribute to member"""
         with MethodContext(self, MA_LOG_PREFIX,
                            'add_member_attribute',
-                           {'member_urn' : member_urn,
-                            'name' : name, 'value' : value,
-                            'self_asserted' : self_asserted},
+                           {'member_urn': member_urn,
+                            'name': name, 'value': value,
+                            'self_asserted': self_asserted},
                            credentials, options, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
@@ -509,17 +515,17 @@ def remove_member_attribute(self,
         """Remove attribute to member"""
         with MethodContext(self, MA_LOG_PREFIX,
                            'remove_member_attribute',
-                           {'member_urn' : member_urn, 'name' : name,
-                            'value' : value},
+                           {'member_urn': member_urn, 'name': name,
+                            'value': value},
                            credentials, options, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
                     self._delegate.remove_member_attribute(mc._client_cert,
-                                                        member_urn,
-                                                        name,
-                                                        credentials,
-                                                        options,
-                                                        mc._session, value)
+                                                           member_urn,
+                                                           name,
+                                                           credentials,
+                                                           options,
+                                                           mc._session, value)
         return mc._result
 
     def create_swap_id(self, member_urn, credentials, options):
diff --git a/test/travis-build b/test/travis-build
index 990aae5..18d566a 100755
--- a/test/travis-build
+++ b/test/travis-build
@@ -1,7 +1,7 @@
 #!/bin/sh
 
-# Limits as of February 14, 2017
-ERROR_LIMIT=2844
+# Limits as of February 22, 2017
+ERROR_LIMIT=2781
 WARNING_LIMIT=46
 
 RESULT=0

From 3dffbf8ff8fbd494a8d3dbf5feeb6c36799b6046 Mon Sep 17 00:00:00 2001
From: Tom Mitchell <tcmitchell@users.noreply.github.com>
Date: Wed, 22 Feb 2017 16:36:43 -0500
Subject: [PATCH 10/11] Remove create_swap_id, no longer necessary

Make identity swapping a single API call. Remove the now extraneous
create_swap_id. Make swap_identities callable by client.py for
testing. Fix up inline docs that mention create_swap_id.
---
 etc/member_authority_policy.json            | 10 ----
 plugins/chapiv1rpc/chapi/MemberAuthority.py | 31 +++--------
 plugins/marm/MAv1Implementation.py          | 60 ++-------------------
 tools/MA_constants.py                       |  1 -
 tools/chapi_utils.py.in                     | 10 ----
 tools/client.py                             |  6 ++-
 6 files changed, 13 insertions(+), 105 deletions(-)

diff --git a/etc/member_authority_policy.json b/etc/member_authority_policy.json
index 5e00252..c91880e 100644
--- a/etc/member_authority_policy.json
+++ b/etc/member_authority_policy.json
@@ -253,16 +253,6 @@
       ]
     },
 
-    "create_swap_id" : {
-      "__DOC__" : "self",
-       "assertions" : [
-         "ME.INVOKING_ON_$MEMBER<-CALLER"
-       ],
-      "policies" : [
-        "ME.MAY_$METHOD<-ME.INVOKING_ON_$SELF"
-      ]
-    },
-
     "swap_identities" : {
       "__DOC__" : "self",
        "assertions" : [
diff --git a/plugins/chapiv1rpc/chapi/MemberAuthority.py b/plugins/chapiv1rpc/chapi/MemberAuthority.py
index 3c75a36..ec76ab8 100644
--- a/plugins/chapiv1rpc/chapi/MemberAuthority.py
+++ b/plugins/chapiv1rpc/chapi/MemberAuthority.py
@@ -528,34 +528,19 @@ def remove_member_attribute(self,
                                                            mc._session, value)
         return mc._result
 
-    def create_swap_id(self, member_urn, credentials, options):
-        """Set a swap nonce attribute on the member for later use
-        to swap identities.
-        """
-        with MethodContext(self, MA_LOG_PREFIX,
-                           'create_swap_id',
-                           {'member_urn': member_urn},
-                           credentials, options, read_only=False) as mc:
-            if not mc._error:
-                mc._result = \
-                    self._delegate.create_swap_id(mc._client_cert, member_urn,
-                                                  credentials, options,
-                                                  mc._session)
-        return mc._result
-
-    def swap_identities(self, member_urn, nonce, credentials, options):
+    def swap_identities(self, source_urn, dest_urn, credentials, options):
         """Swap identities by making this user point to the identity with
         the matching nonce.
         """
         with MethodContext(self, MA_LOG_PREFIX,
                            'swap_identities',
-                           {'member_urn': member_urn, 'nonce': nonce},
+                           {'source_urn': source_urn, 'dest_urn': dest_urn},
                            credentials, options, read_only=False) as mc:
             if not mc._error:
                 mc._result = \
-                    self._delegate.swap_identities(mc._client_cert, member_urn,
-                                                   nonce, credentials, options,
-                                                   mc._session)
+                    self._delegate.swap_identities(mc._client_cert, source_urn,
+                                                   dest_urn, credentials,
+                                                   options, mc._session)
         return mc._result
 
 
@@ -677,10 +662,6 @@ def remove_member_attribute(self, client_cert, member_urn, att_name, \
                                     credentials, options, session, att_value=None):
         raise CHAPIv1NotImplementedError('')
 
-    def create_swap_id(self, client_cert, member_urn, credentials, options,
-                       session):
-        raise CHAPIv1NotImplementedError('')
-
-    def swap_identities(self, client_cert, member_urn, nonce, credentials,
+    def swap_identities(self, client_cert, source_urn, dest_urn, credentials,
                         options, session):
         raise CHAPIv1NotImplementedError('')
diff --git a/plugins/marm/MAv1Implementation.py b/plugins/marm/MAv1Implementation.py
index adb21d0..a45a000 100644
--- a/plugins/marm/MAv1Implementation.py
+++ b/plugins/marm/MAv1Implementation.py
@@ -1726,68 +1726,14 @@ def _delete_attr(self, session, uid, name, value=None):
         q = self._make_attr_query(session, uid, name, value)
         return q.delete()
 
-    def create_swap_id(self, client_cert, member_urn, credentials,
-                       options, session):
-        """Create a swap id that can be used later with swap_identities
-        to swap two accounts. The account that calls this method is the
-        source of the identity swap. The account that calls swap_identities
-        with this swap_id is the destination of the identity swap.
-
-        For now this operation is limited to accounts from the GPO IdP.
-        """
-        # find the uid
-        member = self._get_first_attr(session, None, MA.MEMBER_URN, member_urn)
-        if not member:
-            raise CHAPIv1ArgumentError('No member with URN ' + member_urn)
-
-        # Determine if the eppn is valid for swapping (is from the GPO IdP)
-        eppn = self._get_first_attr(session, member.member_id, MA.MEMBER_EPPN)
-        if not eppn:
-            msg = 'No EPPN found for %s (%s)'
-            chapi_warn(MA_LOG_PREFIX, msg % (member_urn, member.member_id))
-            raise CHAPIv1ArgumentError('No EPPN for %s' % (member_urn))
-        eppn = eppn.value
-        eppn_regex = '.*@gpolab.bbn.com$'
-        if not re.match(eppn_regex, eppn):
-            msg = 'EPPN %s is invalid for swap operation, must be from GPO IdP'
-            chapi_warn(MA_LOG_PREFIX, msg % (eppn))
-            raise CHAPIv1ArgumentError('Account is invalid for swap')
-
-        # Does this user already have a nonce? If so, return it
-        nonce = self._get_first_attr(session, member.member_id,
-                                     MA.SWAP_ID_ATTR)
-        if nonce:
-            return self._successReturn(nonce.value)
-
-        # Avoid duplicate nonces. Get all nonces, then generate random id
-        # until it is not in the list of all nonces
-        nonces = [n.value for n in self._get_all_attrs(session, None,
-                                                       MA.SWAP_ID_ATTR, None)]
-        nonce = random_id(8)
-        while nonce in nonces:
-            nonce = random_id(8)
-
-        nonce_attr = MemberAttribute(MA.SWAP_ID_ATTR, nonce, member.member_id,
-                                     False)
-        session.add(nonce_attr)
-        ts = datetime.datetime.utcnow().replace(microsecond=0)
-        ts_attr = MemberAttribute(MA.SWAP_TS_ATTR, ts, member.member_id,
-                                  False)
-        session.add(ts_attr)
-        return self._successReturn(nonce)
-
     def swap_identities(self, client_cert, source_urn, dest_urn, credentials,
                         options, session):
         """Swaps two identities by swapping their ePPNs. Will
-        also swap the email addresses if they differ.
-
-        The source account is identified by an id which has previously
-        been established by a call to `create_swap_id`.
+        also swap the email addresses if they differ. Both identities
+        are identified by their URNs.
 
         Also adds an attribute to the destination account indicating that it
-        has already been swapped. Accounts can only be swapped once. The
-        value of this attribute is a timestamp to indicate when the swap
-        occurred.
+        has already been swapped.
         """
         # find the uid
         dest = self._get_first_attr(session, None, MA.MEMBER_URN, dest_urn)
diff --git a/tools/MA_constants.py b/tools/MA_constants.py
index 10a0288..5593b3c 100644
--- a/tools/MA_constants.py
+++ b/tools/MA_constants.py
@@ -195,7 +195,6 @@
 USER_CRED_LIFE_YEARS = 1  # See MA.get_user_credential
 # FIXME: username regex
 
-SWAP_ID_ATTR = 'gpo_swap_id'
 SWAP_TS_ATTR = 'gpo_swap_ts'
 SWAP_FROM_ATTR = 'gpo_swap_from'
 MEMBER_URN = 'MEMBER_URN'
diff --git a/tools/chapi_utils.py.in b/tools/chapi_utils.py.in
index 2a9a2e3..a8105cd 100644
--- a/tools/chapi_utils.py.in
+++ b/tools/chapi_utils.py.in
@@ -28,8 +28,6 @@ from email.mime.text import MIMEText
 from email.header import Header
 import os.path
 import datetime
-import string
-import random
 from chapi_log import *
 
 
@@ -122,11 +120,3 @@ def get_implementation_info(log_prefix):
         "code_release_date": str(code_timestamp),
         "site_update_date": str(code_timestamp)
     }
-
-
-def random_id(length=6):
-    chars = string.digits + string.uppercase
-    result = ''
-    for i in range(length):
-        result += chars[random.randint(0, 35)]
-    return result
diff --git a/tools/client.py b/tools/client.py
index a1f6c89..a00ea63 100644
--- a/tools/client.py
+++ b/tools/client.py
@@ -87,6 +87,8 @@ def parseOptions(args):
                       default=None)
     parser.add_option("--uuid3_arg", help="third UUID argument for some calls",
                       default=None)
+    parser.add_option("--urn2_arg", help="second URN argument for some calls",
+                      default=None)
     parser.add_option("--file_arg", help="FILE argument for some calls",
                       default=None)
     parser.add_option("--options", help="JSON of options argument",
@@ -422,11 +424,11 @@ def main(args=sys.argv, do_print=True):
                     member_eppn, project_id, slice_id)
 
     # MA Swap nonce method
-    elif opts.method in ['set_swap_nonce', 'swap_identities']:
+    elif opts.method in ['swap_identities']:
         options = {}
         (result, msg) = \
             _do_ssl(framework, suppress_errors, reason, fcn, opts.urn,
-                    opts.string_arg, opts.credentials, options)
+                    opts.urn2_arg, opts.credentials, options)
 
     # Methods that take attributes and options
     elif client_attributes:

From 76c27537237bbe95ee2003809d6b811e36773e9f Mon Sep 17 00:00:00 2001
From: Tom Mitchell <tcmitchell@users.noreply.github.com>
Date: Wed, 1 Mar 2017 07:16:54 -0500
Subject: [PATCH 11/11] Note addition of swap_identities in Changes file

Add support for swapping identities to support IdP changes.
---
 CHANGES.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/CHANGES.md b/CHANGES.md
index 9f4a99d..1f583b7 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -6,6 +6,8 @@
 
 * Fix authorization for get_requests_for_context
   ([#536](https://github.com/GENI-NSF/geni-ch/issues/536))
+* Add support for swapping identities to support IdP changes
+  ([#557](https://github.com/GENI-NSF/geni-ch/issues/557))
 
 ## Installation Notes