Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Itp from LoginProtection #1604

Merged
merged 5 commits into from
Dec 8, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Unreleased
* Log a deprecation warning for the use of incompatible controller concerns [#1560](https://github.com/Shopify/shopify_app/pull/1560)
* Fixes bug with expired sessions for embedded apps returning a 500 instead of 401 [#1580](https://github.com/Shopify/shopify_app/pull/1580)
* Generator properly handles uninstall [#1597](https://github.com/Shopify/shopify_app/pull/1597)
* Remove `Itp` from `LoginProtection`. See the upgrading docs for more information. [#1604](https://github.com/Shopify/shopify_app/pull/1604)
klenotiw marked this conversation as resolved.
Show resolved Hide resolved

21.2.0 (Oct 25, 2022)
----------
Expand Down
8 changes: 8 additions & 0 deletions docs/Upgrading.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ This file documents important changes needed to upgrade your app's Shopify App v

[General Advice](#general-advice)

[Unreleased](#unreleased)

[Upgrading to `v20.3.0`](#upgrading-to-v2030)

[Upgrading to `v20.2.0`](#upgrading-to-v2020)
Expand Down Expand Up @@ -36,6 +38,12 @@ We also recommend the use of a staging site which matches your production enviro

If you do run into issues, we recommend looking at our [debugging tips.](https://github.com/Shopify/shopify_app/blob/main/docs/Troubleshooting.md#debugging-tips)

## Unreleased
klenotiw marked this conversation as resolved.
Show resolved Hide resolved
The `Itp` controller concern has been removed from `LoginProtection`.
klenotiw marked this conversation as resolved.
Show resolved Hide resolved
If any of your controllers are dependant on methods from `Itp` then you can include `ShopifyApp::Itp` directly.
You may notice a deprecation notice saying, `Itp will be removed in an upcoming version`.
This is because we intend on removing `Itp` completely in `v22.0.0`, but this will work in the meantime.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope this is the right place?

I was also thinking of adding the names of the methods like set_test_cookie, set_top_level_oauth_cookie, clear_top_level_oauth_cookie, ....

So it would be easy to search for that method being removed in the upgrading doc. How do you feel about that?

## Upgrading to `v20.3.0`
Calling `LoginProtection#current_shopify_domain` will no longer raise an error if there is no active session. It will now return a nil value. The internal behavior of raising an error on OAuth redirect is still in place, however. If you were calling `current_shopify_domain` in authenticated actions and expecting an error if nil, you'll need to do a presence check and raise that error within your app.

Expand Down
2 changes: 0 additions & 2 deletions lib/shopify_app/controller_concerns/login_protection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
module ShopifyApp
module LoginProtection
extend ActiveSupport::Concern
include ShopifyApp::Itp
include ShopifyApp::SanitizedParams

included do
Expand All @@ -17,7 +16,6 @@ module LoginProtection
ShopifyApp::Logger.deprecated(message, "22.0.0")
end

after_action :set_test_cookie
rescue_from ShopifyAPI::Errors::HttpResponseError, with: :handle_http_error
end

Expand Down