Known devices #403
Known devices #403
Conversation
The code has been simplified by replacing the use of UCIContain.listOptions with checks to see if (UCIContainer.value instanceof Array). Also bugs identified and fixed. Background. While implementing new functionality to store uci list values, problems were encountered. While UCIContainer.set() accomodated for uci list values using UCIContainer.listOptions, UCIContainer.getScriptCommands() did not refer to UCIContainer.listOptions and did not return the correct commands to set uci list values. While investigating and correcting this problem, various other corrections and simplifications were undertaken. UCIContainer.set() is recoded UCIContainer.listOptions is replaced with instanceof Array checks UCIContainer.values is declared as an Object rather than an Array UCIContainer.clone() corrected clone of Array values
The code has been made more simple and robust by removing the need to maintain keys separately. The use of keys[] has been replaced by tests on values.hasOwnProperty(key) as recommended at: http://stackoverflow.com/questions/11040472/check-if-object-property- exists-using-a-variable#11040508
The code has been made more simple and robust by removing the need to maintain keys separately. The use of keys[] has been replaced by tests on values.hasOwnProperty(key) as recommended at: http://stackoverflow.com/questions/11040472/check-if-object-property- exists-using-a-variable#11040508
device.sh is implemented to enable DeviceNames to be associated with one or more MACs, and to enable DeviceNames to be assigned to Device Groups.
completed the initial i18n implementation in devices.sh
Gargoyle - Connections - Devices
rename SelectMAC validate GroupName implement Save Changes to uci
would have been nice if a extra } at the end of .js would generate a warning!
dosfstools 3.0.27 not found in repositories
…into known-devices
- various js tidy-up and bug fix - correct device lable - css layout for device & group table columns
The Edit Button is done a little differently than std Gargoyle. In std Gargoyle the Edit Button pops up and populates a form for editing. Here, the Edited Row is Removed and the fields in the ADD form are populated with the existing info so that the user can adjust and re-add. This approach seems intuitive for the User and makes the JS code simpler and smaller. If editing a really large table then the User may be confused when the row dissapears while not being able to see the Add Row at the top?
The Gagoyle admin can use a Device Group to define a Quota by selecting "Only the following Host(s) and providing an IP, or IP range, or a Device Group. The Group can be selected or entered manually and it is validated an saved thru the existing processes. The Device Group MACs are included in the validation to avoid quota overlaps involving MACs. To integrate the MACs validation, the groupsValid parameter was added to the function addAddressStringToTable() and this cascaded small adjustments to other JavaScript files which called this function.
Each Device Group is reflected by an ipset of the same name. The ipset is utilised in iptables to implement quota policy for a Device Group. IP addresses are added and deleted from ipsets when a new dhcp lease is assigned by setting dhcp.dnsmasq.dhcp_script
The dnsmasq dhcp-script is not available thru OpenWRT uci but it is possible to suppliment the uci settings with /etc/dnsmasq.conf https://wiki.openwrt.org/doc/uci/dhcp?&#using_plain_dnsmasqconf
- Reset all tables & selects after Save - Add new Devices & Groups to Select Options while editing
|
Look forward to testing this! |
The Known Devices table is populated with entries from uci and from /etc/ethers & /etc/hosts. When the user Saves Changes then all devices are saved into uci and /etc/ethers & /etc/hosts are recreated without redundant entries.
An OpenWRT patch is backported to "Allow UCI dhcp classifier to accept a list of MAC" https://patchwork.ozlabs.org/patch/569678/ Recording each Device Group additionally as a uci dhcp mac classifier enables the utilization of the dnsmasq --dhcp-option and makes 3 functions in common.js redundant. https://wiki.openwrt.org/doc/uci/dhcp#classifying_clients_and_assigning_individual_options http://www.thekelleys.org.uk/dnsmasq/docs/dnsmasq-man.html
|
dnsmasq draws Host info from 3 places: uci & /etc/hosts & /etc/ethers. The prior commit makes a shift to uci being the primary place to store and manipulate Host information. This shift to uci is important because the association between Host and Group and networkid are all stored within the uci system. I will now revisit the other few places where Gargoyle draws info from /etc/hosts and/or /etc/ethers and make a similar adjustment. |
|
This is a good change. |
Potential Wake On Lan hosts are now drawn from the uci system first before drawing from arp, dhcp.leases and ethers
…nto known-devices resolved conflicts: package/gargoyle/files/www/js/dhcp.js
Thank you lyrz
The restiction on the Device and Group names are different (based on uci and ipset respectively) and a little confusing so it is important to guide the user.
thank you ericwong
…nto known-devices Resolved Conflicts: package/plugin-gargoyle-i18n-English-EN/files/www/i18n/English- EN/strings.js
|
By the way, I haven't forgotten about this. My general policy is to merge simpler pull requests first, so that's why other simpler things are getting merged before this. I still think this is a good work and will definitely get around to merging this in the near future. |
|
Thanks for the feedback. I have been running this development for a couple of months now without an issue. |
…nto known-devices Resolved Conflicts: package/gargoyle/files/www/dhcp.sh package/gargoyle/files/www/js/common.js package/gargoyle/files/www/js/dhcp.js package/gargoyle/files/www/js/qos.js package/gargoyle/files/www/js/quotas.js package/gargoyle/files/www/quotas.sh
|
For future reference, the fields used to add a new element to a table should NEVER be used for editing. An editor should pop up a new window (now, if you ever want to implement this with javascript rather than popping up a new browser window that would be fine, and probably better -- the old mechanism is a bit clunky I admit). For now I'm going to remove the "edit" option for the devices/groups -- it can be added back later so long as this is not done with the initial controls used to add the device to the table. |
So is this a stylistic thing, or data integrity, or security or what? Just interested to understand your reasoning? |
|
First, you're removing the row from the table. That's NOT the behavior expected with edit. If I hit edit on multiple rows in rapid succession the way you have it, I lose data. That's bad -- really, really, really, egregiously bad. Second, consistency is important. Every other table in gargoyle pops up a window when you hit edit, so that is what should happen here. As I said, if you want to implement in a more modern way without actually creating a new browser window but creating the new element with javascript, that would be fine and probably an improvement. But, wherever there is an edit button, when you click it, a new window should open. |
Corrected and strengthened saveChanges() to handle whitespace properly and reject unknown host names
|
@ericpaulbishop please see bugfix in prior commit which should be included in you Known Devices evaluation branch |
…nto known-devices
|
Anything preventing this from being merged? |
|
It probably needs to be rebased for the current code set. |
|
This definitely needs to be rebased to fit the 10mths of development since last commit. I have not been following the changes to the UI in detail so I have no idea how much effort might be involved. I have been running the Known Groups branch for these 10+ months with zero issues. |
|
As this is a clean merge without conflicts, according to GH, a rebase should go smoothly AFAIK. |
|
common.js, all of the .sh files, all of the template files, all of the .css files, at a MINIMUM, will need to be updated to suit the current development. |
|
I'm also really against people using programs to help them develop, that try to "auto clean" files. |
|
Is anyone still working on this? I'd love to have it for my system. |
|
Not at this stage. |
The idea is to be able to set policy (Quota, QoS, Restrictions etc) over a Group of Known Devices (owned by an individual for example). To achieve this, the Known Devices need to be identified by their MAC address' and then arranged into Groups. Both Devices and Groups can be named by the Gargoyle Administrator.
Up until now, the best way for a Gargoyle admin to apply policy to users with a number of devices has been to assign a static IP address to each device such that each users' devices are in an IP range. Then, the policy is applied to the IP range, and the IP range is displayed throughout the Gargoyle GUI. Known Devices and Groups enable the Gargoyle Admin to work with meaningful names rather than IP ranges, and relieves the need to assign static IP addresses.
Status
Technical