Skip to content

Fix: Resolve multi-user write permission issues in Samba server#1

Merged
rahulkatiyar19955 merged 2 commits intofiledash:mainfrom
abhaysingh-auraml:Feature/Updated-mutilple-user-permission
Aug 6, 2025
Merged

Fix: Resolve multi-user write permission issues in Samba server#1
rahulkatiyar19955 merged 2 commits intofiledash:mainfrom
abhaysingh-auraml:Feature/Updated-mutilple-user-permission

Conversation

@abhaysingh-auraml
Copy link
Contributor

Fix: Resolve multi-user write permission issues in Samba server

Problem:

Only the first connected user could write to shares
All directories were owned by the same UID (1000:1000)
Subsequent users were denied write access
Root Cause:

start_samba.sh was overriding per-user ownership with HOST_UID:HOST_GID
Missing force user/group configuration in share definitions
Users.conf had example users instead of actual users
Solution:

Removed problematic chown -R $HOST_UID:$HOST_GID /mount/storage from start_samba.sh
Added proper per-user directory ownership logic in init_users.sh
Updated users.conf with real users (abhay, auraml, neuralzome, etc.)
Added force user/group settings to ensure proper file ownership
Each user now gets unique UID (2000+) with correct directory ownership
Result:

All users can now write to their respective directories simultaneously
Proper access control: users cannot access each other's directories
Admin user maintains access to all directories
File ownership correctly reflects the creating user

- Remove conflicting ownership override in start_samba.sh
- Add proper per-user directory ownership in init_users.sh
- Update users.conf with actual users and force user/group settings
- Enable concurrent write access for all users to their respective directories
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes multi-user write permission issues in a Samba server by removing problematic global ownership commands and implementing per-user directory ownership. The changes ensure that multiple users can write to their respective shares simultaneously without interfering with each other.

  • Removed global ownership override that was causing all directories to be owned by the same UID
  • Added per-user directory ownership logic to properly set ownership for each user's share
  • Enhanced user initialization to support proper access control between users

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
start_samba.sh Removed global chown command that was overriding per-user ownership
init_users.sh Added logic to set proper ownership for each user's individual directory

done < <(grep -E '^[^#\[].*:.*' /etc/samba/users.conf)

# Set proper ownership for each user's directory
echo "Setting proper ownership for user directories..."
Copy link

Copilot AI Aug 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The while loop reads from /etc/samba/users.conf without proper error handling. If the file doesn't exist or is empty, the loop will fail silently. Consider adding a check to ensure the file exists before processing.

Suggested change
echo "Setting proper ownership for user directories..."
echo "Setting proper ownership for user directories..."
if [ ! -s /etc/samba/users.conf ]; then
echo "Error: /etc/samba/users.conf does not exist or is empty."
exit 1
fi

Copilot uses AI. Check for mistakes.
Comment on lines +59 to +71
while read -r line; do
# Look for share section headers [sharename]
if [[ $line =~ ^\[([^]]+)\]$ ]]; then
share_name="${BASH_REMATCH[1]}"
share_dir="/mount/storage/$share_name"

if [ -d "$share_dir" ] && id "$share_name" &>/dev/null; then
echo "Setting ownership for $share_dir to $share_name:$share_name"
chown -R "$share_name:$share_name" "$share_dir"
chmod -R 775 "$share_dir"
fi
fi
done < /etc/samba/users.conf
Copy link

Copilot AI Aug 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regex pattern assumes share names in brackets correspond to usernames, but this may not always be true in Samba configurations. Share names and usernames can be different. Consider verifying that the share_name is actually a valid user before proceeding.

Suggested change
while read -r line; do
# Look for share section headers [sharename]
if [[ $line =~ ^\[([^]]+)\]$ ]]; then
share_name="${BASH_REMATCH[1]}"
share_dir="/mount/storage/$share_name"
if [ -d "$share_dir" ] && id "$share_name" &>/dev/null; then
echo "Setting ownership for $share_dir to $share_name:$share_name"
chown -R "$share_name:$share_name" "$share_dir"
chmod -R 775 "$share_dir"
fi
fi
done < /etc/samba/users.conf
# Parse users.conf to map shares to valid users and set ownership accordingly
current_share=""
valid_user=""
while IFS= read -r line; do
# Look for share section headers [sharename]
if [[ $line =~ ^\[([^]]+)\]$ ]]; then
# If we have a previous share and valid user, set ownership
if [[ -n "$current_share" && -n "$valid_user" ]]; then
share_dir="/mount/storage/$current_share"
if [ -d "$share_dir" ] && id "$valid_user" &>/dev/null; then
echo "Setting ownership for $share_dir to $valid_user:$valid_user"
chown -R "$valid_user:$valid_user" "$share_dir"
chmod -R 775 "$share_dir"
fi
fi
current_share="${BASH_REMATCH[1]}"
valid_user=""
elif [[ $line =~ ^[[:space:]]*valid[[:space:]]+users[[:space:]]*=[[:space:]]*([^[:space:]]+) ]]; then
# Extract the first username from the valid users line
valid_user="${BASH_REMATCH[1]}"
fi
done < /etc/samba/users.conf
# After the loop, handle the last share section
if [[ -n "$current_share" && -n "$valid_user" ]]; then
share_dir="/mount/storage/$current_share"
if [ -d "$share_dir" ] && id "$valid_user" &>/dev/null; then
echo "Setting ownership for $share_dir to $valid_user:$valid_user"
chown -R "$valid_user:$valid_user" "$share_dir"
chmod -R 775 "$share_dir"
fi
fi

Copilot uses AI. Check for mistakes.
share_name="${BASH_REMATCH[1]}"
share_dir="/mount/storage/$share_name"

if [ -d "$share_dir" ] && id "$share_name" &>/dev/null; then
Copy link

Copilot AI Aug 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition checks if the user exists but doesn't validate if the user was actually created in the previous loop. This could lead to inconsistent behavior if users.conf contains share sections that don't correspond to users defined in the same file.

Suggested change
if [ -d "$share_dir" ] && id "$share_name" &>/dev/null; then
# Only set ownership if share_name is in the created users list
if [ -d "$share_dir" ] && grep -qx "$share_name" "$created_users_tmp"; then

Copilot uses AI. Check for mistakes.
@rahulkatiyar19955 rahulkatiyar19955 merged commit 5f2503b into filedash:main Aug 6, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants