Skip to content

Commit

Permalink
Merge pull request #3791 from Shopify/theme-patch-proxy-main-2
Browse files Browse the repository at this point in the history
Fix `shopify theme dev` and `shopify theme console` proxies following session changes and bring back the legacy `shopify theme push` implementation in CI/CD workflows
  • Loading branch information
karreiro authored Apr 29, 2024
2 parents 38ca012 + d5a05e7 commit 1ef19f6
Show file tree
Hide file tree
Showing 10 changed files with 62 additions and 32 deletions.
5 changes: 5 additions & 0 deletions .changeset/clean-badgers-deny.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/theme': patch
---

- Fix the `shopify theme dev` proxy to use the development theme, even when users have a browser session with the live theme loaded
6 changes: 6 additions & 0 deletions .changeset/cuddly-cows-hug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/theme': patch
---

- Fix `shopify theme dev` and `shopify theme console` proxies following session changes
- Bring the legacy `shopify theme push` implementation in CI/CD workflows
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ class DevServer
]

class Proxy
SESSION_COOKIE_NAME = "_secure_session_id"
SESSION_COOKIE_REGEXP = /#{SESSION_COOKIE_NAME}=(\h+)/
SESSION_COOKIE_NAME = "_shopify_essential"
SESSION_COOKIE_REGEXP = /#{SESSION_COOKIE_NAME}=([^;]*)(;|$)/
SESSION_COOKIE_MAX_AGE = 60 * 60 * 23 # 1 day - leeway of 1h
IGNORED_ENDPOINTS = %w[
shopify/monorail
Expand Down Expand Up @@ -58,7 +58,8 @@ def call(env)
query = URI.decode_www_form(env["QUERY_STRING"])
replace_templates = build_replacement_param(env)

clean_sfr_cache(env, query, headers)
response = set_preview_theme_id(env, query, headers)
return serve_response(response, env) if response

response = if replace_templates.any?
# Pass to SFR the recently modified templates in `replace_templates` or
Expand Down Expand Up @@ -87,16 +88,14 @@ def call(env)
@core_endpoints << env["PATH_INFO"]
end

body = patch_body(env, response.body)
body = [body] unless body.respond_to?(:each)
[response.code, headers, body]
serve_response(response, env, headers)
end

def secure_session_id
if secure_session_id_expired?
@ctx.debug("Refreshing preview _secure_session_id cookie")
@ctx.debug("Refreshing preview _shopify_essential cookie")
response = request("HEAD", "/", query: [[:preview_theme_id, theme_id]])
@secure_session_id = extract_secure_session_id_from_response_headers(response)
@secure_session_id = extract_shopify_essential_from_response_headers(response)
@last_session_cookie_refresh = Time.now
end

Expand All @@ -105,7 +104,13 @@ def secure_session_id

private

def clean_sfr_cache(env, query, headers)
def serve_response(response, env, headers = get_response_headers(response, env))
body = patch_body(env, response.body)
body = [body] unless body.respond_to?(:each)
[response.code, headers, body]
end

def set_preview_theme_id(env, query, headers)
if env["PATH_INFO"].start_with?("/password")
@cache_cleaned = false
return
Expand Down Expand Up @@ -215,7 +220,7 @@ def secure_session_id_expired?
Time.now - @last_session_cookie_refresh >= SESSION_COOKIE_MAX_AGE
end

def extract_secure_session_id_from_response_headers(headers)
def extract_shopify_essential_from_response_headers(headers)
return unless headers["set-cookie"]

headers["set-cookie"][SESSION_COOKIE_REGEXP, 1]
Expand All @@ -235,9 +240,9 @@ def get_response_headers(response, env)
response_headers["location"].gsub!(%r{(https://#{shop})}, "http://#{host(env)}")
end

new_session_id = extract_secure_session_id_from_response_headers(response_headers)
new_session_id = extract_shopify_essential_from_response_headers(response_headers)
if new_session_id
@ctx.debug("New _secure_session_id cookie from response")
@ctx.debug("New _shopify_essential cookie from response")
@secure_session_id = new_session_id
@last_session_cookie_refresh = Time.now
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ module ShopifyCLI
module Theme
class Repl
class Api
SESSION_COOKIE_NAME = DevServer::Proxy::SESSION_COOKIE_NAME

attr_reader :ctx, :url, :repl

def initialize(ctx, url, repl)
Expand Down Expand Up @@ -60,7 +62,7 @@ def liquid_template
end

def cookie
@cookie ||= "storefront_digest=#{repl.storefront_digest}; _secure_session_id=#{repl.secure_session_id}"
@cookie ||= "storefront_digest=#{repl.storefront_digest}; #{SESSION_COOKIE_NAME}=#{repl.secure_session_id}"
end

def shop
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def call(env)
@env = env
@env["PATH_INFO"] = PASSWORD_PAGE_PATH if redirect_to_password?(@env)

return @app.call(@env) if password_page?(@env)
return @app.call(@env) if password_page?(@env) || (storefront_session.nil? || secure_session.nil?)

authenticate!

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def test_proxy_to_sfr
.to_return(
status: 200,
headers: {
"Set-Cookie" => "_secure_session_id=abcd1234",
"Set-Cookie" => "_shopify_essential=abcd1234",
}
)
stub_sfr = stub_request(:get, "https://dev-theme-server-store.myshopify.com/?_fd=0&pb=0")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,13 @@ def test_get_is_proxied_to_theme_access_api_when_password_is_provided
.with(headers: { "X-Shopify-Shop" => store })
.to_return(
status: 200,
headers: { "Set-Cookie" => "_secure_session_id=#{SECURE_SESSION_ID}" },
headers: { "Set-Cookie" => "_shopify_essential=#{SECURE_SESSION_ID}" },
)
stub_request(:get, "https://theme-kit-access.shopifyapps.com/cli/sfr/?_fd=0&pb=0")
.with(
headers: {
"Content-Length" => "0",
"Cookie" => "_secure_session_id=deadbeef",
"Cookie" => "_shopify_essential=deadbeef",
"X-Shopify-Shop" => store,
},
)
Expand Down Expand Up @@ -163,28 +163,28 @@ def test_refreshes_session_cookie_on_expiry

def test_update_session_cookie_when_returned_from_backend
stub_session_id_request
new_secure_session_id = "#{SECURE_SESSION_ID}2"
new_shopify_essential = "#{SECURE_SESSION_ID}2"

# POST response returning a new session cookie (Set-Cookie)
stub_request(:post, "https://dev-theme-server-store.myshopify.com/account/login?_fd=0&pb=0")
.with(
headers: {
"Cookie" => "_secure_session_id=#{SECURE_SESSION_ID}",
"Cookie" => "_shopify_essential=#{SECURE_SESSION_ID}",
},
)
.to_return(
status: 200,
body: "",
headers: {
"Set-Cookie" => "_secure_session_id=#{new_secure_session_id}",
"Set-Cookie" => "_shopify_essential=#{new_shopify_essential}",
},
)

# GET / passing the new session cookie
stub_request(:get, "https://dev-theme-server-store.myshopify.com/?_fd=0&pb=0")
.with(
headers: {
"Cookie" => "_secure_session_id=#{new_secure_session_id}",
"Cookie" => "_shopify_essential=#{new_shopify_essential}",
},
)
.to_return(status: 200)
Expand Down Expand Up @@ -319,24 +319,24 @@ def test_hop_to_hop_headers_are_removed_from_proxied_response
end
end

def test_replaces_secure_session_id_cookie
def test_replaces_shopify_essential_cookie
stub_request(:get, "https://dev-theme-server-store.myshopify.com/?_fd=0&pb=0")
.with(
headers: {
"Cookie" => "_secure_session_id=#{SECURE_SESSION_ID}",
"Cookie" => "_shopify_essential=#{SECURE_SESSION_ID}",
},
)

stub_session_id_request
request.get("/",
"HTTP_COOKIE" => "_secure_session_id=a12cef")
"HTTP_COOKIE" => "_shopify_essential=a12cef")
end

def test_appends_secure_session_id_cookie
def test_appends_shopify_essential_cookie
stub_request(:get, "https://dev-theme-server-store.myshopify.com/?_fd=0&pb=0")
.with(
headers: {
"Cookie" => "cart_currency=CAD; secure_customer_sig=; _secure_session_id=#{SECURE_SESSION_ID}",
"Cookie" => "cart_currency=CAD; secure_customer_sig=; _shopify_essential=#{SECURE_SESSION_ID}",
},
)

Expand Down Expand Up @@ -373,7 +373,7 @@ def test_pass_pending_templates_to_storefront
"Accept-Encoding" => "none",
"Authorization" => "Bearer TOKEN",
"Content-Type" => "application/x-www-form-urlencoded",
"Cookie" => "_secure_session_id=#{SECURE_SESSION_ID}",
"Cookie" => "_shopify_essential=#{SECURE_SESSION_ID}",
"Host" => "dev-theme-server-store.myshopify.com",
"X-Forwarded-For" => "",
"User-Agent" => "Shopify CLI",
Expand Down Expand Up @@ -597,7 +597,7 @@ def request
def default_proxy_headers(domain = "myshopify.com")
{
"Accept-Encoding" => "none",
"Cookie" => "_secure_session_id=#{SECURE_SESSION_ID}",
"Cookie" => "_shopify_essential=#{SECURE_SESSION_ID}",
"Host" => "dev-theme-server-store.#{domain}",
"X-Forwarded-For" => "",
"User-Agent" => "Shopify CLI",
Expand All @@ -614,7 +614,7 @@ def stub_session_id_request(domain = "myshopify.com")
.to_return(
status: 200,
headers: {
"Set-Cookie" => "_secure_session_id=#{SECURE_SESSION_ID}",
"Set-Cookie" => "_shopify_essential=#{SECURE_SESSION_ID}",
},
)
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require "test_helper"

require "shopify_cli/theme/dev_server/proxy"
require "shopify_cli/theme/theme_admin_api"
require "shopify_cli/theme/repl/api"

Expand Down Expand Up @@ -31,7 +32,7 @@ def test_request_when_logged_in_with_theme_access
headers: {
"Authorization" => "Bearer",
"Content-Type" => "application/x-www-form-urlencoded",
"Cookie" => "storefront_digest=storefront_session_5678; _secure_session_id=secure_session_id_1234",
"Cookie" => "storefront_digest=storefront_session_5678; _shopify_essential=secure_session_id_1234",
"User-Agent" => "Shopify CLI",
},
)
Expand All @@ -57,7 +58,7 @@ def test_request_when_not_logged_in_with_theme_access
},
headers: {
"Content-Type" => "application/x-www-form-urlencoded",
"Cookie" => "storefront_digest=storefront_session_5678; _secure_session_id=secure_session_id_1234",
"Cookie" => "storefront_digest=storefront_session_5678; _shopify_essential=secure_session_id_1234",
"X-Shopify-Shop" => "store.myshopify.com",
},
)
Expand Down
11 changes: 11 additions & 0 deletions packages/theme/src/cli/commands/theme/push.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ describe('Push', () => {
const path = '/my-theme'

describe('run with CLI 3 implementation', () => {
test('should run the CLI 2 implementation if the password flag is provided', async () => {
// Given
const theme = buildTheme({id: 1, name: 'Theme', role: 'development'})!

// When
await runPushCommand(['--password', '123'], path, adminSession, theme)

// Then
expectCLI2ToHaveBeenCalledWith(`theme push ${path} --development-theme-id ${theme.id}`)
})

test('should pass call the CLI 3 command', async () => {
// Given
const theme = buildTheme({id: 1, name: 'Theme', role: 'development'})!
Expand Down
2 changes: 1 addition & 1 deletion packages/theme/src/cli/commands/theme/push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export default class Push extends ThemeCommand {

const developmentThemeManager = new DevelopmentThemeManager(adminSession)

if (!flags.stable) {
if (!flags.stable && !flags.password) {
const {live, development, unpublished, path, nodelete, theme, publish, json, force, ignore, only} = flags

let selectedTheme: Theme
Expand Down

0 comments on commit 1ef19f6

Please sign in to comment.