stevermeister / ngx-cookie-service

Angular (4.2+ ...12) service for cookies. Originally based on the `ng2-cookies` library.
https://www.npmjs.com/package/ngx-cookie-service
MIT License
547 stars 90 forks source link

Cookie not being set in Chrome versions > 80 with ngx-cookie-service 2.4.0 #86

Closed MorleyMinde closed 4 years ago

MorleyMinde commented 4 years ago

Expected Behavior

Using ngx-cookie-service 2.4.0, when a cookie is being set with key, value, Chrome browser should set the cookies in all browsers. this.cookieService.set('runOffline', 'true');

Actual Behavior

No cookie is being set in the chrome browsers with version > 80(the latest version).

Reason

For some reason there are other libraries that are creating cookies and the cookies found in document.cookie does not start with a space. So setting a cookie in this condition does not actually set the cookie.

Steps to Reproduce the Problem

Just download the latest version of chrome and set the cookie

Specifications

Steps to Reproduce the Problem

Set the cookie the traditional way by leaving a space initially eg.

document.cookie = " runOffline=true; " + document.cookie

Notice the space in the beginning for some reason works well.

gbardwell commented 4 years ago

Same issue.

kanelloc commented 4 years ago

Same issue here

lhopkins-tucasi commented 4 years ago

Same issue here

cacc559230 commented 4 years ago

Same issue

cristiansandru11 commented 4 years ago

same issue. Also tried to set secure to true and sameSite to None

Sravan-Nayini commented 4 years ago

we are facing prod issue due to this bug. Can someone fix and release new version ? I am using angular version 6

Yohandah commented 4 years ago

Doing so :

  this._cookieService.set(
      "loginToken",
      user.loginToken,
      "a date object"
    );

Result in this : image

image

package.json "ngx-cookie-service": "^2.4.0",

I guess this is not working because when setting a cookie with just a key and a value doesn't set the Secure flag !!

@matheusnascgomes @stevermeister

stevermeister commented 4 years ago

@Yohandah could you please check version 3.0.0?

Yohandah commented 4 years ago

@stevermeister Oh ! I see it was published yesterday I didn't see !

When I set the cookie with ngx cookie service it doesn't fire the warning! But when I refresh the page the warning appears again, this is the only cookie set on this page so I believe this is the one firing the warning ? When I delete it the warning disappears. And the secure column still isn't set while samesite is.

Anyone else can try to reproduce ?

marcin-piechaczek commented 4 years ago

Version 3.0 still doesn't work

macOS, Chrome Version 80.0.3987.132 (Official Build) (64-bit)

temporary fix: pass all arguments when setting a new cookie. this.cookieService.set('name', 'John', null, null, null, null, null);

stevermeister commented 4 years ago

@marcin-piechaczek could you please create a sandbox for it? I've tested with my Chrome 80 and it did work

Yohandah commented 4 years ago

@stevermeister I made a repo where you can see the warning : https://github.com/Yohandah/ngx-cookie-bug-secure

stevermeister commented 4 years ago

@Yohandah are you sure that you pushed all your changes? I see empty CLI project https://github.com/Yohandah/ngx-cookie-bug-secure/blob/master/src/app/app.component.ts

Yohandah commented 4 years ago

@stevermeister yeah my bad, it's good now

stevermeister commented 4 years ago

yes, I'm on it now. so sameSite should go with secure secure is only possible via https correct?

Yohandah commented 4 years ago

@stevermeister I believe that the default mode for samesite should be Samesite = lax (which is the browsers behavior if Samesite attribute isn't set, which wasn't the case before) so the secure flag doesn't need to be set with lax. I think that today ngx-cookie-service is setting Samesite to none if not specified ?

Then if the developer wants, he can override samesite with Samesite = none, and there : the secure flag is ALWAYS required (for latest versions of evergreen browsers from now on) => Rejected if secure not set

Cookies with the secure flag will only be accessible via https yes.

source: https://web.dev/samesite-cookies-explained/

stevermeister commented 4 years ago

ok, so it should help - https://github.com/stevermeister/ngx-cookie-service/pull/88/files?

Yohandah commented 4 years ago

@stevermeister yes it should fix it.

But in the case where a cookie is set with the samesite = none (via the cookieService.set method) the secure flag always has to be set. So I don't know if ngx-cookie-service should override the secure flag if mode is none

Doing this will result with a reject from the browser :

this._cookieService.set(
cookieName,
cookieValue,
expirationDate,
'/',
someData,
false, <- secure flag
'None' <- sameSite property
);

Doing this will work

this._cookieService.set(
cookieName,
cookieValue,
expirationDate,
'/',
someData,
true, <- secure flag
'None' <- sameSite property
);

Doing this will work

this._cookieService.set(
cookieName,
cookieValue,
expirationDate,
'/',
someData,
true, <- secure flag
'Strict' <- sameSite property
);
stevermeister commented 4 years ago

could you prepare PR for it? I think we can overwrite with a warning.

Yohandah commented 4 years ago

@stevermeister I'm not used to contribute please let me know if I need to change something in my code (especially about the warning message or unit tests, I wasn't able to write a check for the secure flag)

https://github.com/stevermeister/ngx-cookie-service/pull/89

TeodorKolev commented 4 years ago

None of those actually works. Chrome 80 unable to set cookie.

stevermeister commented 4 years ago

@TeodorKolev we need your sandbox with an example

freepirat11 commented 4 years ago

Still not working on 80

stevermeister commented 4 years ago

@freepirat11 sandbox?

freepirat11 commented 4 years ago

response_container_BBPPID{font-family: initial; font-size:initial; color: initial;} Currently moving to angular 9. Maybe it will work Razvan Popescu From: notifications@github.comSent: March 21, 2020 20:16To: ngx-cookie-service@noreply.github.comReply-to: reply@reply.github.comCc: popescu.razvan.92@gmail.com; mention@noreply.github.comSubject: Re: [stevermeister/ngx-cookie-service] Cookie not being set in Chrome versions > 80 with ngx-cookie-service 2.4.0 (#86)

@freepirat11 sandbox?

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or unsubscribe.

nitink-dev commented 4 years ago

It is not working for me also, Somebody has the solution? Unable to set cookies in chrome > 80.

nitink-dev commented 4 years ago

It is not working for me also, Somebody has the solution? Unable to set cookies in chrome > 80.

worked for me after upgrading version to 3.0.1, Thanks :)

pom11 commented 4 years ago

@stevermeister

WARNING in ./node_modules/ngx-cookie-service/fesm5/ngx-cookie-service.js 151:26-44 "export 'ɵɵdefineInjectable' was not found in '@angular/core'

WARNING in ./node_modules/ngx-cookie-service/fesm5/ngx-cookie-service.js 151:116-124 "export 'ɵɵinject' was not found in '@angular/core'

WARNING in ./node_modules/ngx-cookie-service/fesm5/ngx-cookie-service.js 151:136-144 "export 'ɵɵinject' was not found in '@angular/core'

Angular CLI: 7.3.9 Node: 13.10.1 OS: linux x64 Angular: 7.2.16 ... animations, common, compiler, compiler-cli, core, forms ... http, language-service, platform-browser ... platform-browser-dynamic, router

Package Version

@angular-devkit/architect 0.13.9 @angular-devkit/build-angular 0.13.9 @angular-devkit/build-optimizer 0.13.9 @angular-devkit/build-webpack 0.13.9 @angular-devkit/core 7.3.9 @angular-devkit/schematics 7.3.9 @angular/cli 7.3.9 @ngtools/webpack 7.3.9 @schematics/angular 7.3.9 @schematics/update 0.13.9 rxjs 6.3.3 typescript 3.2.4 webpack 4.29.0

{ "name": "app", "version": "0.0.1", "scripts": { "ng": "ng", "start": "ng serve -o", "build": "ng build", "test": "ng test", "lint": "ng lint", "e2e": "ng e2e" }, "private": true, "dependencies": { "@angular/animations": "~7.2.0", "@angular/common": "~7.2.0", "@angular/compiler": "~7.2.0", "@angular/core": "~7.2.0", "@angular/forms": "~7.2.0", "@angular/http": "^7.2.16", "@angular/platform-browser": "~7.2.0", "@angular/platform-browser-dynamic": "~7.2.0", "@angular/router": "~7.2.0", "asciichart": "^1.5.7", "blessed": "^0.1.81", "blessed-contrib": "^4.8.16", "classlist.js": "^1.1.20150312", "core-js": "^2.5.4", "jquery": "^3.4.1", "ngx-bootstrap-slider": "^1.7.0", "ngx-cookie-service": "^3", "rxjs": "~6.3.3", "stream": "0.0.2", "tslib": "^1.9.0", "web-animations-js": "^2.3.2", "zone.js": "~0.8.26" }, "devDependencies": { "@angular-devkit/build-angular": "~0.13.0", "@angular/cli": "~7.3.3", "@angular/compiler-cli": "~7.2.0", "@angular/language-service": "~7.2.0", "@types/node": "~8.9.4", "@types/jasmine": "~2.8.8", "@types/jasminewd2": "~2.0.3", "codelyzer": "~4.5.0", "jasmine-core": "~2.99.1", "jasmine-spec-reporter": "~4.2.1", "karma": "~4.0.0", "karma-chrome-launcher": "~2.2.0", "karma-coverage-istanbul-reporter": "~2.0.1", "karma-jasmine": "~1.1.2", "karma-jasmine-html-reporter": "^0.2.2", "protractor": "~5.4.0", "ts-node": "~7.0.0", "tslint": "~5.11.0", "typescript": "~3.2.2" } }

DOCKERFILE FROM node:13-alpine as build RUN apk add --no-cache g++ make libsass libsass-dev python build-base COPY /app-master /app-master WORKDIR /app-master EXPOSE 443 RUN npm install @angular/cli@8 @angular/core@8 -g RUN npm cache clean --force RUN npm install --save-dev RUN npm rebuild node-sass@4.13.0 --force RUN npm install @angular/http RUN rm -r package-lock.json && npm install

stevermeister commented 4 years ago

3.0.1?

freepirat11 commented 4 years ago

response_container_BBPPID{font-family: initial; font-size:initial; color: initial;} Yes. Forced update from 2.4. Razvan Popescu From: notifications@github.comSent: March 24, 2020 18:52To: ngx-cookie-service@noreply.github.comReply-to: reply@reply.github.comCc: popescu.razvan.92@gmail.com; mention@noreply.github.comSubject: Re: [stevermeister/ngx-cookie-service] Cookie not being set in Chrome versions > 80 with ngx-cookie-service 2.4.0 (#86)

3.0.1?

—You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or unsubscribe.

stevermeister commented 4 years ago

have you tried Angular version 8 or 9?

pom11 commented 4 years ago

response_container_BBPPID{font-family: initial; font-size:initial; color: initial;} Didnt managed to succesfully update. Still trying... No workaround for angular7? From: notifications@github.comSent: March 24, 2020 19:13To: ngx-cookie-service@noreply.github.comReply-to: reply@reply.github.comCc: parsecoriginalmastercraft@gmail.com; comment@noreply.github.comSubject: Re: [stevermeister/ngx-cookie-service] Cookie not being set in Chrome versions > 80 with ngx-cookie-service 2.4.0 (#86)

have you tried Angular version 8 or 9?

—You are receiving this because you commented.Reply to this email directly, view it on GitHub, or unsubscribe.

stevermeister commented 4 years ago

No workaround for angular7?

sorry, don't have time to support all the versions, trying to do my best to fix all the issues in the latest one. I did also hard work to update it from the old version to the nearest one. It looks like no fast solution for you for now.

DoanVanThuong commented 4 years ago

My Angular version is 8 and lib version 2.3.0 it still not working

robbiemu commented 3 years ago

I think that a solution to this problem would be to default to 'sameSite=Lax' like in the current version, or else to add 'secure' by default in addition to 'sameSite=None' which is already being added.

But this does bring up a couple of questions: why did it default to None? It is actually noted in the changelog. the reversion to Lax is not. What was the 2.4 branch about? it is not in the changelog. I don't see separate github branches here for the different releases, do you have to manually rerelease it?

And most importantly: Does anyone really need 2.x ? I mean did you upgrade the major number because it is actually incompatible? Because the easiest thing for everyone would be to just upgrade to the latest if that was available to them. Without a minor or even a major semver branch, I don't imagine it is too easy to release security patches and major bug patches, etc