diff --git a/account/account.go b/account/account.go index 7b5f9435a11..70527d0f6ee 100644 --- a/account/account.go +++ b/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 diff --git a/account/account_test.go b/account/account_test.go index d5223f76119..da0457f8d47 100644 --- a/account/account_test.go +++ b/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,7 +83,7 @@ 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{}}, } @@ -94,9 +91,8 @@ func TestGetAccount(t *testing.T) { 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()) diff --git a/config/config.go b/config/config.go index a8d276a15c2..cf3591dd564 100644 --- a/config/config.go +++ b/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) diff --git a/endpoints/cookie_sync.go b/endpoints/cookie_sync.go index 250d5fd07d3..bbee3847014 100644 --- a/endpoints/cookie_sync.go +++ b/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 diff --git a/endpoints/cookie_sync_test.go b/endpoints/cookie_sync_test.go index 29fe74b273c..80de2b5ff50 100644 --- a/endpoints/cookie_sync_test.go +++ b/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, }, } diff --git a/endpoints/events/account_test.go b/endpoints/events/account_test.go index 7efb8af782b..8477b43b49b 100644 --- a/endpoints/events/account_test.go +++ b/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)) }) } } diff --git a/endpoints/events/event.go b/endpoints/events/event.go index e8084f1b273..7f923eda09d 100644 --- a/endpoints/events/event.go +++ b/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 { diff --git a/endpoints/events/event_test.go b/endpoints/events/event_test.go index 95711ba1328..7f129634221 100644 --- a/endpoints/events/event_test.go +++ b/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,35 +30,30 @@ 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) { @@ -67,15 +61,13 @@ func (e *eventsMockAnalyticsModule) LogNotificationEventObject(ne *analytics.Not 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 diff --git a/endpoints/openrtb2/amp_auction.go b/endpoints/openrtb2/amp_auction.go index 2879610d8e4..aa80d0756c3 100644 --- a/endpoints/openrtb2/amp_auction.go +++ b/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 diff --git a/endpoints/openrtb2/amp_auction_test.go b/endpoints/openrtb2/amp_auction_test.go index 87033f71b64..89d9c472925 100644 --- a/endpoints/openrtb2/amp_auction_test.go +++ b/endpoints/openrtb2/amp_auction_test.go @@ -98,8 +98,6 @@ func TestGoodAmpRequests(t *testing.T) { if test.Config != nil { cfg.BlacklistedApps = test.Config.BlacklistedApps cfg.BlacklistedAppMap = test.Config.getBlacklistedAppMap() - cfg.BlacklistedAccts = test.Config.BlacklistedAccounts - cfg.BlacklistedAcctMap = test.Config.getBlackListedAccountMap() cfg.AccountRequired = test.Config.AccountRequired } @@ -121,7 +119,7 @@ func TestGoodAmpRequests(t *testing.T) { // Assertions if assert.Equal(t, test.ExpectedReturnCode, recorder.Code, "Expected status %d. Got %d. Amp test file: %s", http.StatusOK, recorder.Code, filename) { if test.ExpectedReturnCode == http.StatusOK { - assert.JSONEq(t, string(test.ExpectedAmpResponse), string(recorder.Body.Bytes()), "Not the expected response. Test file: %s", filename) + assert.JSONEq(t, string(test.ExpectedAmpResponse), recorder.Body.String(), "Not the expected response. Test file: %s", filename) } else { assert.Equal(t, test.ExpectedErrorMessage, recorder.Body.String(), filename) } @@ -148,11 +146,6 @@ func TestAccountErrors(t *testing.T) { storedReqID: "1", filename: "account-malformed/malformed-acct.json", }, - { - description: "Blocked account", - storedReqID: "1", - filename: "blacklisted/blacklisted-site-publisher.json", - }, } for _, tt := range tests { @@ -169,9 +162,7 @@ func TestAccountErrors(t *testing.T) { test.endpointType = AMP_ENDPOINT cfg := &config.Configuration{ - BlacklistedAccts: []string{"bad_acct"}, - BlacklistedAcctMap: map[string]bool{"bad_acct": true}, - MaxRequestSize: maxSize, + MaxRequestSize: maxSize, } cfg.MarshalAccountDefaults() @@ -2328,7 +2319,7 @@ func TestSendAmpResponse_LogsErrors(t *testing.T) { account := &config.Account{DebugAllow: true} reqWrapper := openrtb_ext.RequestWrapper{BidRequest: test.request} - labels, ao = sendAmpResponse(test.writer, test.hookExecutor, &exchange.AuctionResponse{BidResponse: test.response}, &reqWrapper, account, labels, ao, nil) + _, ao = sendAmpResponse(test.writer, test.hookExecutor, &exchange.AuctionResponse{BidResponse: test.response}, &reqWrapper, account, labels, ao, nil) assert.Equal(t, ao.Errors, test.expectedErrors, "Invalid errors.") assert.Equal(t, test.expectedStatus, ao.Status, "Invalid HTTP response status.") diff --git a/endpoints/openrtb2/auction.go b/endpoints/openrtb2/auction.go index 810741982ef..b625c0ff54c 100644 --- a/endpoints/openrtb2/auction.go +++ b/endpoints/openrtb2/auction.go @@ -101,7 +101,7 @@ func NewEndpoint( return nil, errors.New("NewEndpoint requires non-nil arguments.") } - defRequest := defReqJSON != nil && len(defReqJSON) > 0 + defRequest := len(defReqJSON) > 0 ipValidator := iputil.PublicNetworkIPValidator{ IPv4PrivateNetworks: cfg.RequestValidation.IPv4PrivateNetworksParsed, @@ -2349,7 +2349,7 @@ func writeError(errs []error, w http.ResponseWriter, labels *metrics.Labels) boo metricsStatus := metrics.RequestStatusBadInput for _, err := range errs { erVal := errortypes.ReadCode(err) - if erVal == errortypes.BlacklistedAppErrorCode || erVal == errortypes.BlacklistedAcctErrorCode { + if erVal == errortypes.BlacklistedAppErrorCode || erVal == errortypes.AccountDisabledErrorCode { httpStatus = http.StatusServiceUnavailable metricsStatus = metrics.RequestStatusBlacklisted break diff --git a/endpoints/openrtb2/auction_benchmark_test.go b/endpoints/openrtb2/auction_benchmark_test.go index a3c19ee0b10..ee74548ea47 100644 --- a/endpoints/openrtb2/auction_benchmark_test.go +++ b/endpoints/openrtb2/auction_benchmark_test.go @@ -147,12 +147,10 @@ func BenchmarkValidWholeExemplary(b *testing.B) { test.endpointType = OPENRTB_ENDPOINT cfg := &config.Configuration{ - MaxRequestSize: maxSize, - BlacklistedApps: test.Config.BlacklistedApps, - BlacklistedAppMap: test.Config.getBlacklistedAppMap(), - BlacklistedAccts: test.Config.BlacklistedAccounts, - BlacklistedAcctMap: test.Config.getBlackListedAccountMap(), - AccountRequired: test.Config.AccountRequired, + MaxRequestSize: maxSize, + BlacklistedApps: test.Config.BlacklistedApps, + BlacklistedAppMap: test.Config.getBlacklistedAppMap(), + AccountRequired: test.Config.AccountRequired, } auctionEndpointHandler, _, mockBidServers, mockCurrencyRatesServer, err := buildTestEndpoint(test, cfg) diff --git a/endpoints/openrtb2/auction_test.go b/endpoints/openrtb2/auction_test.go index c071d6b0950..b5631c93d3d 100644 --- a/endpoints/openrtb2/auction_test.go +++ b/endpoints/openrtb2/auction_test.go @@ -160,8 +160,6 @@ func runJsonBasedTest(t *testing.T, filename, desc string) { if test.Config != nil { cfg.BlacklistedApps = test.Config.BlacklistedApps cfg.BlacklistedAppMap = test.Config.getBlacklistedAppMap() - cfg.BlacklistedAccts = test.Config.BlacklistedAccounts - cfg.BlacklistedAcctMap = test.Config.getBlackListedAccountMap() cfg.AccountRequired = test.Config.AccountRequired } cfg.MarshalAccountDefaults() @@ -5803,7 +5801,7 @@ func TestSendAuctionResponse_LogsErrors(t *testing.T) { ao := analytics.AuctionObject{} account := &config.Account{DebugAllow: true} - labels, ao = sendAuctionResponse(writer, test.hookExecutor, test.response, test.request, account, labels, ao) + _, ao = sendAuctionResponse(writer, test.hookExecutor, test.response, test.request, account, labels, ao) assert.Equal(t, ao.Errors, test.expectedErrors, "Invalid errors.") assert.Equal(t, test.expectedStatus, ao.Status, "Invalid HTTP response status.") diff --git a/endpoints/openrtb2/sample-requests/account-required/with-account/required-blacklisted-acct.json b/endpoints/openrtb2/sample-requests/account-required/with-account/required-blacklisted-acct.json deleted file mode 100644 index 894a92fdb27..00000000000 --- a/endpoints/openrtb2/sample-requests/account-required/with-account/required-blacklisted-acct.json +++ /dev/null @@ -1,91 +0,0 @@ -{ - "description": "Account is required but request comes with a blacklisted account id", - "config": { - "accountRequired": true, - "blacklistedAccts": ["bad_acct"] - }, - "mockBidRequest": { - "id": "some-request-id", - "user": { - "ext": { - "consent": "gdpr-consent-string", - "prebid": { - "buyeruids": { - "appnexus": "override-appnexus-id-in-cookie" - } - } - } - }, - "app": { - "id": "cool_app", - "publisher": { - "id": "bad_acct" - } - }, - "regs": { - "ext": { - "gdpr": 1 - } - }, - "imp": [ - { - "id": "some-impression-id", - "banner": { - "format": [ - { - "w": 300, - "h": 250 - }, - { - "w": 300, - "h": 600 - } - ] - }, - "ext": { - "appnexus": { - "placementId": 12883451 - }, - "districtm": { - "placementId": 105 - }, - "rubicon": { - "accountId": 1001, - "siteId": 113932, - "zoneId": 535510 - } - } - } - ], - "tmax": 500, - "ext": { - "prebid": { - "aliases": { - "districtm": "appnexus" - }, - "bidadjustmentfactors": { - "appnexus": 1.01, - "districtm": 0.98, - "rubicon": 0.99 - }, - "cache": { - "bids": {} - }, - "targeting": { - "includewinners": false, - "pricegranularity": { - "precision": 2, - "ranges": [ - { - "max": 20, - "increment": 0.10 - } - ] - } - } - } - } - }, - "expectedReturnCode": 503, - "expectedErrorMessage": "Invalid request: Prebid-server has disabled Account ID: bad_acct, please reach out to the prebid server host.\n" -} diff --git a/endpoints/openrtb2/sample-requests/blacklisted/blacklisted-app-publisher.json b/endpoints/openrtb2/sample-requests/account-required/with-account/required-disabled-acct.json similarity index 86% rename from endpoints/openrtb2/sample-requests/blacklisted/blacklisted-app-publisher.json rename to endpoints/openrtb2/sample-requests/account-required/with-account/required-disabled-acct.json index ef7a93b8bad..31a0021b7c1 100644 --- a/endpoints/openrtb2/sample-requests/blacklisted/blacklisted-app-publisher.json +++ b/endpoints/openrtb2/sample-requests/account-required/with-account/required-disabled-acct.json @@ -1,7 +1,7 @@ { - "description": "This is a perfectly valid request except that it comes with a blacklisted app publisher account", + "description": "Account is required but request comes with a disabled account id", "config": { - "blacklistedAccts": ["bad_acct"] + "accountRequired": true }, "mockBidRequest": { "id": "some-request-id", @@ -18,7 +18,7 @@ "app": { "id": "cool_app", "publisher": { - "id": "bad_acct" + "id": "disabled_acct" } }, "regs": { @@ -86,5 +86,5 @@ } }, "expectedReturnCode": 503, - "expectedErrorMessage": "Invalid request: Prebid-server has disabled Account ID: bad_acct, please reach out to the prebid server host.\n" + "expectedErrorMessage": "Invalid request: Prebid-server has disabled Account ID: disabled_acct, please reach out to the prebid server host.\n" } diff --git a/endpoints/openrtb2/sample-requests/blacklisted/blacklisted-site-publisher.json b/endpoints/openrtb2/sample-requests/blacklisted/blacklisted-site-publisher.json deleted file mode 100644 index cdec20d22ba..00000000000 --- a/endpoints/openrtb2/sample-requests/blacklisted/blacklisted-site-publisher.json +++ /dev/null @@ -1,90 +0,0 @@ -{ - "description": "This is a perfectly valid request except that it comes with a blacklisted site publisher account", - "config": { - "blacklistedAccts": ["bad_acct"] - }, - "mockBidRequest": { - "id": "some-request-id", - "user": { - "ext": { - "consent": "gdpr-consent-string", - "prebid": { - "buyeruids": { - "appnexus": "override-appnexus-id-in-cookie" - } - } - } - }, - "site": { - "id": "cool_site", - "publisher": { - "id": "bad_acct" - } - }, - "regs": { - "ext": { - "gdpr": 1 - } - }, - "imp": [ - { - "id": "some-impression-id", - "banner": { - "format": [ - { - "w": 300, - "h": 250 - }, - { - "w": 300, - "h": 600 - } - ] - }, - "ext": { - "appnexus": { - "placementId": 12883451 - }, - "districtm": { - "placementId": 105 - }, - "rubicon": { - "accountId": 1001, - "siteId": 113932, - "zoneId": 535510 - } - } - } - ], - "tmax": 500, - "ext": { - "prebid": { - "aliases": { - "districtm": "appnexus" - }, - "bidadjustmentfactors": { - "appnexus": 1.01, - "districtm": 0.98, - "rubicon": 0.99 - }, - "cache": { - "bids": {} - }, - "targeting": { - "includewinners": false, - "pricegranularity": { - "precision": 2, - "ranges": [ - { - "max": 20, - "increment": 0.10 - } - ] - } - } - } - } - }, - "expectedReturnCode": 503, - "expectedErrorMessage": "Invalid request: Prebid-server has disabled Account ID: bad_acct, please reach out to the prebid server host.\n" -} diff --git a/endpoints/openrtb2/test_utils.go b/endpoints/openrtb2/test_utils.go index 3d249dc8d59..2d22f1e4ebe 100644 --- a/endpoints/openrtb2/test_utils.go +++ b/endpoints/openrtb2/test_utils.go @@ -84,7 +84,6 @@ type testCase struct { type testConfigValues struct { AccountRequired bool `json:"accountRequired"` AliasJSON string `json:"aliases"` - BlacklistedAccounts []string `json:"blacklistedAccts"` BlacklistedApps []string `json:"blacklistedApps"` DisabledAdapters []string `json:"disabledAdapters"` CurrencyRates map[string]map[string]float64 `json:"currencyRates"` @@ -932,7 +931,6 @@ func (s mockCurrencyRatesClient) handle(w http.ResponseWriter, req *http.Request return } w.Write(currencyServerJsonResponse) - return } // mockBidderHandler carries mock bidder server information that will be read from JSON test files @@ -994,7 +992,6 @@ func (b mockBidderHandler) bid(w http.ResponseWriter, req *http.Request) { return } w.Write(serverJsonResponse) - return } // mockAdapter is a mock impression-splitting adapter @@ -1154,18 +1151,6 @@ func (tc *testConfigValues) getBlacklistedAppMap() map[string]bool { return blacklistedAppMap } -func (tc *testConfigValues) getBlackListedAccountMap() map[string]bool { - var blacklistedAccountMap map[string]bool - - if len(tc.BlacklistedAccounts) > 0 { - blacklistedAccountMap = make(map[string]bool, len(tc.BlacklistedAccounts)) - for _, account := range tc.BlacklistedAccounts { - blacklistedAccountMap[account] = true - } - } - return blacklistedAccountMap -} - // exchangeTestWrapper is a wrapper that asserts the openrtb2 bid request just before the HoldAuction call type exchangeTestWrapper struct { ex exchange.Exchange @@ -1278,10 +1263,10 @@ func buildTestEndpoint(test testCase, cfg *config.Configuration) (httprouter.Han storedResponseFetcher = empty_fetcher.EmptyFetcher{} } - var accountFetcher stored_requests.AccountFetcher - accountFetcher = &mockAccountFetcher{ + accountFetcher := &mockAccountFetcher{ data: map[string]json.RawMessage{ "malformed_acct": json.RawMessage(`{"disabled":"invalid type"}`), + "disabled_acct": json.RawMessage(`{"disabled":true}`), }, } diff --git a/endpoints/openrtb2/video_auction.go b/endpoints/openrtb2/video_auction.go index a2a519fa8df..7b2a7a5295a 100644 --- a/endpoints/openrtb2/video_auction.go +++ b/endpoints/openrtb2/video_auction.go @@ -244,7 +244,7 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re for _, podEr := range podErrors { resPodErr = append(resPodErr, strings.Join(podEr.ErrMsgs, ", ")) } - err := errors.New(fmt.Sprintf("all pods are incorrect: %s", strings.Join(resPodErr, "; "))) + err := fmt.Errorf("all pods are incorrect: %s", strings.Join(resPodErr, "; ")) errL = append(errL, err) handleError(&labels, w, errL, &vo, &debugLog) return @@ -403,7 +403,7 @@ func handleError(labels *metrics.Labels, w http.ResponseWriter, errL []error, vo var status int = http.StatusInternalServerError for _, er := range errL { erVal := errortypes.ReadCode(er) - if erVal == errortypes.BlacklistedAppErrorCode || erVal == errortypes.BlacklistedAcctErrorCode { + if erVal == errortypes.BlacklistedAppErrorCode || erVal == errortypes.AccountDisabledErrorCode { status = http.StatusServiceUnavailable labels.RequestStatus = metrics.RequestStatusBlacklisted break @@ -555,7 +555,7 @@ func buildVideoResponse(bidresponse *openrtb2.BidResponse, podErrors []PodError) if adPod == nil { adPod = &openrtb_ext.AdPod{ PodId: podId, - Targeting: make([]openrtb_ext.VideoTargeting, 0, 0), + Targeting: make([]openrtb_ext.VideoTargeting, 0), } adPods = append(adPods, adPod) } @@ -620,22 +620,15 @@ func getVideoStoredRequestId(request []byte) (string, error) { func mergeData(videoRequest *openrtb_ext.BidRequestVideo, bidRequest *openrtb2.BidRequest) error { if videoRequest.Site != nil { bidRequest.Site = videoRequest.Site - if &videoRequest.Content != nil { - bidRequest.Site.Content = &videoRequest.Content - } + bidRequest.Site.Content = &videoRequest.Content } if videoRequest.App != nil { bidRequest.App = videoRequest.App - if &videoRequest.Content != nil { - bidRequest.App.Content = &videoRequest.Content - } - } - - if &videoRequest.Device != nil { - bidRequest.Device = &videoRequest.Device + bidRequest.App.Content = &videoRequest.Content } + bidRequest.Device = &videoRequest.Device bidRequest.User = videoRequest.User if len(videoRequest.BCat) != 0 { @@ -768,7 +761,7 @@ func (deps *endpointDeps) validateVideoRequest(req *openrtb_ext.BidRequestVideo) err := errors.New("request missing required field: PodConfig.Pods") errL = append(errL, err) } - podErrors := make([]PodError, 0, 0) + podErrors := make([]PodError, 0) podIdsSet := make(map[int]bool) for ind, pod := range req.PodConfig.Pods { podErr := PodError{} diff --git a/endpoints/openrtb2/video_auction_test.go b/endpoints/openrtb2/video_auction_test.go index da4522ebe0c..f3135ff48db 100644 --- a/endpoints/openrtb2/video_auction_test.go +++ b/endpoints/openrtb2/video_auction_test.go @@ -292,7 +292,7 @@ func TestVideoEndpointNoPods(t *testing.T) { deps := mockDeps(t, ex) deps.VideoAuctionEndpoint(recorder, req, nil) - errorMessage := string(recorder.Body.Bytes()) + errorMessage := recorder.Body.String() assert.Equal(t, recorder.Code, 500, "Should catch error in request") assert.Equal(t, "Critical error while running the video endpoint: request missing required field: PodConfig.DurationRangeSec request missing required field: PodConfig.Pods", errorMessage, "Incorrect request validation message") @@ -739,7 +739,7 @@ func TestVideoBuildVideoResponsePodErrors(t *testing.T) { func TestVideoBuildVideoResponseNoBids(t *testing.T) { openRtbBidResp := openrtb2.BidResponse{} - podErrors := make([]PodError, 0, 0) + podErrors := make([]PodError, 0) openRtbBidResp.SeatBid = make([]openrtb2.SeatBid, 0) bidRespVideo, err := buildVideoResponse(&openRtbBidResp, podErrors) assert.NoError(t, err, "Error should be nil") @@ -808,7 +808,7 @@ func TestHandleError(t *testing.T) { { description: "Blocked account - return 503 with blocked metrics status", giveErrors: []error{ - &errortypes.BlacklistedAcct{}, + &errortypes.AccountDisabled{}, }, wantCode: 503, wantMetricsStatus: metrics.RequestStatusBlacklisted, @@ -1275,40 +1275,6 @@ func mockDeps(t *testing.T, ex *mockExchangeVideo) *endpointDeps { } } -func mockDepsInvalidPrivacy(t *testing.T, ex *mockExchangeVideo) *endpointDeps { - return &endpointDeps{ - fakeUUIDGenerator{}, - ex, - mockBidderParamValidator{}, - &mockVideoStoredReqFetcher{}, - &mockVideoStoredReqFetcher{}, - &mockAccountFetcher{data: mockVideoAccountData}, - &config.Configuration{MaxRequestSize: maxSize, - AccountDefaults: config.Account{ - Privacy: config.AccountPrivacy{ - AllowActivities: &config.AllowActivities{ - TransmitPreciseGeo: config.Activity{Rules: []config.ActivityRule{ - {Condition: config.ActivityCondition{ComponentName: []string{"bidderA.BidderB.bidderC"}}}, - }}, - }, - }, - }, - }, - &metricsConfig.NilMetricsEngine{}, - analyticsConf.NewPBSAnalytics(&config.Analytics{}), - map[string]string{}, - false, - []byte{}, - openrtb_ext.BuildBidderMap(), - ex.cache, - regexp.MustCompile(`[<>]`), - hardcodedResponseIPValidator{response: true}, - empty_fetcher.EmptyFetcher{}, - hooks.EmptyPlanBuilder{}, - nil, - } -} - func mockDepsAppendBidderNames(t *testing.T, ex *mockExchangeAppendBidderNames) *endpointDeps { deps := &endpointDeps{ fakeUUIDGenerator{}, diff --git a/endpoints/setuid_test.go b/endpoints/setuid_test.go index 897538c35e5..18099193045 100644 --- a/endpoints/setuid_test.go +++ b/endpoints/setuid_test.go @@ -1244,27 +1244,6 @@ func TestSetUIDEndpointMetrics(t *testing.T) { a.On("LogSetUIDObject", &expected).Once() }, }, - { - description: "Blocked account", - uri: "/setuid?bidder=pubmatic&uid=123&account=blocked_acct", - cookies: []*usersync.Cookie{}, - syncersBidderNameToKey: map[string]string{"pubmatic": "pubmatic"}, - gdprAllowsHostCookies: true, - expectedResponseCode: 400, - expectedMetrics: func(m *metrics.MetricsEngineMock) { - m.On("RecordSetUid", metrics.SetUidAccountBlocked).Once() - }, - expectedAnalytics: func(a *MockAnalytics) { - expected := analytics.SetUIDObject{ - Status: 400, - Bidder: "pubmatic", - UID: "", - Errors: []error{errCookieSyncAccountBlocked}, - Success: false, - } - a.On("LogSetUIDObject", &expected).Once() - }, - }, { description: "Invalid account", uri: "/setuid?bidder=pubmatic&uid=123&account=unknown", @@ -1549,9 +1528,6 @@ func makeRequest(uri string, existingSyncs map[string]string) *http.Request { func doRequest(req *http.Request, analytics analytics.PBSAnalyticsModule, metrics metrics.MetricsEngine, syncersBidderNameToKey map[string]string, gdprAllowsHostCookies, gdprReturnsError, gdprReturnsMalformedError, cfgAccountRequired bool, maxCookieSize int, priorityGroups [][]string) *httptest.ResponseRecorder { cfg := config.Configuration{ AccountRequired: cfgAccountRequired, - BlacklistedAcctMap: map[string]bool{ - "blocked_acct": true, - }, AccountDefaults: config.Account{}, UserSync: config.UserSync{ PriorityGroups: priorityGroups, diff --git a/errortypes/code.go b/errortypes/code.go index ef8287f2d7d..5cbf3f860b4 100644 --- a/errortypes/code.go +++ b/errortypes/code.go @@ -9,7 +9,7 @@ const ( BadServerResponseErrorCode FailedToRequestBidsErrorCode BidderTemporarilyDisabledErrorCode - BlacklistedAcctErrorCode + AccountDisabledErrorCode AcctRequiredErrorCode NoConversionRateErrorCode MalformedAcctErrorCode diff --git a/errortypes/errortypes.go b/errortypes/errortypes.go index 01a5cca4af7..18b70634c48 100644 --- a/errortypes/errortypes.go +++ b/errortypes/errortypes.go @@ -79,23 +79,20 @@ func (err *BlacklistedApp) Severity() Severity { return SeverityFatal } -// BlacklistedAcct should be used when a request account ID matches an entry in the BlacklistedAccts -// environment variable array -// -// These errors will be written to http.ResponseWriter before canceling execution -type BlacklistedAcct struct { +// AccountDisabled should be used when a request an account is specifically disabled in account config. +type AccountDisabled struct { Message string } -func (err *BlacklistedAcct) Error() string { +func (err *AccountDisabled) Error() string { return err.Message } -func (err *BlacklistedAcct) Code() int { - return BlacklistedAcctErrorCode +func (err *AccountDisabled) Code() int { + return AccountDisabledErrorCode } -func (err *BlacklistedAcct) Severity() Severity { +func (err *AccountDisabled) Severity() Severity { return SeverityFatal }