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

Session data getting lost on using TokenRedirect #82

Closed
talkwithdeveloper opened this issue Feb 3, 2023 · 31 comments
Closed

Session data getting lost on using TokenRedirect #82

talkwithdeveloper opened this issue Feb 3, 2023 · 31 comments
Labels
blade-engine This issue is using blade as the template engine bug Something isn't working

Comments

@talkwithdeveloper
Copy link

For bug reporting only! If you're posting a feature request or discussion, please ignore.

Expected Behavior

The session data should be retained.

Please describe the behavior you are expecting.

I am using a tokenRedirect inside a controller. I am using a non-SPA.

I have set session before it.

Post token redirect I am unable to retreive the added values from session as it does not exist.

Current Behavior

As mentioned above.

Steps to Reproduce

Please provide detailed steps for reproducing the issue.

  1. Just create a route and add the following code to it to add session.
 session()->put('data','hello');
 return Redirect::tokenRedirect('home');
  1. Just try to fetch the data inside your home route's blade.
@if (session()->exists('data'))
    {{ dd(session()->all()) }}
@endif

Context

Please provide any relevant information about your setup. This is important in case the issue is not reproducible except for under certain conditions.

  • Package Version: ^17.4
  • Laravel Version: ^8.75
  • PHP Version: 7.4.33
  • Template Engine: Blade
  • Using a toolset (Docker, Laradock, Vagrant, etc.): None

Failure Logs

Please include any relevant log snippets or files here.

@talkwithdeveloper talkwithdeveloper added bug Something isn't working unconfirmed Bug has not been reproduced yet labels Feb 3, 2023
@Kyon147 Kyon147 added the help wanted Extra attention is needed label Feb 3, 2023
@talkwithdeveloper
Copy link
Author

@Kyon147 can you confirm this bug, if possible?

@Kyon147
Copy link
Owner

Kyon147 commented Feb 3, 2023

A quick google search has return this https://laravel.com/docs/master/session#flash-data

Has your tried this method? To see if it works as an alternative?

@talkwithdeveloper
Copy link
Author

A quick google search has return this https://laravel.com/docs/master/session#flash-data

Has your tried this method? To see if it works as an alternative?

No this doesn't. Also, I tried on a non shopify route with normal redirect and it works fine as expected. Session data is retained on that.

@Kyon147
Copy link
Owner

Kyon147 commented Feb 3, 2023

A quick google search has return this https://laravel.com/docs/master/session#flash-data
Has your tried this method? To see if it works as an alternative?

No this doesn't. Also, I tried on a non shopify route with normal redirect and it works fine as expected. Session data is retained on that.

Can you send me what you did that worked?

@talkwithdeveloper
Copy link
Author

talkwithdeveloper commented Feb 4, 2023

@Kyon147

I mean if you just do this:

  session([
            'data'=> 'hello'
        ]);
        return Redirect::to('/some-route');

On wiritng the logic of some-route just do:

dd(session()->all());

You would see it would work as expected, the session would be retained.

@Kyon147 Kyon147 removed the unconfirmed Bug has not been reproduced yet label Feb 4, 2023
@Kyon147
Copy link
Owner

Kyon147 commented Feb 4, 2023

@talkwithdeveloper I can confirm using the token redirect, the session is lost.

I had a quick look at the cause but could not narrow anything down at the moment. I won't have a lot of time for the next week - so if you can help debug this issue it would be helpful.

If you find a solution, open a PR and I'll take a look.

@Kyon147 Kyon147 added the blade-engine This issue is using blade as the template engine label Feb 4, 2023
@talkwithdeveloper
Copy link
Author

@talkwithdeveloper I can confirm using the token redirect, the session is lost.

I had a quick look at the cause but could not narrow anything down at the moment. I won't have a lot of time for the next week - so if you can help debug this issue it would be helpful.

If you find a solution, open a PR and I'll take a look.

Sure I would try my best to narrow it ASAP.

@Ammusingh
Copy link

Ammusingh commented Feb 23, 2023

how to send data from post method using tokenRoute in laravel,
get method is working properly, please tell about the post form data @Kyon147 @talkwithdeveloper

@Kyon147
Copy link
Owner

Kyon147 commented Feb 23, 2023

Session data is being lost at the moment, as this ticket is still unresolved.

Feel free to open a PR with a solution if you find one.

@talkwithdeveloper
Copy link
Author

Hi any updates on this one?

@Kyon147
Copy link
Owner

Kyon147 commented Apr 9, 2023

@talkwithdeveloper it's not been looked at by myself in much detail yet. It's on my long list of things and being an issue on blade templates I don't actively see it because I mainly use this package as an SPA.

When I did take out an hour to look at it, I can confirm the session was lost but I followed through using xDebug and couldn't pin point the issue at the time.

I'm hoping someone else can take a deeper look, preferably someone who uses blade like yourself and open a PR, otherwise it will just have to wait until I get to it on my to-do list.

@KirillBuziuk
Copy link

It's still a pretty relevant issue... its solution would help a lot

@KirillBuziuk
Copy link

KirillBuziuk commented May 18, 2023 via email

Repository owner deleted a comment from kashif-sol May 18, 2023
@Kyon147
Copy link
Owner

Kyon147 commented May 18, 2023

@kashif-sol This package is stable, enough for me to be able to run three Shopify apps successfully using it. I am however only one coding human, so I can't fix all bugs as soon as they come up and try to prioritise the urgent ones that would effect an app from loading.

Any package is going to have bugs, like all software on the internet. You are free to use Shopify's packages if you wish, that choice is yours and yours alone.

Please refrain from unnecessary spam comments like "close the package" etc as it is just not constructive or helpful.

Hope you have a good day and happy coding.

@michaellehmkuhl
Copy link
Contributor

@Kyon147 I don't have a solution to this issue, but I think I understand at least part of what might be happening.

When we're running an embedded app in the Shopify admin, Shopify essentially forces us to hand over management of the user login session to the Shopify admin. So on every request, the VerifyShopify class checks with Shopify to make sure the user is still logged in on the Shopify side. However, on the Laravel side, we get a new Laravel session every pageload, because there's nothing that ties the Shopify session token to the Laravel session ID.

I'm using the verify.shopify middleware to handle my Shopify login persistence.

I was able to verify that I am getting a brand new Laravel session every pageload by logging session()->getId() after $this->auth->login($shop) in VerifyShopify::loginShopFromToken() which is running on each pageload. While the Laravel session ID changes with each pageload (and creates a new entry in my sessions table in the database), the Shopify session token remains constant ($context->getSessionToken()->toNative()) across pageloads (at least until it expires - see below), but unique to my Shopify admin login session.

So it seems that the missing piece here is some sort of lookup table to tie together the Shopify session to the Laravel session, so we can reuse the same Laravel session based on the Shopify session. The other tricky bit is probably that Shopify's session tokens have a lifetime of only one minute.

I do see that loginShopFromToken() seems to have the ability to accept a session= query string parameter from the request to set a session ID, but that parameter seems to always be null for me. And unless I'm missing it, I don't think that session ID is being used in the package to link to a Laravel session to persist Laravel session data across pageloads.

It's totally possible that there's user error on my part somewhere, but if so I hope documenting what I'm seeing here might help someone else out.

@Kyon147
Copy link
Owner

Kyon147 commented Jun 2, 2023

Hey @michaellehmkuhl

That's some good investigation, and really useful.

Yeah the session is coming off the back of the JWT token authentication which lasts one minute as you say. I'll have a think about how we could potentially keep session data for longer - it could be refactoring the way loginShopFromToken works or another route.

Appreciate you taking the time to delve into this a bit more.

@SachinBahukhandi
Copy link
Contributor

Hey @michaellehmkuhl

That's some good investigation, and really useful.

Yeah the session is coming off the back of the JWT token authentication which lasts one minute as you say. I'll have a think about how we could potentially keep session data for longer - it could be refactoring the way loginShopFromToken works or another route.

Appreciate you taking the time to delve into this a bit more.

But I tried to find on how the Redirector class' originally works and it retains the session.

@Kyon147
Copy link
Owner

Kyon147 commented Jun 2, 2023

@SachinBahukhandi So normal redirects keep session, but the tokenRedirect is not?

@SachinBahukhandi
Copy link
Contributor

@SachinBahukhandi So normal redirects keep session, but the tokenRedirect is not?

Strange but true.

@SachinBahukhandi
Copy link
Contributor

I used this package to generate the backtrace:

@michaellehmkuhl
Copy link
Contributor

In my case, I'm not able to persist a Laravel session at all. Each request initiates a new Laravel session (using v18.0.1, by the way).

@Kyon147
Copy link
Owner

Kyon147 commented Jun 3, 2023

I'll spend a little bit of time this weekend looking into it, thanks for all your research and notes.

@SachinBahukhandi
Copy link
Contributor

I'll spend a little bit of time this weekend looking into it, thanks for all your research and notes.

Any updates on this one? If someone could help with the development I would do the rest of the things! Much appreciate the help in advance!

@CodaemonHaroon
Copy link

Any updates on this? I am also unable to get the session values which i stored. I have also removed verify.shopify middleware to test.
I have laravel version 8 with ossiset package.
And also starnge thing is happening with me is. When i am login in with laravel login blade after redirecting the shopify user is returned not the laravel logged in user. Also tried applying auth Middleware but then it will not redirect to Dashboard page.
Hope any can help me.

@Kyon147
Copy link
Owner

Kyon147 commented Jul 12, 2023

I'm still investigating this at the moment but not had a lot of time to really dive into it.

In terms of your issue @CodaemonHaroon you have to use the verify.shopify middleware otherwise you won't get the logged in shop. It is mandatory on all routes that you want to get current Shop data or api calls.

Redirect wise I am not sure what your issue is but check you are passing the host param.

@Kyon147
Copy link
Owner

Kyon147 commented Jul 28, 2023

Hi everyone,

I think I have found the issue, it is actually down to the browsers and the secure cookie polices now. I remember us having this issue before JWT was implemented.

You have to make sure the laravel app has the correct session cookie setting inside session.php

To get it to work I had to change these values.

Secure has to be true
'secure' => env('SESSION_SECURE_COOKIE', true),

Same site needs to be none
'same_site' => 'none',

APP_URL has to be the correct url set to the app you are loading.

Most important of all, you have to be on https chrome and other browsers no longer allow insecure cookies being set even when you set secure to none it seems.

All these now meant I could use Session::falsh() to save a message. Session::put() also worked to save data across the pages.

Before
image

After on new page after deleing and setting the flash message.
image

@Kyon147 Kyon147 removed the help wanted Extra attention is needed label Jul 28, 2023
@Kyon147
Copy link
Owner

Kyon147 commented Jul 31, 2023

Closing this now as multiple tests are working for me, but can re-open if the issue comes back.

@Kyon147 Kyon147 closed this as completed Jul 31, 2023
@CodaemonHaroon
Copy link

@Kyon147, hello i have installed latest version of the package for this, created a fresh setup. the App is installed successfully.
php => 8.1
laravel => 8
"kyon147/laravel-shopify": "^19.0"
I am setting the session but it is not coming on the other controller.

use Illuminate\Support\Facades\Session;
USED THESE below code of line to set sessions.
$request->session()->put('username', $user->username);
Session::flash('shop_user', 'abc');
Session::put('username', 'xyz');
$request->session()->push('username', 'xyz');
FOR Accessing
Session::get('username')
BUT NO LUCK

I have made the changes in config/session.php file as you said.

@Kyon147
Copy link
Owner

Kyon147 commented Aug 3, 2023

@CodaemonHaroon as it is a cookie / browser issue and not a package issue there might be other things with your set up or code or even the browser that might require changing or additional updates.

I've confirmed myself that it's not a package issue and provided the most likely steps to resolve the issue.

If you need specific help, outside it being an issue with the package which I know believe it is not as I can get it working. Can you provide more information on your set up in our Discord.

Someone or myself can then look at it when free to do so.

Update:
To give a bit of insight.

You have to make sure that the session cookie is staying the same.

If it changes then for some reason the app is unable to set the cookie and a new session is created each time in Laravel.

Also, is this a local app, is it on a server. Are you on http or HTTPS? There's a lot of variables to consider.

@CodaemonHaroon
Copy link

@Kyon147 Hello, can you please help/provide me the code for how to set the Session and use across my all blades files while redirecting.

@Kyon147
Copy link
Owner

Kyon147 commented Sep 11, 2023

@CodaemonHaroon have you check this PR which another user has fixed the issue #133

Try adding the code from the PR mentioned to pass along the host, then sort out the cookies for the app and see if that solves the issue.

The PR for the host in @sessionToken will be deployed this week properly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blade-engine This issue is using blade as the template engine bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants