In Foreman, you can set a host’s associations to ones that are not in the host’s own org. That results in some weird behaviour, especially once Organization.current is set. In Katello, Organization.current is always set, so you end up with things like Foreman thinking a host has no domain and not validating the FQDN because it thinks it has periods that aren’t part of the domain name.
I can think of a bunch of ways to fix this, but not sure what would be the best. Should we validate a host’s associations are in the assigned taxonomy when creating it? This would have rejected the host’s creation with an error message about Domain not found, for example.
But of course the org could be later removed from domain, which would trigger this periods validation thing again. Could we ignore Organization.current for validating a host’s name?
Any other ideas?
Thanks!
> ::Organization.current
=> nil
> host.domain
+----+-----------------+----------+-------------------------+-------------------------+--------+
| id | name | fullname | created_at | updated_at | dns_id |
+----+-----------------+----------+-------------------------+-------------------------+--------+
| 1 | zpm.example.com | | 2018-04-19 16:28:25 UTC | 2018-04-19 16:28:25 UTC | |
+----+-----------------+----------+-------------------------+-------------------------+--------+
1 row in set
> host.domain.organizations.include?(host.organization)
=> false
> host.valid?
=> true
> ::Organization.current = host.organization
2018-04-23T13:15:38 [D|app|] Setting current organization thread-local variable to Default Organization
+----+---------+---------+---------+---------+---------+---------+---------+---------+---------+---------+
| id | name | type | crea... | upda... | igno... | desc... | label | ance... | title | mani... |
+----+---------+---------+---------+---------+---------+---------+---------+---------+---------+---------+
| 1 | Defa... | Orga... | 2018... | 2018... | | | Defa... | | Defa... | |
+----+---------+---------+---------+---------+---------+---------+---------+---------+---------+---------+
1 row in set
> host.reload
> host.domain
=> nil
i> host.valid?
2018-04-23T13:16:09 [W|app|] Not queueing Host::Managed: ["Name must not include periods"]
2018-04-23T13:16:09 [W|app|] Not queueing Host::Managed: ["Name must not include periods"]
2018-04-23T13:16:09 [W|app|] Not queueing Host::Managed: ["Name must not include periods"]
=> false
I think the reasonable answer to this would be to display a modal with the inconsistencies (e.g: domain assigned to host but not in host’s organization) and showing a button like “Fix loc/org mismatches”
maybe in that case we can ensure the domain is part of the ORG? (instead of
dropping validations?) other than that, do you happen to know why this
happened to begin with? (e.g. domain was created but not in the org?)
Like everything with taxonomies, it’s Pandora’s box. The existing mechanism for determining mismatches via the TaxHost service only works on a persisted host, which means it’s useless for validations on a new host. It relies heavily on AR queries using the host’s id, which it doesn’t have at that point. I can try to refactor that, although I worry if I succeed here I’ll be making the taxonomy user experience worse.
The original domain was created via a different puppet-reported host, that’s a pretty common scenario. I can fix the underlying short name check to look at domains unscoped which would repair the odd complaint about dots. That’s not fixing the deeper issue, though.
Is it really a security issue? Reducing the friction when working with taxonomies would be a win IMHO. If you have permissions to create a host with the domain example.com, then you should be able to pull example.com into your organization. If a user should only be allowed to create hosts in other.com, then you should set a filter as such, in which case you’d never get to the step where mismatches are reconciled.
Also, it can be turned of in the PR I have open that implements this.
I think this is a security issue. While it would be very comfortable, it means we drop organizations isolation completely. I like the idea of adding a validation and rejecting save, if domain is not available in host context. If I recall correctly, there’s assign_organization and assign_location permission that we should check, when association changes. This is a general problem though, we don’t check any associations on any object, we assign all ids that are sent as parameters and we only rely that user doesn’t have an easy way to find these ids.
