Skip to:

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#4761 closed defect (bug) (fixed)

Modify ajax url to work properly with FORCE_SSL_ADMIN

Reported by: will_c's profile will_c Owned by:
Milestone: 1.7 Priority: normal
Severity: major Version: 1.7
Component: Core Keywords:


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.

Attachments (2)

admin_ssl_ajax_fix_patch.txt (1022 bytes) - added by will_c 12 years ago.
4761.01.patch (1.1 KB) - added by r-a-y 11 years ago.
Revert to admin_url(), but use second parameter to define the scheme

Download all attachments as: .zip

Change History (16)

#1 @DJPaul
12 years ago

  • Milestone changed from Awaiting Review to 1.7

#2 @will_c
12 years ago

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

#3 @DJPaul
12 years ago

Kinda relevant: #WP12400

#4 @r-a-y
11 years ago

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.


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

Last edited 11 years ago by r-a-y (previous) (diff)

#5 @r-a-y
11 years ago

  • Keywords commit added

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 another dev can give some feedback on this, that would be great. Otherwise, I'll commit this.

Thanks for contributing, will_c.

Last edited 11 years ago by r-a-y (previous) (diff)

#6 @boonebgorges
11 years ago

Sounds good to me, r-a-y.

#7 @r-a-y
11 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(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.

#8 @boonebgorges
11 years ago

  • Keywords needs-patch needs-unit-tests added; dev-feedback commit removed
  • Resolution fixed deleted
  • Severity changed from normal to major
  • Status changed from closed to reopened

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*.

#9 @r-a-y
11 years ago

  • Keywords dev-feedback added

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.

Last edited 11 years ago by r-a-y (previous) (diff)

11 years ago

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

#10 @r-a-y
11 years ago

  • Keywords has-patch added; needs-patch removed

#11 @boonebgorges
11 years ago

  • Keywords needs-unit-tests dev-feedback has-patch removed

This looks good, r-a-y. I wrote some tests for it and it looks like it's passing all the problematic cases:

#12 @boonebgorges
11 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(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:

Partially reverts r6790. See #4850.

Fixes #4761

Props r-a-y

#13 @will_c
11 years ago

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

#14 @boonebgorges
11 years ago

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

See #4761

Note: See TracTickets for help on using tickets.