WebGUI User Input Validators

[Martinski4GitHub]

More functionality improvements in WebGUI code to have validators for the login password & postponement days input text boxes.

[Martinski4GitHub]

@ExtremeFiretop,

Well, this morning I woke up with an "itch" to improve the input validator for the postponement days input text box, and then I added another one for the password text box. The latter is just a very simple string length validator to make sure that at least we have some reasonable, non-empty password string before being allowed to save it.

Here are some screenshots:

Enjoy the day, bud!!

[TheS1R]

@ExtremeFiretop,

Well, this morning I woke up with an "itch" to improve the input validator for the postponement days input text box, and then I added another one for the password text box. The latter is just a very simple string length validator to make sure that at least we have some reasonable, non-empty password string before being allowed to save it.

Here are some screenshots:

Enjoy the day, bud!!

One throught on the validation of password strength... I don't think that it is necessary. One does not change the router password here, but rather it is just saved for the script to log into the router while performing its job. The job of validating password strength ultimately lies upon the router firmware itself. Asus criteria appears to be that password must be between 5 and 32 characters, period — your check is already done by the router when password is changed. A better check, if possible, would be to verify that password matches current password (i.e., does login succeed) — this would need to be different than method that script uses as it simulates login to web UI.

For Set Directory for F/W Updates field, is there a way to verify that the directory actually exists (i.e., drive inserted and mounted)? when field is changed?

[ExtremeFiretop]

A better check, if possible, would be to verify that password matches current password (i.e., does login succeed) — this would need to be different than method that script uses as it simulates login to web UI.

For Set Directory for F/W Updates field, is there a way to verify that the directory actually exists (i.e., drive inserted and mounted)? when field is changed?

Keep in mind the WebUI will always be a more simplified implementation of the shell script, no matter what addon.

Because we have less creative freedom with the WebUI. It's a castrated HTTP server and half the stuff I try from my regular XAMPP install might not work here... Because of that, lots of the features (like the password test feature) may not be implemented or not to the same degree.

Now that being said, I do believe there may be a nvram value for the password we could compare the file value too? Am I wrong? Idk I'm not at my desk but it could be something to look into.

Similar situation with the USB drive, I'm not sure what type of access we have from the WebUI to see local directory files, probably not much access is my assumption.

[ExtremeFiretop]

@Martinski4GitHub

End of the line for the AC models!

https://www.snbforums.com/threads/announcement-asuswrt-merlin-386_x-devices-are-no-longer-supported.93555/

[TheS1R]

A better check, if possible, would be to verify that password matches current password (i.e., does login succeed) — this would need to be different than method that script uses as it simulates login to web UI.
For Set Directory for F/W Updates field, is there a way to verify that the directory actually exists (i.e., drive inserted and mounted)? when field is changed?

Keep in mind the WebUI will always be a more simplified implementation of the shell script, no matter what addon.

Because we have less creative freedom with the WebUI. It's a castrated HTTP server and half the stuff I try from my regular XAMPP install might not work here... Because of that, lots of the features (like the password test feature) may not be implemented or not to the same degree.

Now that being said, I do believe there may be a nvram value for the password we could compare the file value too? Am I wrong? Idk I'm not at my desk but it could be something to look into.

Similar situation with the USB drive, I'm not sure what type of access we have from the WebUI to see local directory files, probably not much access is my assumption.

I just looked for a password nvram variable — it doesn't appear to exist. I only see VPN client passwords.

USB drive mount names looks more promising:

usb_path_sda=2-1
usb_path_sda1=2-1
usb_path_sda1_label=TheS1RsUSB
usb_path_sda_label= 
usb_path_sdb=1-2
usb_path_sdb1=1-2
usb_path_sdb1_label=bckupTheS1R
usb_path_sdb_label=

The check might be as simple as matching similar to the following for my drive used by MerlinAU:

nvram show | grep usb | grep label | grep TheS1RsUSB

[ExtremeFiretop]

I just looked for a password nvram variable — it doesn't appear to exist. I only see VPN client passwords.

That's unfortunate and makes the job a lot harder on that front. But actually now that I think about it, it makes sense why we prompt the user to enter the password in the core shell script instead of grabbing it from nvram.

USB drive mount names looks more promising:

usb_path_sda=2-1
usb_path_sda1=2-1
usb_path_sda1_label=TheS1RsUSB
usb_path_sda_label= 
usb_path_sdb=1-2
usb_path_sdb1=1-2
usb_path_sdb1_label=bckupTheS1R
usb_path_sdb_label=

The check might be as simple as matching similar to the following for my drive used by MerlinAU:

nvram show | grep usb | grep label | grep TheS1RsUSB

We can probably do a basic check against the root directory with some nvram info we have. Would need to be tested

[ExtremeFiretop]

I just looked for a password nvram variable — it doesn't appear to exist. I only see VPN client passwords.

I think I was thinking of the WiFi password itself which I know exists in nvram.

[ExtremeFiretop]

@Martinski4GitHub

The only thing weird I found after testing this PR was this:
When entering a new password I see our eye twice now?

I don't remember seeing this bug after fixing the bug you reported yesterday.. (and yes I tested filling the password box with a net new password yesterday.)

I'll try to poke at this again, if it's really too complicated to get right we can rollback this too.

[ExtremeFiretop]

@Martinski4GitHub

Interestingly, this issue is browser specific. The edge browser is inserting it's own "eye" to view the password.
Outside of the code we added, so if I try with Firefox, I don't have the issue, apparently Chrome doesn't either.

But Edge will "insert" it's own eye into this box now for some reason.

[ExtremeFiretop]

I managed to re-create the issue on my version, for some reason I just had to backout/empty the password twice in a row.
Now my version also gives this double eye situation. I say it can be ignored.

[TheS1R]

I managed to re-create the issue on my version, for some reason I just had to backout/empty the password twice in a row. Now my version also gives this double eye situation. I say it can be ignored.

Yes, Edge specific, and if you click on right eye, left eye disappears. Both eyes appear to be functional. I'm going crosseyed!

[ExtremeFiretop]

I managed to re-create the issue on my version, for some reason I just had to backout/empty the password twice in a row. Now my version also gives this double eye situation. I say it can be ignored.

Yes, Edge specific, and if you click on right eye, left eye disappears. Both eyes appear to be functional. I'm going crosseyed!

Both eyes work for me as well LOL. Not sure why Edge feels the need to insert it's own sometimes.
But considering chrome and Firefox don't do it, this report can be #ignored

[TheS1R]

I managed to re-create the issue on my version, for some reason I just had to backout/empty the password twice in a row. Now my version also gives this double eye situation. I say it can be ignored.

Yes, Edge specific, and if you click on right eye, left eye disappears. Both eyes appear to be functional. I'm going crosseyed!

Both eyes work for me as well LOL. Not sure why Edge feels the need to insert it's own sometimes. But considering chrome and Firefox don't do it, this report can be #ignored

Cuz it's Micro$oft!

[TheS1R]

I managed to re-create the issue on my version, for some reason I just had to backout/empty the password twice in a row. Now my version also gives this double eye situation. I say it can be ignored.

Yes, Edge specific, and if you click on right eye, left eye disappears. Both eyes appear to be functional. I'm going crosseyed!

Both eyes work for me as well LOL. Not sure why Edge feels the need to insert it's own sometimes. But considering chrome and Firefox don't do it, this report can be #ignored

It wan't obvious at first since you need to delete all existing text in input field before second eye appears. Easy to miss!

[ExtremeFiretop]

I managed to re-create the issue on my version, for some reason I just had to backout/empty the password twice in a row. Now my version also gives this double eye situation. I say it can be ignored.

Yes, Edge specific, and if you click on right eye, left eye disappears. Both eyes appear to be functional. I'm going crosseyed!

Both eyes work for me as well LOL. Not sure why Edge feels the need to insert it's own sometimes. But considering chrome and Firefox don't do it, this report can be #ignored

It wan't obvious at first since you need to delete all existing text in input field before second eye appears. Easy to miss!

Agreed. For some reason on my version I did delete all the text but it still didn't show.

Now I think it's because of how I deleted the text. If I select it all and backspace, it shows up, if I select it all and just start typing again it's hit or miss lol.

#Microsoft

[TheS1R]

I managed to re-create the issue on my version, for some reason I just had to backout/empty the password twice in a row. Now my version also gives this double eye situation. I say it can be ignored.

Yes, Edge specific, and if you click on right eye, left eye disappears. Both eyes appear to be functional. I'm going crosseyed!

Both eyes work for me as well LOL. Not sure why Edge feels the need to insert it's own sometimes. But considering chrome and Firefox don't do it, this report can be #ignored

It wan't obvious at first since you need to delete all existing text in input field before second eye appears. Easy to miss!

Agreed. For some reason on my version I did delete all the text but it still didn't show.

Now I think it's because of how I deleted the text. If I select it all and backspace, it shows up, if I select it all and just start typing again it's hit or miss lol.

#Microsoft

#Micro$oftSux

[ExtremeFiretop]

I managed to re-create the issue on my version, for some reason I just had to backout/empty the password twice in a row. Now my version also gives this double eye situation. I say it can be ignored.

Yes, Edge specific, and if you click on right eye, left eye disappears. Both eyes appear to be functional. I'm going crosseyed!

Both eyes work for me as well LOL. Not sure why Edge feels the need to insert it's own sometimes. But considering chrome and Firefox don't do it, this report can be #ignored

It wan't obvious at first since you need to delete all existing text in input field before second eye appears. Easy to miss!

Agreed. For some reason on my version I did delete all the text but it still didn't show.
Now I think it's because of how I deleted the text. If I select it all and backspace, it shows up, if I select it all and just start typing again it's hit or miss lol.
#Microsoft

#Micro$oftSux

As long as it's not "our bug" I don't care 🤣

[Martinski4GitHub]

@ExtremeFiretop,
Well, this morning I woke up with an "itch" to improve the input validator for the postponement days input text box, and then I added another one for the password text box. The latter is just a very simple string length validator to make sure that at least we have some reasonable, non-empty password string before being allowed to save it.
Here are some screenshots:
...

One throught on the validation of password strength... I don't think that it is necessary. One does not change the router password here, but rather it is just saved for the script to log into the router while performing its job. The job of validating password strength ultimately lies upon the router firmware itself. Asus criteria appears to be that password must be between 5 and 32 characters, period — your check is already done by the router when password is changed. A better check, if possible, would be to verify that password matches current password (i.e., does login succeed) — this would need to be different than method that script uses as it simulates login to web UI.

Apparently, you have completely misunderstood the point and the purpose of the simple password validator. It’s there to prevent a new user from saving, accidentally or mistakenly, an entire configuration setup without a proper password (e.g. with an empty password), which could happen upon doing an initial "fresh" configuration from the WebGUI.

Note that I clearly & explicitly stated in my PR that it was a "simple" length validator so it’s certainly not meant to be a password strength validator, at all, period.

For Set Directory for F/W Updates field, is there a way to verify that the directory actually exists (i.e., drive inserted and mounted)? when field is changed?

I know that @ExtremeFiretop has graciously given you access to the very early pre-alpha snapshots of our initial development phase, and I understand you are trying to help, but I highly recommend not becoming the annoyingly cliched "backseat driver" who comments on things not yet done or completed while knowing that we're still on the long journey to start doing them or completing them. I know that may not be your intention but the net effect of being the "backseat driver" is still the same. Providing your testing results at this early stage is fine, but only up to a point.

Remember that we work on this hobby project only on our spare time, whenever we have a free hour or two here and there, so features and functionality are completed very slowly and in small "sections" at a time. Just because a commit is made to support a specific feature or functionality, it does not mean that we are done & finished with that feature or functionality. Wait until a Beta release is officially issued before commenting on what is then perceived as incomplete.

And also remember: this is not my first rodeo.

[Martinski4GitHub]

I managed to re-create the issue on my version, for some reason I just had to backout/empty the password twice in a row. Now my version also gives this double eye situation. I say it can be ignored.

Yes, Edge specific, and if you click on right eye, left eye disappears. Both eyes appear to be functional. I'm going crosseyed!

Both eyes work for me as well LOL. Not sure why Edge feels the need to insert it's own sometimes. But considering chrome and Firefox don't do it, this report can be #ignored

It wan't obvious at first since you need to delete all existing text in input field before second eye appears. Easy to miss!

Agreed. For some reason on my version I did delete all the text but it still didn't show.
Now I think it's because of how I deleted the text. If I select it all and backspace, it shows up, if I select it all and just start typing again it's hit or miss lol.
#Microsoft

#Micro$oftSux

As long as it's not "our bug" I don't care 🤣

Early this evening, my friend from work tested the latest development version snapshot of the MerlinAU WebGUI, and reported that there's another "wrinkle" with the latest password text box implementation where the "eye" image/button is placed within the text box on the right side.

Apparently, when some commonly used password managers are activated on your browser, they put their own button on the right side and within the text box as well, effectively putting it on top of the "eye" image/button and making it difficult to click right on it.

Normally, this is not really a problem after you have already stored your password string in the password manager; but before that, it interferes with the functionality of the "eye" button when you're entering the password string the first time. Obviously, when the "eye" image/button is on the outside of the text box, there's no problem at all as shown in the following images from the router as well.

So now the question is: How do we address this?
So it looks to me like we have at least 2 options:

1) Leave the current implementation "as is" and wait to see how much of a problem all these "issues" become when the WebGUI is released to the general public.

2) Fix the issue now using the original implementation (i.e. "eye" image/button outside the text box) so we can "nip the problems in the bud" to avoid future complaints and "bug" reports from some users not being able to use the "eye" button due to where it is placed, or the one where you see 2 "eye" buttons in the text box.

Granted, the above issue is not due to a bug in the WebGUI code itself, but from the user's perspective there's a problem nevertheless, and it affects their user experience.

At this point, my preference would be option 2) above to take it off the list of possible issues in the future, and we can move on to other things.

[ExtremeFiretop]

Granted, the above issue is not due to a bug in the WebGUI code itself, but from the user's perspective there's a problem nevertheless, and it affects their user experience.

At this point, my preference would be option 2) above to take it off the list of possible issues in the future, and we can move on to other things.

This is the second "visual wrinkle" with this implementation, and I've lost the love for it to be honest.
First issue was edge inserting it's own eye beside our eye, but at least both worked and nothing interfered with each other. It was browser specific (using a very unpopular browser) and so I was willing to let it slide.

But if it's causing other wrinkles elsewhere I say we just roll it back honestly. Back to your lesson from yesterday, if there is a technical reason to change the design, lets do it. I rather people not complain about any "interference" with the button period.

[ExtremeFiretop]

We gave it a shot; and that's all that matters; we found issues, we will roll it back.
The original code I used didn't have any of these issues.

Let me know if you want me to roll it back, or if your already on it @Martinski4GitHub :)

[Martinski4GitHub]

Granted, the above issue is not due to a bug in the WebGUI code itself, but from the user's perspective there's a problem nevertheless, and it affects their user experience.
At this point, my preference would be option 2) above to take it off the list of possible issues in the future, and we can move on to other things.

This is the second "visual wrinkle" with this implementation, and I've lost the love for it to be honest. First issue was edge inserting it's own eye beside our eye, but at least both worked and nothing interfered with each other. It was browser specific (using a very unpopular browser) and so I was willing to let it slide.

But if it's causing other wrinkles elsewhere I say we just roll it back honestly. Back to your lesson from yesterday, if there is a technical reason to change the design, lets do it. I rather people not complain about any "interference" with the button period.

I fully agree, and that's my take as well. Having 2 separate "eye" buttons that still work is one thing, but having another button on top of our "eye" button causing interference (not due to our fault) is not just annoying but would also look "bad" on the user experience of our WebGUI, and we'll likely get some user "complaints" and "bug" reports that we'll have to respond to, whether is our fault or not. I'd rather fix it now, and move on.

[Martinski4GitHub]

We gave it a shot; and that's all that matters; we found issues, we will roll it back. The original code I used didn't have any of these issues.

Let me know if you want me to roll it back, or if your already on it @Martinski4GitHub :)

You go ahead and do it whenever you get a chance later today. No rush.
I have something else "cooking in the oven" and I'm not using my development box at the moment (I'm on my iPad in bed and will go to sleep in about 5 minutes).

Have a good night, bud!!

[ExtremeFiretop]

We gave it a shot; and that's all that matters; we found issues, we will roll it back. The original code I used didn't have any of these issues.
Let me know if you want me to roll it back, or if your already on it @Martinski4GitHub :)

You go ahead and do it whenever you get a chance later today. No rush. I have something else "cooking in the oven" and I'm not using my development box at the moment (I'm on my iPad in bed and will go to sleep in about 5 minutes).

Have a good night, bud!!

Totally understandable.

I'm currently without sleep tonight unfortunately. I laid down for 3 hours tossing and turning, and then got up looked at my phone and saw your messages.

I may as well get to it now. Goodnight Bud! I'll let you keep cooking whatever is in the oven on your side 😉

[TheS1R]

@ExtremeFiretop,
Well, this morning I woke up with an "itch" to improve the input validator for the postponement days input text box, and then I added another one for the password text box. The latter is just a very simple string length validator to make sure that at least we have some reasonable, non-empty password string before being allowed to save it.
Here are some screenshots:
...

One throught on the validation of password strength... I don't think that it is necessary. One does not change the router password here, but rather it is just saved for the script to log into the router while performing its job. The job of validating password strength ultimately lies upon the router firmware itself. Asus criteria appears to be that password must be between 5 and 32 characters, period — your check is already done by the router when password is changed. A better check, if possible, would be to verify that password matches current password (i.e., does login succeed) — this would need to be different than method that script uses as it simulates login to web UI.

Apparently, you have completely misunderstood the point and the purpose of the simple password validator. It’s there to prevent a new user from saving, accidentally or mistakenly, an entire configuration setup without a proper password (e.g. with an empty password), which could happen upon doing an initial "fresh" configuration from the WebGUI.

Note that I clearly & explicitly stated in my PR that it was a "simple" length validator so it’s certainly not meant to be a password strength validator, at all, period.

For Set Directory for F/W Updates field, is there a way to verify that the directory actually exists (i.e., drive inserted and mounted)? when field is changed?

I know that @ExtremeFiretop has graciously given you access to the very early pre-alpha snapshots of our initial development phase, and I understand you are trying to help, but I highly recommend not becoming the annoyingly cliched "backseat driver" who comments on things not yet done or completed while knowing that we're still on the long journey to start doing them or completing them. I know that may not be your intention but the net effect of being the "backseat driver" is still the same. Providing your testing results at this early stage is fine, but only up to a point.

Remember that we work on this hobby project only on our spare time, whenever we have a free hour or two here and there, so features and functionality are completed very slowly and in small "sections" at a time. Just because a commit is made to support a specific feature or functionality, it does not mean that we are done & finished with that feature or functionality. Wait until a Beta release is officially issued before commenting on what is then perceived as incomplete.

And also remember: this is not my first rodeo.

Got it. Apologize, and I will do better.

[Martinski4GitHub]

@ExtremeFiretop,
Well, this morning I woke up with an "itch" to improve the input validator for the postponement days input text box, and then I added another one for the password text box. The latter is just a very simple string length validator to make sure that at least we have some reasonable, non-empty password string before being allowed to save it.
Here are some screenshots:
...

One throught on the validation of password strength... I don't think that it is necessary. One does not change the router password here, but rather it is just saved for the script to log into the router while performing its job. The job of validating password strength ultimately lies upon the router firmware itself. Asus criteria appears to be that password must be between 5 and 32 characters, period — your check is already done by the router when password is changed. A better check, if possible, would be to verify that password matches current password (i.e., does login succeed) — this would need to be different than method that script uses as it simulates login to web UI.

Apparently, you have completely misunderstood the point and the purpose of the simple password validator. It’s there to prevent a new user from saving, accidentally or mistakenly, an entire configuration setup without a proper password (e.g. with an empty password), which could happen upon doing an initial "fresh" configuration from the WebGUI.
Note that I clearly & explicitly stated in my PR that it was a "simple" length validator so it’s certainly not meant to be a password strength validator, at all, period.

For Set Directory for F/W Updates field, is there a way to verify that the directory actually exists (i.e., drive inserted and mounted)? when field is changed?

I know that @ExtremeFiretop has graciously given you access to the very early pre-alpha snapshots of our initial development phase, and I understand you are trying to help, but I highly recommend not becoming the annoyingly cliched "backseat driver" who comments on things not yet done or completed while knowing that we're still on the long journey to start doing them or completing them. I know that may not be your intention but the net effect of being the "backseat driver" is still the same. Providing your testing results at this early stage is fine, but only up to a point.
Remember that we work on this hobby project only on our spare time, whenever we have a free hour or two here and there, so features and functionality are completed very slowly and in small "sections" at a time. Just because a commit is made to support a specific feature or functionality, it does not mean that we are done & finished with that feature or functionality. Wait until a Beta release is officially issued before commenting on what is then perceived as incomplete.
And also remember: this is not my first rodeo.

Got it. Apologize, and I will do better.

Apology accepted.
Now, let's move on and continue to have fun with this project. :>).