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: support for csrf tokens that apply automatically #317

Merged
merged 4 commits into from
Sep 9, 2021

Conversation

james2doyle
Copy link
Contributor

These additions allow supporting CSRF tokens for apps using Laravel Sanctum. You can provide a URL that gives you the token, which it then applies, to each URL being called.

This should address #243.

There are probably some docs updates here just to call this feature out, but I can do these when/if this is accepted.

Copy link
Contributor

@shalvah shalvah left a comment

Choose a reason for hiding this comment

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

Honestly don't know if this works, because I don't use Sanctum, but I'll take your word for it (although I will test that non-Sanctum still works). Nice work, but a few notes.

Instructions for updating docs: https://scribe.knuckles.wtf/laravel/contributing#updating-documentation. At the very least, you'll need to update the config reference and add a section called "CSRF and Laravel Sanctum" under this. Do your best but don't obsess over the details, since I'll probably make some finishing touches.

config/scribe.php Outdated Show resolved Hide resolved
resources/views/themes/default/index.blade.php Outdated Show resolved Hide resolved
null coalesce for possible missing config value

Co-authored-by: Shalvah <[email protected]>
config/scribe.php Outdated Show resolved Hide resolved
config/scribe.php Outdated Show resolved Hide resolved
@shalvah
Copy link
Contributor

shalvah commented Sep 9, 2021

Thanks! Please update the docs.

@james2doyle
Copy link
Contributor Author

Great! Got a PR open here: knuckleswtf/scribe-docs#4

@shalvah
Copy link
Contributor

shalvah commented Sep 9, 2021

I made some changes to the code you pushed: a56eafe. It seems good, but you should pull and verify that it works for you.

Also, I changed the default value of csrf_cookie_name` to match what Laravel's docs say:

During this request, Laravel will set an XSRF-TOKEN cookie containing the current CSRF token. This token should then be passed in an X-XSRF-TOKEN header on subsequent requests

Apparently, the csrf_cookie_name that Laravel sets is by default XSRF-TOKEN, not X-XSRF-TOKEN (that is the header/cookie that we set.

And now I'm wondering if we need that csrf_cookie_name setting at all. I don't see any way to change the CSRF token Sanctum uses, so I think that setting is not needed.

@shalvah
Copy link
Contributor

shalvah commented Sep 9, 2021

removing the csrf_cookie_name in 249e6e6. I see that it may be useful for other non-Sanctum SPA systems, but that means we'd also have to add csrf_header_name too, and I want to keep things simple for now. The CSRF URL is the one that's most likely to vary, so I'll keep only that for now.

Okay, closing shop for now, so I've tagged v3.1.0.0. Let me know if something's broken.

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