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

Reload page instead of file list when getting 401 authentification error #30141

Merged
merged 2 commits into from
Dec 8, 2021

Conversation

CarlSchwan
Copy link
Member

@CarlSchwan CarlSchwan commented Dec 7, 2021

When getting a 401 error, instead of reloading the file list and hopping we are now
authenticated, we reload the page completely. This triggers a redirection to the login
page automatically with the correct ?redirect_url= in the URL, so that once logged the
user goes back to the place they were previously.

Test plan

  1. Open file webui in two tabs
  2. Logout in one of the two tabs
  3. In the other tabs, try to open an folder

Signed-off-by: Carl Schwan carl@carlschwan.eu

When reaching the root dir, instead of reloading the file list we reload
the page completely. This trigger a redirection to the login page automatically
with the correct ?redirect_url= in thr url.

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
@CarlSchwan CarlSchwan added bug 3. to review Waiting for reviews labels Dec 7, 2021
@CarlSchwan CarlSchwan requested review from juliusknorr and a team December 7, 2021 16:26
@CarlSchwan CarlSchwan self-assigned this Dec 7, 2021
@CarlSchwan CarlSchwan requested review from artonge and julien-nc and removed request for a team December 7, 2021 16:26
@CarlSchwan
Copy link
Member Author

Hmm actually this only handle the case there we are one folder deep.

@CarlSchwan CarlSchwan marked this pull request as draft December 7, 2021 16:40
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
@CarlSchwan
Copy link
Member Author

Hmm actually this only handle the case there we are one folder deep.

I changed that to not even checking if we are on the root directory. A 401 should be enough to detect an authentication problem.

From my local testing, 401 only happens when there is an authentication problem. 404 is when the folder is missing, 403 is when the user doesn't have access to the folder (e.g. file_accesscontrol)

@CarlSchwan CarlSchwan marked this pull request as ready for review December 7, 2021 16:51
@CarlSchwan CarlSchwan changed the title Stop reloading file list when getting 401 error on root dir Stop reloading file list when getting 401 authentification error Dec 7, 2021
@CarlSchwan
Copy link
Member Author

/backport to stable23

@CarlSchwan
Copy link
Member Author

/backport to stable22

@CarlSchwan
Copy link
Member Author

/backport to stable21

@CarlSchwan
Copy link
Member Author

CarlSchwan commented Dec 7, 2021

We also need to add error handling in apps/viewer/src/services/FileInfo.js...

@CarlSchwan CarlSchwan changed the title Stop reloading file list when getting 401 authentification error Reload page instead of file list when getting 401 authentification error Dec 8, 2021
@juliusknorr juliusknorr added this to the Nextcloud 24 milestone Dec 8, 2021
@juliusknorr
Copy link
Member

Failure unrelated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants