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

Added auth through Laravel Passport auth server #335

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

Conversation

KyleMassacre
Copy link

No description provided.

@matheusrocha89
Copy link
Contributor

Hi, @KyleMassacre.
Thanks for your contribution. Just want to ask you about some points:

  • Did you test it to see if it's working correctly?
  • Since this is specific to Laravel Passport plugin I think it's better to rename to laravelPassport or something similar since passport is a very generic name. There is a middleware for express to make authentication called passport too, so another name will be more clear for developers that will use.

@KyleMassacre
Copy link
Author

Sorry for the late reply.

Did you test it to see if it's working correctly?

Yes, I did test test it and it worked. One thing I noticed is that for a brief second some sort of 404 error pops up before the in app browser closes and I couldnt figure out what that was but after looking at other modules I didnt know what was different.

Did you happen to find an error by chance?

Since this is specific to Laravel Passport plugin I think it's better to rename to laravelPassport or something similar since passport is a very generic name. There is a middleware for express to make authentication called passport too, so another name will be more clear for developers that will use.

I went ahead and change it to laravelPassport and pushed it

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