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.

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

Improve error messages when missing identity cookies #838

Closed
wants to merge 16 commits into from

Conversation

carterworks
Copy link
Contributor

@carterworks carterworks commented Mar 18, 2022

Description

The goal of this ticket was to improve the error messages when a identity cookie is not set after a Konductor response. We identified two potential causes for this

  1. The provided organization ID is incorrect
    • new error message: "An identity for organization adobeOrg@Adobe was not found. Valid organizations on this page are: adobeOrg2@Adobe, adobeOrg3@Adobe"
  2. The current apex domain doesn't allow third-party cookies from the edge domain.
    • new error message: "An identity was not set properly because edge domain example.com does not match apex domain adobe.com, and adobe.com may not allow third-party cookies."

("example.com" is used as an example of an edge domain and "adobe.com" is used as an example of an apex domain).

In addition to these two new error messages, the catch-all error message has been updated. It now reads as "An identity was not set properly. Please verify that cookies returned from demdex.net can be set on this page."

Related Issue

PDCL-7279–Better error message when Web SDK can't find identity cookie

Currently, when an interact request goes out where the Web SDK expects to get an identity cookie, if the cookie is not there, this message is generated:

"An identity was not set properly. Please verify that the org ID *** configured in Alloy matches the org ID specified in the edge configuration."

This message can lead users down the wrong path. We should actually look for cookies starting with kndctr_ and see if there is an org ID mismatch.

This problem could also be caused by collecting using a CNAME that is not first party to the current location. Can we check to see if the edgeDomain ends with the apex domain of the current location? We could log a warning in such a case when we don't get the identity cookie back.

When neither of these problems are found how about a message like this: "An identity was not set properly. Please verify the cookies returned from ${edgeDomain} can be set on this page."

Motivation and Context

Previous customers have found that the existing error message was not actionable. These new error message provide more useful next steps for the customer.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Improvement (non-breaking change which does not add functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA or I'm an Adobe employee.
  • I have made any necessary test changes and all tests pass.
  • I have run the Sandbox successfully.

@carterworks
Copy link
Contributor Author

Failing functional tests will be resolved when pull request #835 is merged

Copy link
Contributor

@jonsnyder jonsnyder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to see some functional tests. Let's talk about this in standup.

orgIdsFromCookies.length > 0 &&
!includes(orgIdsFromCookies, orgId)
) {
errorMessage = `An identity for organzation ${orgId} was not found. Valid organizations on this page are: ${orgIdsFromCookies.join(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Valid organizations on this page" may not be true. Another page could have set a cookie. Maybe, "Valid organizations on this domain". But that may be wrong too as there could be a third party cookie that you are reading. Ideally we would only look at the cookies returned via cookieTransfer when in 3rd party collection mode.

export default (domain, apexDomain) => {
const regex = new RegExp(`${apexDomain}$`);
return regex.test(domain);
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is equivalent to the existing util "endsWith"

import injectExtractOrgIdsFromCookies from "../../../../../src/components/Identity/injectExtractOrgIdsFromCookies";
import { cookieJar } from "../../../../../src/utils";

describe("injectExtractOrIdsFromCookies", () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo "OrIds" => "OrgIds"

"97D1F3F459CE0AD80A495CBE"
]);
});
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a test for when there are no cookies?

", "
)}`;
} else if (!domainMatchesApex(edgeDomain, apexDomain)) {
errorMessage = `An identity was not set properly because edge domain ${edgeDomain} does not match apex domain ${apexDomain}, and ${apexDomain} may not allow third-party cookies.`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to show this message when the customer is using third party data collection like adobedc.net, but I don't want to recreate the logic that Konductor uses to determine when to send cookies via cookie transfer and when to send them via set-cookie headers.

@jfkhoury
Copy link
Contributor

First of all, good work on this fix @carterworks , anything Identity & cookies can be painful.

Question:

It seems that both of these exceptions can/are being handled by Edge:

  1. Non-matching ORG Id
  2. Using third party edge domain

Should we test to see if Konductor already returns an error for mismatched Org ID & Datastream ID, and how it handles 2, and simply log their error?

I'm asking because we are working towards a future where Alloy won't even care about the Identity cookie, so why not try to simplify this logic now.

Thoughts?

@jonsnyder @ninaceban

@carterworks
Copy link
Contributor Author

@jfkhoury

From what I can tell, Konductor has an error message for missing/incorrect datastream IDs, but not for organization IDs.

Also, I cannot figure out a way to block cookies from being set. I've tried iframe stuff (can't figure out how to run TestCafé in both a browser page and an iframe inside that page at the same time), content security policy stuff (can't block cookies with CSP–you can only block third-party cookies with SameSite=None attributes on the cookies and that is independent from CSP) and regular third party domain usage (Konductor just sends a state:store payload when the apex doesn't match the edge, so what would have been a third-party cookie when set by the Set-Cookie response header becomes a first-party cookie when set via Javascript). I'm leaning towards giving up since I don't think the small benefit of a test for that fail case is worth spending additional days on it.

@carterworks
Copy link
Contributor Author

Closed in favor of the more simple PR #846, which adds a functional test and tweaks the language of the error message slightly.

@carterworks carterworks deleted the PDCL-7279 branch March 30, 2022 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants