Bug 1191913
| Summary: | user should not be allowed to create interface with invalid mac e.g. any random string | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Red Hat Satellite | Reporter: | Sachin Ghai <sghai> | ||||||||||
| Component: | WebUI | Assignee: | Marek Hulan <mhulan> | ||||||||||
| WebUI sub component: | Foreman | QA Contact: | Tazim Kolhar <tkolhar> | ||||||||||
| Status: | CLOSED CURRENTRELEASE | Docs Contact: | |||||||||||
| Severity: | high | ||||||||||||
| Priority: | unspecified | CC: | bkearney, cwelton, mhulan, mmccune, ohadlevy, sghai, tkolhar | ||||||||||
| Version: | 6.1.0 | Keywords: | Reopened, Triaged | ||||||||||
| Target Milestone: | Unspecified | ||||||||||||
| Target Release: | Unused | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Whiteboard: | |||||||||||||
| Fixed In Version: | Doc Type: | Bug Fix | |||||||||||
| Doc Text: | Story Points: | --- | |||||||||||
| Clone Of: | Environment: | ||||||||||||
| Last Closed: | 2015-08-12 13:58:07 UTC | Type: | Bug | ||||||||||
| Regression: | --- | Mount Type: | --- | ||||||||||
| Documentation: | --- | CRM: | |||||||||||
| Verified Versions: | Category: | --- | |||||||||||
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||||||||
| Embargoed: | |||||||||||||
| Bug Depends On: | |||||||||||||
| Bug Blocks: | 1193460, 1193987, 1194641 | ||||||||||||
| Attachments: |
|
||||||||||||
The request you attached is just JS call to update the UI. It does not save anything, it just syncs the modal window with the host form. I tested both host primary interface and other interfaces with invalid MAC address. Upon form submit, they MAC address fields are cleared and there's validation error that it can't be blank. Therefore I'm closing this. Feel free to ask if there are any questions. I didn't see if UI raising any validation error even on form submit.
I discovered a host and click on provision and define a new interface with any random text and submit the form. Form was submitted successfully and Provisioning started and didn't get any validation error.
Processing by DiscoveredHostsController#update as */*
Parameters: {"utf8"=>"✓", "authenticity_token"=>"CkViytgWQfCS/GGRqyeML9iWFyRhMicfsPVehnMc7sU=", "host"=>{"name"=>"mac525400acef58", "hostgroup_id"=>"1", "environment_id"=>"5", "content_source_id"=>"1", "puppet_ca_proxy_id"=>"1", "puppet_proxy_id"=>"1", "puppetclass_ids"=>[""], "managed"=>"true", "progress_report_id"=>"[FILTERED]", "type"=>"Host::Managed", "domain_id"=>"1", "realm_id"=>"", "mac"=>"52:54:00:ac:ef:58", "subnet_id"=>"1", "ip"=>"192.168.100.107", "interfaces_attributes"=>{"new_1424242522520"=>{"_destroy"=>"false", "type"=>"Nic::Managed", "mac"=>"sachin ghai", "identifier"=>"eth0:0", "name"=>"", "domain_id"=>"", "subnet_id"=>"1", "ip"=>"192.168.100.107", "managed"=>"1", "virtual"=>"1", "tag"=>"0", "attached_to"=>"eth0:0"}, "new_interfaces"=>{"_destroy"=>"false", "type"=>"Nic::Managed", "mac"=>"", "identifier"=>"", "name"=>"", "domain_id"=>"", "subnet_id"=>"", "ip"=>"", "managed"=>"1", "virtual"=>"0", "tag"=>"", "attached_to"=>""}}, "architecture_id"=>"1", "operatingsystem_id"=>"1", "build"=>"1", "medium_id"=>"8", "ptable_id"=>"7", "disk"=>"", "root_pass"=>"[FILTERED]", "is_owned_by"=>"3-Users", "enabled"=>"1", "model_id"=>"2", "comment"=>"", "overwrite"=>"false"}, "hostgroup"=>{"lifecycle_environment_id"=>"1", "content_view_id"=>"5"}, "id"=>"mac525400acef58"}
Create DHCP reservation for mac525400acef58.englab.pnq.redhat.com-52:54:00:ac:ef:58/192.168.100.107
Add DNS A record for mac525400acef58.englab.pnq.redhat.com/192.168.100.107
Add DNS PTR record for 192.168.100.107/mac525400acef58.englab.pnq.redhat.com
Add the TFTP configuration for mac525400acef58.englab.pnq.redhat.com
Fetching required TFTP boot files for mac525400acef58.englab.pnq.redhat.com
Redirected to https://dhcp201-167.englab.pnq.redhat.com/hosts/mac525400acef58.englab.pnq.redhat.com
ForemanDiscovery: Rebooting mac525400acef58
ForemanDiscovery: reboot result: successful
See in above logs mac set as sachin ghai
Later, when I edit the host, the mac field was empty. Looks it just silently removes the invalid mac? is it the expected behavior ?
In discovered host it works in a same way in my instance. Invalid mac address gets cleared and if it's required for an interface I see validation issue upon host form submit. If it's not, then there's no validation error and the form saves (with blank mac, so the save is expected I think) Sachin is there any other concern? I think this is NOTABUG and it does not block 1193987 in any way. Marek, My only concern is the way we are validating the mac is misleading.
Case1: I discovered a host and click on provision. A new host edit form opened up. Then added a physical managed interface with mac = 'invalid_mac' and form raised error "mac can't be blank". Its misleading
see in below logs mac was set as "mac"=>"invalid_mac",
Processing by DiscoveredHostsController#update as */*
Parameters: {"utf8"=>"✓", "authenticity_token"=>"s4LVwIN5a8iAOOmYlEDFI9ezLaGvLTdihpUIVIdlVUs=", "host"=>{"name"=>"mac5254000d6420", "hostgroup_id"=>"1", "lifecycle_environment_id"=>"1", "content_view_id"=>"2", "environment_id"=>"2", "content_source_id"=>"1", "puppet_ca_proxy_id"=>"1", "puppet_proxy_id"=>"1", "managed"=>"true", "progress_report_id"=>"[FILTERED]", "type"=>"Host::Managed", "domain_id"=>"1", "realm_id"=>"", "mac"=>"52:54:00:0d:64:20", "subnet_id"=>"1", "ip"=>"192.168.100.15", "interfaces_attributes"=>{"0"=>{"_destroy"=>"false", "type"=>"Nic::Managed", "mac"=>"invalid_mac", "identifier"=>"eth1", "name"=>"", "domain_id"=>"1", "subnet_id"=>"1", "ip"=>"192.168.100.16", "managed"=>"1", "virtual"=>"0", "tag"=>"", "attached_to"=>""}, "new_interfaces"=>{"_destroy"=>"false", "type"=>"Nic::Managed", "mac"=>"", "identifier"=>"", "name"=>"", "domain_id"=>"", "subnet_id"=>"", "ip"=>"", "managed"=>"1", "virtual"=>"0", "tag"=>"", "attached_to"=>""}}, "architecture_id"=>"1", "build"=>"1", "disk"=>"", "root_pass"=>"[FILTERED]", "is_owned_by"=>"3-Users", "enabled"=>"1", "model_id"=>"2", "comment"=>"", "overwrite"=>"false"}, "discovered_host"=>{"puppetclass_ids"=>[""], "operatingsystem_id"=>"1", "medium_id"=>"7", "ptable_id"=>"7"}, "id"=>"mac5254000d6420"}
DHCP records -52:54:00:0d:64:20/192.168.100.14 already exists
Failed to save: Mac can't be blank, Conflict DHCP records -52:54:00:0d:64:20/192.168.100.14 already exists
It would be better if we validate add_interface form as soon as user update the field instead of on form submit. In short, my only concern is we should raise validation errors instead of silently failing them. Comment8 clearly state the wrong configuration for eth0:0(grep without mac). We should really raise validation error. Please let me know what do you think ? Sachin, change to run validation before the form is submit to the server is quite big and should be systematic, not implemented just on one form. It would require JS based validations generated based on server side validations. From your last comment I understood that the issue is that managed Alias can be created without specifying MAC. This results in wrong DEVICE name in interface config file. I agree that this is a bug (which probably applies to VLAN as well). But it's already reported as https://bugzilla.redhat.com/show_bug.cgi?id=1193460 so I think this one should be closed. For the record, this is not related to discovery in any way as the validation rules are same. FAILEDQA: # rpm -qa | grep foreman ruby193-rubygem-foreman_hooks-0.3.7-2.el7sat.noarch foreman-selinux-1.7.2.13-1.el7sat.noarch ruby193-rubygem-foreman_docker-1.2.0.9-1.el7sat.noarch ruby193-rubygem-foreman-tasks-0.6.12.3-1.el7sat.noarch rubygem-hammer_cli_foreman_tasks-0.0.3.3-1.el7sat.noarch foreman-debug-1.7.2.17-1.el7sat.noarch foreman-libvirt-1.7.2.17-1.el7sat.noarch ruby193-rubygem-foreman_bootdisk-4.0.2.10-1.el7sat.noarch foreman-compute-1.7.2.17-1.el7sat.noarch foreman-ovirt-1.7.2.17-1.el7sat.noarch rubygem-hammer_cli_foreman_bootdisk-0.1.2.5-1.el7sat.noarch foreman-postgresql-1.7.2.17-1.el7sat.noarch qe-sat6-rhel71.usersys.redhat.com-foreman-client-1.0-1.noarch qe-sat6-rhel71.usersys.redhat.com-foreman-proxy-1.0-1.noarch ruby193-rubygem-foreman_gutterball-0.0.1.9-1.el7sat.noarch foreman-1.7.2.17-1.el7sat.noarch foreman-gce-1.7.2.17-1.el7sat.noarch ruby193-rubygem-foreman_discovery-2.0.0.9-1.el7sat.noarch rubygem-hammer_cli_foreman-0.1.4.9-1.el7sat.noarch foreman-vmware-1.7.2.17-1.el7sat.noarch rubygem-hammer_cli_foreman_discovery-0.0.1.7-1.el7sat.noarch foreman-proxy-1.7.2.4-1.el7sat.noarch qe-sat6-rhel71.usersys.redhat.com-foreman-proxy-client-1.0-1.noarch ruby193-rubygem-foreman-redhat_access-0.1.0-1.el7sat.noarch steps: 1. New Host --> Add_interface add any value as mac address screen shot attached foreman-debug attached Created attachment 1019603 [details]
Mac address
Created attachment 1019611 [details]
foreman-debug attached
This was tested on old version that does not include the fix. Moving back to POST, I think it should be moved to MODIFIED when there's a compose with the patch. If there is such compose, please move to MODIFIED. VERIFIED: # rpm -qa | grep foreman ruby193-rubygem-foreman_docker-1.2.0.11-1.el7sat.noarch rubygem-hammer_cli_foreman-0.1.4.11-1.el7sat.noarch foreman-proxy-1.7.2.4-1.el7sat.noarch ruby193-rubygem-foreman_bootdisk-4.0.2.13-1.el7sat.noarch foreman-1.7.2.19-1.el7sat.noarch ruby193-rubygem-foreman_discovery-2.0.0.13-1.el7sat.noarch rubygem-hammer_cli_foreman_tasks-0.0.3.4-1.el7sat.noarch foreman-libvirt-1.7.2.19-1.el7sat.noarch foreman-postgresql-1.7.2.19-1.el7sat.noarch ibm-hs22-05.rhts.eng.brq.redhat.com-foreman-proxy-client-1.0-1.noarch foreman-debug-1.7.2.19-1.el7sat.noarch foreman-compute-1.7.2.19-1.el7sat.noarch foreman-vmware-1.7.2.19-1.el7sat.noarch rubygem-hammer_cli_foreman_bootdisk-0.1.2.7-1.el7sat.noarch foreman-ovirt-1.7.2.19-1.el7sat.noarch ruby193-rubygem-foreman-redhat_access-0.1.0-1.el7sat.noarch ruby193-rubygem-foreman-tasks-0.6.12.5-1.el7sat.noarch rubygem-hammer_cli_foreman_discovery-0.0.1.9-1.el7sat.noarch ruby193-rubygem-foreman_hooks-0.3.7-2.el7sat.noarch ruby193-rubygem-foreman_gutterball-0.0.1.9-1.el7sat.noarch foreman-selinux-1.7.2.13-1.el7sat.noarch ibm-hs22-05.rhts.eng.brq.redhat.com-foreman-client-1.0-1.noarch ibm-hs22-05.rhts.eng.brq.redhat.com-foreman-proxy-1.0-2.noarch foreman-gce-1.7.2.19-1.el7sat.noarch steps: 1. New Host --> Add_interface Add invalid mac Error displayed screenshot attached Created attachment 1024193 [details]
invalid mac address
This bug is slated to be released with Satellite 6.1. This bug was fixed in version 6.1.1 of Satellite which was released on 12 August, 2015. |
Created attachment 990803 [details] see the mac field contains string "sachin" and form is saved. Description of problem: I created a new interface and used a random string for mac. It was accepted and form was saved. However user should n't be allowed to create interface with invalid mac. A proper validation needs to be set on mac field. Version-Release number of selected component (if applicable): Satellite-6.1.0-RHEL-6-20150210.0 Sat6.1.0 beta snap2 How reproducible: always Steps to Reproduce: 1. New Host --> Add_interface 2. 3. Actual results: user can create interface with invalid mac Expected results: user should n't be allowed to create interface with invalid mac. A proper validation needs to be set on mac field. Additional info: production.log: Processing by InterfacesController#new as JS Parameters: {"host"=>{"interfaces_attributes"=>{"new_1423727783901"=>{"type"=>"Nic::Managed", "mac"=>"sachin", "identifier"=>"", "name"=>"", "domain_id"=>"", "subnet_id"=>"", "ip"=>"", "managed"=>"1", "username"=>"", "password"=>"[FILTERED]", "provider"=>"IPMI"}}}, "fakepassword"=>"[FILTERED]", "_"=>"1423728170887"} Rendered nic/_base_form.html.erb (16.8ms) Rendered nic/_virtual_form.html.erb (175.9ms) Rendered nic/manageds/_managed.html.erb (196.8ms) Rendered nic/new.js.erb (199.6ms) Completed 200 OK in 215ms (Views: 200.1ms | ActiveRecord: 2.5ms) see mac set as "mac"=>"sachin"