Skip to content

Commit

Permalink
[3.59] Fix shopify theme dev and shopify theme console proxies fo…
Browse files Browse the repository at this point in the history
…llowing session changes and bring the legacy `shopify theme push` implementation in CI/CD workflows
  • Loading branch information
karreiro committed Apr 22, 2024
1 parent a70ec32 commit 3deb6a4
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 26 deletions.
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 @@ -94,9 +94,9 @@ def call(env)

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 Down Expand Up @@ -215,7 +215,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 +235,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 @@ -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 3deb6a4

Please sign in to comment.