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: Add support for secure attribute of local/refresh provider cookies #729

Merged
merged 7 commits into from
May 16, 2024

Conversation

matteioo
Copy link
Contributor

@matteioo matteioo commented Apr 7, 2024

πŸ”— Linked issue

#728

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This PR adds the possibility to optionally set the secure attribute of the local/refresh-provider cookies to true. Thus guaranteeing, that the cookie is only sent over HTTPS. I've already updated the docs to take the changes into consideration.

As this is my first contribution to a public repo like this one, I am glad for any feedback and sorry in advance if I categorized this PR and issue wrongly or forgot to add something necessary.

PS: I've also moved a command which was wrongly on a line below (explanation of the duration in minutes regarding the maxAgeInSeconds field).

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have added tests (if possible).
  • I have updated the documentation accordingly.

@zoey-kaiser zoey-kaiser requested review from phoenix-ru and zoey-kaiser and removed request for phoenix-ru May 9, 2024 10:49
@zoey-kaiser zoey-kaiser added enhancement An improvement that needs to be added provider-local An issue with the local provider provider-refresh An issue with the refresh provider labels May 9, 2024
phoenix-ru
phoenix-ru previously approved these changes May 16, 2024
@phoenix-ru
Copy link
Collaborator

Hi @matteioo, thank you for your contribution. I have done both functional and code review, it all looks good to me!

@phoenix-ru phoenix-ru merged commit f3fc581 into sidebase:main May 16, 2024
4 checks passed
@matteioo
Copy link
Contributor Author

Hi @matteioo, thank you for your contribution. I have done both functional and code review, it all looks good to me!

Thank you very much. I'm very happy to hear that, as this is my first open source contribution to a public repository.

Copy link
Contributor Author

@matteioo matteioo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My formatting was based on the _rawTokenCookie in the refresh provider:

https://github.com/sidebase/nuxt-auth/blob/main/src/runtime/composables/refresh/useAuthState.ts#L16-L25

If you want, I can change the formatting of the _rawTokenCookie so that it matches this updated version.

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 provider-local An issue with the local provider provider-refresh An issue with the refresh provider
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants