users: Avoid reading /etc/shadow

pkg/lib/hooks.js

@@ -190,12 +190,10 @@ export function useFileWithError(path, options, hook_options) {
         const handle = cockpit.file(path, memo_options);
         handle.watch((data, tag, error) => {
             setContentAndError([data || false, error || false]);
-            if (!data && memo_hook_options && memo_hook_options.log_errors)
+            if (!data && memo_hook_options?.log_errors)
                 console.warn("Can't read " + path + ": " + (error ? error.toString() : "not found"));
         });
-        return function () {
-            handle.close();
-        };
+        return handle.close;
     }, [path, memo_options, memo_hook_options]);
 
     return content_and_error;

pkg/users/account-details.js

@@ -17,10 +17,7 @@
  * along with Cockpit; If not, see <http://www.gnu.org/licenses/>.
  */
 
-import cockpit from 'cockpit';
 import React, { useState, useEffect, useRef } from 'react';
-import { superuser } from "superuser";
-import { apply_modal_dialog } from "cockpit-components-dialog.jsx";
 
 import { Button } from "@patternfly/react-core/dist/esm/components/Button/index.js";
 import { Checkbox } from "@patternfly/react-core/dist/esm/components/Checkbox/index.js";
@@ -38,27 +35,23 @@ import { TextInput } from "@patternfly/react-core/dist/esm/components/TextInput/
 import { Spinner } from "@patternfly/react-core/dist/esm/components/Spinner/index.js";
 import { Popover } from "@patternfly/react-core/dist/esm/components/Popover/index.js";
 import { ExclamationCircleIcon, HelpIcon, UndoIcon } from '@patternfly/react-icons';
+
+import cockpit from 'cockpit';
+import { superuser } from "superuser";
+import * as timeformat from "timeformat.js";
+import { apply_modal_dialog } from "cockpit-components-dialog.jsx";
+
 import { show_unexpected_error } from "./dialog-utils.js";
 import { delete_account_dialog } from "./delete-account-dialog.js";
 import { account_expiration_dialog, password_expiration_dialog } from "./expiration-dialogs.js";
 import { account_shell_dialog } from "./shell-dialog.js";
 import { set_password_dialog, reset_password_dialog } from "./password-dialogs.js";
 import { AccountLogs } from "./account-logs-panel.jsx";
 import { AuthorizedKeys } from "./authorized-keys-panel.js";
-import * as timeformat from "timeformat.js";
+import { get_locked } from "./utils.js";
 
 const _ = cockpit.gettext;
 
-function get_locked(name) {
-    return cockpit.spawn(["passwd", "-S", name], { environ: ["LC_ALL=C"], superuser: "require" })
-            .catch(() => "")
-            .then(content => {
-                const status = content.split(" ")[1];
-                // libuser uses "LK", shadow-utils use "L".
-                return status && (status == "LK" || status == "L");
-            });
-}
-
 function get_expire(name) {
     function parse_expire(data) {
         let account_expiration = '';
@@ -103,7 +96,7 @@ function get_expire(name) {
             .then(parse_expire);
 }
 
-export function AccountDetails({ accounts, groups, shadow, current_user, user, shells }) {
+export function AccountDetails({ accounts, groups, current_user, user, shells }) {
     const [expiration, setExpiration] = useState(null);
     useEffect(() => {
         get_expire(user).then(setExpiration);
@@ -114,7 +107,7 @@ export function AccountDetails({ accounts, groups, shadow, current_user, user, s
             get_expire(user).then(setExpiration);
         });
         return handle.close;
-    }, [user, accounts, shadow]);
+    }, [user, accounts]);
 
     const [edited_real_name, set_edited_real_name] = useState(null);
     const [committing_real_name, set_committing_real_name] = useState(false);
@@ -153,7 +146,6 @@ export function AccountDetails({ accounts, groups, shadow, current_user, user, s
                                this is a workaround for different ways of handling a locked account
                                https://github.com/cockpit-project/cockpit/issues/1216
                                https://bugzilla.redhat.com/show_bug.cgi?id=853153
-                               This seems to be fixed in fedora 23 (usermod catches the different locking behavior)
                             */
                                 if (locked != value && !dont_retry_if_stuck) {
                                     console.log("Account locked state doesn't match desired value, trying again.");

pkg/users/users.js

@@ -20,16 +20,18 @@ import '../lib/patternfly/patternfly-5-cockpit.scss';
 import 'polyfills'; // once per application
 import 'cockpit-dark-theme'; // once per page
 
-import cockpit from 'cockpit';
 import React, { useMemo, useState, useEffect } from 'react';
 import { createRoot } from 'react-dom/client';
-import { superuser } from "superuser";
 
+import cockpit from 'cockpit';
+import { superuser } from "superuser";
 import { usePageLocation, useLoggedInUser, useFile, useInit } from "hooks.js";
 import { etc_passwd_syntax, etc_group_syntax, etc_shells_syntax } from "pam_user_parser.js";
+import { EmptyStatePanel } from "cockpit-components-empty-state.jsx";
+
+import { get_locked } from "./utils.js";
 import { AccountsMain } from "./accounts-list.js";
 import { AccountDetails } from "./account-details.js";
-import { EmptyStatePanel } from "cockpit-components-empty-state.jsx";
 
 import "./users.scss";
 
@@ -53,7 +55,6 @@ function AccountsPage() {
     const [isGroupsExpanded, setIsGroupsExpanded] = useState(false);
     const { path } = usePageLocation();
     const accounts = useFile("/etc/passwd", { syntax: etc_passwd_syntax });
-    const shadow = useFile("/etc/shadow", { superuser: true });
     const groups = useFile("/etc/group", { syntax: etc_group_syntax });
     const shells = useFile("/etc/shells", { syntax: etc_shells_syntax });
     const current_user_info = useLoggedInUser();
@@ -86,15 +87,18 @@ function AccountsPage() {
 
     const [details, setDetails] = useState(null);
     useInit(() => {
-        getLogins(shadow).then(setDetails);
+        getLogins().then(setDetails);
 
         // Watch `/var/run/utmp` to register when user logs in or out
-        const handle = cockpit.file("/var/run/utmp", { superuser: "try", binary: true });
-        handle.watch(() => {
-            getLogins(shadow).then(setDetails);
-        });
-        return handle;
-    }, [shadow], null, handle => handle.close());
+        const handleUtmp = cockpit.file("/var/run/utmp", { superuser: "try", binary: true });
+        handleUtmp.watch(() => getLogins().then(setDetails), { read: false });
+
+        // Watch /etc/shadow to register lock/unlock/expire changes; but avoid reading it, it's sensitive data
+        const handleShadow = cockpit.file("/etc/shadow", { superuser: "try" });
+        handleShadow.watch(() => getLogins().then(setDetails), { read: false });
+
+        return [handleUtmp, handleShadow];
+    }, [], null, handles => handles.forEach(handle => handle.close()));
 
     // lastlog uses same sorting as /etc/passwd therefore arrays can be combined based on index
     const accountsInfo = useMemo(() => {
@@ -140,17 +144,13 @@ function AccountsPage() {
         );
     } else if (path.length === 1) {
         return (
-            <AccountDetails accounts={accountsInfo} groups={groupsExtraInfo} shadow={shadow || []}
+            <AccountDetails accounts={accountsInfo} groups={groupsExtraInfo}
                             current_user={current_user_info?.name} user={path[0]} shells={shells} />
         );
     } else return null;
 }
 
-function get_locked(name, shadow) {
-    return Boolean((shadow || '').split('\n').find(line => line.startsWith(name + ':!')));
-}
-
-async function getLogins(shadow) {
+async function getLogins() {
     let lastlog = "";
     try {
         lastlog = await cockpit.spawn(["lastlog"], { environ: ["LC_ALL=C"] });
@@ -167,21 +167,21 @@ async function getLogins(shadow) {
     }
 
     // drop header and last empty line with slice
-    const promises = lastlog.split('\n').slice(1, -1).map(line => {
+    const promises = lastlog.split('\n').slice(1, -1).map(async line => {
         const splitLine = line.split(/ +/);
         const name = splitLine[0];
-        const isLocked = get_locked(name, shadow);
+        const isLocked = await get_locked(name);
 
         if (line.indexOf('**Never logged in**') > -1) {
-            return Promise.resolve({ name, loggedIn: false, lastLogin: null, isLocked });
+            return { name, loggedIn: false, lastLogin: null, isLocked };
         }
 
+        const loggedIn = currentLogins.includes(name);
+
         const date_fields = splitLine.slice(-5);
         // this is impossible to parse with Date() (e.g. Firefox does not work with all time zones), so call `date` to parse it
         return cockpit.spawn(["date", "+%s", "-d", date_fields.join(' ')], { environ: ["LC_ALL=C"], err: "out" })
-                .then(out => {
-                    return { name, loggedIn: currentLogins.includes(name), lastLogin: parseInt(out) * 1000, isLocked };
-                })
+                .then(out => ({ name, loggedIn, lastLogin: parseInt(out) * 1000, isLocked }))
                 .catch(e => console.warn(`Failed to parse date from lastlog line '${line}': ${e.toString()}`));
     });
 

pkg/users/utils.js

@@ -0,0 +1,10 @@
+import cockpit from 'cockpit';
+
+export const get_locked = name =>
+    cockpit.spawn(["passwd", "-S", name], { environ: ["LC_ALL=C"], superuser: "require" })
+            .then(content => {
+                const status = content.split(" ")[1];
+                // libuser uses "LK", shadow-utils use "L".
+                return status == "LK" || status == "L";
+            })
+            .catch(() => null);

test/verify/check-pages

@@ -317,6 +317,9 @@ OnCalendar=daily
             })""", language)
             b.wait_attr(".index-page", "lang", language)
 
+        # the quick iteration starts/stops tracer in quick succession, before it can finish
+        self.allow_browser_errors("Tracer failed:.*internal-error")
+
     def testPtBRLocale(self):
         m = self.machine
         b = self.browser