-
Notifications
You must be signed in to change notification settings - Fork 176
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
Error occuring on authentication of non embeded app #375
Comments
Hi
Yes, that is correct, I changed the null to zero and the whole thing sprang
to life.
Jules
…On Tue, 29 Oct 2024, 10:18 Matteo Depalo, ***@***.***> wrote:
Hi @julesbl <https://github.com/julesbl>, just to clarify, you made the
changes in this PR <#293>
and it worked?
—
Reply to this email directly, view it on GitHub
<#375 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANGHN2SEHVIAUZ22TRPN4DZ55OHNAVCNFSM6AAAAABPXD5IR6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBTHAYDONJTGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I just wanted to jump in and mention I'm facing the same error and applying the fix in PR #293 resolves the issue for me. |
For posterity, you can side-step this issue for now by providing your own function to Example function buildAppAuthUrl(string $storeDomain)
{
$shopifyOAuthCookie = new ShopifyOAuthCookie();
$cookieFunction = [$shopifyOAuthCookie, 'set'];
return OAuth::begin(
isOnline: false,
shop: $shopDomain,
redirectPath: '/shopify/auth/callback',
setCookieFunction: $cookieFunction,
);
}
class ShopifyOAuthCookie
{
public static function set(OAuthCookie $cookie): bool
{
return setcookie(
$cookie->getName(),
$cookie->getValue(),
$cookie->getExpire() ?? 0,
"",
"",
$cookie->isSecure(),
$cookie->isHttpOnly(),
);
}
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When using the package for OAuth of an offline/not embedded app, the function OAUTH::callback() attempts to use setcookie() with a null value for expires: parameter.
The same issue was raised here
#291
This issue is still extant as the pull request failed, I made the changes indicated and it works, however if I update my version I will be back to a fail again.
The text was updated successfully, but these errors were encountered: