Allow for current_shopify_domain to be nil with authenticated requests

[nelsonwittwer]

What this PR does

There is a bug with expired tokens that causes our CSP header to throw an error because current_shopify_domain isn't able to be retrieved because there is no current active session (it's expired).

Because LoginProtection#current_shopify_domain previously has been throwing and error when the domain couldn't be found from the session, this was throwing and exception before it could return a 401 - unauthorized response.

LoginProtection#current_shopify_domain

This changes a few things that I'll want to make sure are acceptable design changes:

1. current_shopify_domain is allowed to be nil within LoginProtection
2. When current_shopify_domain is required for 302 - redirects in LoginProtection we will move responsibility for raising and error to those redirect calls.

Fetch requests don't evaluate to true with the requirest.hr? helper

The other problem preventing us from returning 401 - unauthorized was that we were doing a full page redirect when the current session wasn't found. Our generated home/index.html.erb used the fetch API and LoginProtection was only looking for xhr requests to handle JS requests 🤦 .

1. Added requseted_by_javascript? conditional to better determine API requests vs browser requests. MDN indicates the official MIME type is text/javascript though we'll support the legacy application/javascript as well.
2. Updated our home_controller/index.html.erb to add the correct content type header to act as a better example.

Reviewer's guide to testing

1. Clone test repo that reproduces expired token
2. Add Content-Type: "text/javascript" header to /fetch request in home_controller/index.html.erb
3. Create test app with that test repo
4. Ensure server responds with expected 401 instead of 500 with bad tokens.

Things to focus on

Is changing the raise behavior in current_shopify_domain acceptable? Or is this too big of a breaking change? If so, we can rescue in FrameAncestors and use our wildcard fallback domain.

Checklist

Before submitting the PR, please consider if any of the following are needed:

• Update CHANGELOG.md if the changes would impact users
• Update README.md, if appropriate.
• Update any relevant pages in /docs, if necessary
• For security fixes, the Disclosure Policy must be followed.

[andyw8]

It feels a bit odd to raise this then immediately rescue it, maybe we could use a guard?

return base_return_address if current_shopify_domain.nil?

[ifyouseewendy]

Hey guys, I'm working on upgrading shopify_app for our app and running into the similar issue. More context here https://github.com/Shopify/script-editor/pull/3145#issuecomment-1319053421 .

After some debugging, I realized that the error comes from frame_ancestors.rb

      content_security_policy do |policy|
        policy.frame_ancestors(-> do
          domain_host = current_shopify_domain || "*.#{::ShopifyApp.configuration.myshopify_domain}"
          "#{ShopifyAPI::Context.host_scheme}://#{domain_host} https://admin.shopify.com"
        end)

that current_shopify_domain raises an ::ShopifyApp::ShopifyDomainNotFound when there is no shopify domain found.

By searching in the shopify_app repo, I noticed this PR happens to be the right fix.

FYI, I pulled the PR and tested. It works as expected. Thank you for the fix 👍