Maximum one bound identity per person for salsa

signon/middleware.py

@@ -66,16 +66,32 @@ class SignonMiddleware:
 
         if getattr(settings, "SIGNON_AUTO_BIND", False) and request.user.is_authenticated:
             from backend.models import Person
+            from signon.models import Identity
             bound = [x.issuer for x in request.signon_identities.values() if x.person is not None]
             if bound:
-                for identity in request.signon_identities.values():
+                for identity in list(request.signon_identities.values()):
+                    provider = identity.get_provider()
                     if identity.person is None:
-                        log.info("%s: auto associated to %s", request.user, identity)
-                        identity.person = request.user
-                        identity.save(
-                            audit_author=Person.objects.get_housekeeper(),
-                            audit_notes=f"Auto associated while logged in with {', '.join(bound)}",
-                        )
+                        auto_associate = True
+
+                        if provider.single_bind:
+                            old = Identity.objects.filter(issuer=identity.issuer, person=request.user).first()
+                            if old is not None:
+                                log.info("%s: skipping association to %s because %s is already associated",
+                                         request.user, identity, old)
+                                auto_associate = False
+
+                        if auto_associate:
+                            log.info("%s: auto associated to %s", request.user, identity)
+                            identity.person = request.user
+                            identity.save(
+                                audit_author=Person.objects.get_housekeeper(),
+                                audit_notes=f"Auto associated while logged in with {', '.join(bound)}",
+                            )
+                        else:
+                            log.info("%s: logging out spurious identity %s", request.user, identity)
+                            provider = provider.bind(request)
+                            provider.logout(identity)
 
         return self.get_response(request)
 

signon/providers.py

@@ -53,6 +53,10 @@ class BoundProvider:
     def get_active_identity(self):
         return self.request.signon_identities.get(self.provider.name)
 
+    def logout(self, identity):
+        self.request.signon_identities.pop(self.provider.name, None)
+        self.request.session.pop(f"signon_identity_{self.provider.name}", None)
+
 
 class Provider:
     """
@@ -61,10 +65,11 @@ class Provider:
     # Class used to create a request-bound version
     bound_class: Type["BoundProvider"] = BoundProvider
 
-    def __init__(self, name: str, label: str, icon: Optional[str] = None):
+    def __init__(self, name: str, label: str, icon: Optional[str] = None, single_bind: bool = False):
         self.name = name
         self.label = label
         self.icon = icon
+        self.single_bind = single_bind
 
     def bind(self, request: django.http.HttpRequest) -> "BoundProvider":
         return self.bound_class(self, request)
@@ -158,8 +163,9 @@ class OIDCProvider(Provider):
             url_userinfo: str,
             url_jwks: str,
             scope: Union[str, Sequence[str]],
-            icon: Optional[str] = None):
-        super().__init__(name=name, label=label, icon=icon)
+            icon: Optional[str] = None,
+            single_bind: bool = False):
+        super().__init__(name=name, label=label, icon=icon, single_bind=single_bind)
         self.client_id = client_id
         self.client_secret = client_secret
         self.url_issuer = url_issuer
@@ -200,9 +206,10 @@ class GitlabProvider(OIDCProvider):
             client_secret: str,
             url: str,
             scope: Optional[Union[str, Sequence[str]]] = None,
-            icon: Optional[str] = None):
+            icon: Optional[str] = None,
+            single_bind: bool = False):
         super().__init__(
-                name=name, label=label, icon=icon,
+                name=name, label=label, icon=icon, single_bind=single_bind,
                 client_id=client_id, client_secret=client_secret,
                 scope=scope if scope is not None else "openid",
                 url_issuer=url,
@@ -210,6 +217,7 @@ class GitlabProvider(OIDCProvider):
                 url_token=f"{url}/oauth/token",
                 url_userinfo=f"{url}/oauth/userinfo",
                 url_jwks=f"{url}/oauth/discovery/keys")
+        self.single_bind = True
 
 
 class BoundDebssoProvider(BoundProvider):
@@ -229,8 +237,9 @@ class DebssoProvider(Provider):
     def __init__(
             self, name: str, label: str,
             icon: Optional[str] = None,
+            single_bind: bool = False,
             env_name: Optional[str] = None):
-        super().__init__(name=name, label=label, icon=icon)
+        super().__init__(name=name, label=label, icon=icon, single_bind=single_bind)
         self.env_name = env_name if env_name is not None else "SSL_CLIENT_S_DN_CN"
 
     def _get_remote_user(self, request: django.http.HttpRequest) -> Optional[str]:

signon/tests/test_authentication.py

@@ -9,7 +9,7 @@ from signon import providers
 
 @override_settings(SIGNON_PROVIDERS=[
     providers.DebssoProvider(name="debsso", label="sso.debian.org"),
-    providers.Provider(name="salsa", label="Salsa"),
+    providers.Provider(name="salsa", label="Salsa", single_bind=True),
 ])
 class TestAuthentication(BaseFixtureMixin, TestCase):
     @classmethod
@@ -239,3 +239,48 @@ class TestAuthentication(BaseFixtureMixin, TestCase):
 
         self.identities.salsa.refresh_from_db()
         self.assertEqual(self.identities.salsa.person, self.persons.dd)
+
+    @override_settings(SIGNON_AUTO_BIND=True)
+    def test_multiple_salsa_accounts(self):
+        # One bound debsso account
+        self.identities.create(
+                "debsso", person=self.persons.dd, issuer="debsso", subject="dd@debian.org", audit_skip=True)
+        # One bound salsa account
+        self.identities.create(
+                "salsa1", person=self.persons.dd, issuer="salsa", subject="1", audit_skip=True)
+        # One unbound salsa account
+        self.identities.create(
+                "salsa2", issuer="salsa", subject="2", audit_skip=True)
+
+        def _instantiate_identities(_self, request):
+            request.signon_identities = {
+                "debsso": self.identities.debsso,
+                "salsa": self.identities.salsa2,
+            }
+
+        with mock.patch("signon.middleware.SignonMiddleware._instantiate_identities", _instantiate_identities):
+            client = self.make_test_client(None)
+            with self.assertLogs() as log:
+                response = client.get(reverse('signon_login'))
+
+        self.assertEqual(log.output, [
+            "INFO:signon.middleware:Dd <dd@example.org>: skipping association to -:salsa:2 because dd:salsa:1 is already associated",
+            'INFO:signon.middleware:Dd <dd@example.org>: logging out spurious identity -:salsa:2',
+        ])
+
+        self.assertEqual(response.status_code, 200)
+
+        request = response.context["request"]
+        self.assertEqual(request.signon_identities, {
+            "debsso": self.identities.debsso,
+        })
+        self.assertEqual(request.user, self.persons.dd)
+
+        self.identities.debsso.refresh_from_db()
+        self.assertEqual(self.identities.debsso.person, self.persons.dd)
+
+        self.identities.salsa1.refresh_from_db()
+        self.assertEqual(self.identities.salsa1.person, self.persons.dd)
+
+        self.identities.salsa2.refresh_from_db()
+        self.assertIsNone(self.identities.salsa2.person)