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

feat(Cookies): Updates WP Cookie Expiration to Same as Session Length #514

Conversation

menno-ll
Copy link
Contributor

@menno-ll menno-ll commented Feb 14, 2024

All Submissions:

Changes proposed in this Pull Request:

Closes #512 .

This change allows the user to, using a filter, make the WP user session length the same as the refresh token expiration.
So if a refresh token is valid for 30 days, the wordpress login cookies will then be set for 30 days instead of the default of 14 days. Please note that the remember-me option needs to be enabled as well, as wordpress does not allow changing the cookie expiration if remember-me is not used.

Please note that this PR requires the code of PR #513 , and also includes that code for now untill PR #513 is merged.

How to test the changes in this Pull Request:

  1. In your theme, add add_filter( 'openid-connect-generic-remember-me', '__return_true' ); to the functions.php file.
  2. In your theme, add add_filter( 'openid-connect-generic-use-token-refresh-expiration', '__return_true' ); to the functions.php file.
  3. Login using the openid connect plugin
  4. Open the development tools of your browser, and look at the cookie expiration. It should not be a session cookie, and it should be valid for the same time as the refresh token of the IDP.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully run tests with your changes locally?

Changelog entry

Added the openid-connect-generic-use-token-refresh-expiration filter to allow theme makers to login users with the same wp login cookie expiration time as the IDP.

@timnolte timnolte self-assigned this Mar 15, 2024
@timnolte timnolte added the status: needs review PR that needs review. label Mar 15, 2024
@timnolte timnolte added status: approved PRs that have been approved and ready to be merged. and removed status: needs review PR that needs review. labels Mar 17, 2024
@timnolte
Copy link
Collaborator

@menno-ll I'm good with merging this in but it looks like your other PR/change that I merged is it now causing a conflict. If you can resolve the conflict I'll get this merged in. Thanks!

@menno-ll
Copy link
Contributor Author

Hi @timnolte, thanks!
I will try to fix the conflict somewhere this week.

…ion_length

Signed-off-by: Menno van den Ende <50165380+menno-ll@users.noreply.github.com>
@menno-ll
Copy link
Contributor Author

Hi @timnolte, I have resolved the conflict.
Please note that the failing unit test is a failed integration with Slack?
It at least doesn't seem to be caused by this PR.

@timnolte
Copy link
Collaborator

@menno-ll yeah, there is an issue with the Coverage Report GitHub Action I'm using when PRs are opened from forks. I have an issue opened with the GitHub Actions maintainer in it. Thanks for fixing the conflict!

@timnolte timnolte changed the title Feature/wp cookie expiration same as session length feat(Session): Updates WP Cookie Expiration to Same as Session Length Mar 22, 2024
@timnolte timnolte changed the title feat(Session): Updates WP Cookie Expiration to Same as Session Length feat(Cookies): Updates WP Cookie Expiration to Same as Session Length Mar 22, 2024
@timnolte timnolte merged commit e0c8229 into oidc-wp:develop Mar 22, 2024
4 of 5 checks passed
@menno-ll
Copy link
Contributor Author

menno-ll commented Apr 3, 2024

Thanks, this will be of good use.
Do you have an idea when the new version will be available in the WP plugin repository?
Thanks in advance!

@timnolte
Copy link
Collaborator

timnolte commented Apr 3, 2024

@menno-ll I was sort of hoping to have the PKCE support in the next release but I will probably just go ahead and cut a new release this week at some point. Might be over the weekend.

@menno-ll
Copy link
Contributor Author

menno-ll commented Apr 4, 2024

All good! But no need to work in the weekend, everybody deserves some days off in a week 😄 .

@timnolte
Copy link
Collaborator

timnolte commented Apr 4, 2024

@menno-ll ah, well, since I really don't have the provisions to work on this during my day job, all of my work and support for this plugin happens during my nights and weekends. 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: approved PRs that have been approved and ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a filter / feature to login using rememberme, which allows longer valid login cookies
2 participants