-
Notifications
You must be signed in to change notification settings - Fork 1.3k
precise error message if password has become invalid after password change #5522
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
Comments
I would like to work on this |
"after a password change" might be difficult to detect from the app. I could be wrong but I imagine MediaWiki doesn't let clients (like this app) know how old a user's password is on the server side, from a security standpoint. What I think we could do instead is:
|
I can understand that. How would it be if, instead of the meaningless error message "Failed" there would be a short keyword list of possible errors and also a link to a long version? |
There are more people who have this problem: |
Would there perhaps be a way to check whether the user is successfully logged in before each upload? |
@OpenGreenStreet We do check if the user is logged in before even allowing the user to add media details of the upload. Here's the code : |
@ShashwatKedia Yes, thank you. I assume this source code refers to the initial login? I am interested in checking the current validity of the login before each individual upload process. From my point of view, this is the only way to recognize whether the password has been changed in the meantime. (Sorry: I have no idea how something like this could be implemented programmatically). |
@OpenGreenStreet No, the above is code is a check in Upload Activity (which is started when an Upload Is triggered) so this is checked every time you try to upload something |
@shashankiitbhu Okay, but then I don't understand why the uploader only receives the unspecific message "Upload error" at the very end of the upload process? |
@OpenGreenStreet What the above piece of code does is it checks if the User is Logged in (If the user's current session is valid or not) and if it's not then it takes the user to the login page, but this happens immediately after you trigger an upload (as soon as Upload Activity is visible to user), so If you can go till the end of Upload Process then this logic isn't getting called at all. |
@shashankiitbhu The problem is that the user assumes that the app does not work. I (and others: Link: https://commons.wikimedia.org/wiki/Commons_talk:Mobile_app#App_broken) only realized after weeks(!) that the upload error was related to my own (long forgotten) last password change. If you're not persistent, you probably think that the app just doesn't work . . . and you will stop using the app or remove it from your smartphone altogether Is there nothing that can be read and checked that only logged-in users can do? (For example, I can only see which user is the administrator when logged in). |
@OpenGreenStreet Yes, If you are asking about what restrictions non-logged-in users have then you can see that by clicking "skip" at the Login Page and a restricted version will where you can not Upload Anything or Review Anything, as you can see below: Comparing it to the logged-in Version we can see that there are many things only logged-in users can do. I think the issue here is that the password is changed from the web frontend when you are already Logged-In on the app and you are then unable to upload anything after that. Is this correct? |
@shashankiitbhu Yes, that's exactly the problem: "I think the issue here is that the password is changed from the web frontend when you are already Logged-In on the app and you are then unable to upload anything after that. Is this correct?" It won't happen to me again. I just want to prevent others from giving up in frustration and no longer using the Commons app (which I really like!). (By the way: For me, the Commons app is a key application that significantly lowers the threshold for uploading images to Wikimedia Commons). |
It looks like login failure upon upload attempt is reported internally, but it gets lost somewhere before reaching the user. I tried this:
There was no error message that mentions log in failure or wrong password. (EDIT: in the user's eyes, I mean.) From Logcat: (a long list of messages)
|
@whym thank you, that's exactly how it was for me and it took me weeks to recognize the connection. |
@whym Yes that is exactly what is happening, we are getting an empty token but that message is getting over generalised to just being an error |
After a thorough examination of the code, I found the following solution that can be Implemented - After the Upload Process is complete we can show a message to the user that their Login Session has become Invalid and we will then send the user to Login Page (which I guess is the correct flow here). Note that this will only take place for the above case (when user's token is Invalid) not in case of any other upload failure. @whym @OpenGreenStreet @nicolas-raoul If this sounds good then I'll make these changes and add the PR here, thanks :) |
Here is how the flow will work after you Make an Upload (with your password changed from the web frontend) : WhatsApp.Video.2024-02-11.at.12.24.30.AM.mp4This way has the following advantages:
|
@shashankiitbhu Thank you for working on this. I generally support your approach - showing the login screen is better than showing a message saying to logout and login. Some minor points:
|
I meant pre-filled and editable. In the "wrong password or invalid token" context while trying to upload something, I assume the app already has the username internally and it is valid. But yes, it is possible that the user might have changed the name on the website, so the username might need to be edited. (I don't know if that would cause the same invalid token error, or something else, though.) |
@whym ok so let's make the username pre-filled, will make the PR soon for this |
@whym @nicolas-raoul Made the changes in the following PR, please take a look at it |
@whym @sivaraam @nicolas-raoul Upon More Examination of the app's behavior when the password is changed by the frontend website I found out that apart from this issue there are a few more that arise due to this change, for example - reviewing something (like nominating an Image for deletion by clicking "No") ends with failure and no precise message about that failure. So all these issues are linked to the user session becoming Invalid and the app not handling it well. These are the logs when I am trying to review a contribution:
Overall what we need to fix here is how we handle this case when the user's Current Session Becomes Invalid, I have the following two solutions :
Please Review both of these options and suggest if there is a better solution than this, Thanks |
That's a great catch! Approch 1 sounds better. |
@whym Great, so going forward with the 1st approach. I think since I am fixing for each case, it'll be better to have separate PRs for each case @nicolas-raoul what's your opinion on this? If having separate PRs works here then can you merge the above PR so that I can add PRs for other cases? |
@nicolas-raoul @whym Just Pinging you to ask if the approach works then can I go forward with the other 2 PRs? handling the other 2 cases? |
@sivaraam @whym For the related two Issues, Should I create a Single Issue Named - Unexpected Behaviour after the Password Change ?? Or Should I just make the PRs and related them to this Issue?
This is one of the Issues |
@whym, @shashankiitbhu
|
Just as an aside: I noticed afterwards that the current version of the app behaves "quite normally" until the category is queried: (I first noticed that something was "weird" because no categories were offered/selectable). |
Thanks for the report, @OpenGreenStreet. I've opened issue #5679 about this. |
The problem persists even with version 5.0.1~da0b2c28e. No meaningful message is displayed after a previous password change via the Wikipedia web interface:
|
Thanks for the feedback! |
*_2: Apparently every upload attempt was recognized by the system as a logon with an incorrect password *_3: For information only: after successfully logging out and successfully logging in again *_4: Successful upload with valid password |
@nicolas-raoul : One more thing: after logging off and on again, all settings are lost (for example for EXIF). Perhaps there could be a menu item "Relogin". |
@OpenGreenStreet I think the invalid login credential is not communicated as clearly as it should with the current set of changes. But you should certainly see a difference in v5.0.1. Specifically, the notification you receive in your notification drawer should have a message mentioning "Your log-in has expired. Please log in again". Do you not see that notification? Of course, it is not the most obvious way to communicate this. We should've at least had a toast mentioning this. It's sad that tapping on the notification doesn't take you to the login screen either. We could certainly improve this 👍🏼 On a side note, I suppose the flow you see when you try to Thank a user for a good image in the "Review" screen would match closely with what you expect. |
@sivaraam Yes, I also think that the information for users could be better. I just noticed that the upload failed. Then I remembered that I had changed my password. Then I logged out and logged in again. Unfortunately, I couldn't find a "log in again" option. As a result, all my settings were lost. |
Ah. The settings lost on each login is indeed unfortunate. I suppose a setting export/import option would be a good value addition that would help address your use case. Feel free to raise a request for it. We could certainly handle it. 👍🏼 |
Not really but it is a good starting point. The relogin would help your specific case but a settings export/import option would be more helpful as it would generally give an option to restore the settings, say, when the user switches to a new device. |
@sivaraam I don't really want to log out, I just want to update my password: hence the "relogin" idea |
The problem that there is no indication of an invalid password still exists with version 5.0.2~05ffd123e. |
What is the user problem or growth opportunity you want to see solved?
I would like there to be a pop-up message (or a clear error message: more than "Failed") if the password is no longer valid after a password change.
How do you know that this problem exists today? Why is this important?
Background: changed my password using the web frontend, but didn't think to change it in the Common app as well. It took me quite a while to recognize the connection...
Who will benefit from it?
all user
Anything else you would like to add?
https://commons.wikimedia.org/wiki/Commons:Mobile_app/Feedback#Feedback_from_Molgreen_for_version_4.2.1~14b6c455b_6
The text was updated successfully, but these errors were encountered: