OpenEVSE / ESP8266_WiFi_v2.x

ESP8266 WiFi for OpenEVSE Version 2.x
https://openevse.openenergymonitor.org
76 stars 46 forks source link

www_password limited to 15-characters but not validated #193

Closed jshank closed 5 years ago

jshank commented 6 years ago

I've now twice been locked out of the web interface. I set the password as "VPIBaqzUoM70CStt" (generated by LastPass). The first time was after a firmware upgrade to 2.7.4. I just did a restart on the unit and my credentials aren't working again. Is this password too complicated? In the administration section, it shows my username (jshank) with a password of _DUMMYPASSWORD.

OpenEVSE WiFi Firmware: | 2.7.4 Flash Size: | 4096K Free RAM: | 18K

jeremypoulter commented 6 years ago

Hi, I am very sorry about this.

It looks like it very well could be the password length. From what I can see the password is limited to 15 chars however we don't look to error or anything if this limit is breached. I think the password will just be cropped so should be VPIBaqzUoM70CSt.

The UI can definitely be improved in regard to validation of inputs, @glynhudson what do you think?

The ___DUMMY_PASSWORD___ is just a magic value used by the UI if you have set a password, so the UI will show something in the password box but you can't view the password by using view source or the browser inspector, etc.

jshank commented 6 years ago

No need to be sorry, it's an amazing interface and thank you for it. I confirmed that typing the first 15 characters of the password worked just fine. An input validation or longer password field would definitely fix this.

jeremypoulter commented 6 years ago

It will probably have to be 15 chars in the short term, changing the length will cause the current saved password to be lost, the structure config is not really designed to be changed

glynhudson commented 6 years ago

Yes, I agree. Let's stick with 15 characters for the moment but add validation to ensure this doesn't catch people out. Thanks for reporting @jshank :+1:

Suriv commented 5 years ago

My proposal, add required, and tittle and maxlength

<input type="text" autocapitalize="none" autocapitalize="none" data-bind="textInput: config.pass" pattern=".{15}" title="15 characters limited" maxlength="15" required="" >

captura de pantalla 2019-01-09 a las 11 06 53 captura de pantalla 2019-01-09 a las 11 04 10

How could the modified data from src be passed to the simulator to verify the changes?

glynhudson commented 5 years ago

Hi @Suriv , thanks for the PR. Nice idea, I've just tested but it seems the text box color stays green even if I enter a long password. I'm still able to save a long password

selection_063

Note: DUMMY_PASSWORD also needs to be changed in web_server.cpp

Suriv commented 5 years ago

Hi @glynhudson

If you help me compile the html and move it to the data or the simulator, I will be able to verify it.

I have done tests in the simulator.

upload video: https://we.tl/t-YSnMWGD03r

Note: Macosx, Chrome: Versión 71.0.3578.98

glynhudson commented 5 years ago

Ah ok, I was testing on the HTTP password field rather than the WiFi PSK field sinc this is what this issue is regarding. The WiFi password can be up to 64 characters long which is the maximum permissible WiFi password length therefore validation is not essentail: https://github.com/OpenEVSE/ESP8266_WiFi_v2.x/blob/a5a43a9f025b00d85671e49b0e42f0fab19d173b/src/config.cpp#L36

The http username / password can only be 16 characters long, validation is needed to ensure user does not try and set a long er username / password. https://github.com/OpenEVSE/ESP8266_WiFi_v2.x/blob/a5a43a9f025b00d85671e49b0e42f0fab19d173b/src/config.cpp#L48

What issues are you having uploading the code? Have you got platformio up and running? Assuming you have seen: https://github.com/OpenEVSE/ESP8266_WiFi_v2.x#firmware-compile--upload

Suriv commented 5 years ago

update

glynhudson commented 5 years ago

Nice work, I've just tested PR. It now works as expected which is great. Couple of issues:

screenshot_20190118-174025 screenshot_20190118-174032

Suriv commented 5 years ago

Hello

I changed the color code required will be blue along with a text indicative of the characters that must be entered, as in the image.

captura de pantalla 2019-01-18 a las 23 50 51

When it is correct it will turn green and the text will disappear.

captura de pantalla 2019-01-18 a las 23 52 30

The configuration of the password field indicates 16, I will just change it to 15.

Where does it indicate that they are 15 characters for the user?

glynhudson commented 5 years ago

The max number of characters is 15. There is no minimum requirement, therefore the fields should be green until the 15 character limit is exceeded when it should turn red and the 16th character should be disallowed.

Suriv commented 5 years ago

The point is that I did not get to 16 because it is not going to leave the field with the max-length property of the input that is set

I have changed the formats, since the red color would be to indicate error, I have put the blue to indicate that the field is required and it becomes green when you reach those 15 characters telling the user that it is correct.

glynhudson commented 5 years ago

Sure, I'm happy with that.

Couple of things:

Suriv commented 5 years ago

Sure, I'm happy with that.

Couple of things:

  • "the field requires 15 characters" should be changed to "15 characters max"
  • there seems to be a duplicate of the username field (maybe this is a browser cache issue? can you replicate)

It was a test to see if a normal or required field. It is already deleted.

And uploaded the latest update

glynhudson commented 5 years ago

Great thanks 👍

Suriv commented 5 years ago

It’s my pleasure