URGENT UPDATE need to shopify_app v18.1.1 or @shopify/koa-shopify-auth v5.0.3

APdevelopments commented 2 years ago

I'm using Laravel 5.8 and Osiset library 12.1

This morning I got this message:

How to solve the problem? Please help, its really urgent

bougsid commented 2 years ago

Have you got any idea about this message?

APdevelopments commented 2 years ago

No, i dont have idea about this message

shreyansh-shroff commented 2 years ago

We are getting the same message for our apps. Does anyone know the solution?

mlgrozev commented 2 years ago

We're getting the same message.

o2b3k commented 2 years ago

We're getting the same message. How to solve this issue ?

paxdigital commented 2 years ago

We're getting the same message.

joeyfreund commented 2 years ago

Hi there, this is Joey from Shopify.

The source of the problem is in these lines. It should use App Bridge to do the redirect (the postMessage call you're seeing is the deprecated EASDK call).

I'm not familiar with this project's codebase, but I can see that it's using App Bridge. So, hopefully it won't be too hard to fix this issue.

In the meantime, let me check with our partner managers and ensure that nobody is getting hit by this issue.

jvillegasmx commented 2 years ago

Getting same message

joeyfreund commented 2 years ago

@osiset / @lucasmichot , are you actively maintaining this repo? (I was wondering, since you're the top contributors).

If so, is this something you'd be able to fix? Also, if you require any help/guidance, please let me know. Thanks 🙏

nathan-hutchinson commented 2 years ago

Have you guys tried this PR? #1075

haseebbutt1999 commented 2 years ago

@joeyfreund sir, are you getting any solution about this issue? URGENT UPDATE need to shopify_app v18.1.1 or @shopify/koa-shopify-auth v5.0.3 I am using osiset 16 and laravel 8. http://joxi.ru/DrlPjWOTK0ZKzr

mlgrozev commented 2 years ago

@osiset / @lucasmichot , are you actively maintaining this repo? (I was wondering, since you're the top contributors).

If so, is this something you'd be able to fix? Also, if you require any help/guidance, please let me know. Thanks 🙏

@joeyfreund, Do you plan to have Laravel Shopify PHP boilerplate and library in the nearest future?

The PHP community will much appreciate it.

stevesweets commented 2 years ago

Have you guys tried this PR? #1075

trying this now, will report back.

stevesweets commented 2 years ago

@joeyfreund is there a way that i can quickly get a re-check of an app, to see if i've resolved the issue?

joeyfreund commented 2 years ago

@mlgrozev , as for your question

Do you plan to have Laravel Shopify PHP boilerplate and library in the nearest future?

This is the Shopify-maintained PHP template - https://github.com/Shopify/shopify-app-php (It is also what developers get when they scaffold a new PHP app using the Shopify CLI)

@lucasmichot , thank you so much for the clarifications (and honesty).

I was mostly told internally that PHP is more a language for rookies

This is really unfortunate to hear 😞 . I don't want this to be the focus of the discussion, so all I'm going to say is that I strongly disagree with this statement.

feel free to make PR and we can review it

I'm looking for a PHP dev here who might be able to help with that.

To everybody else in this thread that are currently seeing the warning banner:

If your app is built with PHP, the shopify_app and/or @shopify/koa-shopify-auth libraries aren't relevant to your app (they are relevant to Rails and Node apps, respectively)
We are still trying to figure out how to best help all partners migrate their apps off of using the (deprecated) EASDK API.
The team is working on an update to the text in the banner to make things clearer, it will also provide a link to contact partner support with specific questions (cc @stevesweets ).

olavoasantos commented 2 years ago

I've opened a PR for this. Could someone help me test it?

Full disclosure, I don't have my PHP environment setup so I couldn't test it properly. When I ran the tests I got this exception which seems unrelated to my work:

1) Osiset\ShopifyApp\Test\Actions\ActivatePlanTest::testRunRecurring
Exception: Missing `laravel/legacy-factories` in composer.json. Please refer to <https://github.com/orchestral/testbench/blob/6.x/README.md#using-legacy-factories>

mlgrozev commented 2 years ago

Despite endless efforts advocating for taking better care of our open-source communities, I was mostly told internally that PHP is more a language for rookies, and that our partners just have to switch to Ruby and TS.

@lucasmichot This statement is so immature... I personally know many great apps in the ecosystem with PHP codebase, some of which had made great exits.

@osiset is a busy father that has been working very hard for years on this project on his free time with zero support. When asking for help, he was sent a t-shirt and some stickers https://opencollective.com/laravel-shopify would have been more welcomed

I'm so happy to hear that, I wish him and his family all the best! He's a great asset to the community and I much appreciate his contribution.

@joeyfreund thanks for clarifying and for having you in this discussion.

jvillegasmx commented 2 years ago

I have applied the PR from @olavoasantos , the app works normally, but the message it is still there, i think it takes some time to shopify to validated again. Thanks!

gnikyt commented 2 years ago

I think I need more info on this. Full page redirect for the auth was always the case and worked. It was originally the way the Ruby gem worked as well. So if thats not the case now, then something obviously needs to be modified. I'll try my best to find out what information I can, but if anyone has links to something helpful that would be good.

Additionally, yes, thank you to everyone for the PRs and supports. I've greatly needed help given my situations with fraud/court things I had to go through. I've been so busy, but I am hoping to finally make time to get back to reviewing the work in the pipeline of the PRs and getting releases out.

stevesweets commented 2 years ago

I think I need more info on this. Full page redirect for the auth was always the case and worked. It was originally the way the Ruby gem worked as well. So if thats not the case now, then something obviously needs to be modified. I'll try my best to find out what information I can, but if anyone has links to something helpful that would be good.

Additionally, yes, thank you to everyone for the PRs and supports. I've greatly needed help given my situations with fraud/court things I had to go through. I've been so busy, but I am hoping to finally make time to get back to reviewing the work in the pipeline of the PRs and getting releases out.

I think they just want this to be redirected with App Bridge, pr #1097 I believe will handle this properly. There's a comment on that pr in particular that references how koa now does it which i think is enough to be convinced that it'll work properly.

@osiset Sorry to hear about the personal stuff going on, I really appreciate all your efforts. If I'm able to assist in anyway, I'm absolutely willing to.

olavoasantos commented 2 years ago

Context

For some context, Shopify is deprecating the EASDK support (it's a grandfather library to App Bridge). With that, apps that emit EASDK events are receiving the warning reported in this thread. While most events are emitted through the main library (i.e EASDK), there are some events manually being emitted by 3rd party libraries (e.g. shopify_app gem, @shopify/koa-shopify-auth NPM package and osiset/laravel-shopify). One common case is the Shopify.API.remoteRedirect event that's used to redirect the app. Since the support for EASDK is under deprecation, libraries need to refactor this event to use the App Bridge to handle the redirect.

How does it affect us?

On the src/resources/views/auth/fullpage_redirect.blade.php view, we're currently manually emitting the Shopify.API.remoteRedirect event:

data = JSON.stringify({
  message: 'Shopify.API.remoteRedirect',
  data: { location: redirectUrl },
});

window.parent.postMessage(data, "https://{{ $shopDomain }}");

Because this event is an EASDK event, the Shopify platform detects that an app is emitting the event and flagging them as using deprecated protocols (which results in the warnings seen by developers using laravel-shopify).

How to fix it?

We can fix it by just using the App Bridge library to handle the redirect. This is basically what the PR is doing.

APdevelopments commented 2 years ago

I implement everything from PR - my app is partial inside iframe and partial need to be full page.

When I want to redirect user to full page I use this link: <a href="/design" target="_top">New Page Design</a> and target="_top" redirect user to full page

but now I got a problem because it wont redirect user - it show page in iframe inside shopify not an external page

Kyon147 commented 2 years ago

My PR https://github.com/osiset/laravel-shopify/pull/1075 should solve this issue as it removes the need to do the full page redirect.

If @osiset is super busy as can't review it @lucasmichot can you review the PR for me please as I don't want to be approving my own.

Kyon147 commented 2 years ago

In terms of maintaining, I am around sometimes in the week and try to send some PRs across for the bigger issues. Like the one above.

Anyone else who wants to submit PRs for issues, I'll be happy to look at them and try and get them into the codebase for a next release.

APdevelopments commented 2 years ago

Interesting that alert message from Shopify admin panel gone but I didnt update anything...

Also I test the code from PR #1075 but it doesnt work for me - didnt redirect to a new page, use the iframe again

haseebbutt1999 commented 2 years ago

Interesting that alert message from Shopify admin panel gone but I didnt update anything...

Also I test the code from PR #1075 but it doesnt work for me - didnt redirect to a new page, use the iframe again

Update the version of osiset to version 17.1. This is the solution of this issue. Also i tried this with my application, its work for me. Recently my app approved, using package 17.1.

Kyon147 commented 2 years ago

Interesting that alert message from Shopify admin panel gone but I did not update anything...

Also I test the code from PR #1075 but it doesn't work for me - didn't redirect to a new page, use the iframe again

@APdevelopments

The PR #1075 is meant to simplify the process of the auth flow. Basically it avoids the fullpage redirect page which should also stop the auth/token route from running before the OAuth which was one of Shopify's complaints. It also removes one redirect from the whole flow which will speed up the auth ever so slightly.

APdevelopments commented 2 years ago

Hi,

My version is 12 and works

uto, 15. mar 2022. 07:19 haseebbutt1999 @.***> је написао/ла:

Interesting that alert message from Shopify admin panel gone but I didnt update anything...

Also I test the code from PR #1075 https://github.com/osiset/laravel-shopify/pull/1075 but it doesnt work for me - didnt redirect to a new page, use the iframe again

Update the version of osiset to version 17.1. This is the solution of this issue. Also i tried this with my application, its work for me. Recently my app approved, using package 17.1.

— Reply to this email directly, view it on GitHub https://github.com/osiset/laravel-shopify/issues/1094#issuecomment-1067606116, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACQHR7RGZ2J4UKFHGSAVVA3VAATYTANCNFSM5PMMWQ2A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

APdevelopments commented 2 years ago

Can you please share example how to redirect user out of Shopify panel to use full screen in browser

uto, 15. mar 2022. 09:12 Perica Aleksov @.***> је написао/ла:

Hi,

My version is 12 and works

uto, 15. mar 2022. 07:19 haseebbutt1999 @.***> је написао/ла:

Interesting that alert message from Shopify admin panel gone but I didnt update anything...

Also I test the code from PR #1075 https://github.com/osiset/laravel-shopify/pull/1075 but it doesnt work for me - didnt redirect to a new page, use the iframe again

Update the version of osiset to version 17.1. This is the solution of this issue. Also i tried this with my application, its work for me. Recently my app approved, using package 17.1.

— Reply to this email directly, view it on GitHub https://github.com/osiset/laravel-shopify/issues/1094#issuecomment-1067606116, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACQHR7RGZ2J4UKFHGSAVVA3VAATYTANCNFSM5PMMWQ2A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

Kyon147 commented 2 years ago

@APdevelopments

This is currently not possible with v17.1 - you can't use this package version for full page apps. You would need to do some work to get it done which Oiset has mentioned in this PR (https://github.com/osiset/laravel-shopify/pull/935) which was being worked on.

APdevelopments commented 2 years ago

Thanks. If I use a full page app then I don't need AppBridge and dont need to use JWT? Is that correct?

On Tue, Mar 15, 2022 at 9:18 AM Luke Walsh @.***> wrote:

@APdevelopments https://github.com/APdevelopments

This is currently not possible with v17.1 - you can't use this package version for full page apps. You would need to do some work to get it done which Oiset has mentioned in this PR (#935 https://github.com/osiset/laravel-shopify/pull/935) which was being worked on.

— Reply to this email directly, view it on GitHub https://github.com/osiset/laravel-shopify/issues/1094#issuecomment-1067690623, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACQHR7QKRDIU7PWYYAEX4MLVABBVBANCNFSM5PMMWQ2A . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you were mentioned.Message ID: @.***>

-- [image: Logo] https://aleksovapps.com/ Perica Aleksov Founder aleksovApps.com M: (+381) 65 33 63 439 E: @.*** [image: LinkedIn icon] https://www.linkedin.com/in/perica-aleksov-a5227327/

stevesweets commented 2 years ago

Quick check in - I wouldn't like to recommend implementing a non-merged PR, but, I have been successful in ridding myself of this warning by implementing pr #1097

Kyon147 commented 2 years ago

Quick check in - I wouldn't like to recommend implementing a non-merged PR, but, I have been successful in ridding myself of this warning by implementing pr #1097

Interesting, my PR actually removes that route with the old ESDK call. So it would be interesting to see as if merged in you won't ever access the full page redirect as it's technically not needed as you can just do a redirect within laravel to send you to the auth.

stevesweets commented 2 years ago

Quick check in - I wouldn't like to recommend implementing a non-merged PR, but, I have been successful in ridding myself of this warning by implementing pr #1097

Interesting, my PR actually removes that route with the old ESDK call. So it would be interesting to see as if merged in you won't ever access the full page redirect as it's technically not needed as you can just do a redirect within laravel to send you to the auth.

Good question. Ill try to get an answer. I may even be able to trial it.

Kyon147 commented 2 years ago

Released in v17.2.0

gnikyt / laravel-shopify