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

using a space at the beginning of otpauth:// generate a wrong OTP #2122

Open
1 task
e075e opened this issue Oct 10, 2022 · 18 comments · May be fixed by #2865
Open
1 task

using a space at the beginning of otpauth:// generate a wrong OTP #2122

e075e opened this issue Oct 10, 2022 · 18 comments · May be fixed by #2865

Comments

@e075e
Copy link

e075e commented Oct 10, 2022

Steps To Reproduce

Bitwarden Android, web interface and Firefox's extension are affected

  1. Open Bitwarden
  2. clone a working entry that has a valid OTP
  3. edit the cloned entry putting a space at the beginning of otpauth://... like this:
    " otpauth://..."
  4. save

Expected Result

two possible results:

  1. a space before otpauth:// is wrong, bitwarden should not generate any OTP but display a warning on the wrong OTP field
  2. bitwarden deals with wrong characters like spaces before otpauth:// and generate a working OTP

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

  • Using a pre-release version of the application.
@e075e e075e added the bug label Oct 10, 2022
@tangowithfoxtrot
Copy link

Thanks, @luca-e075e!

I've confirmed the behavior and marked this as being reproducible internally.

@jayg2309
Copy link

Is someone working on this?
Can i work on this?

@djsmith85
Copy link
Contributor

@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.

@jayg2309
Copy link

Okay thankyou

@flooxo
Copy link

flooxo commented Oct 24, 2023

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 :)

@FlorianLang06
Copy link

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.

@FlorianLang06
Copy link

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.

@flooxo
Copy link

flooxo commented Nov 9, 2023

OK, thanks for the note. Feel free to fix it if you already know where in the code :)

@FlorianLang06
Copy link

Ok, but I have to get the project up and running first on my PC.

@flooxo
Copy link

flooxo commented Nov 9, 2023

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 :)

@FlorianLang06
Copy link

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

@FlorianLang06
Copy link

@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.

@flooxo
Copy link

flooxo commented Nov 9, 2023

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

@FlorianLang06
Copy link

FlorianLang06 commented Nov 9, 2023

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

public async Task<string> GetCodeAsync(string key)

You can also search for the code where a entry get saved and trim the string before saving

@flooxo
Copy link

flooxo commented Nov 9, 2023

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?

@FlorianLang06
Copy link

Yes, but I think trim it at the generation is also necessary for keys which are already stored wrong.

@flooxo flooxo linked a pull request Nov 11, 2023 that will close this issue
5 tasks
@FlorianLang06
Copy link

@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.

@flooxo
Copy link

flooxo commented Nov 12, 2023

Sure, i'll give it a try. Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants