Skip to content

Commit

Permalink
login: Split alert into title/description
Browse files Browse the repository at this point in the history 
This makes it conformant to the current PF design.

Split the host and login alerts accordingly. Give some additional
instruction for `no-cockpit`.

Bring the DOM structure closer to current PF5's alert, and
adjust/simplify CSS accordingly. Thanks Garrett for working this out!

Also tighten the error message checks in the tests, in particular for
check-ws-bastion.

Rework the "no-cockpit" message: Telling the user which OSes are
supported is nice at some level, but doesn't help operationally when
they want to connect to *that* server. Rather, suggest to install
cockpit-system, which is the recommended way out of this error.
  • Loading branch information
@martinpitt
martinpitt committed Oct 23, 2024
1 parent bfc4a81 commit e361d9c
Show file tree
Hide file tree
Showing 9 changed files with 134 additions and 101 deletions.
41 changes: 26 additions & 15 deletions pkg/static/login.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
</head>

<body class="login-pf">
<div id="banner" class="pf-v5-c-alert pf-m-info pf-m-inline dialog-error" aria-label="inline danger alert" hidden="true">
<svg fill="currentColor" viewBox="0 0 448 512" aria-hidden="true">
<path d="M224 512c35.32 0 63.97-28.65 63.97-64H160.03c0 35.35 28.65 64 63.97 64zm215.39-149.71c-19.32-20.76-55.47-51.99-55.47-154.29 0-77.7-54.48-139.9-127.94-155.16V32c0-17.67-14.32-32-31.98-32s-31.98 14.33-31.98 32v20.84C118.56 68.1 64.08 130.3 64.08 208c0 102.3-36.15 133.53-55.47 154.29-6 6.45-8.66 14.16-8.61 21.71.11 16.4 12.98 32 32.1 32h383.8c19.12 0 32-15.6 32.1-32 .05-7.55-2.61-15.27-8.61-21.71z" />
</svg>
<span id="banner-message" class="pf-v5-c-alert__title"></span>
<div id="banner" class="pf-v5-c-alert pf-m-info pf-m-inline dialog-error" aria-label="inline info alert" hidden="true">
<div class="pf-v5-c-alert__icon">
<svg fill="currentColor" viewBox="0 0 448 512" aria-hidden="true">
<path d="M224 512c35.32 0 63.97-28.65 63.97-64H160.03c0 35.35 28.65 64 63.97 64zm215.39-149.71c-19.32-20.76-55.47-51.99-55.47-154.29 0-77.7-54.48-139.9-127.94-155.16V32c0-17.67-14.32-32-31.98-32s-31.98 14.33-31.98 32v20.84C118.56 68.1 64.08 130.3 64.08 208c0 102.3-36.15 133.53-55.47 154.29-6 6.45-8.66 14.16-8.61 21.71.11 16.4 12.98 32 32.1 32h383.8c19.12 0 32-15.6 32.1-32 .05-7.55-2.61-15.27-8.61-21.71z" />
</svg>
</div>
<span id="banner-message" class="pf-v5-c-alert__description"></span>
</div>

<span id="badge"></span>
Expand All @@ -27,12 +29,19 @@
<h1 id="brand" class="hide-before"></h1>

<div id="error-group" class="pf-v5-c-alert pf-m-danger pf-m-inline dialog-error noscript" aria-label="inline danger alert">
<svg fill="currentColor" viewBox="0 0 512 512" aria-hidden="true">
<path d="M504 256c0 136.997-111.043 248-248 248S8 392.997 8 256C8 119.083 119.043 8 256 8s248 111.083 248 248zm-248 50c-25.405 0-46 20.595-46 46s20.595 46 46 46 46-20.595 46-46-20.595-46-46-46zm-43.673-165.346l7.418 136c.347 6.364 5.609 11.346 11.982 11.346h28.546c6.373 0 11.635-4.982 11.982-11.346l7.418-136c.375-6.874-5.098-12.654-11.982-12.654h-63.383c-6.884 0-12.356 5.78-11.981 12.654z" />
</svg>
<h2 id="login-error-message" class="pf-v5-c-alert__title">
<span class="noscript" translate="yes">Please enable JavaScript to use the Web Console.</span>
</h2>
<div class="pf-v5-c-alert__icon">
<svg fill="currentColor" viewBox="0 0 512 512" aria-hidden="true">
<path d="M504 256c0 136.997-111.043 248-248 248S8 392.997 8 256C8 119.083 119.043 8 256 8s248 111.083 248 248zm-248 50c-25.405 0-46 20.595-46 46s20.595 46 46 46 46-20.595 46-46-20.595-46-46-46zm-43.673-165.346l7.418 136c.347 6.364 5.609 11.346 11.982 11.346h28.546c6.373 0 11.635-4.982 11.982-11.346l7.418-136c.375-6.874-5.098-12.654-11.982-12.654h-63.383c-6.884 0-12.356 5.78-11.981 12.654z" />
</svg>
</div>
<p class="pf-v5-c-alert__title">
<span id="login-error-title" class="pf-v5-screen-reader"></span>
</p>
<div class="pf-v5-c-alert__description">
<p id="login-error-message">
<span class="noscript" translate="yes">Please enable JavaScript to use the Web Console.</span>
</p>
</div>
</div>

<div class="unsupported-browser" id="unsupported-browser" hidden="true">
Expand Down Expand Up @@ -64,7 +73,7 @@ <h3 translate="yes">Or use a bundled browser</h3>
</details>
</div>

<div id="info-group" class="pf-v5-c-alert pf-m-info pf-m-inline dialog-error" aria-label="inline danger alert" hidden="true">
<div id="info-group" class="pf-v5-c-alert pf-m-info pf-m-inline dialog-error" aria-label="inline info alert" hidden="true">
<svg fill="currentColor" viewBox="0 0 512 512" aria-hidden="true">
<path d="M256 8C119.043 8 8 119.083 8 256c0 136.997 111.043 248 248 248s248-111.003 248-248C504 119.083 392.957 8 256 8zm0 110c23.196 0 42 18.804 42 42s-18.804 42-42 42-42-18.804-42-42 18.804-42 42-42zm56 254c0 6.627-5.373 12-12 12h-88c-6.627 0-12-5.373-12-12v-24c0-6.627 5.373-12 12-12h12v-64h-12c-6.627 0-12-5.373-12-12v-24c0-6.627 5.373-12 12-12h64c6.627 0 12 5.373 12 12v100h12c6.627 0 12 5.373 12 12v24z" />
</svg>
Expand All @@ -77,9 +86,11 @@ <h2 id="login-info-message" class="pf-v5-c-alert__title"></h2>
<div id="hostkey-group" class="form-group" hidden="true">
<h1 id="hostkey-title"></h1>
<div id="hostkey-warning-group" class="pf-v5-c-alert pf-m-warning pf-m-inline dialog-error" aria-label="inline warning alert" hidden="true">
<svg fill="currentColor" viewBox="0 0 576 512" aria-hidden="true"><path d="M569.52 440.01c18.46 32-4.71 71.99-41.58 71.99H48.05c-36.93 0-60-40.05-41.57-71.99L246.42 24c18.47-32.01 64.72-31.96 83.16 0L569.52 440zM288 354a46 46 0 100 92 46 46 0 000-92zm-43.67-165.35l7.41 136A12 12 0 00263.74 336h48.54a12 12 0 0011.98-11.35l7.42-136A12 12 0 00319.7 176h-63.38a12 12 0 00-11.98 12.65z"/></svg>
<h2 translate="yes" class="pf-v5-c-alert__title">Changed keys are often the result of an operating system reinstallation. However, an unexpected change may indicate a third-party attempt to intercept your connection.</h2>
</div>
<div class="pf-v5-c-alert__icon">
<svg fill="currentColor" viewBox="0 0 576 512" aria-hidden="true"><path d="M569.52 440.01c18.46 32-4.71 71.99-41.58 71.99H48.05c-36.93 0-60-40.05-41.57-71.99L246.42 24c18.47-32.01 64.72-31.96 83.16 0L569.52 440zM288 354a46 46 0 100 92 46 46 0 000-92zm-43.67-165.35l7.41 136A12 12 0 00263.74 336h48.54a12 12 0 0011.98-11.35l7.42-136A12 12 0 00319.7 176h-63.38a12 12 0 00-11.98 12.65z"/></svg>
</div>
<span translate="yes" class="pf-v5-c-alert__description">Changed keys are often the result of an operating system reinstallation. However, an unexpected change may indicate a third-party attempt to intercept your connection.</span>
</div>
<p id="hostkey-message-1"></p>
<p translate="yes">To ensure that your connection is not intercepted by a malicious third-party, please verify the host key fingerprint:</p>
<pre id="hostkey-fingerprint"></pre>
Expand Down
42 changes: 22 additions & 20 deletions pkg/static/login.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -543,15 +543,17 @@ function debug(...args) {
id("login-info-message").textContent = "";
}

function login_failure(msg, form) {
function login_failure(title, msg, form) {
clear_errors();
if (msg) {
if (title) {
/* OAuth failures are always fatal */
if (oauth) {
fatal(msg);
fatal(title);
} else {
show_form(form || "login");
id("login-error-title").textContent = title;
id("login-error-message").textContent = msg;
hideToggle("#error-group .pf-v5-c-alert__description", !msg);
show("#error-group");
}
}
Expand All @@ -565,12 +567,14 @@ function debug(...args) {
}
}

function host_failure(msg) {
function host_failure(title, msg) {
if (!login_machine) {
login_failure(msg);
} else {
clear_errors();
id("login-error-title").textContent = title;
id("login-error-message").textContent = msg;
hideToggle("#error-group .pf-v5-c-alert__description", !msg);
show("#error-group");
toggle_options(null, true);
show_form("login");
Expand Down Expand Up @@ -853,7 +857,7 @@ function debug(...args) {

function call_converse() {
id("login-button").removeEventListener("click", call_converse);
login_failure(null, "hostkey");
login_failure(null, null, "hostkey");
// cockpit-beiboot sends only a placeholder, defer to login-data in setup_localstorage()
if (key.endsWith(" login-data")) {
login_data_host = key_host;
Expand Down Expand Up @@ -905,12 +909,12 @@ function debug(...args) {
function call_converse() {
id("conversation-input").removeEventListener("keydown", key_down);
id("login-button").removeEventListener("click", call_converse);
login_failure(null, "conversation");
login_failure(null, null, "conversation");
converse(prompt_data.id, id("conversation-input").value);
}

function key_down(e) {
login_failure(null, "conversation");
login_failure(null, null, "conversation");
if (e.which == 13) {
call_converse();
}
Expand Down Expand Up @@ -1009,13 +1013,13 @@ function debug(...args) {
const user = trim(id("login-user-input").value);
fatal(format(_("The server refused to authenticate '$0' using password authentication, and no other supported authentication methods are available."), user));
} else if (xhr.statusText.indexOf("terminated") > -1) {
login_failure(_("Authentication failed: Server closed connection"));
login_failure(_("Authentication failed"), _("Server closed connection"));
} else if (xhr.statusText.indexOf("no-host") > -1) {
host_failure(_("Unable to connect to that address"));
} else if (xhr.statusText.indexOf("unknown-hostkey") > -1) {
host_failure(_("Refusing to connect. Hostkey is unknown"));
host_failure(_("Refusing to connect"), _("Hostkey is unknown"));
} else if (xhr.statusText.indexOf("unknown-host") > -1) {
host_failure(_("Refusing to connect. Host is unknown"));
host_failure(_("Refusing to connect"), _("Host is unknown"));
} else if (xhr.statusText.indexOf("invalid-hostkey") > -1) {
/* ssh/ferny/beiboot immediately fail in this case, it's not a conversation;
* ask the user for confirmation and try again */
Expand All @@ -1026,24 +1030,22 @@ function debug(...args) {
} else {
// but only once, to avoid loops
debug("send_login_request(): invalid-hostkey, and already retried, giving up");
host_failure(_("Refusing to connect. Hostkey does not match"));
host_failure(_("Refusing to connect"), _("Hostkey does not match"));
}
} else if (is_conversation) {
login_failure(_("Authentication failed"));
} else {
login_failure(_("Wrong user name or password"));
login_failure(_("Authentication failed"), _("Wrong user name or password"));
}
}
} else if (xhr.status == 403) {
login_failure(_(decodeURIComponent(xhr.statusText)) || _("Permission denied"));
const status = decodeURIComponent(xhr.statusText).trim();
login_failure(_("Permission denied"), status === "Permission denied" ? "" : status);
} else if (xhr.status == 500 && xhr.statusText.indexOf("no-cockpit") > -1) {
// always show what's going on
let message = format(_("A compatible version of Cockpit is not installed on $0."), login_machine || "localhost");
// in beiboot mode we get some more info
const error = JSON.parse(xhr.responseText);
if (error.supported)
message += " " + format(_("This is only supported for $0 on the target machine."), error.supported);
login_failure(message);
login_failure(
_("Cockpit not installed on remote host"),
format(_("Install at least the cockpit-system package on $0 to connect."), login_machine || "localhost")
);
} else if (xhr.statusText) {
fatal(decodeURIComponent(xhr.statusText));
} else {
Expand Down
81 changes: 42 additions & 39 deletions pkg/static/login.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -416,61 +416,61 @@ label.checkbox {
inline-size: 1.5rem;
}

/* Alerts */
// Alerts: structure

.pf-v5-c-alert {
color: #151515;
position: relative;
grid-template-columns: max-content 1fr max-content;
grid-template-rows: 1fr auto;
grid-template-areas:
"icon title action"
". content content";
background-color: #fff;
margin-block: 0 1.5rem;
margin-inline: 0;
display: grid;
border: 3px solid #009596;
border-width: 2px 0 0;
box-shadow: rgb(3 3 3 / 16%) 0 0.5rem 1rem 0, rgb(3 3 3 / 8%) 0 0 0.5rem 0;
grid-template: "icon content" / auto 1fr;
gap: 0.5rem 1rem;
align-items: start;
}

.pf-v5-c-alert.pf-m-inline {
box-shadow: none;
.pf-v5-c-alert__title,
.pf-v5-c-alert__description {
grid-column: content;
}

.pf-v5-c-alert > svg {
grid-area: icon;
block-size: 1.125rem;
inline-size: 1.125rem;
margin-block: 1.25rem 1rem;
margin-inline: 1rem;
float: inline-start;
color: #009596;
}
// Alerts: styling

.pf-v5-c-alert {
margin: 1rem;
padding-block: 0.75rem;
padding-inline: 1rem;
border-block-start: 2px solid;

@supports (display: grid) {
.pf-v5-c-alert > svg {
float: none;
margin-inline-end: 0;
p {
margin: 0;
}
}

.pf-v5-c-alert__icon {
// vertically align the icon's top edge with the text; https://blog.kizu.dev/cap-height-align/
margin-block-start: calc((1cap - 1ex) / 2);
}

.pf-v5-c-alert__icon svg {
display: block;
block-size: 1.125rem;
inline-size: 1.125rem;
}

.pf-v5-c-alert__title {
grid-area: title;
font-size: 1rem;
margin: 1rem;
font-weight: 700; // pf-v5-global--FontWeight--bold
}

// Alert: colors for types

.pf-v5-c-alert.pf-m-inline.pf-m-danger {
background: #faeae8;
border-color: #c9190b;
}

.pf-v5-c-alert.pf-m-danger > svg {
.pf-v5-c-alert.pf-m-danger svg {
color: #c9190b;
}

.pf-v5-c-alert.pf-m-danger .pf-v5-c-alert__title {
.pf-v5-c-alert.pf-m-danger .pf-v5-c-alert__title,
.pf-v5-c-alert.pf-m-danger .pf-v5-c-alert__description {
color: #a30000;
}

Expand All @@ -479,11 +479,12 @@ label.checkbox {
border-color: #f0ab00;
}

.pf-v5-c-alert.pf-m-warning > svg {
.pf-v5-c-alert.pf-m-warning svg {
color: #f0ab00;
}

.pf-v5-c-alert.pf-m-warning .pf-v5-c-alert__title {
.pf-v5-c-alert.pf-m-warning .pf-v5-c-alert__title,
.pf-v5-c-alert.pf-m-warning .pf-v5-c-alert__description {
color: #795600;
}

Expand All @@ -492,11 +493,12 @@ label.checkbox {
border-color: #73bcf7;
}

.pf-v5-c-alert.pf-m-info > svg {
.pf-v5-c-alert.pf-m-info svg {
color: #73bcf7;
}

.pf-v5-c-alert.pf-m-info .pf-v5-c-alert__title {
.pf-v5-c-alert.pf-m-info .pf-v5-c-alert__title,
.pf-v5-c-alert.pf-m-info .pf-v5-c-alert__description {
color: #004368;
}

Expand Down Expand Up @@ -1002,7 +1004,8 @@ input[type="password"] + .login-password-toggle .password-hide {
}

.pf-v5-c-alert.pf-m-danger > svg,
.pf-v5-c-alert.pf-m-danger .pf-v5-c-alert__title {
.pf-v5-c-alert.pf-m-danger .pf-v5-c-alert__title,
.pf-v5-c-alert.pf-m-danger .pf-v5-c-alert__description {
color: #fe5142;
}

Expand Down
5 changes: 3 additions & 2 deletions test/verify/check-client
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ exec "$@"
b.wait_text("#conversation-prompt", "[email protected]'s password: ")
b.set_val("#conversation-input", "wrong")
b.click("#login-button")
b.wait_in_text("#login-error-message", "Authentication failed")
b.wait_in_text("#login-error-title", "Authentication failed")
b.wait_val("#server-field", "10.111.113.2")

# connect to most recent host
Expand Down Expand Up @@ -217,7 +217,8 @@ exec "$@"
# unreachable host
b.set_val("#server-field", "unknownhost")
b.click("#login-button")
b.wait_in_text("#login-error-message", "Host is unknown")
b.wait_text("#login-error-title", "Refusing to connect")
b.wait_text("#login-error-message", "Host is unknown")
b.wait_val("#server-field", "unknownhost")
# does not appear in recent hosts
b.wait_in_text("#recent-hosts-list", "10.111.113.2")
Expand Down
3 changes: 2 additions & 1 deletion test/verify/check-loopback
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ class TestLoopback(testlib.MachineCase):
b.set_val('#login-user-input', "admin")
b.set_val('#login-password-input', "foobar")
b.click('#login-button')
b.wait_in_text('#login-error-message', "Wrong user name or password")
b.wait_text("#login-error-title", "Authentication failed")
b.wait_text('#login-error-message', "Wrong user name or password")

self.allow_journal_messages("Cannot run program cockpit-bridge: No such file or directory",
".*server offered unsupported authentication methods: public-key.*")
Expand Down
11 changes: 6 additions & 5 deletions test/verify/check-shell-multi-machine
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,8 @@ class TestMultiMachine(testlib.MachineCase):
b.wait_visible("#user-group")
b.wait_visible("#option-group")
b.wait_visible("#server-group")
b.wait_in_text("#login-error-message", "Hostkey does not match")
b.wait_text("#login-error-title", "Refusing to connect")
b.wait_text("#login-error-message", "Hostkey does not match")

# Clear bad host key in /etc and set bad host key in
# localStorage.
Expand Down Expand Up @@ -365,9 +366,8 @@ class TestMultiMachine(testlib.MachineCase):
if m.image not in ["arch", "debian-testing"]:
m2.execute("sed -i '/^VERSION_ID/ s/$/1/' /etc/os-release")
b.try_login(password="alt-password")
b.wait_in_text('#login-error-message', "A compatible version of Cockpit is not installed on 10.111.113.2")
source_os = m.execute('. /etc/os-release; echo "$ID $VERSION_ID"').strip()
b.wait_in_text('#login-error-message', f"This is only supported for {source_os} on the target machine")
b.wait_text('#login-error-title', "Cockpit not installed on remote host")
b.wait_text('#login-error-message', "Install at least the cockpit-system package on 10.111.113.2 to connect.")

fix_bridge(m2)

Expand All @@ -384,7 +384,8 @@ class TestMultiMachine(testlib.MachineCase):
b.wait_not_visible("#server-group")
b.click('#login-button')
b.wait_visible("#server-group")
b.wait_in_text("#login-error-message", "Refusing to connect")
b.wait_text("#login-error-title", "Refusing to connect")
b.wait_text("#login-error-message", "Host is unknown")

# Might happen when we switch away.
self.allow_hostkey_messages()
Expand Down
Loading

0 comments on commit e361d9c

Please sign in to comment.