-
Notifications
You must be signed in to change notification settings - Fork 504
Cookie consent preferences #861
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
Conversation
… on login components
Conflicts: src/app/app-routing.module.ts src/app/shared/log-in/log-in.component.ts
|
This pull request introduces 1 alert when merging 69704e6 into cd6c5b7 - view on LGTM.com new alerts:
|
Conflicts: src/app/app-routing.module.ts
69704e6 to
d4027cc
Compare
|
This pull request introduces 1 alert when merging d4027cc into dd03745 - view on LGTM.com new alerts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LotteHofstede I noticed that even if the user does not accept cookies clicking on deny, after he has logged in the dspace.agreements.cookies metadata is still set as if he had accepted only the mandatory ones. How should the application behave if the user does not accept the cookie conditions?
|
The user can't refuse the mandatory cookies, they're required for the site to function. So even if you click deny all, you can't help but accept the mandatory cookies. Their toggles are also disabled in the UI. That approach is GDPR compliant as long as our mandatory cookies are actually required for the site to function. |
|
@artlowel Ok, thanks. So I think this PR can be integrated:
|
tdonohue
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good to me. The code changes in this PR are less than they seem (as it looks to include all of the changes from #856). So, I reviewed the code via a diff of the branches.
I also tested it, and it works as described. Also made sure to test the scenario where you accept cookies prior to login, and I verified that the accepted cookies are then copied to your user account after you authenticate.
Overall, looks good to me. Thanks @LotteHofstede !
References
Description
This PR adds a cookie preference pop-up using Klaro and saves them per account.
Instructions for Reviewers
To test this feature, make sure the metadata field
dspace.agreements.cookiesexist on the REST server you're testing with. I already created this field on https://dspace7.4science.it/server/api.On opening the application, a small dialog in the bottom right corner (or just at the bottom on mobile) shows, where the user can choose to accept/decline/customise their cookie consent preferences.
Clicking 'customise' should open a more detailed pop-up where you can see what cookies are used for DSpace and disable them. Some of the cookies are required for DSpace to work and can not be disabled.
When a user logs in, the preferences selected are copied to a cookie specifically used for this user and stored in the metadata field
dspace.agreements.cookiesof the user.When the user already has preferences defined in the
dspace.agreements.cookiesfield, these settings are used when the user logs in and copied to a cookie specifically used for the authenticated user.When the authenticated user changes their consent preferences later, the metadata field value for the user is updated.
The cookie consent preferences can be changed by the user at any time clicking the 'Cookie settings' link in the footer.
I have already added a link for the "End user agreement" and "Privacy policy" in the footer, however these pages will be added by different PRs.
Checklist
yarn run lintpackage.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.