Shopify / shopify_app

A Rails Engine for building Shopify Apps
MIT License
1.76k stars 683 forks source link

LoginProtection: Extract private method set_session_return_to #1860

Closed jaredbeck closed 3 months ago

jaredbeck commented 3 months ago

What this PR does

Apps which use cookies for session storage, and thus have a 4kB session data limit, may choose to override this method to prevent excessively-long query strings from causing a CookieOverflow error.

Currently, apps must override the entire redirect_to_login method. To make gem updates easier, we want to override a smaller area.

Reviewer's guide to testing

This PR does not change functionality.

Things to focus on

Confirm that the method extraction was performed correctly, ie. without changing functionality.

Checklist

Please let me know if any of the following are necessary in this case. It's not immediately clear to me if they are necessary.

jaredbeck commented 3 months ago

CLA signed

jaredbeck commented 3 months ago

That sounds great. I like the idea of having a method that simply returns a URL, instead of setting the session property.

lizkenyon commented 3 months ago

@jaredbeck If you are able to make those changes to this PR, we will prioritize getting it reviewed and merged!

If not you can close this PR, and I will create a ticket for the team to tackle this, but transparently we won't get to this immediately.

jaredbeck commented 3 months ago

How does this look?