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

Adds Option to define if a cookie should be set on login. #85

Merged
merged 5 commits into from
Oct 25, 2022

Conversation

henrikwirth
Copy link
Collaborator

@henrikwirth henrikwirth commented Mar 7, 2020

So the idea is to give an option to enable this behaviour.

One option would be to be able to define something in wp-config.php like so:

define( 'GRAPHQL_JWT_AUTH_SET_COOKIES', true );

Then the authenticate_user function would use wp_signon with cookies set to true instead of wp_authenticate.

I for example need the cookie to be set, so I can use it for the protected Downloads in WooCommerce.

Could be related to this issue: #73
We could use this approach to also set the RefreshToken as a HttpOnly cookie. Then you could decide to use the JWT in a HttpOnly environment if you need/want to, or still just take it from the response and handle it yourself.

Open for suggestions on how to approach this though. This one works fine for me so far.

@henrikwirth henrikwirth requested a review from jasonbahl March 7, 2020 17:22
@henrikwirth
Copy link
Collaborator Author

Maybe this would rather make sense as a boolean filter in the registration/login mutation?

Copy link

@renatonascalves renatonascalves left a comment

Choose a reason for hiding this comment

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

PHPDoc: correct inline comments.

src/Auth.php Outdated Show resolved Hide resolved
src/Auth.php Outdated Show resolved Hide resolved
henrikwirth and others added 3 commits March 10, 2020 13:19
Co-Authored-By: Renato Alves <renato@alley.co>
Co-Authored-By: Renato Alves <renato@alley.co>
Copy link

@victors1681 victors1681 left a comment

Choose a reason for hiding this comment

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

I did that change before noticed this PR.. looking good.

src/Auth.php Outdated Show resolved Hide resolved
@jasonbahl jasonbahl merged commit b08e06e into develop Oct 25, 2022
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.

4 participants