Change action to POST in readme #53
Conversation
|
Why? This makes it inconsistent with the example/config.ru and the integration tests. |
|
I didn't check the files, |
|
This login form is only the start of the oauth process. It won't actually log the user into Shopify or install the app in the Shopify store. I'm not sure what action you are worried about being forged. Besides, changing the form isn't going to prevent any vulnerability, since it won't change the fact that the endpoint accepts GET requests. This library doesn't even work with the change you are proposing, since that endpoint looks for the shop parameter in the GET parameters. |
|
This is a long story but here we go... The action that is vulnerable to csrf is the login action. If someone has already signed into an app at any moment in the past, they will go through the oauth flow on shopify's side without being prompted. This leads to the situation where if anyone visits the /auth/shopify?shop=... path on a app where they have already signed in, they will be signed in again. For regular shopify apps, we would accept the get request but validate the signature provided by shopify before initiating the login (we don't do it currently). In some other cases where the app itself initiate the login to shopify (app store, theme store, EU, etc) we should validate the csrf token and use a POST request. So far I have fixed the app store which is the only of our oauth apps to not be vulnerable to CSRF for the login action... |
|
To allow the functionality for regular apps which use get request, this change doesn't make sense. Fixing other shopify apps which are vulnerable to login CSRF would be the initial step before bringing this change ? I had a look at some of the app, only app store uses a POST request to login with shopify. |
No description provided.