Remove Deprecated Config: Account Blacklist (prebid#3156)

account/account.go

@@ -4,7 +4,9 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+
 	"github.com/prebid/go-gdpr/consentconstants"
+
 	"github.com/prebid/prebid-server/config"
 	"github.com/prebid/prebid-server/errortypes"
 	"github.com/prebid/prebid-server/metrics"
@@ -15,17 +17,12 @@ import (
 
 // GetAccount looks up the config.Account object referenced by the given accountID, with access rules applied
 func GetAccount(ctx context.Context, cfg *config.Configuration, fetcher stored_requests.AccountFetcher, accountID string, me metrics.MetricsEngine) (account *config.Account, errs []error) {
-	// Check BlacklistedAcctMap until we have deprecated it
-	if _, found := cfg.BlacklistedAcctMap[accountID]; found {
-		return nil, []error{&errortypes.BlacklistedAcct{
-			Message: fmt.Sprintf("Prebid-server has disabled Account ID: %s, please reach out to the prebid server host.", accountID),
-		}}
-	}
 	if cfg.AccountRequired && accountID == metrics.PublisherUnknown {
 		return nil, []error{&errortypes.AcctRequired{
 			Message: fmt.Sprintf("Prebid-server has been configured to discard requests without a valid Account ID. Please reach out to the prebid server host."),
 		}}
 	}
+
 	if accountJSON, accErrs := fetcher.FetchAccount(ctx, cfg.AccountDefaultsJSON(), accountID); len(accErrs) > 0 || accountJSON == nil {
 		// accountID does not reference a valid account
 		for _, e := range accErrs {
@@ -79,7 +76,7 @@ func GetAccount(ctx context.Context, cfg *config.Configuration, fetcher stored_r
 		setDerivedConfig(account)
 	}
 	if account.Disabled {
-		errs = append(errs, &errortypes.BlacklistedAcct{
+		errs = append(errs, &errortypes.AccountDisabled{
 			Message: fmt.Sprintf("Prebid-server has disabled Account ID: %s, please reach out to the prebid server host.", accountID),
 		})
 		return nil, errs

account/account_test.go

@@ -48,19 +48,16 @@ func TestGetAccount(t *testing.T) {
 		// expected error, or nil if account should be found
 		err error
 	}{
-		// Blacklisted account is always rejected even in permissive setup
-		{accountID: "bad_acct", required: false, disabled: false, err: &errortypes.BlacklistedAcct{}},
-
 		// empty pubID
 		{accountID: unknown, required: false, disabled: false, err: nil},
 		{accountID: unknown, required: true, disabled: false, err: &errortypes.AcctRequired{}},
-		{accountID: unknown, required: false, disabled: true, err: &errortypes.BlacklistedAcct{}},
+		{accountID: unknown, required: false, disabled: true, err: &errortypes.AccountDisabled{}},
 		{accountID: unknown, required: true, disabled: true, err: &errortypes.AcctRequired{}},
 
 		// pubID given but is not a valid host account (does not exist)
 		{accountID: "doesnt_exist_acct", required: false, disabled: false, err: nil},
 		{accountID: "doesnt_exist_acct", required: true, disabled: false, err: nil},
-		{accountID: "doesnt_exist_acct", required: false, disabled: true, err: &errortypes.BlacklistedAcct{}},
+		{accountID: "doesnt_exist_acct", required: false, disabled: true, err: &errortypes.AccountDisabled{}},
 		{accountID: "doesnt_exist_acct", required: true, disabled: true, err: &errortypes.AcctRequired{}},
 
 		// pubID given and matches a valid host account with Disabled: false
@@ -72,10 +69,10 @@ func TestGetAccount(t *testing.T) {
 		{accountID: "invalid_acct_ipv6_ipv4", required: true, disabled: false, err: nil, checkDefaultIP: true},
 
 		// pubID given and matches a host account explicitly disabled (Disabled: true on account json)
-		{accountID: "disabled_acct", required: false, disabled: false, err: &errortypes.BlacklistedAcct{}},
-		{accountID: "disabled_acct", required: true, disabled: false, err: &errortypes.BlacklistedAcct{}},
-		{accountID: "disabled_acct", required: false, disabled: true, err: &errortypes.BlacklistedAcct{}},
-		{accountID: "disabled_acct", required: true, disabled: true, err: &errortypes.BlacklistedAcct{}},
+		{accountID: "disabled_acct", required: false, disabled: false, err: &errortypes.AccountDisabled{}},
+		{accountID: "disabled_acct", required: true, disabled: false, err: &errortypes.AccountDisabled{}},
+		{accountID: "disabled_acct", required: false, disabled: true, err: &errortypes.AccountDisabled{}},
+		{accountID: "disabled_acct", required: true, disabled: true, err: &errortypes.AccountDisabled{}},
 
 		// pubID given and matches a host account that has a malformed config
 		{accountID: "malformed_acct", required: false, disabled: false, err: &errortypes.MalformedAcct{}},
@@ -86,17 +83,16 @@ func TestGetAccount(t *testing.T) {
 		// account not provided (does not exist)
 		{accountID: "", required: false, disabled: false, err: nil},
 		{accountID: "", required: true, disabled: false, err: nil},
-		{accountID: "", required: false, disabled: true, err: &errortypes.BlacklistedAcct{}},
+		{accountID: "", required: false, disabled: true, err: &errortypes.AccountDisabled{}},
 		{accountID: "", required: true, disabled: true, err: &errortypes.AcctRequired{}},
 	}
 
 	for _, test := range testCases {
 		description := fmt.Sprintf(`ID=%s/required=%t/disabled=%t`, test.accountID, test.required, test.disabled)
 		t.Run(description, func(t *testing.T) {
 			cfg := &config.Configuration{
-				BlacklistedAcctMap: map[string]bool{"bad_acct": true},
-				AccountRequired:    test.required,
-				AccountDefaults:    config.Account{Disabled: test.disabled},
+				AccountRequired: test.required,
+				AccountDefaults: config.Account{Disabled: test.disabled},
 			}
 			fetcher := &mockAccountFetcher{}
 			assert.NoError(t, cfg.MarshalAccountDefaults())

config/config.go

@@ -67,9 +67,6 @@ type Configuration struct {
 	// Array of blacklisted apps that is used to create the hash table BlacklistedAppMap so App.ID's can be instantly accessed.
 	BlacklistedApps   []string `mapstructure:"blacklisted_apps,flow"`
 	BlacklistedAppMap map[string]bool
-	// Array of blacklisted accounts that is used to create the hash table BlacklistedAcctMap so Account.ID's can be instantly accessed.
-	BlacklistedAccts   []string `mapstructure:"blacklisted_accts,flow"`
-	BlacklistedAcctMap map[string]bool
 	// Is publisher/account ID required to be submitted in the OpenRTB2 request
 	AccountRequired bool `mapstructure:"account_required"`
 	// AccountDefaults defines default settings for valid accounts that are partially defined
@@ -757,13 +754,6 @@ func New(v *viper.Viper, bidderInfos BidderInfos, normalizeBidderName func(strin
 		c.BlacklistedAppMap[c.BlacklistedApps[i]] = true
 	}
 
-	// To look for a request's account id in O(1) time, we fill this hash table located in the
-	// the BlacklistedAccts field of the Configuration struct defined in this file
-	c.BlacklistedAcctMap = make(map[string]bool)
-	for i := 0; i < len(c.BlacklistedAccts); i++ {
-		c.BlacklistedAcctMap[c.BlacklistedAccts[i]] = true
-	}
-
 	// Migrate combo stored request config to separate stored_reqs and amp stored_reqs configs.
 	resolvedStoredRequestsConfig(&c)
 

endpoints/cookie_sync.go

@@ -367,7 +367,7 @@ func combineErrors(errs []error) error {
 	for _, err := range errs {
 		// preserve knowledge of special account errors
 		switch errortypes.ReadCode(err) {
-		case errortypes.BlacklistedAcctErrorCode:
+		case errortypes.AccountDisabledErrorCode:
 			return errCookieSyncAccountBlocked
 		case errortypes.AcctRequiredErrorCode:
 			return errCookieSyncAccountInvalid

endpoints/cookie_sync_test.go

@@ -1938,7 +1938,7 @@ func TestCombineErrors(t *testing.T) {
 		},
 		{
 			description:    "Special Case: blocked (rejected via block list)",
-			givenErrorList: []error{&errortypes.BlacklistedAcct{}},
+			givenErrorList: []error{&errortypes.AccountDisabled{}},
 			expectedError:  errCookieSyncAccountBlocked,
 		},
 		{
@@ -1953,7 +1953,7 @@ func TestCombineErrors(t *testing.T) {
 		},
 		{
 			description:    "Special Case: multiple special cases, first one wins",
-			givenErrorList: []error{&errortypes.BlacklistedAcct{}, &errortypes.AcctRequired{}, &errortypes.MalformedAcct{}},
+			givenErrorList: []error{&errortypes.AccountDisabled{}, &errortypes.AcctRequired{}, &errortypes.MalformedAcct{}},
 			expectedError:  errCookieSyncAccountBlocked,
 		},
 	}

endpoints/events/account_test.go

@@ -2,7 +2,6 @@ package events
 
 import (
 	"errors"
-	"fmt"
 	"io"
 	"net/http"
 	"net/http/httptest"
@@ -15,19 +14,18 @@ import (
 	"github.com/prebid/prebid-server/metrics"
 	"github.com/prebid/prebid-server/stored_requests"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestHandleAccountServiceErrors(t *testing.T) {
 	tests := map[string]struct {
-		fetcher   *mockAccountsFetcher
-		cfg       *config.Configuration
-		accountID string
-		want      struct {
-			code     int
-			response string
-		}
+		fetcher      *mockAccountsFetcher
+		cfg          *config.Configuration
+		accountID    string
+		wantCode     int
+		wantResponse string
 	}{
-		"badRequest": {
+		"bad-request": {
 			fetcher: &mockAccountsFetcher{
 				Fail:  true,
 				Error: errors.New("some error"),
@@ -40,16 +38,11 @@ func TestHandleAccountServiceErrors(t *testing.T) {
 					TimeoutMS: int64(2000), AllowUnknownBidder: false,
 				},
 			},
-			accountID: "testacc",
-			want: struct {
-				code     int
-				response string
-			}{
-				code:     400,
-				response: "Invalid request: some error\nInvalid request: Prebid-server could not verify the Account ID. Please reach out to the prebid server host.\n",
-			},
+			accountID:    "testacc",
+			wantCode:     400,
+			wantResponse: "Invalid request: some error\nInvalid request: Prebid-server could not verify the Account ID. Please reach out to the prebid server host.\n",
 		},
-		"malformedAccountConfig": {
+		"malformed-account-config": {
 			fetcher: &mockAccountsFetcher{
 				Fail:  true,
 				Error: &errortypes.MalformedAcct{},
@@ -60,34 +53,25 @@ func TestHandleAccountServiceErrors(t *testing.T) {
 					TimeoutMS: int64(2000), AllowUnknownBidder: false,
 				},
 			},
-			accountID: "malformed_acct",
-			want: struct {
-				code     int
-				response string
-			}{
-				code:     500,
-				response: "Invalid request: The prebid-server account config for account id \"malformed_acct\" is malformed. Please reach out to the prebid server host.\n",
-			},
+			accountID:    "malformed_acct",
+			wantCode:     500,
+			wantResponse: "Invalid request: The prebid-server account config for account id \"malformed_acct\" is malformed. Please reach out to the prebid server host.\n",
 		},
-		"serviceUnavailable": {
+		"service-unavailable": {
 			fetcher: &mockAccountsFetcher{
 				Fail: false,
 			},
 			cfg: &config.Configuration{
-				BlacklistedAcctMap: map[string]bool{"testacc": true},
-				MaxRequestSize:     maxSize,
+				AccountDefaults: config.Account{},
+				AccountRequired: true,
+				MaxRequestSize:  maxSize,
 				VTrack: config.VTrack{
 					TimeoutMS: int64(2000), AllowUnknownBidder: false,
 				},
 			},
-			accountID: "testacc",
-			want: struct {
-				code     int
-				response string
-			}{
-				code:     503,
-				response: "Invalid request: Prebid-server has disabled Account ID: testacc, please reach out to the prebid server host.\n",
-			},
+			accountID:    "disabled_acct",
+			wantCode:     503,
+			wantResponse: "Invalid request: Prebid-server has disabled Account ID: disabled_acct, please reach out to the prebid server host.\n",
 		},
 		"timeout": {
 			fetcher: &mockAccountsFetcher{
@@ -106,19 +90,13 @@ func TestHandleAccountServiceErrors(t *testing.T) {
 					AllowUnknownBidder: false,
 				},
 			},
-			accountID: "testacc",
-			want: struct {
-				code     int
-				response string
-			}{
-				code:     504,
-				response: "Invalid request: context deadline exceeded\nInvalid request: Prebid-server could not verify the Account ID. Please reach out to the prebid server host.\n",
-			},
+			accountID:    "testacc",
+			wantCode:     504,
+			wantResponse: "Invalid request: context deadline exceeded\nInvalid request: Prebid-server could not verify the Account ID. Please reach out to the prebid server host.\n",
 		},
 	}
 
 	for name, test := range tests {
-
 		handlers := []struct {
 			name string
 			h    httprouter.Handle
@@ -137,13 +115,11 @@ func TestHandleAccountServiceErrors(t *testing.T) {
 				// execute
 				handler.h(recorder, handler.r, nil)
 				d, err := io.ReadAll(recorder.Result().Body)
-				if err != nil {
-					t.Fatal(err)
-				}
+				require.NoError(t, err)
 
 				// validate
-				assert.Equal(t, test.want.code, recorder.Result().StatusCode, fmt.Sprintf("Expected %d", test.want.code))
-				assert.Equal(t, test.want.response, string(d))
+				assert.Equal(t, test.wantCode, recorder.Result().StatusCode)
+				assert.Equal(t, test.wantResponse, string(d))
 			})
 		}
 	}

endpoints/events/event.go

@@ -206,7 +206,7 @@ func HandleAccountServiceErrors(errs []error) (status int, messages []string) {
 
 		errCode := errortypes.ReadCode(er)
 
-		if errCode == errortypes.BlacklistedAppErrorCode || errCode == errortypes.BlacklistedAcctErrorCode {
+		if errCode == errortypes.BlacklistedAppErrorCode || errCode == errortypes.AccountDisabledErrorCode {
 			status = http.StatusServiceUnavailable
 		}
 		if errCode == errortypes.MalformedAcctErrorCode {

endpoints/events/event_test.go

@@ -20,7 +20,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-// Mock Analytics Module
 type eventsMockAnalyticsModule struct {
 	Fail    bool
 	Error   error
@@ -31,51 +30,44 @@ func (e *eventsMockAnalyticsModule) LogAuctionObject(ao *analytics.AuctionObject
 	if e.Fail {
 		panic(e.Error)
 	}
-	return
 }
 
 func (e *eventsMockAnalyticsModule) LogVideoObject(vo *analytics.VideoObject) {
 	if e.Fail {
 		panic(e.Error)
 	}
-	return
 }
 
 func (e *eventsMockAnalyticsModule) LogCookieSyncObject(cso *analytics.CookieSyncObject) {
 	if e.Fail {
 		panic(e.Error)
 	}
-	return
 }
 
 func (e *eventsMockAnalyticsModule) LogSetUIDObject(so *analytics.SetUIDObject) {
 	if e.Fail {
 		panic(e.Error)
 	}
-	return
 }
 
 func (e *eventsMockAnalyticsModule) LogAmpObject(ao *analytics.AmpObject) {
 	if e.Fail {
 		panic(e.Error)
 	}
-	return
 }
 
 func (e *eventsMockAnalyticsModule) LogNotificationEventObject(ne *analytics.NotificationEvent) {
 	if e.Fail {
 		panic(e.Error)
 	}
 	e.Invoked = true
-
-	return
 }
 
-// Mock Account fetcher
 var mockAccountData = map[string]json.RawMessage{
 	"events_enabled":  json.RawMessage(`{"events": {"enabled":true}}`),
 	"events_disabled": json.RawMessage(`{"events": {"enabled":false}}`),
 	"malformed_acct":  json.RawMessage(`{"events": {"enabled":"invalid type"}}`),
+	"disabled_acct":   json.RawMessage(`{"disabled": true}`),
 }
 
 type mockAccountsFetcher struct {
@@ -102,7 +94,7 @@ func (maf mockAccountsFetcher) FetchAccount(ctx context.Context, defaultAccountJ
 		return nil, []error{maf.Error}
 	}
 
-	return nil, []error{stored_requests.NotFoundError{accountID, "Account"}}
+	return nil, []error{stored_requests.NotFoundError{ID: accountID, DataType: "Account"}}
 }
 
 // Tests

endpoints/openrtb2/amp_auction.go

@@ -77,7 +77,7 @@ func NewAmpEndpoint(
 		return nil, errors.New("NewAmpEndpoint requires non-nil arguments.")
 	}
 
-	defRequest := defReqJSON != nil && len(defReqJSON) > 0
+	defRequest := len(defReqJSON) > 0
 
 	ipValidator := iputil.PublicNetworkIPValidator{
 		IPv4PrivateNetworks: cfg.RequestValidation.IPv4PrivateNetworksParsed,
@@ -208,7 +208,7 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h
 		metricsStatus := metrics.RequestStatusBadInput
 		for _, er := range errL {
 			errCode := errortypes.ReadCode(er)
-			if errCode == errortypes.BlacklistedAppErrorCode || errCode == errortypes.BlacklistedAcctErrorCode {
+			if errCode == errortypes.BlacklistedAppErrorCode || errCode == errortypes.AccountDisabledErrorCode {
 				httpStatus = http.StatusServiceUnavailable
 				metricsStatus = metrics.RequestStatusBlacklisted
 				break