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

show login page when session expires #56

Closed
prescriptionlifeline opened this issue Jun 2, 2024 · 4 comments
Closed

show login page when session expires #56

prescriptionlifeline opened this issue Jun 2, 2024 · 4 comments

Comments

@prescriptionlifeline
Copy link

prescriptionlifeline commented Jun 2, 2024

I have Route::get('files', 'FilesController@view')->name('files'); in my routes/backpack/custom.php file and that config/backpack/base.php has route_prefix set to admin. As such, to access the page created by this project, I go to /admin/file.

So let's say I do just that. I then walk away and do something else and when I come back the session has expired and I'm locked out. /admin/file doesn't know that I've been logged out so I'll come back to that same page. If I then try to upload files I get this error:

Unable to connect to backend.
HTTP error 401

Ok cool, that all looks good, altho I think maybe redirecting to the login page with a helpful error message of something like "Your session has expired - please log back in to continue" might be more useful.

Anyway, let's say that I kept the /admin/file tab open and then logged back in in a new tab. After I login I go back to the /admin/file tab. Instead of reloading I just try to start uploading a folder. At that point the folders will be created but the file won't be uploaded. It look like all GET requests to /admin/files/connector succeed but all POST requests to /admin/files/connector fail.

Thanks to the Network tab of Developer tools in Chrome I can see that the HTTP POST requests to /admin/files/connector are failing with 419 | PAGE EXPIRED. The HTTP POST requests all have cmd set to "upload" whereas cmd for HTTP GET requests it would appear that cmd can be set to any number of options including, but not necessarily limited to, mkdir, ls, size, parents and open.

Unless /admin/file were to show the login screen immediately when your session expired (which would fix both issues) I think the next best solution to this is to just reload the page whenever an HTTP POST request would fail. Altho it'd be nice to do this test before elFinder does any operations, because, right now, when you're upload a folder, in particular, elFinder sends a bunch of HTTP GET requests first that create al the subfolders. It just can't actually upload any files into those subfolders so it silently fails, leaving you with a bunch of empty subfolders vs filled subfolders as you might be expecting.

Another solution would be to make it so that both POST and GET requests fail instead of just POST requests. Then show a popup error from the get go just as is done when you're not logged in at all.

The problem with the current approach is that it, as I already said, it silently fails. These silent failures could undermine confidence in the whole tool if you don't realize what's happening.

@pxpm
Copy link
Contributor

pxpm commented Jun 3, 2024

Hello @prescriptionlifeline

Thanks for the question and for such a detailed explanation.

I think I understand the issue that you raise, but could that be related to your setup ?

What's the reason to have registered a new route ? Why not configure the route in:

'route' => [
?

How about the access, are you properly validating the access ?

'access' => 'Barryvdh\Elfinder\Elfinder::checkAccess',

You can create your own access validation, you can find the original file here:
https://github.com/barryvdh/laravel-elfinder/blob/master/src/Elfinder.php

Let me know if that helps. In the meantime I will investigate on my side too if there is something else we (Backpack) can do to ease things here.

Cheers

@prescriptionlifeline
Copy link
Author

prescriptionlifeline commented Jun 5, 2024

How about the access, are you properly validating the access ?

Looking at https://github.com/barryvdh/laravel-elfinder/blob/master/src/Elfinder.php it looks like that's more of a way to simulate chmod. None-the-less I tried it out.

I opened up mywebsite.com/config/elfinder.php and changed access to from 'Barryvdh\Elfinder\Elfinder::checkAccess' to
'App\Http\Helpers\Elfinder::checkAccess'. I then created a new file, mywebsite.com/app/Helpers/Elfinder.php, with the following contents:

<?php
namespace App\Http\Helpers;

class Elfinder
{
    public static function checkAccess($attr, $path, $data, $volume)
    {
        exit('THIS FAR');
        return request()->user()->isAdmin == 1;
}

That didn't do anything. Like I opened up Google Chrome's Network tab and all the requests to /admin/files/connector were still succeeding when I would have expected them all to be outputting "THIS FAR" and that's it.

And even if they had succeeded in outputting "THIS FAR", idk that the request()->user()->isAdmin == 1 check would do anything anyway. I already have a class that does that in middleware_class in config/backpack/base.php and it seems to work. As documented in my original post I already get this error when I'm logged out:

Unable to connect to backend.
HTTP error 401

The problem I'm having is when I'm still logged in but the session is not the same one that I had when I originally opened the ElFinder page.

@prescriptionlifeline
Copy link
Author

What's the reason to have registered a new route ?

In doing git blame on mywebsite.com/routes/backpack/custom.php it looks like that route was initially added as a placeholder for a file management page and it looks like that line was just never removed from the route when the decision was made to go with Laravel-Backpack/FileManager.

I commented that line out and the behavior I described in my orig post persists. Namely that when the session is different that POST requests to /admin/files/connector (which are mainly produced by file uploads) fail with a "419 | PAGE EXPIRED". And the way I'm simulating this is by using this sequence of steps:

  1. I go to the ElFinder page
  2. I open up a new tab, log out of my Laravel instance from that tab and then log back in
  3. I go back to the ElFinder tab and, without reloading it, I create a directory (which works because it's a GET request) and I then try to upload a file (which fails because it's a POST request)

@pxpm
Copy link
Contributor

pxpm commented Jun 10, 2024

@prescriptionlifeline I have been investigating this, and I think that's what should happen.

You login first, your CSRF token is ABC. You logout and then login again, your CSRF token is now CBA.
In the page that you didn't "reload", it will still send ABC as your token (it's not aware of the change), and you will get a 419 response stating that the tokens does not match, application was expecting CBA from the logged in user.

The GET endpoint works because it doesn't need the CSRF token, only need you to be logged in.

I am not sure about a solution for this, maybe you can implement a setTimeout or something on the page, that tracks user actions, if the user don't do nothing for like 59m, refresh the page before the 60m mark where the session expires.

Other than that, maybe try to intercept the request on a middleware and setup some cookie or some session thing you can use on the frontend to refresh the page if for example the tokens don't match.

I don't think there is nothing us (backpack) can do here, but if you have a solution that is not something huge to maintain, feel free to submit a PR and we can talk about it 🙏

Cheers

@pxpm pxpm closed this as completed Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

2 participants