On Mon, 2012-10-29 at 15:45 +0100, Alexander Marx wrote:
Hey Alex,
I just installed the files on my system. This is my first impression:
Over the entire application, the wording is very poor. It is hard to understand what is going on - even for someone who is experienced with what the GUI does. Buttons say "static net", headlines are "add net". What kind of net? What is a static net?
Well, my first suggestion was to name it ccd net which also didn't find your acceptance. Your comment was that we should completely hide things like ccd from the user. thats what i did. What do you think, the names should be than? Should there be another hint on top of the page which explains the use of the button?!
Well, this is very tricky. I cannot come up with an idea at the moment. But I think that "CCD" is just confusing. Users don't care what is working under the hood and what it is called. Also, looking up "CCD" leads to "client config directory" which does not suggest that one is able to configure static IP addresses.
The problem with "static net" is "net". I would like something as "static network address pools". It explains everything that is needed to get the user to click on the button. However, this is very long and the buttons already clobber the interface.
We could possible drop the restart button, which makes not a lot sense to me or just rearrange things in that area.
The form above the listing where you create a new network does not look like the forms on top of other pages like the port forwarding.
I have no idea what i should change here, there are only two fields to fill in. What are you missing on the page? The only thing i can imagine is the black bar between the fields and the submitbutton.
Yeah, the bar and move the "add net" button to the right. Can you manage that it just says "add"?
Maybe: <input type="hidden" name="ACTION" value="add-net"> <input type="submit" value="Add" ...>
Why is the max. number of fixed leases shown in that table? I cannot see any use for that over here.
Admins are able to see directly how many roadwarriors are possible in the configured net. if it does not fit, they can delete the network and choose a greater one. By the way that should be an information, that the admins do NOT choose a net thats too great for only a few needed roadwarriors. I can delte the column if you prefer it. But if i use the whole width for the table, it looks poor with only 2 columns....
Okay. Then leave it there.
The list of host addresses where to pick from is cool, but the subnet mask is _always_ /30. No matter what the actual network size is.
yes. Here are two possibillities: i can only use the ip's or add a /30 behind them, Whats your favour?
I'd like the /30 so we can easily see that there is a reason why not every single address from the pool network can be chosen.
Maybe the other guys can give their opinion on this.
What does checking the redirect gw option change? Can I check it for multiple networks? Is it bound to a network or is it just an option to enable the gateway redirection for this client in general?
What does "net to route" do?
Well That is roadwarriorspecific, you are adding a roadwarrior at this form. So the options are redirect gateway (All traffic from the client goes through the tunnel, even Webconnections) And the othe roption is the iroute option that forces a roadwarrior to route its net. With this option it is possible to use a printer from the roadwarrriors network in a RDP connection for example.
Hmm. Okay.
Can we move those to an extra from at the bottom of the page, because those are also handy for the dynamic pool. The interface suggests that I can check multiple "redirect gw" boxes which in fact I can not.
The text input to add an additional route could also be a text area where I can put more than one route? Like that one at "advanced options".
I had to create /var/ipfire/ovpn/ccd (nobody:nobody) manually.
That need to be fixed. The file should be created by the extension.
The CGI script *could* take care of it. However this directory should also be created by the updater. But sometimes people restore backups and I don't know if someone can manage to delete that directory.
I rather don't like the header of the configuration files in the ccd directory. Is there a reason why the parameters are separated by "=" than "," in ccd.conf?
It is a hash, not a hasharray because of only Key-Value (see server.conf)
What happens when I enter two networks with the same description? What happens when I enter no description at all?
The source code has got very many comments in German which is not very convenient, because not all developers do speak German.
Ok. a general question: Well i can rewrite them or leave them out. I don't know what your rules are for comments in sourcecode
We really like good comments in the code. But as you can see the inherited code from the WUI is not very well written and often not commented at all.
So new rule: Code nicely that other can understand what is happening.
Could you also remove those
########### #A.Marx CCD---add route into server.conf-------------------------------- ###########
things?
We will see it in the git commit, what has been changed and the code does not need to be cluttered. ovpnmain.cgi already has more than 4500 lines of code.
Michael
-
Michael Tremer