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

Add refresh token functionality #24

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

arnorietdijk
Copy link

  • Added jwt-refresh-token-bundle
  • Updated readme with how to configure the refresh token
Production Changes From To Compare
gesdinet/jwt-refresh-token-bundle NEW v0.9.1

Copy link
Member

@RonRademaker RonRademaker left a comment

Choose a reason for hiding this comment

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

I'd prefer to have this optional, ie. allow the user to choose to activate refresh tokens or not

@arnorietdijk
Copy link
Author

arnorietdijk commented Nov 24, 2020

I'd prefer to have this optional, ie. allow the user to choose to activate refresh tokens or not

They have to add it their self to the files named in the readme, so they have a choice whether they use it or not. Maybe it's better to point it out more that the refresh token is optional?

@RonRademaker
Copy link
Member

I'd prefer to have this optional, ie. allow the user to choose to activate refresh tokens or not

They have to add it their self to the files named in the readme, so they have a choice whether they use it or not. Maybe it's better to point it out more that the refresh token is optional?

And maybe add the dependency to suggest?

@arnorietdijk
Copy link
Author

I'd prefer to have this optional, ie. allow the user to choose to activate refresh tokens or not

They have to add it their self to the files named in the readme, so they have a choice whether they use it or not. Maybe it's better to point it out more that the refresh token is optional?

And maybe add the dependency to suggest?

Which dependency?

@RonRademaker
Copy link
Member

I'd prefer to have this optional, ie. allow the user to choose to activate refresh tokens or not

They have to add it their self to the files named in the readme, so they have a choice whether they use it or not. Maybe it's better to point it out more that the refresh token is optional?

And maybe add the dependency to suggest?

Which dependency?

gesdinet/jwt-refresh-token-bundle

@arnorietdijk
Copy link
Author

I'd prefer to have this optional, ie. allow the user to choose to activate refresh tokens or not

They have to add it their self to the files named in the readme, so they have a choice whether they use it or not. Maybe it's better to point it out more that the refresh token is optional?

And maybe add the dependency to suggest?

Which dependency?

gesdinet/jwt-refresh-token-bundle

So just to be clear, you want to be able to add the bundle/dependency yourself if you want to use it instead of having it already required in the bundle?

@RonRademaker
Copy link
Member

Yes, composer has a suggests option

@arnorietdijk
Copy link
Author

Yes, composer has a suggests option

First time I heard about it but I agree with this way of adding the functionality.
I have changed it in the latest commit.

@RonRademaker RonRademaker requested a review from a team November 24, 2020 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants