#4761 closed defect (bug) (fixed)
Modify ajax url to work properly with FORCE_SSL_ADMIN
Reported by: | will_c | Owned by: | |
---|---|---|---|
Milestone: | 1.7 | Priority: | normal |
Severity: | major | Version: | 1.7 |
Component: | Core | Keywords: | |
Cc: |
Description
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)
Change History (16)
#4
@
12 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.
Reference:
https://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/link-template.php#L1994
#5
@
12 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.
#7
@
12 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
@
12 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
@
12 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.
#11
@
12 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: https://github.com/buddypress/BuddyPress-Unit-Tests/commit/5cec5a154f568c83fd97f1e8ce5333a9183f2a4d
#12
@
12 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: https://github.com/buddypress/BuddyPress-Unit-Tests/commit/5cec5a154f568c83fd97f1e8ce5333a9183f2a4d
Partially reverts r6790. See #4850.
Fixes #4761
Props r-a-y
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.