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

menno-ll commented 7 months ago

All Submissions:

[x] Have you followed the plugin Contributing guideline?
[x] Does your code follow the WordPress' coding standards?
[x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?

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:

In your theme, add add_filter( 'openid-connect-generic-remember-me', '__return_true' ); to the functions.php file.
In your theme, add add_filter( 'openid-connect-generic-use-token-refresh-expiration', '__return_true' ); to the functions.php file.
Login using the openid connect plugin
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:

[x] Have you added an explanation of what your changes do and why you'd like us to include them?
[x] Have you written new tests for your changes, as applicable?
[x] 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 commented 6 months ago

@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 commented 6 months ago

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

menno-ll commented 6 months ago

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 commented 6 months ago

@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!

menno-ll commented 6 months ago

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 commented 6 months ago

@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 commented 6 months ago

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

timnolte commented 6 months ago

@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. 😉

oidc-wp / openid-connect-generic