Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add invalid ccpa notification in debug error response #787

Merged
merged 7 commits into from
Dec 11, 2020
Merged

Add invalid ccpa notification in debug error response #787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0732810
Add invalid ccpa notification in debug error response
DGarbar Jul 3, 2020
f19c296
Merge remote-tracking branch 'opensource/master' into ccpa-errors-in-…
DGarbar Oct 23, 2020
9073ab2
Fix merge with master
DGarbar Oct 23, 2020
07c6d09
Fix possible error
DGarbar Oct 23, 2020
2ae27a1
Merge branch 'master' into ccpa-errors-in-response
Nov 20, 2020
53e52ad
Merge branch 'master' into ccpa-errors-in-response
Dec 3, 2020
5fba771
Merge branch 'master' into ccpa-errors-in-response
Dec 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/main/java/org/prebid/server/auction/AmpRequestFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,7 @@ private static Integer debugFromQueryStringParam(RoutingContext context) {
/**
* Extracts parameters from http request and overrides corresponding attributes in {@link BidRequest}.
*/
private BidRequest overrideParameters(BidRequest bidRequest, HttpServerRequest request,
List<String> errors) {
private BidRequest overrideParameters(BidRequest bidRequest, HttpServerRequest request, List<String> errors) {
final String requestConsentParam = request.getParam(CONSENT_PARAM);
final String requestGdprConsentParam = request.getParam(GDPR_CONSENT_PARAM);
final String consentString = ObjectUtils.firstNonNull(requestConsentParam, requestGdprConsentParam);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/prebid/server/auction/AuctionRequestFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ Future<AuctionContext> toAuctionContext(RoutingContext routingContext,

return accountFrom(bidRequest, timeout, routingContext)
.compose(account -> privacyEnforcementService.contextFromBidRequest(
bidRequest, account, requestTypeMetric, timeout)
bidRequest, account, requestTypeMetric, timeout, errors)
.map(privacyContext -> AuctionContext.builder()
.routingContext(routingContext)
.uidsCookie(uidsCookieService.parseFromRequest(routingContext))
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/prebid/server/auction/PrivacyEnforcementService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ public PrivacyEnforcementService(BidderCatalog bidderCatalog,
}

Future<PrivacyContext> contextFromBidRequest(
BidRequest bidRequest, Account account, MetricName requestType, Timeout timeout) {
BidRequest bidRequest, Account account, MetricName requestType, Timeout timeout, List<String> errors) {

final Privacy privacy = privacyExtractor.validPrivacyFrom(bidRequest);
final Privacy privacy = privacyExtractor.validPrivacyFrom(bidRequest, errors);

final Device device = bidRequest.getDevice();
final String ipAddress = device != null ? device.getIp() : null;
Expand Down
34 changes: 22 additions & 12 deletions src/main/java/org/prebid/server/privacy/PrivacyExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
import org.prebid.server.proto.request.CookieSyncRequest;
import org.prebid.server.proto.request.PreBidRequest;

import java.util.List;

/**
* GDPR-aware utilities
*/
Expand All @@ -40,12 +42,13 @@ public class PrivacyExtractor {
* </ul><p>
* And construct {@link Privacy} from them. Use default values in case of invalid value.
*/
public Privacy validPrivacyFrom(BidRequest bidRequest) {
return extractPrivacy(bidRequest.getRegs(), bidRequest.getUser());
public Privacy validPrivacyFrom(BidRequest bidRequest, List<String> errors) {
return extractPrivacy(bidRequest.getRegs(), bidRequest.getUser(), errors);
}

@Deprecated
public Privacy validPrivacyFrom(PreBidRequest preBidRequest) {
return extractPrivacy(preBidRequest.getRegs(), preBidRequest.getUser());
return extractPrivacy(preBidRequest.getRegs(), preBidRequest.getUser(), null);
}

public Privacy validPrivacyFrom(CookieSyncRequest request) {
Expand All @@ -54,17 +57,17 @@ public Privacy validPrivacyFrom(CookieSyncRequest request) {
final String gdprConsent = request.getGdprConsent();
final String usPrivacy = request.getUsPrivacy();

return toValidPrivacy(gdpr, gdprConsent, usPrivacy, null);
return toValidPrivacy(gdpr, gdprConsent, usPrivacy, null, null);
}

public Privacy validPrivacyFromSetuidRequest(HttpServerRequest request) {
final String gdpr = request.getParam(SETUID_GDPR_PARAM);
final String gdprConsent = request.getParam(SETUID_GDPR_CONSENT_PARAM);

return toValidPrivacy(gdpr, gdprConsent, null, null);
return toValidPrivacy(gdpr, gdprConsent, null, null, null);
}

private Privacy extractPrivacy(Regs regs, User user) {
private Privacy extractPrivacy(Regs regs, User user, List<String> errors) {
final ExtRegs extRegs = regs != null ? regs.getExt() : null;
final ExtUser extUser = user != null ? user.getExt() : null;

Expand All @@ -74,27 +77,34 @@ private Privacy extractPrivacy(Regs regs, User user) {
final String usPrivacy = extRegs != null ? extRegs.getUsPrivacy() : null;
final Integer coppa = regs != null ? regs.getCoppa() : null;

return toValidPrivacy(gdpr, consent, usPrivacy, coppa);
return toValidPrivacy(gdpr, consent, usPrivacy, coppa, errors);
}

private static Privacy toValidPrivacy(String gdpr, String consent, String usPrivacy, Integer coppa) {
private static Privacy toValidPrivacy(String gdpr,
String consent,
String usPrivacy,
Integer coppa,
List<String> errors) {
final String validGdpr = ObjectUtils.notEqual(gdpr, "1") && ObjectUtils.notEqual(gdpr, "0")
? DEFAULT_GDPR_VALUE
: gdpr;
final String validConsent = consent == null ? DEFAULT_CONSENT_VALUE : consent;
final Ccpa validCcpa = usPrivacy == null ? DEFAULT_CCPA_VALUE : toValidCcpa(usPrivacy);
final Ccpa validCcpa = usPrivacy == null ? DEFAULT_CCPA_VALUE : toValidCcpa(usPrivacy, errors);
final Integer validCoppa = coppa == null ? DEFAULT_COPPA_VALUE : coppa;

return Privacy.of(validGdpr, validConsent, validCcpa, validCoppa);
}

private static Ccpa toValidCcpa(String usPrivacy) {
private static Ccpa toValidCcpa(String usPrivacy, List<String> errors) {
try {
Ccpa.validateUsPrivacy(usPrivacy);
return Ccpa.of(usPrivacy);
} catch (PreBidException e) {
// TODO add error to PBS response, not only in logs (See PR #758)
logger.debug("CCPA consent {0} has invalid format: {1}", usPrivacy, e.getMessage());
final String message = String.format("CCPA consent %s has invalid format: %s", usPrivacy, e.getMessage());
logger.debug(message);
if (errors != null) {
errors.add(message);
}
return DEFAULT_CCPA_VALUE;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/prebid/server/auction/AuctionRequestFactoryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public void setUp() {
given(timeoutResolver.resolve(any())).willReturn(2000L);
given(timeoutResolver.adjustTimeout(anyLong())).willReturn(1900L);

given(privacyEnforcementService.contextFromBidRequest(any(), any(), any(), any()))
given(privacyEnforcementService.contextFromBidRequest(any(), any(), any(), any(), any()))
.willReturn(Future.succeededFuture(PrivacyContext.of(
Privacy.of("0", EMPTY, Ccpa.EMPTY, 0),
TcfContext.empty())));
Expand Down Expand Up @@ -1693,7 +1693,7 @@ public void shouldEnrichRequestWithIpAddressAndCountryAndSaveAuctionContext() {
.geoInfo(GeoInfo.builder().vendor("v").country("ua").build())
.build(),
"ip");
given(privacyEnforcementService.contextFromBidRequest(any(), any(), any(), any()))
given(privacyEnforcementService.contextFromBidRequest(any(), any(), any(), any(), any()))
.willReturn(Future.succeededFuture(privacyContext));

// when
Expand Down
7 changes: 4 additions & 3 deletions src/test/java/org/prebid/server/auction/PrivacyEnforcementServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import java.time.Clock;
import java.time.Instant;
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -131,7 +132,7 @@ public void contextFromBidRequestShouldReturnCoppaContext() {

// when
final Future<PrivacyContext> privacyContext = privacyEnforcementService.contextFromBidRequest(
bidRequest, Account.empty("account"), null, null);
bidRequest, Account.empty("account"), null, null, new ArrayList<>());

// then
FutureAssertion.assertThat(privacyContext).succeededWith(
Expand Down Expand Up @@ -165,7 +166,7 @@ public void contextFromBidRequestShouldReturnTcfContext() {

// when
final Future<PrivacyContext> privacyContext = privacyEnforcementService.contextFromBidRequest(
bidRequest, Account.empty(accountId), requestType, null);
bidRequest, Account.empty(accountId), requestType, null, new ArrayList<>());

// then
final Privacy privacy = Privacy.of("1", "consent", Ccpa.of("1YYY"), 0);
Expand Down Expand Up @@ -207,7 +208,7 @@ public void contextFromBidRequestShouldReturnTcfContextWithIpMasked() {

// when
final Future<PrivacyContext> privacyContext = privacyEnforcementService.contextFromBidRequest(
bidRequest, Account.empty("account"), MetricName.openrtb2web, null);
bidRequest, Account.empty("account"), MetricName.openrtb2web, null, new ArrayList<>());

// then
final Privacy privacy = Privacy.of("1", "consent", Ccpa.of("1YYY"), 0);
Expand Down
48 changes: 33 additions & 15 deletions src/test/java/org/prebid/server/privacy/PrivacyExtractorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import org.prebid.server.proto.request.CookieSyncRequest;
import org.prebid.server.proto.request.PreBidRequest;

import java.util.ArrayList;

import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.given;
Expand All @@ -37,7 +39,8 @@ public void setUp() {
@Test
public void shouldReturnGdprEmptyValueWhenRegsIsNull() {
// given and when
final String gdpr = privacyExtractor.validPrivacyFrom(BidRequest.builder().build()).getGdpr();
final String gdpr =
privacyExtractor.validPrivacyFrom(BidRequest.builder().build(), new ArrayList<>()).getGdpr();

// then
assertThat(gdpr).isEmpty();
Expand All @@ -47,7 +50,7 @@ public void shouldReturnGdprEmptyValueWhenRegsIsNull() {
public void shouldReturnGdprEmptyValueWhenRegsExtIsNull() {
// given and when
final String gdpr = privacyExtractor.validPrivacyFrom(
BidRequest.builder().regs(Regs.of(null, null)).build())
BidRequest.builder().regs(Regs.of(null, null)).build(), new ArrayList<>())
.getGdpr();

// then
Expand All @@ -60,7 +63,8 @@ public void shouldReturnGdprEmptyValueWhenRegsExtGdprIsNoEqualsToOneOrZero() {
final Regs regs = Regs.of(null, ExtRegs.of(2, null));

// when
final String gdpr = privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).build()).getGdpr();
final String gdpr =
privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).build(), new ArrayList<>()).getGdpr();

// then
assertThat(gdpr).isEmpty();
Expand All @@ -72,7 +76,8 @@ public void shouldReturnGdprOneWhenExtRegsContainsGdprOne() {
final Regs regs = Regs.of(null, ExtRegs.of(1, null));

// when
final String gdpr = privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).build()).getGdpr();
final String gdpr =
privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).build(), new ArrayList<>()).getGdpr();

// then
assertThat(gdpr).isEqualTo("1");
Expand All @@ -84,7 +89,8 @@ public void shouldReturnGdprZeroWhenExtRegsContainsGdprZero() {
final Regs regs = Regs.of(null, ExtRegs.of(0, null));

// when
final String gdpr = privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).build()).getGdpr();
final String gdpr =
privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).build(), new ArrayList<>()).getGdpr();

// then
assertThat(gdpr).isEqualTo("0");
Expand All @@ -93,7 +99,8 @@ public void shouldReturnGdprZeroWhenExtRegsContainsGdprZero() {
@Test
public void shouldReturnConsentEmptyValueWhenExtUserIsNull() {
// given and when
final String consent = privacyExtractor.validPrivacyFrom(BidRequest.builder().build()).getConsentString();
final String consent = privacyExtractor.validPrivacyFrom(BidRequest.builder().build(), new ArrayList<>())
.getConsentString();

// then
assertThat(consent).isEmpty();
Expand All @@ -105,8 +112,9 @@ public void shouldReturnConsentEmptyValueWhenUserConsentIsNull() {
final User user = User.builder().ext(ExtUser.builder().build()).build();

// when
final String consent = privacyExtractor.validPrivacyFrom(BidRequest.builder().user(user).build())
.getConsentString();
final String consent =
privacyExtractor.validPrivacyFrom(BidRequest.builder().user(user).build(), new ArrayList<>())
.getConsentString();

// then
assertThat(consent).isEmpty();
Expand All @@ -118,23 +126,28 @@ public void shouldReturnConsentWhenUserContainsConsent() {
final User user = User.builder().ext(ExtUser.builder().consent("consent").build()).build();

// when
final String consent = privacyExtractor.validPrivacyFrom(BidRequest.builder().user(user).build())
.getConsentString();
final String consent =
privacyExtractor.validPrivacyFrom(BidRequest.builder().user(user).build(), new ArrayList<>())
.getConsentString();

// then
assertThat(consent).isEqualTo("consent");
}

@Test
public void shouldReturnDefaultCcpaIfNotValid() {
public void shouldReturnDefaultCcpaWhenNotValidAndAddError() {
// given
final Regs regs = Regs.of(null, ExtRegs.of(null, "invalid"));
final ArrayList<String> errors = new ArrayList<>();

// when
final Ccpa ccpa = privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).build()).getCcpa();
final Ccpa ccpa =
privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).build(), errors).getCcpa();

// then
assertThat(ccpa).isEqualTo(Ccpa.EMPTY);
assertThat(errors).containsOnly(
"CCPA consent invalid has invalid format: us_privacy must contain 4 characters");
}

@Test
Expand All @@ -143,7 +156,9 @@ public void shouldReturnDefaultCoppaIfNull() {
final Regs regs = Regs.of(null, null);

// when
final Integer coppa = privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).build()).getCoppa();
final Integer coppa =
privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).build(), new ArrayList<>())
.getCoppa();

// then
assertThat(coppa).isZero();
Expand All @@ -155,7 +170,9 @@ public void shouldReturnCoppaIfNotNull() {
final Regs regs = Regs.of(42, null);

// when
final Integer coppa = privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).build()).getCoppa();
final Integer coppa =
privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).build(), new ArrayList<>())
.getCoppa();

// then
assertThat(coppa).isEqualTo(42);
Expand All @@ -168,7 +185,8 @@ public void shouldReturnPrivacyWithParametersExtractedFromBidRequest() {
final User user = User.builder().ext(ExtUser.builder().consent("consent").build()).build();

// when
final Privacy privacy = privacyExtractor.validPrivacyFrom(BidRequest.builder().regs(regs).user(user).build());
final Privacy privacy = privacyExtractor
.validPrivacyFrom(BidRequest.builder().regs(regs).user(user).build(), new ArrayList<>());

// then
assertThat(privacy).isEqualTo(Privacy.of("0", "consent", Ccpa.of("1Yn-"), 0));
Expand Down