Modify ajax url to work properly with FORCE_SSL_ADMIN

[will_c]

With FORCE_SSL_ADMIN enabled on my site using BP 1.7-trunk and WP 3.5 I am experiencing issues with AJAX that are preventing the activity feed from functioning properly (I cannot make new updates or comment on existing activity items). I believe this problem is caused by having the ajax url using the https protocol rather than http.

To replicate this issue, turn on FORCE_ADMIN_SSL in your wp-config file and then try to post to the newsfeed (while browsing your site over http). In my testing, I receive an error saying:

"Are you sure you want to do this?
Please try again."

If you then switch to https browsing or disable FORCE_SSL_ADMIN, you should be able to post to the activity feed again.

I believe the issue is that admin_url() (in bp-core/bp-core-cssjs.php and bp-templates/bp-legacy/buddypress-functions.php) returns the https variant when FORCE_SSL_ADMIN is enabled (as it should). This causes problems in my setup for sites that are using http on the frontend.

My proposed solution would be to switch to using network_site_url('/wp-admin/admin-ajax.php'), which would also account for multisite installations, or using site_url('/wp-admin/admin-ajax.php') if it's preferable to only deal with single site installs at the moment.

This is my first patch for BuddyPress, so let me know if there are reporting conventions that I should follow in the future. Also, I realize there are probably other approaches to fixing this issue or there may be other logic behind the use of admin_url that I'm missing, so I'd appreciate any feedback.

[will_c]

I was about to report a similar issue with bbPress 2.2.3 (which uses the same approach), but found out that bbPress 2.3 includes a new way to handle AJAX. It is included in ​http://bbpress.svn.wordpress.org/trunk/includes/common/ajax.php. It looks like bbPress now uses it's own AJAX handler and circumvents the reliance on admin-ajax.php.

I realize this might take a lot of work to implement for BP 1.7, but may be relevant for future releases. I'm happy to submit a new enhancement ticket for this approach, but still think my original suggestion would be a good patch for 1.7 because it will prevent errors for people using force_ssl_admin.

Just wanted to bring this up as an alternate way to address this issue. Not sure if it's already been under discussion previously.

[DJPaul]

Kinda relevant: ​#WP12400

[r-a-y]

will_c: Appreciate the patch. I don't have a SSL install to currently verify this, but your thoughts are valid.

If we don't address this for 1.7, you can temporarily filter 'admin_url' and force the AJAX url to use non-SSL.

Reference:
​https://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/link-template.php#L1994

Update: Was pretty simple to set up a SSL cert on my local install. Will be testing this some time this week.

[r-a-y]

will_c: I have duplicated your problem and your fix works.

The question, as you noted in your ticket, is whether we should use site_url() vs. network_site_url().

Since the FORCE_SSL_ADMIN define is in wp-config.php, it probably makes sense to use network_site_url().

If other dev can give some feedback on this, that would be great. Otherwise, I'll commit this.

Thanks for contributing, will_c.

[boonebgorges]

Sounds good to me, r-a-y.

[r-a-y]

(In [6790]) Use network_site_url() to build the 'ajaxurl' JS variable.

Previously, for sites on HTTPS and using the 'FORCE_SSL_ADMIN' constant,
when viewing the frontend of the site on HTTP, the 'ajaxurl' JS
variable would still use HTTPS due to the usage of admin_url().

This would break most BP AJAX actions that use check_admin_referer() to
verify the nonce.

Switching admin_url() to use network_site_url() fixes this problem.

Fixes #4761. Props will_c.

[boonebgorges]

The changes introduced in r6790 do not work properly when BP_ROOT_BLOG is defined as something other than the default. That's because network_site_url() will always point to the main site's wp-admin, and on some setups, some network users do not have access to any wp-admin URLs on the main site. Thus, for those users, AJAX requests fail silently. See #4850

I'll try to figure something out (and write some tests so we don't get in this mess again). But in the meantime, if we can't figure out a solution we are confident with, we should revert r6790 for 1.7, since the HTTPS issue here is not a regression, but breaking AJAX for alternative root blog setups *is*.

[r-a-y]

How about?

admin_url( 'admin-ajax.php', is_ssl() ? 'admin' : 'http' )

The second parameter in admin_url() can be used to enforce which scheme to use.

I've tested this with FORCE_SSL_ADMIN defined and the AJAX URL does not use SSL when the frontend of the site is on HTTP.

[r-a-y]

Revert to admin_url(), but use second parameter to define the scheme

[boonebgorges]

This looks good, r-a-y. I wrote some tests for it and it looks like it's passing all the problematic cases: ​https://github.com/buddypress/BuddyPress-Unit-Tests/commit/5cec5a154f568c83fd97f1e8ce5333a9183f2a4d

[boonebgorges]

(In [6826]) Improvements in the way ajaxurl is determined

Introduces bp_core_ajax_url() for easier unit testing

Switches bp_core_ajax_url() back to using admin_url(), but now it's more
sensitive to various HTTPS setups, including FORCE_SSL_ADMIN.

Corresponding unit test: ​https://github.com/buddypress/BuddyPress-Unit-Tests/commit/5cec5a154f568c83fd97f1e8ce5333a9183f2a4d

Partially reverts r6790. See #4850.

Fixes #4761

Props r-a-y

[will_c]

Apologies for the trouble boonebgorges. Thanks to you and r-a-y for the solution.

[boonebgorges]

In 7059:

Fixes test_bp_core_ajax_url() for new multisite unit test global manipulation

The changes introduced in r7058, which adds improvemens to go_to() for use on
multisite installations, uncovered a limitation in the test_bp_core_ajax_url()
test. Basically, it shows that bp_core_ajax_url() will always point to
admin-ajax.php of the current site, *not* the root blog. This should not be a
problem in most cases, though we should keep an eye out in the future to see
whether problems arise on mapped domains, where cross-domain AJAX calls may be
affected.

See #4761