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

sameSite isn't proxied to cookie store when using adaptive store #2230

Closed
makepanic opened this issue Aug 10, 2020 · 2 comments
Closed

sameSite isn't proxied to cookie store when using adaptive store #2230

makepanic opened this issue Aug 10, 2020 · 2 comments

Comments

@makepanic
Copy link
Contributor

makepanic commented Aug 10, 2020

Hi,

I was trying to configure the sameSite property (which was introduced in #1848) on an adaptive session store.

It seems like the sameSite property isn't proxied to the cookie store when it's created:

https:/simplabs/ember-simple-auth/blob/master/packages/ember-simple-auth/addon/session-stores/adaptive.js#L124

If I simply add sameSite to the properties list of getProperties, it works as expected.

Would it be ok if I create a PR to add the property there or do you also want an additional test (it might be a bit complicated because in adaptive store test, the cookie store is created outside the _createStore method)?

@marcoow
Copy link
Member

marcoow commented Aug 19, 2020

Yes, a PR for that would be cool but I think that'd need a number of things:

  • adding the new property to the adaptive store and documenting it
  • passing it to the cookie store
  • adding a test for that behavior (I know the adaptive store tests are a mess – if you need help with that, let me know)

makepanic added a commit to makepanic/ember-simple-auth that referenced this issue Aug 27, 2020
marcoow pushed a commit to makepanic/ember-simple-auth that referenced this issue Apr 29, 2022
marcoow pushed a commit to makepanic/ember-simple-auth that referenced this issue Apr 29, 2022
marcoow pushed a commit to makepanic/ember-simple-auth that referenced this issue Apr 29, 2022
@marcoow
Copy link
Member

marcoow commented Apr 29, 2022

This was fixed in #2312

@marcoow marcoow closed this as completed Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants