search

webcompat / webcompat.com

Source code for webcompat.com
https://webcompat.com
360 stars 191 forks source link

Automatic login is performed when reporting via GH #3054

Open softvision-sergiulogigan opened 5 years ago

softvision-sergiulogigan commented 5 years ago

Environment: Browser / Version: Firefox Nightly 72.0a1 (2019-10-24) Operating System: Windows 10 Pro

Prerequisites:

Steps to Reproduce:

  1. Navigate to https://staging.webcompat.com/issues/new
  2. Follow the flow to log a new issue.
  3. Click on the "Report via GitHub" button.

Expected Behavior: The GitHub login page is opened.

Actual Behavior: The issue is logged using the GH username. A login is performed.

Notes:

  1. Screenshot attached.

Watchers: @softvision-sergiulogigan @softvision-oana-arbuzov @cipriansv

Screen capture

miketaylr commented 5 years ago

@softvision-sergiulogigan I think this is our current behavior in form v1 as well. What would you expect to happen in this situation?

softvision-sergiulogigan commented 5 years ago

What would you expect to happen in this situation?

@miketaylr if a log out is performed from any page, it should remain that way, and not perform an auto-login without any confirmation from the user.

miketaylr commented 5 years ago

OK, sounds reasonable to me. But we will need to re-think how to achieve this. Currently "logging in with GitHub" is really about obtaining OAuth permission from GitHub to do things on your behalf. Once you do that, you never really do it again (unless you revoke access, or the developer revokes access for everything) -- it's sort of a permanent permission state.

"Logged out" on webcompat.com doesn't really mean "permission is revoked", it just means "not logged in". If that makes any sense...

karlcow commented 5 years ago

It could indeed create an issue on a shared computer model/environment (classroom). Someone files an issue, someone logs out. Then the next person comes.

@miketaylr when the person is logged out. What about removing the user from session.db.

https://github.com/webcompat/webcompat.com/blob/55a196c64b0ae58a21251735c539af75869ee04f/webcompat/views.py#L111-L116

with session_db.remove(user) and session_db.commit() OR DELETE /authorizations/:authorization_id https://developer.github.com/v3/oauth_authorizations/#delete-an-authorization

karlcow commented 5 years ago

hmm delete authorization might not be a good idea either. Because if the person is logged in another context, we just removed it with this.

miketaylr commented 5 years ago

So far, we don't have evidence of a non-ninja bug hunter complaining about this behavior (which doesn't mean nobody has complained about it...). So, a valid bug, but probably low priority.