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.

Sign up for GitHub

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

Jump to bottom

logout redirects to login #5558

Merged
merged 4 commits into from Jan 19, 2018
Merged

logout redirects to login #5558

merged 4 commits into from Jan 19, 2018

Conversation

will-moore
Copy link
Member

When you logout, we previously tried to redirect to /webclient/. If public user was enabled for /webclient then you'd be auto logged in again (when you might have wanted to login as another user etc).
Now, we redirect to the login page.
When Public user is not enabled, the workflow is the same, since logout would then try to redirect to /webclient/ which would then redirect to /webclient/login/?url=%2Fwebclient%2F and then back to /webclient on login.

https://trello.com/c/AaY4cwkn/98-go-to-webclient-login-after-logout

Testing this PR

  1. In webclient, logout

  2. Check that you are redirected to /webclient/login/ (NB: no ?url=%2Fwebclient%2F)

@will-moore will-moore closed this Oct 24, 2017
@will-moore
logout redirects to login
24eed6f 
This now works even when a Public user is enabled.
Instead of auto-logging in a public user.
https://trello.com/c/AaY4cwkn/98-go-to-webclient-login-after-logout
@will-moore will-moore reopened this Oct 24, 2017
@joshmoore joshmoore changed the title Merge pull request #5553 from will-moore/fix_guest_username_check logout redirects to login Oct 24, 2017
@jburel jburel added the develop label Nov 14, 2017
rgozim
rgozim approved these changes Nov 14, 2017
View reviewed changes
Copy link
Member

@rgozim rgozim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested using public user and hitting the 'logout' button. Browser is directed to the login page, rather than back to the /webclient page.

@will-moore
Copy link
Member Author

Still some Robot tests failing - I need to add more fixes...

@sbesson sbesson added this to the 5.4.2 milestone Nov 20, 2017
@will-moore will-moore mentioned this pull request Dec 13, 2017
Merged
@will-moore
Copy link
Member Author

@jburel - The failure you're seeing at https://10.0.51.135:32776/job/OMERO-robot/22/robot/Web%20&%20Web/Web/Annotate%20Test/Test%20Search%20Results%20File%20Annotations/ should be fixed in this PR with 795bc26. Do you have this PR merged in your tests?

@will-moore will-moore changed the base branch from develop to workflows January 10, 2018 12:54
@will-moore
Copy link
Member Author

Changing the PR target to include it in the robot tests fixed the test on Chrome: https://10.0.51.135:32776/job/OMERO-robot/24/robot/Web%20&%20Web/Web_1/Annotate%20Test/Test%20Search%20Results%20File%20Annotations/
but not on Firefox: https://10.0.51.135:32776/job/OMERO-robot/24/robot/Web%20&%20Web/Web/Annotate%20Test/Test%20Search%20Results%20File%20Annotations/

@will-moore will-moore mentioned this pull request Jan 11, 2018
Merged
@joshmoore
Copy link
Member

Ah, this is against workflows now. Shall we delay the other public_user items until after 5.4.2 then?

@rgozim
Copy link
Member

rgozim commented Jan 17, 2018 •
edited

Checked on web-dev-merge 👍

@jburel
Copy link
Member

jburel commented Jan 17, 2018

@rgozim this PR is not included in web-dev-merge

@will-moore
Copy link
Member Author

Hmmm - strange. It seemed to be working earlier (clicking logout redirected to login page) when I saw @rgozim test this.
Maybe something else was going on. Will need to check but can't seem to access web-dev-merge now.

@jburel
Copy link
Member

jburel commented Jan 17, 2018

I can log in as user-1 w/o issue

@will-moore will-moore changed the base branch from workflows to develop January 18, 2018 10:06
@will-moore
Copy link
Member Author

It was a problem with VPN from home.
Today, with this PR base now develop, I am seeing the expected behaviour on web-dev-merge:

  • Logged-in as e.g. user-3, logout button takes me to /webclient/login/.

Previously this "appeared" to be working when testing logout as PUBLIC USER because the logout is a POST action that is not permitted by PUBLIC user, so the 403 redirects to login (public user is not actually logged out).
This can be seen when PUBLIC user logs out, the url they are redirected to is
/webclient/login/?url=%2Fwebclient%2F instead of simply /webclient/login/ as above.

@joshmoore
Copy link
Member

Merging for 5.4.2

@joshmoore joshmoore merged commit a49d37e into ome:develop Jan 19, 2018
@joshmoore joshmoore deleted the logout_redirects_to_login branch January 19, 2018 09:38
@jburel
Copy link
Member

jburel commented Jan 19, 2018

Confirmation: Ran the robots w/o this PR and the changes were not needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgozim rgozim rgozim approved these changes

Assignees
No one assigned
Labels
develop
Projects
None yet
Milestone
5.4.2
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@will-moore @joshmoore @rgozim @jburel @sbesson