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

[DEPR]: Marketing site login and user info cookies #32343

Open
robrap opened this issue Jun 1, 2023 · 5 comments
Open

[DEPR]: Marketing site login and user info cookies #32343

robrap opened this issue Jun 1, 2023 · 5 comments
Labels
depr Proposal for deprecation & removal per OEP-21

Comments

@robrap
Copy link
Contributor

robrap commented Jun 1, 2023

Proposal Date

2023-06-15

Target Ticket Acceptance Date

2023-06-30

Earliest Open edX Named Release Without This Functionality

Redwood - 2024-04

Rationale

The following marketing site cookies were deprecated in code long ago, but they did not go through a DEPR process.

  • EDXMKTG_LOGGED_IN_COOKIE_NAME
  • EDXMKTG_USER_INFO_COOKIE_NAME

The 2U private marketing site is using these cookies, and is blocking removal. However, it is unknown at this time if anyone else in the community is using these cookies.

This would resolve potential security issues, login related bugs, performance and stability issues given that the user info cookie is ~1k, which is a large part of our cookie header size budget.

Removal

Copying details from ARCHBOM-1172...

Once the Marketing site is updated to use our new header component, we can remove:

  • EDXMKTG_LOGGED_IN_COOKIE_NAME
  • EDXMKTG_USER_INFO_COOKIE_NAME

Notes:

  • Here is a link to code related to setting these deprecated cookies.
  • The EDXMKTG_LOGGED_IN_COOKIE_NAME has an existing alternative and simply should no longer be used. In place of EDXMKTG_LOGGED_IN_COOKIE_NAME, we should be using frontend-auth code from frontend-platform to determine if the user is authenticated and to get basic information on the user. (This happens to use JWT cookies behind the scenes, but that should be encapsulated away.)
    • For EDXMKTG_USER_INFO_COOKIE_NAME, there is not yet an existing alternative.
      • Using EDXMKTG_USER_INFO_COOKIE_NAME (until replaced):
        • Do not use this cookie to determine if the user is logged in. See notes about frontend-auth.
        • Only use this cookie for supplemental data if you have already checked that the user is authenticated using frontend-auth. For additional security, only use data from this cookie if the user matches the authenticated user, although that should be the case.
      • Replacing EDXMKTG_USER_INFO_COOKIE_NAME:
        • Some of the required data may already be returned from frontend-auth code.
        • For data that is specific to this cookie, we should consider API calls that can use a local cache in place of a cookie.
        • Not using a cookie would help our cookie size problems, because this is a big one.
        • For data helpful to all MFEs, this could be loaded from frontend-platform.
        • We probably don’t want to add more to the JWT cookie.
        • For data helpful only to the marketing site, this data could be loaded from Prospectus.

Replacement

Details included in earlier section.

Deprecation

It is already marked as deprecated.

Migration

No response

Additional Info

Additional notes:

  • The legacy marketing cookies that are created at login predate our use of JWT cookies.
  • The decision to deprecate the marketing cookies is captured in the code via names like DEPRECATED_LOGGED_IN_COOKIE_NAMES.
  • The original JIRA ticket was Note: This ticket used to be ARCH-245`, which is what was used in the login cookie code comments.
  • It seems the mobile app ran into discrepancies with when and if the marketing site considers a user to be logged in, based on this outdated cookie.
@github-actions github-actions bot added the depr Proposal for deprecation & removal per OEP-21 label Jun 1, 2023
@omar-nelc
Copy link

Thanks for the announcement. @robrap I usually recommend this cookie to customers needing marketing site integration.

I also need to check with @felipemontoya on the use of Marketing Site User Info cookie on customer sites. He'll respond to this ticket.

Please keep this on hold until we get a definitive "no" from the eduNEXT folks.

@robrap
Copy link
Contributor Author

robrap commented Jun 20, 2023

@omar-nelc

  1. Thanks for letting me know others are using this.
  2. This work is not yet planned.
  3. Once there is a real alternative, it will be simple to make a more clear deprecation timeline where the community has time to transition before the actual removal of the user info cookie.

@felipemontoya
Copy link
Member

Thanks @robrap for organizing this DEPR. This cookie is used in the wordpress plugin for marketing site connections (https://wordpress.org/plugins/edunext-openedx-integrator/).
We are in the process of splitting that plugin into an ecommerce only part and one for the header and other marketing info.

I don't think we use all the information contained in the EDXMKTG_USER_INFO_COOKIE_NAME so I will look into reading this info from the JWT cookie.
Do you know if the 2U private marketing is also considering to move to JWTs in the future?

@robrap
Copy link
Contributor Author

robrap commented Jun 20, 2023

Thanks @felipemontoya. That's helpful context. The 2U marketing site already uses the auth JWT, but the user info cookie contains a lot of information that is not contained in the auth JWT.

There is no agreed upon design, but my thoughts are captured in the original description around EDXMKTG_USER_INFO_COOKIE_NAME. I imagine that MFEs could use either additional config (where appropriate) or a new API call to gather the same information that was in this cookie. The contents could be cached in the browser cache. There is no need for this data to be in a cookie, because it doesn't need to be sent back to the server. In this approach, I don't see how a JWT fits in.

@robrap
Copy link
Contributor Author

robrap commented Jun 20, 2023

In case I haven't made this extremely clear, there should be no timeline concerns. Although "Redwood - 2024-04" was listed as the earliest removal, there is no rush on the full removal, and there is plenty of time to discuss how and if this work could be accomplished.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
depr Proposal for deprecation & removal per OEP-21
Projects
Status: Proposed
Development

No branches or pull requests

3 participants