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

feat: local with cookie session #564

Closed
wants to merge 4 commits into from

Conversation

benjipott
Copy link

A simple PR to prevent create a new provider like #495 and set session in header

Closes # .

Checklist:

  • issue number linked above after pound (#)
    • replace "Closes " with "Contributes to" or other if this PR does not close the issue
  • manually checked my feature / checking not applicable
  • wrote tests / testing not applicable
  • attached screenshots / screenshot not applicable

@benjipott benjipott force-pushed the feat-local-cookies-options branch 3 times, most recently from e02becd to 36e55ce Compare November 13, 2023 22:02
@benjipott benjipott mentioned this pull request Nov 13, 2023
4 tasks
@benjipott benjipott force-pushed the feat-local-cookies-options branch 7 times, most recently from 0f0a1ec to 0669c3a Compare November 14, 2023 11:32
@zoey-kaiser
Copy link
Member

Hi @benjipott 👋

Thanks for your contribution! Feel free to let me know, when I can do a first review!

@benjipott benjipott force-pushed the feat-local-cookies-options branch from deace67 to 98406bb Compare December 4, 2023 11:37
@benjipott
Copy link
Author

@zoey-kaiser happy to see if that can help

@zoey-kaiser
Copy link
Member

Hi @benjipott 👋

I will need to discuss this internally first. My only reservations with this PR would be that cookie based authentication is less secure then session based authentication. Therefore we would need to discuss if we want to provide this feature and if we do, how we could ensure developers know that a cookie based solution is less secure

https://dev.to/emmykolic/cookies-based-authentication-vs-session-based-authentication-1f6

@benjipott
Copy link
Author

Sure but the real target of this PR is to allow all devs to keep their integration choices based on their backend infrastructure.

And for me, session token is better and more secure than cookies stored on devices.

Actually, useCookie is implemented without option like domain, secure configuration.

Keep in mind, if you don't give developers a choice, they won't use your tools.

Happy to help.
Cheers 🍻

@benjipott benjipott force-pushed the feat-local-cookies-options branch from 98406bb to ee58fcf Compare December 5, 2023 11:08
@benjipott benjipott force-pushed the feat-local-cookies-options branch from ee58fcf to 28016db Compare December 5, 2023 11:13
@zoey-kaiser
Copy link
Member

Sure but the real target of this PR is to allow all devs to keep their integration choices based on their backend infrastructure.

I definitely agree! I can also see some situations in which a cookie provider would make more sense.

My current plan is the following:

@zoey-kaiser zoey-kaiser added p2 Nice to have enhancement An improvement that needs to be added provider-new A new provider labels Feb 23, 2024
@benjipott benjipott closed this Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An improvement that needs to be added p2 Nice to have provider-new A new provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants