-
Notifications
You must be signed in to change notification settings - Fork 48
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
Missing 'state' variable in verifyRequest #20
Comments
The better course of action (per Shopify's documentation) is not to build the hash test from the currently-known required pieces, but rather to build it with the query string when you extract the hmac and signature fields. I haven't had a chance to submit a patch yet (though it seems Josh has abandoned this project), but here's the function I recently implemented in a project (it's in a Laravel project, so is using some of its features, but should be easily adaptable to one's own needs), that should be more future-proof than Josh's original implementation. function verifyRequest(Request $request) {
// Per the Shopify docs:
// Everything except hmac and signature...
$signature = $request->except(['hmac', 'signature']);
// Sorted lexilogically...
ksort($signature);
// Hashed per hmac standards, using sha256 and the shared secret
$test = hash_hmac('sha256', http_build_query($signature), config('services.shopify_secret'));
// Verified when equal
return $request->input('hmac') === $test;
} |
@ShaunaGordon Due to several circumstances, the project is being refactored on my personal account - once it's ready, I will be updating the readme with a link to the new one :) So abandoned - kinda. But will be maintained elsewhere shortly. :) |
@ShaunaGordon Thanks for the code! I had to recently come back to this and was thinking about more or less the same solution. @joshrps looking forward to the update. |
According to the shopify OAuth documentation the authorization request includes a 'state' variable: https://{shop}.myshopify.com/admin/oauth/authorize?client_id={api_key}&scope={scopes}&redirect_uri={redirect_uri}&state={nonce}&grant_options[]={option}
When this redirects to my server the api code does not check for the state variable so the 'verifyRequest' method will return false even if that refresh token matches with the request url.
I can monkeypatch it to include it in API.php line 64:
$queryString = http_build_query(array('code' => $da['code'], 'shop' => $da['shop'], 'state' => $da['state'], 'timestamp' => $da['timestamp']));
Maybe this is something you will want to add or at least make it optional if the 'state' variable is omitted in the request (even though its strongly suggested you don't do this).
The text was updated successfully, but these errors were encountered: