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

Logins redirect to Membership Options Page when site-wide SSL in use #1009

Closed
raamdev opened this issue Oct 27, 2016 · 9 comments
Closed

Logins redirect to Membership Options Page when site-wide SSL in use #1009

raamdev opened this issue Oct 27, 2016 · 9 comments

Comments

@raamdev
Copy link
Contributor

raamdev commented Oct 27, 2016

We have had several reports of an issue where users would incorrectly be redirected to the Membership Options Page when logging in, instead of being redirected to the Login Welcome Page as would be expected. I have seen many reports like this over the past several months, but I always chocked them up to a plugin conflict, that is until I saw these two posts on the forums:

Those indicate that the default s2Member configuration causes this mysterious redirect problem when a site is configured to use SSL. The reports vary with how the sites are configured to use site-wide SSL, but my feeling is that this issue is big enough and widespread enough that it deserves attention.

With SSL on the rise, I don't see any reason s2Member should be forcing non-SSL redirects and I propose we change this default to "No, do NOT modify". I also propose that we force this change with the next release and update this option to "No, do NOT modify".

2016-10-27_17-00-59

@raamdev
Copy link
Contributor Author

raamdev commented Oct 27, 2016

Noting that I have NOT reproduced this issue myself yet. I wanted to file this report immediately as I have seen these exact same symptoms at least a dozen times. I'm marking this as needs testing.

jaswrks pushed a commit that referenced this issue Oct 28, 2016
Force non-SSL login off by default. See: #1009
jaswrks pushed a commit that referenced this issue Oct 28, 2016
Force non-SSL login off by default. See: #1009
@jaswrks
Copy link
Contributor

jaswrks commented Oct 28, 2016

Next Release Changelog:

  • SSL Compatibility & Option Deprecation: In previous versions of s2Member there was a setting in the UI that allowed you to force non-SSL redirects to the Login Welcome Page. By popular demand, this setting has been deprecated and removed from the UI.

    New Approach: The new approach taken in the latest release of s2Member is to automatically detect when a non-SSL redirection should occur, and when it should not occur (i.e., when the default WordPress core behavior should remain as-is).

    s2Member does this by looking at your configured siteurl option in WordPress. If it begins with https:// (indicating that your entire site is served over SSL), non-SSL redirects will no longer be forced by s2Member, which resolves problems on many sites that serve their entire site over SSL (a growing trend over the past couple years).

    Conversely, if your configured siteurl option in WordPress does NOT begin with https:// (e.g., just plain http://), then a non-SSL redirect is forced, as necessary, in order to avoid login cookie conflicts; i.e., the old behavior is preserved by this automatic detection.

    Overall, this new approach improves compatibility with WordPress core, particularly on sites that serve all of their pages over https:// (as recommended by Google).

    Backward Compatibility: As noted previously, the old option that allowed you to configure s2Member to force non-SSL redirects to the Login Welcome Page has been officially deprecated and removed from the UI. However, the old option does still exist internally, but only for backward compatibility. A WordPress filter is exposed that allows developers to alter the old setting if necessary. You can use the filter to force a true or false value.

    <?php
    add_filter('ws_plugin__s2member_login_redirection_always_http', '__return_true');
    // OR add_filter('ws_plugin__s2member_login_redirection_always_http', '__return_false');

@jaswrks
Copy link
Contributor

jaswrks commented Oct 28, 2016

@raamdev This has been completed. Please review and close if you are satisfied with the changelog above.

@jaswrks jaswrks self-assigned this Oct 28, 2016
@KTS915
Copy link

KTS915 commented Oct 28, 2016

This also takes care of #977.

@raamdev
Copy link
Contributor Author

raamdev commented Oct 28, 2016

@jaswsinc writes...

s2Member does this by looking at your configured siteurl option in WordPress. If it begins with https:// (indicating that your entire site is served over SSL), non-SSL redirects will no longer be forced by s2Member
[...]
Conversely, if your configured siteurl option in WordPress does NOT begin with https:// (e.g., just plain http://), then a non-SSL redirect is forced

Hmmm, my first thought when I read that was to wonder if "Force SSL" plugins that might be in use out there would still cause the original problem (Login Welcome Page being redirected to Membership Options Page). If those plugins don't change the siteurl in WordPress Core, then the above fix won't help at all, correct?

Here's a list of SSL plugins in the WordPress Plugin Directory, and here's one with 60,000+ active installs.

We should review the source code of a few of those plugins to see if the above fix is going to be sufficient.

@KTS915 writes...

This also takes care of #977.

Thanks for the heads up! :-) I've closed that issue in favor of this one.

@jaswrks
Copy link
Contributor

jaswrks commented Nov 11, 2016

Updating changelog to the following. A last-minute update to the work completed previously improves this further by also looking at the FORCE_SSL_LOGIN and FORCE_SSL_ADMIN constants in WordPress via the force_ssl_admin() function.


  • SSL Compatibility & Option Deprecation: In previous versions of s2Member there was a setting in the UI that allowed you to force non-SSL redirects to the Login Welcome Page. By popular demand, this setting has been deprecated and removed from the UI.

    New Approach: The new approach taken in the latest release of s2Member is to automatically detect when a non-SSL redirection should occur, and when it should not occur (i.e., when the default WordPress core behavior should remain as-is).

    s2Member does this by looking at the FORCE_SSL_LOGIN and FORCE_SSL_ADMIN settings in WordPress, and also at your configured siteurl option in WordPress. If you are not forcing SSL logins, or your siteurl begins with https:// (indicating that your entire site is served over SSL), non-SSL redirects will no longer be forced by s2Member, which resolves problems on many sites that serve their entire site over SSL (a growing trend over the past couple years).

    Conversely, if FORCE_SSL_LOGIN or FORCE_SSL_ADMIN are true, and your configured siteurl option in WordPress does NOT begin with https:// (e.g., just plain http://), then a non-SSL redirect is forced, as necessary, in order to avoid login cookie conflicts; i.e., the old behavior is preserved by this automatic detection.

    Overall, this new approach improves compatibility with WordPress core, particularly on sites that serve all of their pages over https:// (as recommended by Google).

    Backward Compatibility: As noted previously, the old option that allowed you to configure s2Member to force non-SSL redirects to the Login Welcome Page has been officially deprecated and removed from the UI. However, the old option does still exist internally, but only for backward compatibility. A WordPress filter is exposed that allows developers to alter the old setting if necessary. You can use the filter to force a true or false value.

    <?php
    add_filter('ws_plugin__s2member_login_redirection_always_http', '__return_true');
    // OR add_filter('ws_plugin__s2member_login_redirection_always_http', '__return_false');

@jaswrks
Copy link
Contributor

jaswrks commented Nov 11, 2016

@raamdev writes...

If those plugins don't change the siteurl in WordPress Core, then the above fix won't help at all, correct?

Plugins like those are forcing SSL to begin with. If you log into the site over SSL (i.e., a plugin like one of those forces SSL), then whenever you log into the site you are already on the https:// protocol and that's how the redirection occurs as well; i.e., following the same protocol that you logged in with.

Compatibility should be improved in this regard now, because now the default behavior in s2Member is simply to use the default WordPress behavior (i.e., keep the same protocol that you are logging in with. Starting with the next release, the only time s2Member will force a change in protocol when redirecting, is when the following conditions are true.

  • force_ssl_admin() returns true in your installation of WordPress; i.e., you have set one of FORCE_SSL_LOGIN (now deprecated) or FORCE_SSL_ADMIN to true. This indicates that logins always occur over https://.
  • And, your configured siteurl does not begin with https://, indicating the front-end of your site is to be served over http:// and not over https://. In this case, the redirect protocol should change. WordPress core uses the same approach when it sets the logged-in cookie. Therefore, a LWP redirection needs to match that behavior so that when a user arrives at your LWP the logged-in cookie can be read as expected.

I do understand what you're saying though. A way of this going wrong is to install a force SSL plugin and also meet the above criteria for a redirection protocol change in s2Member. However, I'm not seeing any way to reliably detect this (more than we already are) across a variety of plugins that force SSL. We would need to do an integration with each plugin for it to work 100% in all cases without issue.

Also, the risk of this happening seems very low to me. If you're using a force SSL plugin, then you are likely not using force_ssl_admin() (there's no need). In which case the protocol change does not occur in s2Member, and the default behavior will do fine; i.e., the protocol does not change when a redirection occurs, so the user remains on an SSL connection.

In scenarios where this does become an issue, the filter we expose can be used to gain more control over this behavior and dictate when/if http:// should be forced on login redirection.

<?php
add_filter('ws_plugin__s2member_login_redirection_always_http', '__return_true');
// OR add_filter('ws_plugin__s2member_login_redirection_always_http', '__return_false');

@renzms
Copy link
Contributor

renzms commented Nov 21, 2016

@jaswsinc @raamdev

Confirmed working in s2Member v161117-RC.

The option no longer appears in the panel and login redirection happens correctly for the respective site configurations. Setting the siteurl to http:// and logging in from http:// will redirect you to http://. The same is true for setting the siteurl to https:// and logging in from https://, it will redirect you to https://.

screen shot 2016-11-21 at 9 19 31 pm

@raamdev
Copy link
Contributor Author

raamdev commented Nov 29, 2016

s2Member v161129 has been released and includes changes from this GitHub Issue. See the v161129 announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#1009).

@raamdev raamdev closed this as completed Nov 29, 2016
@wpsharks wpsharks locked and limited conversation to collaborators Nov 29, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants