-
Notifications
You must be signed in to change notification settings - Fork 27
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
using a space at the beginning of otpauth:// generate a wrong OTP #2122
Comments
Thanks, @luca-e075e! I've confirmed the behavior and marked this as being reproducible internally. |
Is someone working on this? |
@jayg2309 Thank you for your interest in contributing. As a starting guide please have a look at our Contribution Guidelines. These will get you started with setting up your development environment and how to proceed with your contribution. Please reference this issue when you create a pull request. |
Okay thankyou |
Is this sill an open issue? I was not able to reproduce it, as the new OTP is generated correctly after adding a space. Otherwise I am happy to help solve the issue :) |
I tested it on my iPhone and in the Chrome extension. In Both cases the issue is still happening. It is important that its an OTP that begins with otpauth://. @flooxo if you want to fix it, feel free to do that or I try to do this. |
I will wait some days and if you don't respond to this, I will fix this issue. I already know how to fix this, but if you want to fix it, I let you fix this. |
OK, thanks for the note. Feel free to fix it if you already know where in the code :) |
Ok, but I have to get the project up and running first on my PC. |
If you give me a hint where in the code something should be changed, I can do it too (it just saves time if i don't have to find out myself first). Whatever suits you best :) |
I try if the setup on my PC is easy, if it isn't easy I give you a hint so you can do it |
@flooxo Do you think its a good idea to remove the space at saving and at generation. At generating for existing entries and saving for new entries. |
Yeah, I guess the expected behavior when cloning an entry with an otp is to still have a valid otp path, right? So I would suggest to just trim the whitespaces |
I can't get xamarin running. @flooxo you can solve this issue. In this method the key must be trimmed because the StartwWith check don't work mobile/src/Core/Services/TotpService.cs Line 21 in 793c5fe
You can also search for the code where a entry get saved and trim the string before saving |
Ok, thanks, I've already looked at that method. I'll have to take a closer look at where the key is saved, because wouldn't it be a better solution if it was already saved correctly beforehand? |
Yes, but I think trim it at the generation is also necessary for keys which are already stored wrong. |
@flooxo would you also try to fix this in the browser extensions? If not I can try it. I think it's a good idea to create an new issue there and attach the link of this issue, so anyone has the context to the issue. |
Sure, i'll give it a try. Thanks :) |
Steps To Reproduce
Bitwarden Android, web interface and Firefox's extension are affected
" otpauth://..."
Expected Result
two possible results:
Actual Result
a wrong OTP is generated
Screenshots or Videos
No response
Additional Context
I found it while copying and pasting the otpauth:// line. Bitwarden interface's extension is small and it's not easy to immediately find what is wrong
Operating System
Android
Operating System Version
10
Device
No response
Build Version
2022.9.1 (5047)
Beta
The text was updated successfully, but these errors were encountered: