Allow specifying domain when deleting cookie

[BastienL]

As discussed on #194, the IntercomRails::ShutdownHelper.intercom_shutdown does not work (except for localhost). The reason is as because the intercom-session-INTERCOM_ID cookie is set with a domain, a :domain needs to be added to edit the cookie (as stated in the official Rails documentation: "Please note that if you specify a :domain when setting a cookie, you must also specify the domain when deleting the cookie".

This PR adds an optional domain param to IntercomRails::ShutdownHelper.intercom_shutdown to fix the issue.

[calleluks]

This PR in combination with code copied from sharetribe/sharetribe#2924 allowed me to properly shut down Intercom when signing out users in development, on staging, and in production:

    # The Rails docs state that
    # (http://api.rubyonrails.org/classes/ActionDispatch/Cookies.html):
    #
    # > if you specify a :domain when setting a cookie, you must also specify
    # > the domain when deleting the cookie
    #
    # Since the Intercom session cookie is set by their client side JavaScript,
    # the code below has been written to perform the same operations in order to
    # decide which host the cookie should be set for.
    #
    # Copied from https://github.com/sharetribe/sharetribe/pull/2924 and then
    # modified to fit the changes to intercom-rails introduced in
    # https://github.com/intercom/intercom-rails/pull/266
    domain_regexp = /[^.]*\.([^.]*|..\...|...\...)$/
    match = domain_regexp.match(request.host_with_port)
    domain = match[0].split(":")[0] if match
    IntercomRails::ShutdownHelper.intercom_shutdown session, cookies, domain

Is there anything we can do to push this forward?

[stevenharman]

Confirmed, this is broken (that is, IntercomRails::ShutdownHelper.intercom_shutdown does not work in production environments). Can we get this fixed?

[stevenharman]

If we can get Intercom to confirm how they calculate the domain when setting the cookie via JavaScript, we can make a corresponding calculation in IntercomRails when removing the cookie. More specifically, do they take the Public Suffix List into account, or just blindly set it as the host (e.g., foo.tld)?

If the former, we might want to look at https://github.com/weppos/publicsuffix-ruby for doing the calculation.

[stevenharman]

Based on a conversation w/Intercom staff, they do not consider the Public Suffix List when setting the domain via JavaScript. They take the 2nd + 1st level domains and set that as the cookie's domain. e.g., an Intercom cookie at app.foo.example would be set as domain = .foo.example. So, we should be able to leverage request.domain to get what we need from w/in the controller. And then the ShutdownHelper can set the domain attribute, so long as it's not localhost.

    def self.intercom_shutdown_helper(cookies, domain = nil)
      nil_session = { value: nil, expires: 1.day.ago }
      nil_session = nil_session.merge(domain: domain) unless domain.nil? || domain == 'localhost'

      if (cookies.is_a?(ActionDispatch::Cookies::CookieJar))
        cookies["intercom-session-#{IntercomRails.config.app_id}"] = nil_session
      else
        controller = cookies
        Rails.logger.info("Warning: IntercomRails::ShutdownHelper.intercom_shutdown_helper takes an instance of ActionDispatch::Cookies::CookieJar as an argument since v0.2.34. Passing a controller is depreciated. See https://github.com/intercom/intercom-rails#shutdown for more details.")
        controller.response.delete_cookie("intercom-session-#{IntercomRails.config.app_id}", nil_session)
      end
    rescue
    end

[stevenharman]

There's been no movement on this from anyone at Intercom/with commit-bit... [edit] I will open a new PR to roll in the tweaks I mentioned in the comment above (accounting for localhost) and hope that gets attention.

[danielhusar]

Closing in favor of #278
Thank you!

[danielhusar]

This is now available in v0.4.0