Properly handle expired decisions with different scopes (#1616)

pkg/apiserver/decisions_test.go

@@ -809,6 +809,56 @@ func TestStreamDecisionStart(t *testing.T) {
 				},
 			},
 		},
+		{
+			TestName:      "delete decisions 8 (127.0.0.2) with captcha",
+			Method:        "DELETE",
+			Route:         "/v1/decisions/8",
+			CheckCodeOnly: true,
+			Code:          200,
+			LenNew:        0,
+			LenDeleted:    0,
+			AuthType:      PASSWORD,
+			DelChecks:     []DecisionCheck{},
+			NewChecks:     []DecisionCheck{},
+		},
+		{
+			TestName:      "127.0.0.2 with captcha should be in deleted now",
+			Method:        "GET",
+			Route:         "/v1/decisions/stream?startup=true",
+			CheckCodeOnly: false,
+			Code:          200,
+			LenNew:        1,
+			LenDeleted:    2,
+			AuthType:      APIKEY,
+			DelChecks: []DecisionCheck{
+				{
+					ID:       int64(1),
+					Origin:   "test",
+					Scenario: "crowdsecurity/test",
+					Value:    "127.0.0.1",
+					Duration: "-", // we check that the time is negative
+					Type:     "ban",
+				},
+				{
+					ID:       int64(8),
+					Origin:   "test",
+					Scenario: "crowdsecurity/test",
+					Value:    "127.0.0.2",
+					Duration: "-",
+					Type:     "captcha",
+				},
+			},
+			NewChecks: []DecisionCheck{
+				{
+					ID:       int64(4),
+					Origin:   "test",
+					Scenario: "crowdsecurity/test",
+					Value:    "127.0.0.2",
+					Duration: "2h59",
+					Type:     "ban",
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {

pkg/database/decisions.go

@@ -207,12 +207,16 @@ func (c *Client) QueryExpiredDecisionsWithFilters(filters map[string][]string) (
 		return []*ent.Decision{}, errors.Wrap(QueryFail, "get expired decisions with filters")
 	}
 	query = query.Where(func(s *sql.Selector) {
-		t := sql.Table(decision.Table)
+		t := sql.Table(decision.Table).As("t1")
 
 		subQuery := sql.Select(t.C(decision.FieldValue)).From(t).Where(sql.GT(t.C(decision.FieldUntil), now))
 		for _, predicate := range predicates {
 			subQuery.Where(predicate)
 		}
+		subQuery.Where(sql.And(
+			sql.ColumnsEQ(t.C(decision.FieldType), s.C(decision.FieldType)),
+			sql.ColumnsEQ(t.C(decision.FieldScope), s.C(decision.FieldScope)),
+		))
 		s.Where(
 			sql.NotIn(
 				s.C(decision.FieldValue),
@@ -230,41 +234,45 @@ func (c *Client) QueryExpiredDecisionsWithFilters(filters map[string][]string) (
 	return data, nil
 }
 
+//The "dedup" is not performed in SQL here because we suck at it, we do it in Go:
+// - Get all decisions (expired or not) with an end time after the last pull from the bouncer
+// - Sort them by increasing expiration date
+// - Iterate over them, keeping only decisions that have expired but only if we don't have an active decision with the same scope/value/type
 func (c *Client) QueryExpiredDecisionsSinceWithFilters(since time.Time, filters map[string][]string) ([]*ent.Decision, error) {
 	now := time.Now().UTC()
 
 	query := c.Ent.Decision.Query().Where(
-		decision.UntilLT(now),
 		decision.UntilGT(since),
 	)
-	query, predicates, err := BuildDecisionRequestWithFilter(query, filters)
+	query, _, err := BuildDecisionRequestWithFilter(query, filters)
 	if err != nil {
 		c.Log.Warningf("QueryExpiredDecisionsSinceWithFilters : %s", err)
 		return []*ent.Decision{}, errors.Wrap(QueryFail, "expired decisions with filters")
 	}
 
-	query = query.Where(func(s *sql.Selector) {
-		t := sql.Table(decision.Table)
+	data, err := query.Order(ent.Asc(decision.FieldValue), ent.Asc(decision.FieldUntil)).All(c.CTX)
+	if err != nil {
+		c.Log.Warningf("QueryExpiredDecisionsSinceWithFilters : %s", err)
+		return []*ent.Decision{}, errors.Wrap(QueryFail, "expired decisions with filters")
+	}
 
-		subQuery := sql.Select(t.C(decision.FieldValue)).From(t).Where(sql.GT(t.C(decision.FieldUntil), now))
-		for _, predicate := range predicates {
-			subQuery.Where(predicate)
+	ret := make([]*ent.Decision, 0)
+	deletedDecisions := make(map[string]*ent.Decision)
+	for _, d := range data {
+		key := fmt.Sprintf("%s:%s:%s", d.Scope, d.Type, d.Value)
+		if d.Until.Before(now) {
+			deletedDecisions[key] = d
+		}
+		if d.Until.After(now) {
+			delete(deletedDecisions, key)
 		}
-		s.Where(
-			sql.NotIn(
-				s.C(decision.FieldValue),
-				subQuery,
-			),
-		)
-	})
-
-	data, err := query.Order(ent.Asc(decision.FieldValue), ent.Desc(decision.FieldUntil)).All(c.CTX)
-	if err != nil {
-		c.Log.Warningf("QueryExpiredDecisionsSinceWithFilters : %s", err)
-		return []*ent.Decision{}, errors.Wrap(QueryFail, "expired decisions with filters")
 	}
 
-	return data, nil
+	for _, d := range deletedDecisions {
+		ret = append(ret, d)
+	}
+
+	return ret, nil
 }
 
 func (c *Client) QueryNewDecisionsSinceWithFilters(since time.Time, filters map[string][]string) ([]*ent.Decision, error) {