3.2-fix users administrator and guest force zero quota and prevent delete #708

Open
wants to merge 2 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

on-jz commented Oct 21, 2013

This patch will prevent setting a quota value other than "0" for users "Administrator" and "Guest" showing an error message on "Users and Computers".

It will also prevent deletion of users "Administrator" and "Guest" showing an error message on "Users and Computers".

on-jz added some commits Oct 21, 2013

@on-jz on-jz Fixed Users 'Administrator' and 'Guest' - prevent deletion, force zero
quota with error message
94b400d
@on-jz on-jz Updated Changelog
89fe020
Contributor

jenkins-zentyal commented Oct 21, 2013

Can one of the admins verify this patch?

Contributor

papajulio commented Oct 21, 2013

Jenkins add to whitelist

@jacalvo jacalvo commented on the diff Oct 23, 2013

main/users/src/EBox/Users/User.pm
@@ -179,6 +185,16 @@ sub set
'advice' => __('User quota must be an integer. To set an unlimited quota, enter zero.'),
);
}
+
+ # Never set quota for 'Administrator' or 'Guest'
+ if ( ($uidNumber eq $AdministratorUidNumber) || ($uidNumber eq $GuestUidNumber) ) {
+ if ($value ne '0') {
+ throw EBox::Exceptions::InvalidData('data' => __('user quota'),
+ 'value' => $value,
+ 'advice' => __('User <b>' . $uid . '</b> requires unlimited quota. To set an unlimited quota, enter zero.'),
@jacalvo

jacalvo Oct 23, 2013

Owner

You can't translate variables, only literal strings, you should use this format:

__x("User {uid} requires...", uid => "$uid")

Also is not a good idea to include html code in the string to translate, so I would also move it to the substitution var.

@jacalvo jacalvo commented on the diff Oct 23, 2013

main/users/src/EBox/Users/User.pm
@@ -218,6 +234,22 @@ sub save
$usersMod->notifyModsLdapUserBase('modifyUser', [ $self, $passwd ], $self->{ignoreMods}, $self->{ignoreSlaves});
}
}
+
+ my $uidNumber = $self->get('uidNumber');
+ my $uid = $self->get('uid');
+
+ my $AdministratorUidNumber = 0;
+ my $GuestUidNumber = 65534;
+
+ if ($changetype eq 'delete') {
+ # Prevent deletion of 'Administrator' and 'Guest'
+ if ( ($uidNumber eq $AdministratorUidNumber) || ($uidNumber eq $GuestUidNumber) ) {
+ throw EBox::Exceptions::InvalidData('data' => __('user quota'),
+ 'advice' => __('User <b>' . $uid . '</b> is a System User and cannot be deleted.'),
@jacalvo

jacalvo Oct 23, 2013

Owner

same as above

@jacalvo jacalvo commented on the diff Oct 23, 2013

main/users/src/EBox/Users/CGI/DeleteUser.pm
my $dn = $self->unsafeParam('dn');
my $user = new EBox::Users::User(dn => $dn);
+ # Prevent deletion of users Administrator, Guest
+ my $samba = EBox::Global->modInstance('samba');
+ my $sid = undef;
+ if (defined ($samba)) {
+ my $object = $samba->ldbObjectFromLDAPObject($user);
+ if (defined ($object) and ($object->sid() =~ /^S-1-5-21-\d+-\d+-\d+-500$/)) {
@jacalvo

jacalvo Oct 23, 2013

Owner

Would be nice to have a comment about 'Administrator' and 'Guest' in that code, or just use a constant or a variable with auto-descriptive name like my $administratorSidRegex = '^S-1-5-21...';

Owner

jacalvo commented Oct 23, 2013

Thank you very much for this contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment