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

Get password from session if not given when joining room #2121

Merged

Conversation

danxuliu
Copy link
Member

This pull request was extracted from the lobby pull request, as the change is not needed for the current lobby UI but it should be kept anyway.

As mentioned in the lobby pull request:

The commit itself introduced a regression when joining a password protected conversation as a logged in user (I have added an acceptance test that shows it); I fixed it for logged in users following the change you made for guests... but please check it :-)

Besides that I have added some acceptance tests that check that it is possible to join a password protected room again without having to enter again the password. @nickvergessen I am not sure if that is the expected behaviour (as it is not exactly what you described and it requires an additional change from what you originally did to not remove the password after joining the room), so please check that too; if that is not the expected behaviour the last fixup commit (the one that keeps the password after joining the room) needs to be dropped and the last acceptance tests adjusted to show that it is needed to authenticate again.

@danxuliu danxuliu added this to the 💚 Next Major milestone Aug 28, 2019
@danxuliu danxuliu requested a review from nickvergessen August 28, 2019 00:49
@danxuliu danxuliu mentioned this pull request Aug 28, 2019
6 tasks
@nickvergessen
Copy link
Member

Besides that I have added some acceptance tests that check that it is possible to join a password protected room again without having to enter again the password.

That is no intended. It should re-ask when you completely finished joining the room. Just not if there happened an error in between the loading of the room and the joining (like there used to be the lobby reload to check if the lobby is still enabled).

Otherwise anyone can visit all your public rooms again as long as the session didn't time out (default 1h).

@nickvergessen nickvergessen force-pushed the get-password-from-session-if-not-given-when-joining-room branch from 64a58eb to 050255d Compare August 28, 2019 07:12
@nickvergessen
Copy link
Member

Rebased and fixed up, now waiting for hours on drone again...

@danxuliu danxuliu force-pushed the get-password-from-session-if-not-given-when-joining-room branch from 050255d to 2eb38ad Compare August 28, 2019 07:15
@danxuliu
Copy link
Member Author

danxuliu commented Aug 28, 2019

That is no intended. It should re-ask when you completely finished joining the room.

Ah, OK; I was not sure because public shares do not need to authenticate again.

Rebased and fixed up

Oops, I force pushed too :-S Anyway the differences are not important. Edit: OK, I forgot the fixup... done now.

nickvergessen and others added 3 commits August 28, 2019 09:19
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu danxuliu force-pushed the get-password-from-session-if-not-given-when-joining-room branch from 2eb38ad to bff5d29 Compare August 28, 2019 07:20
@nickvergessen
Copy link
Member

So if we both did the some changes, I expect the tests to pass too => merging

@nickvergessen nickvergessen merged commit c9f390d into master Aug 28, 2019
@delete-merged-branch delete-merged-branch bot deleted the get-password-from-session-if-not-given-when-joining-room branch August 28, 2019 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants