Deduplicate user authenticated clients during logout.

ory · Aug 17, 2019 · 0a1aa2e · 0a1aa2e
1 parent 17aa7b9
commit 0a1aa2e
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 2 deletions.
diff --git a/consent/manager_memory.go b/consent/manager_memory.go
@@ -477,10 +477,13 @@ func (m *MemoryManager) ListUserAuthenticatedClientsWithBackChannelLogout(ctx co
 	m.m["consentRequests"].RLock()
 	defer m.m["consentRequests"].RUnlock()
 
+	clientsMap := make(map[string]bool)
+
 	var rs []client.Client
 	for _, cr := range m.consentRequests {
-		if cr.Subject == subject && len(cr.Client.BackChannelLogoutURI) > 0 {
+		if cr.Subject == subject && len(cr.Client.BackChannelLogoutURI) > 0 && !clientsMap[cr.Client.GetID()] {
 			rs = append(rs, *cr.Client)
+			clientsMap[cr.Client.GetID()] = true
 		}
 	}
 

diff --git a/consent/manager_sql.go b/consent/manager_sql.go
@@ -528,7 +528,7 @@ func (m *SQLManager) ListUserAuthenticatedClientsWithBackChannelLogout(ctx conte
 
 func (m *SQLManager) listUserAuthenticatedClients(ctx context.Context, subject string, channel string) ([]client.Client, error) {
 	var ids []string
-	if err := m.DB.SelectContext(ctx, &ids, m.DB.Rebind(fmt.Sprintf(`SELECT c.id FROM hydra_client as c JOIN hydra_oauth2_consent_request as r ON (c.id = r.client_id) WHERE r.subject=? AND c.%schannel_logout_uri!='' and c.%schannel_logout_uri IS NOT NULL`, channel, channel)), subject); err != nil {
+	if err := m.DB.SelectContext(ctx, &ids, m.DB.Rebind(fmt.Sprintf(`SELECT DISTINCT(c.id) FROM hydra_client as c JOIN hydra_oauth2_consent_request as r ON (c.id = r.client_id) WHERE r.subject=? AND c.%schannel_logout_uri!='' and c.%schannel_logout_uri IS NOT NULL`, channel, channel)), subject); err != nil {
 		if err == sql.ErrNoRows {
 			return nil, errors.WithStack(x.ErrNotFound)
 		}

diff --git a/consent/manager_test_helpers.go b/consent/manager_test_helpers.go
@@ -598,6 +598,39 @@ func ManagerTests(m Manager, clientManager client.Manager, fositeManager x.Fosit
 				assert.EqualValues(t, "fk-client-LUACWFCL-5", clients[0].ClientID)
 			})
 
+			t.Run("case=ListUserAuthenticatedClientsWithFrontChannelLogout-duplicateLogin", func(t *testing.T) {
+				c, h := MockConsentRequest("LUACWFCL-DUPLICCATE", false, 0, false, false, false)
+				c.Client.BackChannelLogoutURI = "http://some-url.com/"
+
+				c.LoginSessionID = ""                                // otherwise we had to create the login session as well..
+				c.Subject = "subjectLUACWFCL-DUPLICCATE"             // otherwise we had to create the login session as well..
+				clientManager.CreateClient(context.TODO(), c.Client) // Ignore errors that are caused by duplication
+
+				for i := 0; i < 2; i++ {
+					lc, _ := MockAuthRequest("LUACWFCL-DUPLICCATE", true)
+					lc.Challenge = fmt.Sprintf("fk-login-challenge-LUACWFCL-DUPLICCATE-%d", i)
+					lc.Verifier = fmt.Sprintf("fk-login-verifier-LUACWFCL-DUPLICCATE-%d", i)
+					lc.SessionID = ""
+					require.NoError(t, m.CreateLoginRequest(context.TODO(), lc))
+
+					c.Challenge = fmt.Sprintf("challenge-LUACWFCL-DUPLICCATE-%d", i)
+					c.LoginChallenge = fmt.Sprintf("fk-login-challenge-LUACWFCL-DUPLICCATE-%d", i)
+					c.Verifier = fmt.Sprintf("verifiers-LUACWFCL-DUPLICCATE-%d", i)
+					require.NoError(t, m.CreateConsentRequest(context.TODO(), c))
+
+					h.Challenge = fmt.Sprintf("challenge-LUACWFCL-DUPLICCATE-%d", i)
+					_, err = m.HandleConsentRequest(context.TODO(), c.Challenge, h)
+					require.NoError(t, err)
+				}
+
+				clients, err := m.ListUserAuthenticatedClientsWithBackChannelLogout(context.TODO(), "subjectLUACWFCL-DUPLICCATE")
+				require.NoError(t, err)
+
+				require.Len(t, clients, 1)
+				assert.EqualValues(t, "http://some-url.com/", clients[0].BackChannelLogoutURI)
+				assert.EqualValues(t, "fk-client-LUACWFCL-DUPLICCATE", clients[0].ClientID)
+			})
+
 			t.Run("case=ListUserAuthenticatedClientsWithBackChannelLogout", func(t *testing.T) {
 				for i := 0; i <= 10; i++ {
 					c, h := MockConsentRequest(fmt.Sprintf("LUACWFBL-%d", i), false, 0, false, false, false)
@@ -626,6 +659,39 @@ func ManagerTests(m Manager, clientManager client.Manager, fositeManager x.Fosit
 				assert.EqualValues(t, "fk-client-LUACWFBL-5", clients[0].ClientID)
 			})
 
+			t.Run("case=ListUserAuthenticatedClientsWithBackChannelLogout-duplicateLogin", func(t *testing.T) {
+				c, h := MockConsentRequest("LUACWFBL-DUPLICCATE", false, 0, false, false, false)
+				c.Client.BackChannelLogoutURI = "http://some-url.com/"
+
+				c.LoginSessionID = ""                                // otherwise we had to create the login session as well..
+				c.Subject = "subjectLUACWFBL-DUPLICCATE"             // otherwise we had to create the login session as well..
+				clientManager.CreateClient(context.TODO(), c.Client) // Ignore errors that are caused by duplication
+
+				for i := 0; i < 2; i++ {
+					lc, _ := MockAuthRequest("LUACWFBL-DUPLICCATE", true)
+					lc.Challenge = fmt.Sprintf("fk-login-challenge-LUACWFBL-DUPLICCATE-%d", i)
+					lc.Verifier = fmt.Sprintf("fk-login-verifier-LUACWFBL-DUPLICCATE-%d", i)
+					lc.SessionID = ""
+					require.NoError(t, m.CreateLoginRequest(context.TODO(), lc))
+
+					c.Challenge = fmt.Sprintf("challenge-LUACWFBL-DUPLICCATE-%d", i)
+					c.LoginChallenge = fmt.Sprintf("fk-login-challenge-LUACWFBL-DUPLICCATE-%d", i)
+					c.Verifier = fmt.Sprintf("verifiers-LUACWFBL-DUPLICCATE-%d", i)
+					require.NoError(t, m.CreateConsentRequest(context.TODO(), c))
+
+					h.Challenge = fmt.Sprintf("challenge-LUACWFBL-DUPLICCATE-%d", i)
+					_, err = m.HandleConsentRequest(context.TODO(), c.Challenge, h)
+					require.NoError(t, err)
+				}
+
+				clients, err := m.ListUserAuthenticatedClientsWithBackChannelLogout(context.TODO(), "subjectLUACWFBL-DUPLICCATE")
+				require.NoError(t, err)
+
+				require.Len(t, clients, 1)
+				assert.EqualValues(t, "http://some-url.com/", clients[0].BackChannelLogoutURI)
+				assert.EqualValues(t, "fk-client-LUACWFBL-DUPLICCATE", clients[0].ClientID)
+			})
+
 			t.Run("case=LogoutRequest", func(t *testing.T) {
 				for k, tc := range []struct {
 					key        string