Skip to content

Allow custom cookie consent value#40213

Merged
pranavkm merged 8 commits into
dotnet:mainfrom
daviddesmet:feat/cookie-consent-custom-value
Feb 15, 2022
Merged

Allow custom cookie consent value#40213
pranavkm merged 8 commits into
dotnet:mainfrom
daviddesmet:feat/cookie-consent-custom-value

Conversation

@daviddesmet
Copy link
Copy Markdown
Contributor

@daviddesmet daviddesmet commented Feb 14, 2022

Allow customizing the cookie consent value

Allows setting a custom value for the cookie consent.

Description

Currently, the cookie consent validation performs an equality comparison against the constant cookie value yes. It will add more flexibility if the value can be customized, especially when supporting an already existing cookie consent.

Fixes #40164

@daviddesmet daviddesmet requested review from a team and Tratcher as code owners February 14, 2022 21:18
@ghost ghost added area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer community-contribution Indicates that the PR has been added by a community member labels Feb 14, 2022
@dnfadmin
Copy link
Copy Markdown

dnfadmin commented Feb 14, 2022 •
edited
Loading

CLA assistant check
All CLA requirements met.

pranavkm
pranavkm reviewed Feb 14, 2022
View reviewed changes
Comment thread src/Security/CookiePolicy/src/ResponseCookiesWrapper.cs Outdated
@daviddesmet
Copy link
Copy Markdown
Contributor Author

daviddesmet commented Feb 14, 2022

@pranavkm The test CreateConsentCookieMatchesGrantConsentCookieWhenCookieValueIsMissing() will need to be updated since the ConsentCookieValue setter no longer accepts an empty value.

@pranavkm
Copy link
Copy Markdown
Contributor

pranavkm commented Feb 14, 2022

Thanks. If you feel like it, it would be nice to add a test for CookieOptions to check it does not allow null / empty values for the consent value.

@Tratcher Tratcher added this to the 7.0-preview3 milestone Feb 14, 2022
@daviddesmet
Copy link
Copy Markdown
Contributor Author

daviddesmet commented Feb 14, 2022

@pranavkm Sure, I will add it!

@daviddesmet
Copy link
Copy Markdown
Contributor Author

daviddesmet commented Feb 14, 2022

@pranavkm @Tratcher Unit Test added when consent value is missing.

@pranavkm pranavkm enabled auto-merge (squash) February 14, 2022 22:31
@daviddesmet
Copy link
Copy Markdown
Contributor Author

daviddesmet commented Feb 14, 2022

@pranavkm The tests that failed didn't like 'ExceptionAssert.ThrowsArgument(Action, string, string)' :(

@pranavkm pranavkm merged commit 8c1d2de into dotnet:main Feb 15, 2022
@ghost ghost modified the milestones: 7.0-preview3, 7.0-preview2 Feb 15, 2022
@Rick-Anderson Rick-Anderson mentioned this pull request Jul 7, 2022
Closed
@Tratcher Tratcher mentioned this pull request Sep 28, 2022
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Tratcher Tratcher Tratcher approved these changes

+1 more reviewer

@pranavkm pranavkm pranavkm left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@Tratcher Tratcher

Labels

area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

7.0-preview2

Development

Successfully merging this pull request may close these issues.

Allow customizing the cookie consent value

4 participants

@daviddesmet @dnfadmin @pranavkm @Tratcher