realmd: Don't directly manipulate the DOM

Both the realmd code and the systemd code would try to to set the
"disabled" attribute of the #system_information_hostname_button
element, and would race each other.

To get better decoupling, the realmd code has been changed to only
export the necessary information to construct the buttons (instead of
modifying the DOM directly with jQuery), and the systemd code now uses
that to render them with React.

As part of that the realmd code hase been moved into pkg/systemd since
that is the only user.

Closes #13369
This commit is contained in:
Marius Vollmer 2020-01-13 16:55:54 +02:00 committed by Martin Pitt
parent 0b949fcbbb
commit f8e80d09a3
12 changed files with 112 additions and 118 deletions

View File

@ -162,7 +162,6 @@ WEBPACK_PACKAGES = \
pcp \
packagekit \
playground \
realmd \
selinux \
shell \
sosreport \

View File

@ -34,7 +34,7 @@ import cockpit from "cockpit";
*/
export function Privileged({ excuse, allowed, placement, tooltipId, children }) {
// wrap into extra <span> so that a disabled child keeps the tooltip working
let contents = <span id={tooltipId}>{ children }</span>;
let contents = <span id={allowed ? null : tooltipId}>{ children }</span>;
if (!allowed) {
contents = (
<OverlayTrigger placement={ placement || "top" }

View File

@ -1,7 +0,0 @@
{
"version": "@VERSION@",
"name": "domain",
"requires": {
"cockpit": "122"
}
}

View File

@ -12,7 +12,7 @@ To contribute to this component, run a test domain which ends
up being rather easy. Install the stuff in ```test/README``` near the
top. And then do the following:
$ bots/vm-run --network ipa
$ bots/vm-run --network services
That runs an IPA domain. Now in another terminal do the following:

View File

@ -264,7 +264,6 @@
</div>
</div>
<script src="../domain/domain.js"></script>
<script src="../performance/performance.js"></script>
</body>
</html>

View File

@ -27,8 +27,9 @@ import $ from "jquery";
import { mustache } from "mustache";
import * as packagekit from "packagekit.js";
import { install_dialog } from "cockpit-components-install-dialog.jsx";
import { PrivilegedButton } from "cockpit-components-privileged.jsx";
import { Privileged, PrivilegedButton } from "cockpit-components-privileged.jsx";
import { ServerTime } from './serverTime.js';
import * as realmd from "./realmd-operation.js";
/* These add themselves to jQuery so just including is enough */
import "patterns";
@ -78,6 +79,8 @@ export class ConfigurationCard extends React.Component {
this.host_keys_show = this.host_keys_show.bind(this);
this.host_keys_hide = this.host_keys_hide.bind(this);
this.realmd = realmd.setup();
}
componentDidMount() {
@ -94,6 +97,8 @@ export class ConfigurationCard extends React.Component {
});
$("#system_information_ssh_keys").on("hide.bs.modal", () => this.host_keys_hide());
this.realmd.addEventListener("changed", () => this.setState({}));
}
systime_setup() {
@ -321,14 +326,27 @@ export class ConfigurationCard extends React.Component {
}
render() {
// We use a Privileged component for its ability to
// conditionally show a tooltip, even when the button is not
// actually disabled, so the "allowed" property really means
// "does not have tooltip" here.
const hostname_tooltip = (this.permission.allowed === false
? cockpit.format(_("The user $0 is not permitted to modify hostnames"),
this.permission.user ? this.permission.user.name : '')
: this.realmd.hostname_button_tooltip);
const hostname_disabled = this.permission.allowed === false || this.realmd.hostname_button_disabled;
const hostname_button = (
<PrivilegedButton variant="link" buttonId="system_information_hostname_button"
tooltipId="system_information_hostname_tooltip"
onClick={ () => $('#system_information_change_hostname').modal('show') }
excuse={ _("The user $0 is not permitted to modify hostnames") }
permission={ this.permission } ariaLabel="edit hostname">
{this.props.hostname !== "" ? _("edit") : _("Set Hostname")}
</PrivilegedButton>);
<Privileged allowed={ !hostname_tooltip }
tooltipId="system_information_hostname_tooltip"
excuse={ hostname_tooltip }>
<Button id="system_information_hostname_button" variant="link"
onClick={ () => $('#system_information_change_hostname').modal('show') }
isInline isDisabled={ hostname_disabled } aria-label="edit hostname">
{this.props.hostname !== "" ? _("edit") : _("Set Hostname")}
</Button>
</Privileged>);
const systime_button = (
<PrivilegedButton variant="link" buttonId="system_information_systime_button"
@ -339,6 +357,23 @@ export class ConfigurationCard extends React.Component {
{ this.state.serverTime }
</PrivilegedButton>);
const domain_tooltip = (this.permission.allowed === false
? cockpit.format(_("The user $0 is not permitted to modify realms"),
this.permission.user ? this.permission.user.name : '')
: this.realmd.button_tooltip);
const domain_disabled = this.permission.allowed === false || this.realmd.button_disabled;
const domain_button = (
<Privileged allowed={ !domain_tooltip }
tooltipId="system_information_domain_tooltip"
excuse={ domain_tooltip }>
<Button id="system_information_domain_button" variant="link"
onClick={ () => this.realmd.clicked() }
isInline isDisabled={ domain_disabled } aria-label="join domain">
{ this.realmd.button_text }
</Button>
</Privileged>);
return (
<Card className="system-configuration">
<CardHeader>{_("Configuration")}</CardHeader>
@ -365,7 +400,7 @@ export class ConfigurationCard extends React.Component {
<tr>
<th scope="row">{_("Domain")}</th>
<td><p id="system-info-domain" /></td>
<td>{domain_button}</td>
</tr>
<tr>

View File

@ -4,7 +4,7 @@ import * as packagekit from "packagekit.js";
import { install_dialog } from "cockpit-components-install-dialog.jsx";
import "patterns";
import operation_html from "raw-loader!./operation.html";
import operation_html from "raw-loader!./realmd-operation.html";
const _ = cockpit.gettext;
@ -16,7 +16,7 @@ var KERBEROS = "org.freedesktop.realmd.Kerberos";
var KERBEROS_MEMBERSHIP = "org.freedesktop.realmd.KerberosMembership";
var REALM = "org.freedesktop.realmd.Realm";
function instance(realmd, mode, realm, button) {
function instance(realmd, mode, realm, state) {
var dialog = jQuery.parseHTML(operation_html)[0];
/* Scope the jQuery selector to our dialog */
@ -287,10 +287,8 @@ function instance(realmd, mode, realm, button) {
}
}
if (operation)
button.attr('disabled', 'disabled');
else
button.removeAttr('disabled');
state.button_disabled = !!operation;
state.dispatchEvent("changed");
if (mode != 'join')
return;
@ -564,14 +562,20 @@ function instance(realmd, mode, realm, button) {
return dialog;
}
function setup() {
export function setup() {
var $ = jQuery;
var element = $("<span>");
var link = $("<button class='pf-c-button pf-m-link pf-m-inline' type='button' aria-label='join domain'>");
element.append(link);
var hostname_link = $("#system_information_hostname_button");
var hostname_tooltip = $("#system_information_hostname_tooltip");
var self = {
button_text: "",
button_tooltip: null,
button_disabled: false,
hostname_button_tooltip: null,
hostname_button_disabled: false,
clicked: handle_link_click
};
cockpit.event_target(self);
var realmd = null;
var realms = null;
@ -582,13 +586,6 @@ function setup() {
var permission = null;
var install_realmd = false;
function setTooltip(message) {
element
.attr('title', message)
.tooltip({ container: 'body' })
.tooltip('fixTitle');
}
function update_realms() {
var text, path, realm;
joined = [];
@ -600,16 +597,17 @@ function setup() {
if (!joined || !joined.length) {
text = _("Join Domain");
hostname_link.removeAttr('disabled');
hostname_tooltip.removeAttr('title');
hostname_tooltip.removeAttr('data-original-title');
self.hostname_button_disabled = false;
self.hostname_button_tooltip = null;
} else {
text = joined.map(function(x) { return x.Name }).join(", ");
hostname_link.attr('disabled', true);
hostname_tooltip.attr('title', _("Host name should not be changed in a domain"))
.tooltip('fixTitle');
self.hostname_button_disabled = true;
self.hostname_button_tooltip = _("Host name should not be changed in a domain");
}
link.text(text);
self.button_text = text;
self.button_disabled = false;
self.button_tooltip = null;
self.dispatchEvent("changed");
}
function setup_realms_proxy() {
@ -622,17 +620,20 @@ function setup() {
/* see if we can install it */
packagekit.detect().then(function (exists) {
if (exists) {
setTooltip("Joining a domain requires installation of realmd");
link.removeAttr("disabled");
self.button_tooltip = _("Joining a domain requires installation of realmd");
self.button_disabled = false;
install_realmd = true;
self.dispatchEvent("changed");
} else {
setTooltip("Cannot join a domain because realmd is not available on this system");
link.attr("disabled", true);
self.button_tooltip = _("Cannot join a domain because realmd is not available on this system");
self.button_disabled = true;
self.dispatchEvent("changed");
}
});
} else {
setTooltip(cockpit.message(options));
link.attr("disabled", true);
self.button_tooltip = cockpit.message(options);
self.button_disabled = true;
self.dispatchEvent("changed");
}
$(realmd).off();
realmd.close();
@ -640,21 +641,6 @@ function setup() {
});
realms = realmd.proxies("org.freedesktop.realmd.Realm");
realms.wait(function() {
if (!realmd)
return;
permission = cockpit.permission({ admin: true });
function update_realm_privileged() {
$(link).update_privileged(permission,
cockpit.format(_("The user <b>$0</b> is not permitted to modify realms"),
permission.user ? permission.user.name : ''), null, element);
}
$(permission).on("changed", update_realm_privileged);
});
$(realms).on("changed", update_realms);
}
@ -663,7 +649,8 @@ function setup() {
.then(function() {
install_realmd = false;
setup_realms_proxy();
element.tooltip('disable');
self.button_tooltip = null;
self.dispatchEvent("changed");
// proceed to domain join dialog after realmd initialized
realms.wait().done(handle_link_click);
})
@ -681,9 +668,9 @@ function setup() {
}
if (joined && joined.length)
dialog = instance(realmd, 'leave', joined[0], link);
dialog = instance(realmd, 'leave', joined[0], self);
else
dialog = instance(realmd, 'join', null, link);
dialog = instance(realmd, 'join', null, self);
$(dialog)
.attr("id", "realms-op")
@ -694,27 +681,15 @@ function setup() {
setup_realms_proxy();
update_realms();
link.on("click", handle_link_click);
element.close = function close() {
self.close = function close() {
if (dialog)
dialog.cancel();
element.remove();
if (realmd)
realmd.close();
if (permission)
permission.close();
};
return element;
return self;
}
/* Hook this in when loaded */
jQuery(function() {
var placeholder = jQuery("#system-info-domain");
if (placeholder.length) {
placeholder.append(setup());
placeholder.removeAttr('hidden');
placeholder.prev().removeAttr('hidden');
}
});

View File

@ -95,7 +95,7 @@ class TestRealms(MachineCase):
if self.machine.image in ["rhel-8-1-distropkg"]:
self.domain_sel = "#system-info-domain a"
else:
self.domain_sel = "#system-info-domain button"
self.domain_sel = "#system_information_domain_button"
def testIpa(self):
m = self.machine
@ -781,7 +781,20 @@ class TestPackageInstall(packagelib.PackageCase):
if self.machine.image in ["rhel-8-1-distropkg"]:
self.domain_sel = "#system-info-domain a"
else:
self.domain_sel = "#system-info-domain button"
self.domain_sel = "#system_information_domain_button"
def checkTooltip(self, text):
b = self.browser
if self.machine.image in ["rhel-8-1-distropkg"]:
b.wait_present("#system-info-domain [data-original-title]")
b.mouse("#system-info-domain span", "mouseover")
b.wait_in_text(".tooltip-inner", text)
b.mouse("#system-info-domain span", "mouseout")
else:
b.wait_present("#system_information_domain_tooltip")
b.mouse("#system_information_domain_tooltip", "mouseover")
b.wait_in_text(".tooltip-inner", text)
b.mouse("#system_information_domain_tooltip", "mouseout")
def testInstall(self):
m = self.machine
@ -797,11 +810,7 @@ class TestPackageInstall(packagelib.PackageCase):
b.wait_present(self.domain_sel + ".disabled")
else:
b.wait_present(self.domain_sel + "[disabled]")
# check tooltip
b.wait_present("#system-info-domain [data-original-title]")
b.mouse("#system-info-domain span", "mouseover")
b.wait_in_text(".tooltip-inner", "realmd is not available on this system")
b.mouse("#system-info-domain span", "mouseout")
self.checkTooltip("realmd is not available on this system")
b.logout()
# case 2: enable PackageKit, but no realmd package available
@ -809,11 +818,7 @@ class TestPackageInstall(packagelib.PackageCase):
self.login_and_go("/system")
# Joining a domain should bring up the install dialog
b.wait_text(self.domain_sel, "Join Domain")
# check tooltip
b.wait_present("#system-info-domain [data-original-title]")
b.mouse("#system-info-domain span", "mouseover")
b.wait_in_text(".tooltip-inner", "requires installation of realmd")
b.mouse("#system-info-domain span", "mouseout")
self.checkTooltip("requires installation of realmd")
b.click(self.domain_sel)
b.wait_in_text(".modal-dialog:contains('Install Software')", "realmd is not available")
@ -831,11 +836,7 @@ class TestPackageInstall(packagelib.PackageCase):
# Joining a domain should bring up the install dialog
b.wait_text(self.domain_sel, "Join Domain")
# check tooltip
b.wait_present("#system-info-domain [data-original-title]")
b.mouse("#system-info-domain span", "mouseover")
b.wait_in_text(".tooltip-inner", "requires installation of realmd")
b.mouse("#system-info-domain span", "mouseout")
self.checkTooltip("requires installation of realmd")
b.click(self.domain_sel)
b.click(".modal-dialog:contains('Install Software') .modal-footer button.btn-primary")
@ -857,11 +858,7 @@ class TestPackageInstall(packagelib.PackageCase):
# Joining a domain should bring up the install dialog
b.wait_text(self.domain_sel, "Join Domain")
# check tooltip
b.wait_present("#system-info-domain [data-original-title]")
b.mouse("#system-info-domain span", "mouseover")
b.wait_in_text(".tooltip-inner", "requires installation of realmd")
b.mouse("#system-info-domain span", "mouseout")
self.checkTooltip("requires installation of realmd")
b.click(self.domain_sel)
# restore realmd service, to pretend that package install completed
@ -878,10 +875,12 @@ class TestPackageInstall(packagelib.PackageCase):
b.wait_popdown("realms-op")
# should not have a tooltip any more
b.mouse("#system-info-domain span", "mouseover")
time.sleep(2)
self.assertFalse(b.is_present(".tooltip-inner"))
if self.machine.image in ["rhel-8-1-distropkg"]:
b.mouse("#system-info-domain span", "mouseover")
time.sleep(2)
self.assertFalse(b.is_present(".tooltip-inner"))
else:
b.wait_not_present("#system_information_domain_tooltip")
if __name__ == '__main__':
test_main()

View File

@ -164,9 +164,6 @@ touch dashboard.list
echo '%dir %{_datadir}/cockpit/pcp' >> pcp.list
find %{buildroot}%{_datadir}/cockpit/pcp -type f >> pcp.list
echo '%dir %{_datadir}/cockpit/realmd' >> system.list
find %{buildroot}%{_datadir}/cockpit/realmd -type f >> system.list
echo '%dir %{_datadir}/cockpit/tuned' >> system.list
find %{buildroot}%{_datadir}/cockpit/tuned -type f >> system.list
@ -221,7 +218,7 @@ touch docker.list
# when not building basic packages, remove their files
%if 0%{?build_basic} == 0
for pkg in base1 branding motd kdump networkmanager realmd selinux shell sosreport ssh static systemd tuned users; do
for pkg in base1 branding motd kdump networkmanager selinux shell sosreport ssh static systemd tuned users; do
rm -r %{buildroot}/%{_datadir}/cockpit/$pkg
rm -f %{buildroot}/%{_datadir}/metainfo/org.cockpit-project.cockpit-${pkg}.metainfo.xml
done
@ -352,7 +349,6 @@ Requires: shadow-utils
Requires: grep
Requires: libpwquality
Requires: /usr/bin/date
Provides: cockpit-realmd = %{version}-%{release}
Provides: cockpit-shell = %{version}-%{release}
Provides: cockpit-systemd = %{version}-%{release}
Provides: cockpit-tuned = %{version}-%{release}

View File

@ -1,4 +1,3 @@
usr/share/cockpit/realmd/
usr/share/cockpit/shell/
usr/share/cockpit/systemd/
usr/share/cockpit/users/

View File

@ -159,8 +159,7 @@ Depends: ${misc:Depends},
libpwquality-tools,
openssl,
Recommends: policykit-1
Provides: cockpit-realmd,
cockpit-shell,
Provides: cockpit-shell,
cockpit-systemd,
cockpit-tuned,
cockpit-users