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 set and clear token methods #554

Merged
merged 3 commits into from
Oct 23, 2023
Merged

Conversation

IsraelOrtuno
Copy link
Contributor

Added getToken and clearToken to local provider useAuthState composable for convenience. It's quite common to have cases where you have to "log in" the user manually when using this provider like when using social authentication, impersonation from a back-office or a custom register process. This PR just adds a couple of methods to make setting the cookie token more convenient and obvious.

Checklist:

  • issue number linked above after pound
  • manually checked my feature / checking not applicable
  • wrote tests / testing not applicable
  • attached screenshots / screenshot not applicable

@zoey-kaiser
Copy link
Member

Hi @IsraelOrtuno 👋

Thank you for your contribution! I think it would be super helpful to create a quick note on this feature in the docs, to explain how to use this improvement. I think it would be good to write a quick section under docs/nuxt-auth/0.6/resources.

@IsraelOrtuno
Copy link
Contributor Author

Hi @IsraelOrtuno 👋

Thank you for your contribution! I think it would be super helpful to create a quick note on this feature in the docs, to explain how to use this improvement. I think it would be good to write a quick section under docs/nuxt-auth/0.6/resources.

Yes I thought so too but wasn't sure where to add it. I have updated the PR with a dedicated docs section which I guess it can be rearranged in the future 👍

@zoey-kaiser
Copy link
Member

Thanks for adding the docs! I think the texts are good and well written, however after some more thinking, I would place them at the bottom of this file with a new heading, called useAuthState() Composable. You could then also mention that this section is meant for use with the local provider.

Another thing I wanted to point out, AFAIK useAuthState is used by both the local and authjs providers, does including this feature break anything or interact with the Authjs provider? If you have not had the chance to test this yet, I can also do it today!

@IsraelOrtuno
Copy link
Contributor Author

I don't think it has anything to do with authjs nor breaking anything. This is a local provider which is likely to be used with custom backend solutions.

I have also updated the docs with a more extensive information about what useAuthState composable does not only for local but for authjs too 👍

Copy link
Member

@zoey-kaiser zoey-kaiser left a comment

Choose a reason for hiding this comment

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

Thank you so much for your contribution. I will merge this now, wait till I close issue #553 and then move on to releasing this in 0.6.0-rc.1!

@zoey-kaiser zoey-kaiser merged commit 6190dec into sidebase:main Oct 23, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants