Bug report: admin_url() returns *mapped* domain with HTTPS URL

Domain Mapping v4.4.0.5, WordPress 4.2.2 in subdir WPMS mode. Admin and login are forced to original domain, cross-domain autologin is enabled, and HTTPS is forced for login and admin pages (but not for front-end pages)

Looks like we have another bug relating to admin_url(), possibly of a similar nature to

https://premium.wpmudev.org/forums/topic/bug-report-admin-bar-links-to-wrong-wp-admin-urls-mapped-domain-possibly-also-sso-problem

admin_url() is returning mapped domains with https:// prefix which, in 99% of cases, will result in a CN mismatch.

In this particular instance, the problem is revealed in WPMUdev Appointments+ (v1.4.7, appointments.php lines 4158 and 5071) in which that plugin is calling wp_localize_script() with 'ajax_url' => admin_url('admin-ajax.php'). Instead of returning:

https://original.com/siteslug/wp-admin/admin-ajax.php

it is returning

https://mapped.com/wp-admin/admin-ajax.php

even when admin_url() is being called with the protocol forced to http://, per this support case here:

https://premium.wpmudev.org/forums/topic/incompatibility-between-appointments-and-domain-mapping-plugins

and so the AJAX call fails because the browser won't connect to POST the AJAX request.

Let me know if there's anything I can do to assist in debugging this case.

Thanks,

DK

  • Jude

    Hi David King

    Hope you had a great weekend.
    Thanks for bringing this to our attention.

    https://original.com/siteslug/wp-admin/admin-ajax.php

    it is returning

    https://mapped.com/wp-admin/admin-ajax.php

    I see Sam has already committed a fix to this issue in your earlier thread. I am going to request his feedback again here on this one to see if this is a bug or intended behavior.

    Please enable support access and keep it active incase we need to check anything.

    Jude

  • David King

    Hi Jude,

    Support access is still enabled and I've extended it.

    to see if this is a bug or intended behavior.

    I'll be curious to hear what he (or she) has to say about it. As I said,

    admin_url() is returning mapped domains with https:// prefix which, in 99% of cases, will result in a CN mismatch.

    The only way an HTTPS request to a mapped domain will work is if the server is loaded with an SSL certificate for every mapped domain, something which (I imagine) would be pretty rare. Anyway, it shouldn't be necessary if all admin functions (which, with the exceptions of e-commerce applications and similar, are the only things that really need to be protected by SSL) are forced to SSL on the original domain. It's one of the beauties of this plugin! :slight_smile:

  • Sam

    Hi @David King

    It resolving the to the mapped domain is intended and should be like this unless you have a reason to prevent this.

    For the https thing, currently it's looking at the scheme of admin and then forcing https or http on it which is troublesome in some cases and is being addressed in 4.4.0.6 which will be release after the test we're currently carrying out.

    Hope this helps,
    Sam

  • David King

    @Sam,

    Being that we've got a live test case (that, I hope, 4.4.0.6 will fix), could I test a release candidate of 4.4.0.6 for you?

    I'm currently blocking on a solution to this (in respect of a job involving Appointments+) so, with any luck, that may be helpful for your testing process and, if 4.4.0.6 does, indeed, fix the problem, then I can resume work on that other job :slight_smile:

  • David King

    @Sam,

    It resolving the to the mapped domain is intended and should be like this unless you have a reason to prevent this.

    Okay, digging into Domain Mapper a bit, I see the definition of filter domain_mapping_admin_url() at classes/class.domainmap.php:131 that reverses the usual behaviour when called for admin-ajax.php, so the result returned is the mapped domain when not in wp-admin. I assume that this has to do with cookies or CORS or similar.

    Whatever the reason, I've identified an issue with this filter. As written, line 166 unconditionally forces the protocol to https:// when Domain Mapping is set to force wp-admin functions to SSL. This is a bug, for two reasons:

    1) It breaks the Wordpress API: function admin_url() takes an optional argument 'scheme' which allows the caller to specify the protocol they want, but this unconditional behaviour breaks that where it might really matter (as in the case with Appointments+); and

    2) as I've said before, https:// requests to a mapped domain will fail because of the CN mismatch that will result unless the web server is configured with an SSL certificate for the mapped domain, which will be very much the minority of cases.

    I therefore offer the following patch to fix this bug:

    --- class.domainmap.php.bk      2015-05-26 00:40:22.000000000 -0500
    +++ class.domainmap.php 2015-05-26 15:12:16.709220591 -0500
    @@ -134,6 +134,17 @@
                    if ( !$_blog_id ) {
                            $_blog_id = $blog_id;
                    }
    +
    +               /* Depending on settings, the URL should be forced to SSL - except in the
    +                * case of the exception for admin-ajax.php.
    +                *
    +                * Returning https:// URLs for mapped domains will only work when a
    +                * suitable SSL certificate for the mapped domain is installed on the
    +                * web server.  Rather than force http:// for AJAX requests, assume the
    +                * caller knows what they want and leave the scheme alone (for that
    +                * minority of cases where the front end/mapped domain is forced SSL). */
    +               $preserve_scheme = false;
    +
                    switch ( $this->options['map_admindomain'] ) {
                            case 'user':
                                    break;
    @@ -157,13 +168,15 @@
                                            if ( !is_admin() ) {
                                                    // swap the original url with the mapped one
                                                    $admin_url = str_replace( $orig_url, $mapped_url, $admin_url );
    +                                               // Leave the scheme alone.
    +                                               $preserve_scheme = true;
                                            }
                                    }
    
                                    break;
                    }
    -
    -               return $this->options['map_force_admin_ssl'] ? set_url_scheme($admin_url, "https") : $admin_url;
    +               return $preserve_scheme ? $admin_url
    +                                       : ($this->options['map_force_admin_ssl'] ? set_url_scheme($admin_url, "https") : $admin_url);
            }
    
            function add_domain_mapping_filters() {
    @@ -855,4 +868,4 @@
    
             return $scheme;
         }
    -}
    \ No newline at end of file
    +}

    (though this reply form has probably mangled the spacing.) If you'd rather it emailed, let me know where to send it.

    An alternative way to implement the fix would be to force the scheme based on the value of map_force_admin_ssl or map_force_frontend_ssl depending on whether the mapped or original domain is served, but that would still break the WordPress API.

    This patch fixes the original problem that prompted this support case as well as the problem that prompted the other case and the subsequent problems I identified here:

    https://premium.wpmudev.org/forums/topic/incompatibility-between-appointments-and-domain-mapping-plugins?replies=8#post-886264

    Let me know what you think. Either this patch, or something like it, needs to be applied to the next release, because I don't see how else to fix the problems I identified in that other support case. Doubtless there are other cases where this is a problem, too.

    • David King

      Here is an alternative implementation that might be a better solution, even if it does break the WP API (but it occurs to me that since DM messes with protocols all the time anyway, it may be less of an issue).

      This implementation forces the protocol to https:// for all admin_url()s (if DM is configured to do so) except for AJAX URLs, in which case it forces the protocol to be the same as that of the page currently being rendered.

      The advantage of this solution is that the filter should do the RightThing™ whether plugin authors know or not that they have to force http:// (note that my previous patch required a patch to Appointments+ also, where this patch should not).

      It's all a question of how much you trust plugin authors to think of this sort of thing. Idk, tough call. I don't much like the idea of taking control out of plugin authors' hands, but OTOH, plugins rarely get the extent of testing required to catch bugs like this, so maybe it's the safer option.

      It's up to you which you prefer to apply. NB: Both patches should be applied against v4.4.0.6.

      --- class.domainmap.php.bk      2015-05-27 08:34:25.454006425 -0500
      +++ class.domainmap.php 2015-05-27 08:33:34.985844557 -0500
      @@ -134,6 +134,10 @@
                      if ( !$_blog_id ) {
                              $_blog_id = $blog_id;
                      }
      +
      +               // is front-end AJAX URL
      +               $is_fe_ajax = false;
      +
                      switch ( $this->options['map_admindomain'] ) {
                              case 'user':
                                      break;
      @@ -157,13 +161,24 @@
                                              if ( !is_admin() ) {
                                                      // swap the original url with the mapped one
                                                      $admin_url = str_replace( $orig_url, $mapped_url, $admin_url );
      +                                               $is_fe_ajax = true;
                                              }
                                      }
      
                                      break;
                      }
      
      -               return $this->options['map_force_admin_ssl'] ? set_url_scheme($admin_url, "https") : $admin_url;
      +               if( $is_fe_ajax ) {
      +                       /* This is a front-end AJAX URL, so force scheme to be same as that of the page currently being rendered
      +                        * in order to avoid avoiding "mixed protocol" errors in client browsers and problems with auth cookies not
      +                        * being sent.
      +                        */
      +                       $admin_url = set_url_scheme( $admin_url, is_ssl() ? 'https' : 'http' );
      +               } elseif( $this->options['map_force_admin_ssl'] ) {
      +                       /* Force scheme per Domain Mapping configuration */
      +                       $admin_url = set_url_scheme( $admin_url, 'https' );
      +               }
      +               return $admin_url;
              }
      
              function add_domain_mapping_filters() {
  • David King

    Multiple certificates is not a technical problem (as you point out, SNI supports this, cloudflare or no), but multiple certificates should not be required in most cases, and DM should support (what I assume is the common) case where the only SSL cert available is that of the original domain.

    Of course, it depends a bit on the circumstances (especially what sort of service is being offered), which vary widely.

    Again though, many thanks for contributing awesomeness to the DM project!

    You're welcome, and thanks for the points!

    PS: @devs, not marking this resolved until one or other of the patches is merged and released.

Thank NAME, for their help.

Let NAME know exactly why they deserved these points.

Gift a custom amount of points.