From b19f716e1b519e0aee7fc0c28f393c19fe440b5c Mon Sep 17 00:00:00 2001
From: Matthew McGarvey <matthewmcgarvey14@gmail.com>
Date: Mon, 12 Apr 2021 13:32:11 -0400
Subject: [PATCH] Ignore cookies that fail decryption (#1470)

---
 spec/lucky/cookies/cookie_jar_spec.cr | 20 ++++++++++---
 spec/spec_helper.cr                   |  2 +-
 src/lucky/cookies/cookie_jar.cr       | 41 +++++++--------------------
 3 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/spec/lucky/cookies/cookie_jar_spec.cr b/spec/lucky/cookies/cookie_jar_spec.cr
index 7b0f097ac..e14965138 100644
--- a/spec/lucky/cookies/cookie_jar_spec.cr
+++ b/spec/lucky/cookies/cookie_jar_spec.cr
@@ -70,14 +70,26 @@ describe Lucky::CookieJar do
     jar.get?(:name).should be_nil
   end
 
-  it "raises helpful error if trying to read unencrypted values" do
+  it "returns nil if fails to decrypt value" do
     jar = Lucky::CookieJar.empty_jar
 
     jar.set_raw(:name, "Jane")
 
-    expect_raises Exception, "cookies.get_raw(:name).value" do
-      jar.get?(:name)
-    end
+    jar.get?(:name).should be_nil
+  end
+
+  it "parses encrypted cookies as expected" do
+    # meant to be a regression test to make sure we don't
+    # accidentally break cookie decryption
+    #
+    # this cookie was created with Lucky 0.27
+    cookie_key = "cookie_key"
+    cookie_value = "bHVja3k=--hY71kbRfob4pb9NS7wJpWKOBRhF+kwYPsHRQQanyXzGSKsCO6MIHCZfRBxDRqqm6"
+    cookies = HTTP::Cookies.new
+    cookies[cookie_key] = cookie_value
+    jar = Lucky::CookieJar.from_request_cookies(cookies)
+
+    JSON.parse(jar.get(cookie_key)).should eq({"key" => "value", "abc" => "123"})
   end
 
   describe "#set" do
diff --git a/spec/spec_helper.cr b/spec/spec_helper.cr
index 4522b74e9..2f316dc8b 100644
--- a/spec/spec_helper.cr
+++ b/spec/spec_helper.cr
@@ -18,7 +18,7 @@ Lucky::Session.configure do |settings|
 end
 
 Lucky::Server.configure do |settings|
-  settings.secret_key_base = Random::Secure.base64(32)
+  settings.secret_key_base = "EPzB4/PA/JZxEhISPr7Ad5X+G73exX+qg8IKFjqwdx0="
   settings.host = "0.0.0.0"
   settings.port = 8080
 end
diff --git a/src/lucky/cookies/cookie_jar.cr b/src/lucky/cookies/cookie_jar.cr
index e023f2dc9..3ddfad7d2 100644
--- a/src/lucky/cookies/cookie_jar.cr
+++ b/src/lucky/cookies/cookie_jar.cr
@@ -1,7 +1,6 @@
 class Lucky::CookieJar
-  MAX_COOKIE_SIZE                = 4096
-  LUCKY_ENCRYPTION_PREFIX        = Base64.strict_encode("lucky") + "--"
-  LEGACY_LUCKY_ENCRYPTION_PREFIX = Base64.encode("lucky") + "--"
+  MAX_COOKIE_SIZE         = 4096
+  LUCKY_ENCRYPTION_PREFIX = Base64.strict_encode("lucky") + "--"
   alias Key = String | Symbol
   private property cookies
   private property set_cookies
@@ -143,41 +142,21 @@ class Lucky::CookieJar
     end
   end
 
-  private def decrypt(cookie_value : String, cookie_name : String) : String
-    if encrypted_with_lucky?(cookie_value)
-      base_64_encrypted_part = cookie_value.lchop(LUCKY_ENCRYPTION_PREFIX)
-      decoded = Base64.decode(base_64_encrypted_part)
-      String.new(encryptor.decrypt(decoded))
-    elsif encrypted_with_legacy?(cookie_value)
-      decrypt(update_from_legacy_value(cookie_value), cookie_name)
-    else
-      raise <<-ERROR
-      It looks like this cookie's value is not encrypted by Lucky. This likely means the cookie was set by something other than Lucky, like a client side script.
-
-      You can access the raw value by using 'get_raw':
-
-          cookies.get_raw(:#{cookie_name}).value
-
-      ERROR
-    end
-  end
+  private def decrypt(cookie_value : String, cookie_name : String) : String?
+    return unless encrypted_with_lucky?(cookie_value)
 
-  private def update_from_legacy_value(value : String) : String
-    decoded_value = URI.decode_www_form(value)
-    LUCKY_ENCRYPTION_PREFIX + decoded_value.lchop(LEGACY_LUCKY_ENCRYPTION_PREFIX)
+    base_64_encrypted_part = cookie_value.lchop(LUCKY_ENCRYPTION_PREFIX)
+    decoded = Base64.decode(base_64_encrypted_part)
+    String.new(encryptor.decrypt(decoded))
+  rescue e
+    # an error happened while decrypting the cookie
+    # we will treat that as if no cookie was passed
   end
 
   private def encrypted_with_lucky?(value : String) : Bool
     value.starts_with?(LUCKY_ENCRYPTION_PREFIX)
   end
 
-  # legacy encrypted values had a \n between the encoded lucky and -- and were also www form encoded
-  # this allows apps made before 0.27.0 to not have to log all users out
-  private def encrypted_with_legacy?(value : String) : Bool
-    decoded_value = URI.decode_www_form(value)
-    decoded_value.starts_with?(LEGACY_LUCKY_ENCRYPTION_PREFIX)
-  end
-
   @_encryptor : Lucky::MessageEncryptor?
 
   private def encryptor : Lucky::MessageEncryptor