Skip to content

Commit

Permalink
Fix enforce-valid-account config logic (#1836)
Browse files Browse the repository at this point in the history
  • Loading branch information
mtuchkova authored Apr 25, 2022
1 parent fa759dc commit 08cbce6
Show file tree
Hide file tree
Showing 8 changed files with 301 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,19 @@

public class EnrichingApplicationSettings implements ApplicationSettings {

private final boolean enforceValidAccount;
private final ApplicationSettings delegate;
private final PriceFloorsConfigResolver priceFloorsConfigResolver;
private final JsonMerger jsonMerger;
private final Account defaultAccount;

public EnrichingApplicationSettings(String defaultAccountConfig,
public EnrichingApplicationSettings(boolean enforceValidAccount,
String defaultAccountConfig,
ApplicationSettings delegate,
PriceFloorsConfigResolver priceFloorsConfigResolver,
JsonMerger jsonMerger) {

this.enforceValidAccount = enforceValidAccount;
this.delegate = Objects.requireNonNull(delegate);
this.jsonMerger = Objects.requireNonNull(jsonMerger);
this.priceFloorsConfigResolver = Objects.requireNonNull(priceFloorsConfigResolver);
Expand All @@ -42,10 +45,13 @@ public Future<Account> getAccountById(String accountId, Timeout timeout) {
if (defaultAccount == null) {
return accountFuture;
}
final Future<Account> mergedWithDefaultAccount = accountFuture
.map(this::mergeAccounts);

return accountFuture
.map(this::mergeAccounts)
.otherwise(mergeAccounts(Account.empty(accountId)));
// In case of invalid account return failed future
return enforceValidAccount
? mergedWithDefaultAccount
: mergedWithDefaultAccount.otherwise(mergeAccounts(Account.empty(accountId)));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,12 +340,14 @@ static class EnrichingSettingsConfiguration {

@Bean
EnrichingApplicationSettings enrichingApplicationSettings(
@Value("${settings.enforce-valid-account}") boolean enforceValidAccount,
@Value("${settings.default-account-config:#{null}}") String defaultAccountConfig,
CompositeApplicationSettings compositeApplicationSettings,
PriceFloorsConfigResolver priceFloorsConfigResolver,
JsonMerger jsonMerger) {

return new EnrichingApplicationSettings(defaultAccountConfig,
return new EnrichingApplicationSettings(enforceValidAccount,
defaultAccountConfig,
compositeApplicationSettings,
priceFloorsConfigResolver,
jsonMerger);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,8 @@ class AccountConfig {
AccountAnalyticsConfig analytics
AccountCookieSyncConfig cookieSync
AccountHooksConfiguration hooks

static getDefaultAccountConfig() {
new AccountConfig(status: AccountStatus.ACTIVE)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class Imp {
}
}

private static Imp getDefaultImp() {
private static Imp getDefaultImp() {
new Imp().tap {
id = UUID.randomUUID()
ext = ImpExt.defaultImpExt
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,6 @@ class Video {
List<Integer> companiontype

static Video getDefaultVideo() {
new Video(mimes: ["video/mp4"], w: 300, h: 200)
new Video(mimes: ["video/mp4"], w: 300, h: 200)
}
}
256 changes: 256 additions & 0 deletions src/test/groovy/org/prebid/server/functional/tests/AccountSpec.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,256 @@
package org.prebid.server.functional.tests

import org.prebid.server.functional.model.AccountStatus
import org.prebid.server.functional.model.config.AccountConfig
import org.prebid.server.functional.model.db.Account
import org.prebid.server.functional.model.db.StoredRequest
import org.prebid.server.functional.model.request.amp.AmpRequest
import org.prebid.server.functional.model.request.auction.BidRequest
import org.prebid.server.functional.model.request.auction.Site
import org.prebid.server.functional.service.PrebidServerException
import org.prebid.server.functional.util.PBSUtils

import static io.netty.handler.codec.http.HttpResponseStatus.UNAUTHORIZED

class AccountSpec extends BaseSpec {

def "PBS should reject request with inactive account"() {
given: "Pbs config with enforce-valid-account and default-account-config"
def pbsService = pbsServiceFactory.getService(
["settings.enforce-valid-account": enforceValidAccount as String])

and: "Inactive account id"
def accountId = PBSUtils.randomNumber
def account = new Account(uuid: accountId, config: new AccountConfig(status: AccountStatus.INACTIVE))
accountDao.save(account)

and: "Default basic BidRequest with inactive account id"
def bidRequest = BidRequest.defaultBidRequest.tap {
site.publisher.id = accountId
}

when: "PBS processes auction request"
pbsService.sendAuctionRequest(bidRequest)

then: "PBS should reject the entire auction"
def exception = thrown(PrebidServerException)
assert exception.statusCode == UNAUTHORIZED.code()
assert exception.responseBody == "Account $accountId is inactive"

where:
enforceValidAccount << [true, false]
}

def "PBS should reject request with unknown account when settings.enforce-valid-account = true"() {
given: "Pbs config with enforce-valid-account and default-account-config"
def pbsService = pbsServiceFactory.getService(
["settings.enforce-valid-account" : "true",
"settings.default-account-config": mapper.encode(defaultAccountConfig)])

and: "Non-existing account id"
def accountId = PBSUtils.randomNumber

and: "Default basic BidRequest with non-existing account id"
def bidRequest = BidRequest.defaultBidRequest.tap {
site.publisher.id = accountId
}

when: "PBS processes auction request"
pbsService.sendAuctionRequest(bidRequest)

then: "Request should fail with an error"
def exception = thrown(PrebidServerException)
assert exception.statusCode == UNAUTHORIZED.code()
assert exception.responseBody == "Unauthorized account id: $accountId"

where:
defaultAccountConfig << [null, AccountConfig.defaultAccountConfig]
}

def "PBS should reject request without account when settings.enforce-valid-account = true"() {
given: "Pbs config with enforce-valid-account and default-account-config"
def pbsService = pbsServiceFactory.getService(
["settings.enforce-valid-account" : "true",
"settings.default-account-config": mapper.encode(defaultAccountConfig)])

and: "Default basic BidRequest without account"
def bidRequest = BidRequest.defaultBidRequest.tap {
site.publisher.id = null
}

when: "PBS processes auction request"
pbsService.sendAuctionRequest(bidRequest)

then: "Request should fail with an error"
def exception = thrown(PrebidServerException)
assert exception.statusCode == UNAUTHORIZED.code()
assert exception.responseBody == "Unauthorized account id: "

where:
defaultAccountConfig << [null, AccountConfig.defaultAccountConfig]
}

def "PBS should not reject request with unknown account when settings.enforce-valid-account = false"() {
given: "Pbs config with enforce-valid-account and default-account-config"
def pbsService = pbsServiceFactory.getService(
["settings.enforce-valid-account" : "false",
"settings.default-account-config": mapper.encode(defaultAccountConfig)])

and: "Default basic BidRequest with non-existing account id"
def bidRequest = BidRequest.defaultBidRequest.tap {
site.publisher.id = accountId
}

when: "PBS processes auction request"
def response = pbsService.sendAuctionRequest(bidRequest)

then: "PBS should not reject the entire auction"
assert !response.seatbid?.isEmpty()

where:
defaultAccountConfig || accountId
null || null
null || PBSUtils.randomNumber
AccountConfig.defaultAccountConfig || null
AccountConfig.defaultAccountConfig || PBSUtils.randomNumber
}

def "PBS AMP should reject request with unknown account when settings.enforce-valid-account = true"() {
given: "Pbs config with enforce-valid-account and default-account-config"
def pbsService = pbsServiceFactory.getService(
["settings.enforce-valid-account" : "true",
"settings.default-account-config": mapper.encode(defaultAccountConfig)])

and: "Default AMP request with non-existing account"
def ampRequest = AmpRequest.defaultAmpRequest.tap {
account = requestAccount
}

and: "Default stored request with non-existing account"
def ampStoredRequest = BidRequest.defaultStoredRequest.tap {
site = Site.defaultSite
site.publisher.id = storedRequestAccount
}

and: "Save storedRequest into DB"
def storedRequest = StoredRequest.getDbStoredRequest(ampRequest, ampStoredRequest)
storedRequestDao.save(storedRequest)

when: "PBS processes amp request"
pbsService.sendAmpRequest(ampRequest)

then: "Request should fail with an error"
def exception = thrown(PrebidServerException)
def resolvedAccount = requestAccount ?: storedRequestAccount
assert exception.statusCode == UNAUTHORIZED.code()
assert exception.responseBody == "Unauthorized account id: $resolvedAccount"

where:
defaultAccountConfig || requestAccount || storedRequestAccount
null || PBSUtils.randomNumber || null
null || null || PBSUtils.randomNumber
AccountConfig.defaultAccountConfig || PBSUtils.randomNumber || null
AccountConfig.defaultAccountConfig || null || PBSUtils.randomNumber
}

def "PBS AMP should reject request without account when settings.enforce-valid-account = true"() {
given: "Pbs config with enforce-valid-account and default-account-config"
def pbsService = pbsServiceFactory.getService(
["settings.enforce-valid-account" : "true",
"settings.default-account-config": mapper.encode(defaultAccountConfig)])

and: "Default AMP request without account"
def ampRequest = AmpRequest.defaultAmpRequest.tap {
account = null
}

and: "Default stored request without account"
def ampStoredRequest = BidRequest.defaultStoredRequest.tap {
site = Site.defaultSite
site.publisher.id = null
}

and: "Save storedRequest into DB"
def storedRequest = StoredRequest.getDbStoredRequest(ampRequest, ampStoredRequest)
storedRequestDao.save(storedRequest)

when: "PBS processes amp request"
pbsService.sendAmpRequest(ampRequest)

then: "Request should fail with an error"
def exception = thrown(PrebidServerException)
assert exception.statusCode == UNAUTHORIZED.code()
assert exception.responseBody == "Unauthorized account id: "

where:
defaultAccountConfig << [null, AccountConfig.defaultAccountConfig]
}

def "PBS AMP should not reject request with unknown account when settings.enforce-valid-account = false"() {
given: "Pbs config with enforce-valid-account and default-account-config"
def pbsService = pbsServiceFactory.getService(
["settings.enforce-valid-account" : "false",
"settings.default-account-config": mapper.encode(defaultAccountConfig)])

and: "Default AMP request with non-existing account"
def ampRequest = AmpRequest.defaultAmpRequest.tap {
account = requestAccount
}

and: "Default stored request with non-existing account"
def ampStoredRequest = BidRequest.defaultStoredRequest.tap {
site = Site.defaultSite
site.publisher.id = null
}

and: "Save storedRequest into DB"
def storedRequest = StoredRequest.getDbStoredRequest(ampRequest, ampStoredRequest)
storedRequestDao.save(storedRequest)

when: "PBS processes amp request"
def response = pbsService.sendAmpRequest(ampRequest)

then: "PBS should not reject request"
assert response.targeting
assert response.ext?.debug?.httpcalls

where:
defaultAccountConfig || requestAccount || storedRequestAccount
null || PBSUtils.randomNumber || null
null || null || PBSUtils.randomNumber
AccountConfig.defaultAccountConfig || PBSUtils.randomNumber || null
AccountConfig.defaultAccountConfig || null || PBSUtils.randomNumber
}

def "PBS AMP should not reject request without account when settings.enforce-valid-account = false"() {
given: "Pbs config with enforce-valid-account and default-account-config"
def pbsService = pbsServiceFactory.getService(
["settings.enforce-valid-account" : "false",
"settings.default-account-config": mapper.encode(defaultAccountConfig)])

and: "Default AMP request without account"
def ampRequest = AmpRequest.defaultAmpRequest.tap {
account = null
}

and: "Default stored request without account"
def ampStoredRequest = BidRequest.defaultStoredRequest.tap {
site = Site.defaultSite
site.publisher.id = null
}

and: "Save storedRequest into DB"
def storedRequest = StoredRequest.getDbStoredRequest(ampRequest, ampStoredRequest)
storedRequestDao.save(storedRequest)

when: "PBS processes amp request"
def response = pbsService.sendAmpRequest(ampRequest)

then: "PBS should not reject request"
assert response.targeting
assert response.ext?.debug?.httpcalls

where:
defaultAccountConfig << [null, AccountConfig.defaultAccountConfig]
}
}
4 changes: 2 additions & 2 deletions src/test/java/org/prebid/server/it/PriceFloorsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@
@RunWith(SpringRunner.class)
@TestPropertySource(
value = {"test-application.properties"},
properties = {"price-floors.enabled=true", "http.port=55555", "admin.port=0"}
properties = {"price-floors.enabled=true", "http.port=8050", "admin.port=0"}
)
public class PriceFloorsTest extends VertxTest {

private static final int APP_PORT = 55555;
private static final int APP_PORT = 8050;
private static final int WIREMOCK_PORT = 8090;

private static final String PRICE_FLOORS = "Price Floors Test";
Expand Down
Loading

0 comments on commit 08cbce6

Please sign in to comment.