diff --git a/CHANGELOG.md b/CHANGELOG.md index b194e41..01f4cbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,19 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.0.2] - 2026-07-18 + +### Fixed +- `setPassword` now uses `chpasswd` for reliable non-interactive password updates. +- `validateUsername` now accepts Linux login names up to 32 characters long. +- `addSSHtoUser` no longer appends duplicate keys to `authorized_keys`. +- `removeUser` and `addUserToGroup` now use `spawnWrapper` for consistent error + handling and timeout support. +- Direct `require('linux-sys-user/lib/promise')` now enforces the Linux platform + guard instead of bypassing it. +- Documentation and GitHub Pages navigation updated to reflect the current API, + including `chpasswd` and the `addUser` string shortcut. + ## [2.0.0] - 2026-07-16 ### Added diff --git a/README.md b/README.md index 37c402f..caad5df 100644 --- a/README.md +++ b/README.md @@ -265,6 +265,9 @@ console.log(user); * callback function(err, userInfo) + A string may be passed instead of an object as a shortcut for + `{ username: string, create_home: true }`. + * linuxUser.getExpiration(username, callback) Gets expiration information about a user, returns an Object: @@ -348,7 +351,10 @@ console.log(user); * username String * password String * callback function(err) - + + Uses `chpasswd` to set the password non-interactively. The system must + have `chpasswd` available (usually provided by `shadow-utils`). + * linuxUser.addGroup(groupname, callback) * groupname String * callback function(err, groupInfo) @@ -386,6 +392,9 @@ console.log(user); * key String * callback function(err, true) + The key is appended to the user's `~/.ssh/authorized_keys` only if it is + not already present. + ### License diff --git a/docs/api.html b/docs/api.html index 6187a1b..54b3dae 100644 --- a/docs/api.html +++ b/docs/api.html @@ -154,6 +154,7 @@

API Reference

  • API Reference
  • Examples
  • Getting Started
    • +
  • Installation
    • @@ -182,7 +183,7 @@

    addUser(args, callback)

    Parameters:
    - args - Object or String
    + args - Object or String. When a string is passed it is treated as { username: string, create_home: true }.
      • username (String) - User name of the user to be created
      • shell (String|null) - Path to login shell, null uses /usr/sbin/nologin
      • create_home (Boolean) - Create home directory
    @@ -346,7 +347,7 @@

    🔐 Password Management

    setPassword(username, password, callback)

    -

    Sets or changes a user's password.

    +

    Sets or changes a user's password using chpasswd.

    Parameters:
    username (String) - Username to set password for
    @@ -380,7 +381,7 @@

    verifySSHKey(key, callback)

    addSSHtoUser(user, key, callback)

    -

    Adds an SSH key to a user's authorized_keys file.

    +

    Adds an SSH key to a user's authorized_keys file. If the key already exists, the call succeeds without creating a duplicate.

    Parameters:
    user (String) - Username
    @@ -509,7 +510,7 @@

    .promise(promisifyFunction)

    -

    © 2024 linux-sys-user. MIT License. Built with ❤️ for the Node.js community.

    +

    © 2026 linux-sys-user. MIT License. Built with ❤️ for the Node.js community.

    diff --git a/docs/examples.html b/docs/examples.html index b5a1fa6..ab2ca5d 100644 --- a/docs/examples.html +++ b/docs/examples.html @@ -207,6 +207,7 @@

    linux-sys-user

  • Home
  • API Reference
  • Examples
    • +
  • Getting Started
  • Installation
  • GitHub
    • @@ -488,7 +489,7 @@

    SSH Key Management

    - Note: The addSSHtoUser method automatically creates the .ssh directory with proper permissions (700) and sets the authorized_keys file permissions to 600. + Note: The addSSHtoUser method automatically creates the .ssh directory with proper permissions (700) and sets the authorized_keys file permissions to 600. The key is only appended if it is not already present, so calling it multiple times with the same key will not create duplicates.
    diff --git a/docs/getting-started.html b/docs/getting-started.html index ea4c52f..60e2799 100644 --- a/docs/getting-started.html +++ b/docs/getting-started.html @@ -174,6 +174,7 @@

    Getting Started

  • API Reference
  • Examples
  • Getting Started
    • +
  • Installation
    • @@ -331,7 +332,7 @@

    ✅ Next Steps

    -

    © 2024 linux-sys-user. MIT License. Built with ❤️ for the Node.js community.

    +

    © 2026 linux-sys-user. MIT License. Built with ❤️ for the Node.js community.

    diff --git a/docs/index.html b/docs/index.html index 6cf3427..ba866e8 100644 --- a/docs/index.html +++ b/docs/index.html @@ -153,6 +153,7 @@

    linux-sys-user

  • API Reference
  • Examples
  • Getting Started
    • +
  • Installation
    • @@ -243,7 +244,7 @@

    Installation

    diff --git a/docs/installation.html b/docs/installation.html index 76a7d6b..12b3cd8 100644 --- a/docs/installation.html +++ b/docs/installation.html @@ -226,6 +226,7 @@

    linux-sys-user

  • Home
  • API Reference
  • Examples
    • +
  • Getting Started
  • Installation
  • GitHub
    • @@ -244,7 +245,7 @@

    ⚠️ Important Prerequisites

  • Operating System: Linux (any distribution)
  • Node.js: Version 6.0.0 or higher
  • Privileges: Root access for most operations
    • -
  • System Commands: useradd, userdel, groupadd, groupdel, usermod, passwd, chage, ssh-keygen
    • +
  • System Commands: useradd, userdel, groupadd, groupdel, usermod, passwd, chpasswd, chage, ssh-keygen
    • @@ -256,7 +257,7 @@

    ⚠️ Important Prerequisites

    whoami # Verify required commands are available -which useradd userdel groupadd groupdel usermod passwd chage ssh-keygen +which useradd userdel groupadd groupdel usermod passwd chpasswd chage ssh-keygen diff --git a/docs/issues.md b/docs/issues.md deleted file mode 100644 index e135d7f..0000000 --- a/docs/issues.md +++ /dev/null @@ -1,328 +0,0 @@ -# Issue Report: linux-sys-user - -This document details issues found during code review of the linux-sys-user module. - ---- - -## 1. Inconsistent Error Handling in `spawnWrapper` - -**Location**: `lib/user.js:26-31` - -**Problem**: The `spawnWrapper` function has inconsistent behavior between spawn errors and normal exit: - -```javascript -_p.on("error", function () { - callback(stderr || true, stdout); // Returns: (stderr || true, stdout) -}); -_p.on("exit", function () { - callback(stderr, stdout); // Returns: (stderr, stdout) -}); -``` - -When a spawn error occurs, `callback` is called with `stderr || true` - ensuring a truthy first argument. But on normal exit, `callback` receives `stderr` directly, which may be an empty string (falsy). - -**Impact**: -- Calling code cannot reliably distinguish between a spawn failure and a successful command that produced no stderr output. -- Functions that check `if (error) callback(error)` will behave inconsistently. - -**Recommendation**: Normalize error handling to always return a consistent format. Consider creating an `Error` object or using a consistent sentinel value: - -```javascript -_p.on("error", function (err) { - callback(new Error(err.message || "Process spawn failed"), stdout); -}); -``` - ---- - -## 2. TypeScript Type Mismatch for `selinux_user` - -**Location**: `lib/user.js:79-81` and `index.d.ts:11` - -**Problem**: The TypeScript definition types `selinux_user` as `boolean`, but the implementation passes it as a string argument to `useradd`: - -```javascript -// lib/user.js:79-81 -selinux_user: function (SEUSER) { - if (SEUSER) { - return ["--selinux-user", SEUSER]; // SEUSER is used as a string - } -}, -``` - -```typescript -// index.d.ts:11 -selinux_user?: boolean; // Typed as boolean -``` - -**Impact**: TypeScript users will receive type errors when passing a valid string value like `"staff_u"`. - -**Recommendation**: Fix the type definition: - -```typescript -selinux_user?: string; -``` - ---- - -## 3. Fragile Date Parsing in `getExpiration` - -**Location**: `lib/user.js:331-350` - -**Problem**: The `getExpiration` function parses `chage --list` output by splitting on newlines and colons, assuming a specific output format: - -```javascript -data = { - changed: new Date(data[0].split(": ")[1]), - passwordExpires: - data[1].split(": ")[1] === "never" - ? null - : new Date(data[1].split(": ")[1]), - // ... assumes data[0] through data[6] exist with exact format -}; -``` - -**Impact**: -- Output format may vary by `chage` version or system locale/language settings. -- If `data` array lacks expected indices, will throw `TypeError: Cannot read property 'split' of undefined`. -- Date strings in different locales may not parse correctly with `new Date()`. - -**Recommendation**: Add defensive checks and locale-aware parsing: - -```javascript -if (!data || data.length < 7) { - return callback(new Error("Unexpected chage output format")); -} - -var parseDate = function(str) { - if (!str || str.trim() === "never") return null; - // Handle locale-specific date formats if needed - return new Date(str); -}; -``` - ---- - -## 4. Unreliable Password Setting via Stdin - -**Location**: `lib/user.js:210-217` - -**Problem**: The `setPassword` function writes to `passwd`'s stdin but may not work reliably: - -```javascript -var _p = spawn("passwd", [username]); -_p.stdin.write(password + "\n"); -_p.stdin.write(password + "\n"); -_p.stdin.end(); -``` - -Some `passwd` implementations may not accept password via stdin, or may behave differently depending on PAM configuration, security policies, or whether stdin is a terminal. - -**Impact**: Password changes may silently fail or behave unexpectedly on some systems. - -**Recommendation**: Consider using `chpasswd` which is designed for non-interactive password input: - -```javascript -var _p = spawn("chpasswd", []); -_p.stdin.write(username + ":" + password + "\n"); -_p.stdin.end(); -``` - -Alternatively, document that this method requires specific system configuration to work. - ---- - -## 5. Potential Command Injection in `addSSHtoUser` - -**Location**: `lib/user.js:387-398` - -**Problem**: The `addSSHtoUser` function uses `exec` with a format string built from user data: - -```javascript -var commands = [ - util.format("mkdir %s/.ssh;", info.homedir), - util.format("touch %s/.ssh/authorized_keys;", info.homedir), - util.format("chown 700 %s/.ssh;", info.homedir), - util.format("chmod 600 %s/.ssh/authorized_keys;", info.homedir), - util.format("chown %s:%s %s/.ssh -R;", info.username, info.username, info.homedir), -].join(" "); -``` - -The comment claims this is safe because "we are not taking any user input data", but `info.homedir` comes from `/etc/passwd` parsing. - -**Impact**: If `/etc/passwd` is compromised or contains malicious data, command injection could occur. - -**Recommendation**: Validate paths before use: - -```javascript -var safePath = function(path) { - // Only allow absolute paths under /home or /root - if (!path || !path.match(/^\/(home|root)\/[\w.-]+$/)) { - return false; - } - return path; -}; - -if (!safePath(info.homedir)) { - return callback(new Error("Unsafe home directory path")); -} -``` - ---- - -## 6. Username Validation Regex Too Restrictive - -**Location**: `lib/user.js:8` - -**Problem**: The username regex rejects valid Linux usernames on some systems: - -```javascript -var validUsernameRegex = /^([a-z_][a-z0-9_-]{0,30})$/; -``` - -Linux allows usernames up to 32 characters (with recent glibc), but this regex: -- Rejects uppercase letters (though unusual, they are allowed in some configurations) -- Rejects usernames starting with numbers -- May reject legitimate edge cases - -**Impact**: Users with valid system accounts may be unable to use this library. - -**Recommendation**: Update regex to match modern Linux standards (POSIX says 1-255 chars, glibc uses 32 max): - -```javascript -var validUsernameRegex = /^([a-z_][a-z0-9_-]{0,30})$/; -// Current: correct for most cases, but could expand to: -var validUsernameRegex = /^([a-z_][a-z0-9_-]{0,30}[a-z0-9_-]?|[a-z_])$/i; -``` - -Or at minimum, document the restriction clearly. - ---- - -## 7. No Process Timeout Handling - -**Location**: `lib/user.js:10-32` (`spawnWrapper`) - -**Problem**: `spawnWrapper` has no mechanism to timeout or kill long-running processes. If a spawned process hangs (e.g., `passwd` prompting for something unexpected), the callback will never fire. - -**Impact**: Applications may hang indefinitely waiting for a callback that never comes. - -**Recommendation**: Add optional timeout support: - -```javascript -function spawnWrapper(command, args, stdin, callback, timeoutMs) { - var timer; - if (timeoutMs) { - timer = setTimeout(function() { - _p.kill(); - callback(new Error("Process timed out"), ""); - }, timeoutMs); - } - // ... existing code ... -} -``` - ---- - -## 8. Module Structure Issue - -**Location**: `lib/promise.js:4` vs `index.js:15` - -**Problem**: When using `require('./lib/promise')` directly, it requires `./user` which triggers the platform check in `index.js` twice: - -```javascript -// lib/promise.js -var lib = require("./user"); // This loads index.js, not lib/user.js directly -``` - -The `require("./user")` in `lib/promise.js` will resolve to `lib/user.js` (via Node's require resolution), but this means: -- The file is `lib/user.js` not `index.js` -- The platform/root checks in `index.js` are bypassed when using promise.js directly - -**Impact**: If someone requires `require('linux-sys-user/lib/promise')` directly, they bypass platform checks. While unlikely, it creates an inconsistent entry point. - -**Recommendation**: This is a minor issue. Consider if direct access to `lib/promise.js` should be supported or blocked. - ---- - -## 9. Missing Error Handling in `addUser` Success Path - -**Location**: `lib/user.js:128-131` - -**Problem**: In `addUser`, if `getUserInfo` fails on line 130, the error is not propagated: - -```javascript -spawnWrapper( - "useradd", - [].concat(args_array, [args.username]), - null, - function (error, data) { - if (error) callback(error); - exports.getUserInfo(args.username, callback); // Error ignored here - } -); -``` - -If `useradd` succeeds but `getUserInfo` fails, the error is silently lost and `callback` may be called twice (once with user info, once with error). - -**Recommendation**: - -```javascript -function (error, data) { - if (error) return callback(error); - exports.getUserInfo(args.username, function(err, user) { - if (err) return callback(err); - callback(null, user); - }); -} -``` - ---- - -## 10. SSH Key Addition Doesn't Respect Existing Keys - -**Location**: `lib/user.js:406-415` - -**Problem**: `addSSHtoUser` appends the key but doesn't check for duplicates: - -```javascript -fs.appendFile( - info.homedir + "/.ssh/authorized_keys", - key, - function (err) { - // Appends regardless of existing entries - } -); -``` - -**Impact**: The same SSH key can be added multiple times to `authorized_keys`. - -**Recommendation**: Check for key existence before appending: - -```javascript -fs.readFile(info.homedir + "/.ssh/authorized_keys", "utf8", function(err, existing) { - if (err && err.code !== 'ENOENT') return callback(err); - var keys = existing ? existing.split('\n') : []; - if (keys.some(function(line) { return line.trim() === key.trim(); })) { - return callback(null, true); // Key already exists - } - fs.appendFile(/* ... */); -}); -``` - ---- - -## Summary Table - -| # | Issue | Severity | Effort | -|---|-------|----------|--------| -| 1 | Inconsistent error handling in spawnWrapper | Medium | Low | -| 2 | TypeScript boolean vs string for selinux_user | Low | Trivial | -| 3 | Fragile date parsing in getExpiration | Medium | Medium | -| 4 | Unreliable setPassword stdin handling | Medium | Medium | -| 5 | Potential path injection in addSSHtoUser | Medium | Low | -| 6 | Username regex too restrictive | Low | Trivial | -| 7 | No process timeout | Low | Low | -| 8 | Module structure issue | Low | N/A | -| 9 | Missing error handling in addUser | Medium | Low | -| 10 | Duplicate SSH keys not checked | Low | Low | diff --git a/issue.md b/issue.md deleted file mode 100644 index 774ee45..0000000 --- a/issue.md +++ /dev/null @@ -1,308 +0,0 @@ -# Issue Report: linux-sys-user - -This document details issues found during code review of the linux-sys-user codebase. - ---- - -## 1. Inconsistent Error Handling in `spawnWrapper` - -**Location**: `lib/user.js:10-32` - -**Problem**: The `spawnWrapper` function has inconsistent error handling behavior. - -```javascript -_p.on("error", function () { - callback(stderr || true, stdout); // Line 26-27 -}); -_p.on("exit", function () { - callback(stderr, stdout); // Line 29-30 -}); -``` - -When a spawn error occurs (e.g., command not found), it passes `stderr || true` to the callback's error parameter. When the process exits normally, it passes just `stderr`. This means: - -- On spawn error: callback receives truthy error (boolean `true` if stderr is empty) -- On exit with error: callback receives stderr string -- On exit with success: callback receives empty string/falsy - -**Impact**: Calling code cannot reliably distinguish between: -1. Process failed to spawn (command doesn't exist) -2. Process ran but returned an error -3. Process succeeded - -**Recommendation**: Normalize error handling to always pass either an `Error` object or `null`: - -```javascript -_p.on("error", function (err) { - callback(err || new Error("Spawn failed"), stdout); -}); -_p.on("exit", function (code) { - if (code !== 0) { - callback(new Error(stderr || `Process exited with code ${code}`), stdout); - } else { - callback(null, stdout); - } -}); -``` - ---- - -## 2. TypeScript Type Mismatch for `selinux_user` - -**Location**: `lib/user.js:79-81` and `index.d.ts:11` - -**Problem**: The TypeScript definition declares `selinux_user` as `boolean`, but the implementation treats it as a string. - -```typescript -// index.d.ts line 11 -selinux_user?: boolean; - -// lib/user.js lines 79-81 -selinux_user: function (SEUSER) { - if (SEUSER) { - return ["--selinux-user", SEUSER]; - } -}, -``` - -The `useradd` command expects `--selinux-user ` where `` is a SELinux user name (e.g., `staff_u`, `user_u`), not a boolean. - -**Impact**: TypeScript users will receive incorrect type hints and may pass boolean values, which will fail at runtime or produce incorrect `useradd` arguments. - -**Recommendation**: Fix the TypeScript type: - -```typescript -selinux_user?: string; -``` - ---- - -## 3. `setPassword` Stdin Handling May Be Unreliable - -**Location**: `lib/user.js:196-218` - -**Problem**: The `setPassword` function writes to stdin of the `passwd` command: - -```javascript -var _p = spawn("passwd", [username]); -_p.stdin.write(password + "\n"); -_p.stdin.write(password + "\n"); -_p.stdin.end(); -``` - -This approach may fail because: -1. `passwd` may prompt for the current password before asking for a new one (depending on context and security settings) -2. stdin buffering may cause the password to be read after the prompt is displayed -3. Some systems have `pam_chauthtok` requirements that need full interactive password change flow - -**Impact**: Password setting may fail silently or inconsistently across different Linux distributions and configurations. - -**Recommendation**: Consider alternatives: -1. Use `chpasswd` command which accepts `username:password` format via stdin: `echo "user:pass" | chpasswd` -2. Use `openssl` to generate a salted hash and directly edit `/etc/shadow` (requires careful implementation) -3. Document this limitation clearly and note it requires specific system configurations - ---- - -## 4. Fragile Date Parsing in `getExpiration` - -**Location**: `lib/user.js:319-354` - -**Problem**: The `getExpiration` function parses output from `chage --list` assuming a specific format: - -```javascript -data = { - changed: new Date(data[0].split(": ")[1]), - passwordExpires: data[1].split(": ")[1] === "never" ? null : new Date(data[1].split(": ")[1]), - // ... -}; -``` - -This parsing is fragile because: -1. It assumes English output with `: ` delimiter -2. System locale changes could localize date strings causing `new Date()` to fail -3. Different `chage` implementations may format output differently - -**Impact**: On non-English systems or systems with custom locale settings, this function will either throw an error or return invalid Date objects. - -**Recommendation**: Either: -1. Document this as a known limitation requiring English locale -2. Use regex patterns that extract the value between `: ` and the newline, then use a locale-aware date parser -3. Parse `/etc/shadow` directly for more reliable data extraction - ---- - -## 5. Potential Path Injection in `addSSHtoUser` - -**Location**: `lib/user.js:377-419` - -**Problem**: The `addSSHtoUser` function uses `exec` with string formatting for shell commands: - -```javascript -var commands = [ - util.format("mkdir %s/.ssh;", info.homedir), - util.format("touch %s/.ssh/authorized_keys;", info.homedir), - util.format("chown 700 %s/.ssh;", info.homedir), - // ... -].join(" "); -exec(commands, function (err, stdout, stderr) { ... }); -``` - -While `info.homedir` comes from `/etc/passwd` parsing (trusted on a properly secured system), if an attacker could modify `/etc/passwd` or if there's a misconfiguration, the homedir could contain shell metacharacters. - -**Impact**: Potential for command injection if homedir contains characters like `;`, `|`, `&`, etc. - -**Recommendation**: Validate homedir path before use: - -```javascript -if (!/^\/[a-zA-Z0-9_\/-]+$/.test(info.homedir)) { - return callback(new Error("Invalid homedir path")); -} -``` - -Or use spawn with argument arrays instead of shell execution. - ---- - -## 6. Commented-Out Code in `setPassword` - -**Location**: `lib/user.js:206-208` - -**Problem**: There is commented-out code suggesting a previous implementation approach: - -```javascript -// spawnWrapper('password', [username], password+'\n'+password+'\n', function(error, data){ -// console.log(error, data) -// }) -``` - -**Impact**: Dead code creates confusion about the intended implementation. This should either be removed or the approach should be documented. - -**Recommendation**: Remove the commented code or document why it was abandoned. - ---- - -## 7. `addUser` Always Calls `getUserInfo` on Error - -**Location**: `lib/user.js:128-131` - -**Problem**: In the `addUser` function: - -```javascript -spawnWrapper("useradd", [].concat(args_array, [args.username]), null, - function (error, data) { - if (error) callback(error); - exports.getUserInfo(args.username, callback); // Called even when error occurred! - } -); -``` - -When `useradd` fails, the error is passed to callback, but then `getUserInfo` is still called (and overwrites the error). If `useradd` fails, `getUserInfo` will return `null` as the user doesn't exist, and the actual error from `useradd` is lost. - -**Impact**: Error messages from `useradd` failures are lost or overwritten. - -**Recommendation**: Return early on error: - -```javascript -if (error) return callback(error); -exports.getUserInfo(args.username, callback); -``` - ---- - -## 8. Same Issue in `addGroup` - -**Location**: `lib/user.js:226-228` - -**Problem**: Same pattern as issue #7: - -```javascript -spawnWrapper("groupadd", [groupname], null, function () { - exports.getGroupInfo(groupname, callback); // No error handling! -}); -``` - -Errors from `groupadd` are silently ignored. The callback receives whatever `getGroupInfo` returns. - -**Impact**: Group creation failures are not reported to the caller. - -**Recommendation**: Add error handling: - -```javascript -spawnWrapper("groupadd", [groupname], null, function (error) { - if (error) return callback(error); - exports.getGroupInfo(groupname, callback); -}); -``` - ---- - -## 9. No Timeout Handling - -**Location**: All spawn-based functions - -**Problem**: None of the `spawn` or `exec` calls implement timeouts. A hung process (e.g., due to system configuration issues, LDAP/SSSD delays, etc.) could cause the library to hang indefinitely. - -**Impact**: Applications using this library may hang waiting for child processes that will never complete. - -**Recommendation**: Add a timeout mechanism: - -```javascript -function spawnWithTimeout(command, args, stdin, timeoutMs, callback) { - var timer; - var p = spawn(command, args); - - if (timeoutMs) { - timer = setTimeout(function() { - p.kill(); - callback(new Error("Process timed out")); - }, timeoutMs); - } - - // ... rest of spawnWrapper logic - _p.on("exit", function() { - if (timer) clearTimeout(timer); - callback(stderr, stdout); - }); -} -``` - ---- - -## 10. Documentation vs Implementation: `addUser` String Argument - -**Location**: `lib/user.js:109-116` and `README.md` - -**Problem**: When `addUser` receives a string, it converts it to an object: - -```javascript -if (typeof args === "string") { - args = { - username: args, - create_home: true, - }; -} -``` - -This is documented in the README (line 51 shows passing an object), but the string shortcut is a convenience that isn't clearly documented. - -**Impact**: Users may not know they can pass just a username string. - -**Recommendation**: Either document this shortcut or remove it for API clarity. - ---- - -## Summary Table - -| # | Issue | Severity | Location | -|---|-------|----------|----------| -| 1 | Inconsistent error handling in spawnWrapper | Medium | lib/user.js:26-30 | -| 2 | TypeScript type mismatch for selinux_user | Low | index.d.ts:11, lib/user.js:79-81 | -| 3 | setPassword stdin handling unreliable | Medium | lib/user.js:196-218 | -| 4 | Fragile date parsing in getExpiration | Medium | lib/user.js:319-354 | -| 5 | Potential path injection in addSSHtoUser | Medium | lib/user.js:387-398 | -| 6 | Commented-out dead code | Low | lib/user.js:206-208 | -| 7 | addUser loses error on failure | Medium | lib/user.js:128-131 | -| 8 | addGroup ignores errors | Medium | lib/user.js:226-228 | -| 9 | No timeout handling | Low | All spawn/exec calls | -| 10 | Undocumented string argument shortcut | Low | lib/user.js:109-116 | diff --git a/lib/promise.js b/lib/promise.js index 2c46e75..da13443 100644 --- a/lib/promise.js +++ b/lib/promise.js @@ -1,5 +1,10 @@ "use strict"; +// Guard against direct require of this internal file on non-Linux platforms. +if (process.platform !== "linux") { + throw new Error("linux-user must be running on Linux"); +} + // Functions that follow the (err, result) callback convention and can be // promisified. `validateUsername` is synchronous (no callback) and must NOT be // promisified -- util.promisify would inject a callback that is never invoked, diff --git a/lib/user.js b/lib/user.js index 5fefced..c279e6d 100644 --- a/lib/user.js +++ b/lib/user.js @@ -3,7 +3,7 @@ var spawn = require("child_process").spawn; var fs = require("fs"); -var validUsernameRegex = /^([a-z_][a-z0-9_-]{0,30})$/; +var validUsernameRegex = /^([a-z_][a-z0-9_-]{0,31})$/; // Default timeout applied to spawned commands so a hung child process // (e.g. LDAP/SSSD delays) cannot hang the library forever. Pass @@ -191,19 +191,7 @@ exports.removeUser = function (username, callback) { return callback(new Error("Invalid username")); } - var settled = false; - var _p = spawn("userdel", ["-rf", username]); - function finish(err) { - if (settled) { - return; - } - settled = true; - callback(err); - } - _p.on("error", finish); - _p.on("exit", function (code) { - finish(code === 0 ? null : new Error("userdel exited with code " + code)); - }); + spawnWrapper("userdel", ["-rf", username], null, callback); }; exports.getUserGroups = function (username, callback) { @@ -276,28 +264,14 @@ exports.setPassword = function (username, password, callback) { return callback(new Error("Invalid username")); } - var stderr = ""; - var settled = false; - var _p = spawn("passwd", [username]); - _p.stdin.write(password + "\n"); - _p.stdin.write(password + "\n"); - _p.stdin.end(); - _p.stderr.on("data", (data) => { - stderr += data.toString(); - }); - - function finish(err) { - if (settled) { - return; - } - settled = true; - callback(err); - } - - _p.on("error", finish); - _p.on("exit", function (code) { - finish(code === 0 ? null : new Error((stderr || "passwd failed").trim())); - }); + // Use chpasswd instead of writing to passwd's stdin. chpasswd is designed + // for non-interactive password updates and accepts "user:password" lines. + spawnWrapper( + "chpasswd", + [], + username + ":" + password + "\n", + callback + ); }; exports.addGroup = function (groupname, callback) { @@ -370,19 +344,7 @@ exports.addUserToGroup = function (username, groupname, callback) { return callback(new Error("Invalid arguments")); } - var settled = false; - var _p = spawn("usermod", ["-a", "-G", groupname, username]); - function finish(err) { - if (settled) { - return; - } - settled = true; - callback(err); - } - _p.on("error", finish); - _p.on("exit", function (code) { - finish(code === 0 ? null : new Error("usermod exited with code " + code)); - }); + spawnWrapper("usermod", ["-a", "-G", groupname, username], null, callback); }; var chage_arg_map = { @@ -537,11 +499,27 @@ exports.addSSHtoUser = function (user, key, callback) { return callback(err); } if (index >= steps.length) { - return fs.appendFile(keyFile, key + "\n", function (err) { - if (err) { - return callback(err); + return fs.readFile(keyFile, "utf8", function (readErr, existing) { + if (readErr && readErr.code !== "ENOENT") { + return callback(readErr); + } + + var normalizedKey = key.trim(); + if (existing) { + var lines = existing.split("\n"); + for (var i = 0; i < lines.length; i++) { + if (lines[i].trim() === normalizedKey) { + return callback(null, true); + } + } } - callback(null, true); + + fs.appendFile(keyFile, key + "\n", function (err) { + if (err) { + return callback(err); + } + callback(null, true); + }); }); } var step = steps[index++]; diff --git a/package.json b/package.json index 8c05dd3..3f27e12 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "linux-sys-user", - "version": "2.0.1", + "version": "2.0.2", "description": "Node.js module for Linux user and group management with zero dependencies", "main": "index.js", "types": "index.d.ts", diff --git a/test/user.test.js b/test/user.test.js index c21160a..a339bc2 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -34,6 +34,13 @@ describe("user.js", function () { it("should accept a valid username", function () { linuxUser.validateUsername("bob").should.be.true; }); + it("should accept a 32-character username", function () { + // Linux login names may be up to 32 characters long. + linuxUser.validateUsername("a1234567890123456789012345678901").should.be.true; + }); + it("should reject a 33-character username", function () { + linuxUser.validateUsername("a12345678901234567890123456789012").should.be.false; + }); }); describe("getUsers", function () { @@ -253,6 +260,16 @@ describe("user.js", function () { } ); }); + + it("should accept a username string shortcut", function (done) { + linuxUser.addUser(testUsername, function (err, user) { + if (err) { + return done(err); + } + user.username.should.equal(testUsername); + done(); + }); + }); }); describe("getGroups", function () { @@ -379,6 +396,7 @@ describe("user.js", function () { if (err) { return done(err); } + data.trim().should.equal(testSSHKeyGood); done(); } ); @@ -386,6 +404,48 @@ describe("user.js", function () { } ); }); + + it("should not add the same SSH key twice", function (done) { + linuxUser.addUser( + { username: testUsername, create_home: true }, + function (err, user) { + if (err) { + return done(err); + } + + linuxUser.addSSHtoUser(testUsername, testSSHKeyGood, function (err) { + if (err) { + return done(err); + } + + linuxUser.addSSHtoUser( + testUsername, + testSSHKeyGood, + function (err, success) { + if (err) { + return done(err); + } + success.should.be.true; + + fs.readFile( + user.homedir + "/.ssh/authorized_keys", + "utf8", + (err, data) => { + if (err) { + return done(err); + } + // File should contain exactly one copy of the key. + data.trim().should.equal(testSSHKeyGood); + data.trim().split("\n").length.should.equal(1); + done(); + } + ); + } + ); + }); + } + ); + }); }); });