Skip to content

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
@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.

3 participants