Skip to content

Cookie issue fix#10666

Open
Lanaparezanin wants to merge 6 commits intodevfrom
lpar-cookie-fix
Open

Cookie issue fix#10666
Lanaparezanin wants to merge 6 commits intodevfrom
lpar-cookie-fix

Conversation

@Lanaparezanin
Copy link
Copy Markdown
Contributor

@Lanaparezanin Lanaparezanin commented Dec 19, 2025

Bug description:
Non-essential cookie __Controller::TempData appears eve though the user hasn't granted their consent.
Test by going to the following link: https://www.nuget.org/users/account/authenticate/return
The unknown error banner shouldn't appear:
image

Fix:
Added check to see if the user has granted consent.

Addresses https://github.com/orgs/NuGet/projects/21/views/1?filterQuery=assignee%3A%40me+milestone%3A%22Sprint+2026-01%22&pane=issue&itemId=147803039&issue=NuGet%7CEngineering%7C6244

@Lanaparezanin Lanaparezanin changed the title [DRAFT] Cookie issue fix Cookie issue fix Feb 2, 2026
@Lanaparezanin Lanaparezanin marked this pull request as ready for review February 2, 2026 18:29
@Lanaparezanin Lanaparezanin requested a review from a team as a code owner February 2, 2026 18:29
@agr
Copy link
Copy Markdown
Contributor

agr commented Feb 2, 2026

How __Controller::TempData cookie is non-essential? That's our only way of showing errors in some cases.

@Lanaparezanin
Copy link
Copy Markdown
Contributor Author

How __Controller::TempData cookie is non-essential? That's our only way of showing errors in some cases.

@agr You could make a case for either, the argument for non-essential is that it is a convenience feature, doesn't break user's workflow, they just don't see the feedback messages. I didn't find any proper definition in ComplianceWeb's documentation about what exactly counts as essential. I can go to their office hours to sort this out, unless someone else has more context?

@joelverhagen
Copy link
Copy Markdown
Member

I personally would argue that error/warning/info messages we put in TempData are essential. They describe status of work the user is doing. It's different from, say, Application Insights browser events sent by our JS.

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