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

Laravel API quickstart #2200

Merged
merged 20 commits into from
Jul 31, 2023
Merged

Laravel API quickstart #2200

merged 20 commits into from
Jul 31, 2023

Conversation

vcampitelli
Copy link
Contributor

@vcampitelli vcampitelli commented May 25, 2023

Adding Laravel API quickstart.

PS: this is a pretty long quickstart, so I tried something different and included some diffs instead of the whole files when we needed to change something that the Laravel project installer already created. Please let me know if you think that's not good and I'll include the whole files instead.

Closes FusionAuth/fusionauth-issues#2283


@vcampitelli vcampitelli added the content large piece of content label May 25, 2023
@vcampitelli vcampitelli requested a review from mooreds May 25, 2023 14:46
@vcampitelli vcampitelli self-assigned this May 25, 2023
site/_layouts/doc.liquid Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
@vcampitelli
Copy link
Contributor Author

@mooreds I've added our newly-published fusionauth/jwt-auth-webtoken-provider library so this is ready for another review. There's still a pending comment about passwords that I need your input.

@vcampitelli vcampitelli requested a review from mooreds July 3, 2023 13:05
Copy link
Contributor

@mooreds mooreds left a comment

Choose a reason for hiding this comment

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

A few changes requested.

site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
site/docs/v1/tech/tutorials/integrate-laravel-api.md Outdated Show resolved Hide resolved
@vcampitelli
Copy link
Contributor Author

@mooreds it seems that I didn't change the article after I started using the hosted backend 🤦 I've now rewritten some things.

@vcampitelli vcampitelli requested a review from mooreds July 4, 2023 15:40
@mooreds
Copy link
Contributor

mooreds commented Jul 17, 2023

Hiya @vcampitelli . A few things:

  • I made a few small wording changes and merged master in.
  • Now that we have moved to the quickstarts page being managed by astro, please update this file with the laravel quickstart
  • I know we went back and forth over how to get the token. I think what you have here is a bit confusing (you log into a page and then get an API result). But because of our move to v3 quickstarts, I'm not sure it is worth changing here.
  • I don't see where you validate the JWT provided? We want to make sure people know to check the aud and iss claim, as well as verify the signature. What am I missing?

@vcampitelli
Copy link
Contributor Author

@mooreds

I made a few small wording changes and merged master in.

Thanks! I also merged it again to update the quickstarts home page.

Now that we have moved to the quickstarts page being managed by astro, please update this file with the laravel quickstart

Done

I know we went back and forth over how to get the token. I think what you have here is a bit confusing (you log into a page and then get an API result). But because of our move to v3 quickstarts, I'm not sure it is worth changing here.

I don't get it why it is confusing. I'm logging in, grabbing the JWT from FusionAuth and passing it to the Laravel API to retrieve the messages. What should I have done differently?

I don't see where you validate the JWT provided? We want to make sure people know to check the aud and iss claim, as well as verify the signature. What am I missing?

The underlying tymon/jwt-auth library verifies the signature using JWKS. But we are not validating those claims. Do you think I should update our Laravel library to do that?

@mooreds
Copy link
Contributor

mooreds commented Jul 26, 2023

I don't get it why it is confusing. I'm logging in, grabbing the JWT from FusionAuth and passing it to the Laravel API to retrieve the messages. What should I have done differently?

I think this will be okay for now, but the two ways we've done this for API quickstarts is:

You chose a third option, which is:

The underlying tymon/jwt-auth library verifies the signature using JWKS. But we are not validating those claims. Do you think I should update our Laravel library to do that?

Yes, we should at a minimum validate the iss and aud claims are what we expect them to be. Here's the guidance we give on validating JWTs: https://fusionauth.io/articles/tokens/building-a-secure-jwt#consuming-a-jwt

Our examples should follow it as best as we can. (I don't know any libraries that validate the typ header, though.)

@vcampitelli
Copy link
Contributor Author

@mooreds

  1. Yeah, you are right. I created it based on the .NET API quickstart, which uses this approach. The sad thing is that it's only being used to call the /me and /messages endpoints, as I'm actually using the cookies from the hosted backend. So I really mixed things up.
  2. I've improved the app & docs to actually validate both aud and iss claims

@vcampitelli vcampitelli requested a review from mooreds July 28, 2023 12:21
Copy link
Contributor

@mooreds mooreds left a comment

Choose a reason for hiding this comment

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

LGTM!

@mooreds
Copy link
Contributor

mooreds commented Jul 28, 2023

@vcampitelli ship it!

🚢 🚢 🚢

@vcampitelli vcampitelli merged commit 19dec6f into master Jul 31, 2023
2 checks passed
@vcampitelli vcampitelli deleted the quickstart/laravel-api branch July 31, 2023 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
content large piece of content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create Laravel API quickstart
2 participants