Bug 353931 - Review Request: xguest - kiosk user setup program
Review Request: xguest - kiosk user setup program
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tomas Mraz
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-26 08:13 EDT by Daniel Walsh
Modified: 2008-09-22 23:30 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-09-22 23:30:47 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tmraz: fedora‑review+
tcallawa: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Daniel Walsh 2007-10-26 08:13:52 EDT
Spec URL: http://people.fedoraproject.org/~dwalsh/xguest/xguest.spec
SRPM URL: ttp://people.fedoraproject.org/~dwalsh/xguest/xguest-1.0.2-1.fc8.src.rpm
Description: xguest is a package that sets up a locked down user for use in
kiosk systems.  The user will be controlled so that all they can use is Firefox
for reaching the internet.
Comment 1 Daniel Walsh 2007-10-26 08:17:30 EDT
SRPM URL: http://people.fedoraproject.org/~dwalsh/xguest/xguest-1.0.2-1.fc8.src.rpm

First off I would like to get this review as fast as possible, to get inclusion
in Fedora 8.  Maybe I am too late already.

The spec file does a couple of things that cause rpmlint to give errors.  These
are discussed at:

https://bugzilla.redhat.com/show_bug.cgi?id=353321

This package could be used in conjunction with Fast User Switching to allow
users to share the machines with others with little risk, as well as in kiosk
environments.
Comment 2 Parag AN(पराग) 2007-10-26 09:46:32 EDT
good to add license file as %doc
whats use of calling "exit 0" at end of each section?
Comment 3 Daniel Walsh 2007-10-26 10:52:22 EDT
Added and removed exit 0's

SRPM URL: http://people.fedoraproject.org/~dwalsh/xguest/xguest-1.0.3-1.fc8.src.rpm
Comment 4 manuel wolfshant 2007-10-26 20:02:46 EDT
Maybe I am just picky, but I am very much against creating files in %post. I
feel that %source1 sepermit.conf, with the appropriate content, copied in %post
to the right place would be cleaner and easier to manage/verify via rpm -qV
Comment 5 Daniel Walsh 2007-10-27 07:16:36 EDT
There are no files being created in post.  sepermit.conf is owned by pam and
xguest is just appending "xguest" to the file.
Comment 6 Daniel Walsh 2007-11-20 06:26:04 EST
So what is the state of this? 

How do we move forward?
Comment 7 Tomas Mraz 2007-12-17 16:50:22 EST
rpmlint -v ../SRPMS/xguest-1.0.5-2.fc8.src.rpm xguest.src: I: checking

rpmlint -v ../RPMS/noarch/xguest-1.0.5-2.fc9.noarch.rpm 
xguest.noarch: I: checking
xguest.noarch: E: use-tmp-in-%post
xguest.noarch: E: use-of-home-in-%post
These two aren't actually a use of it but adding the namespace configuration to
namespace.conf. As soon as pam_namespace will support config files in
namespace.d this should be changed. Perhaps you should save a backup of the
existing namespace.conf so it will be possible to restore it after the future
change. Of course the backup should be owned by the package as %ghost.

xguest.noarch: E: preun-without-chkconfig /etc/rc.d/init.d/xguest
That's a real error and it should be fixed. chkconfig --del should be run for
xguest.

xguest.noarch: W: service-default-enabled /etc/rc.d/init.d/xguest
xguest.noarch: E: no-status-entry /etc/rc.d/init.d/xguest
xguest.noarch: W: no-reload-entry /etc/rc.d/init.d/xguest
I think these are OK. Status and reload entries do not make much sense as xguest
is not a daemon but script containing just some bind mounts. Whether it should
be enabled by default or not is debatable but as the %post script creates the
xguest user account I think that enabling the polyinstantiation to work for him
without further admins actions (except reboot or start of the script for the
first time) is fine.

xguest.noarch: W: uncompressed-zip /etc/desktop-profiles/xguest.zip
That's OK as it is only 177kB anyway.

Comment about the wording of Summary and Description:
It should be describing what the package does so IMO it should be more like:
Summary: Creates xguest user as a locked down user
%description
Installing this package sets up the xguest user to be used as a temporary
account to switch to or as a kiosk user account. The account is disabled unless
SELinux is in enforcing mode. The user is only allowed to log in via gdm.
The home and temporary directories of the user will be polyinstantiated and
mounted on tmpfs.

More notes:
The URL: http://people.fedoraproject.org/~dwalsh/xguest/%{name}-%{version}.tar.bz2
points to nonexistent file.
There is no %defattr(...) on the beginning of %files.
Comment 8 Daniel Walsh 2007-12-17 17:36:16 EST
SRPM URL: 
http://people.fedoraproject.org/~dwalsh/xguest/xguest-1.0.6-1.fc9.src.rpm

Has been updated with all your suggested fixes.
Comment 9 Tomas Mraz 2007-12-18 03:43:36 EST
rpmlint -v ../SRPMS/xguest-1.0.6-1.fc8.src.rpm 
xguest.src: I: checking

rpmlint -v ../RPMS/noarch/xguest-1.0.6-1.fc8.noarch.rpm 
xguest.noarch: I: checking
xguest.noarch: E: use-tmp-in-%post
xguest.noarch: E: use-of-home-in-%post
xguest.noarch: W: service-default-enabled /etc/rc.d/init.d/xguest
xguest.noarch: W: no-reload-entry /etc/rc.d/init.d/xguest
xguest.noarch: W: uncompressed-zip /etc/desktop-profiles/xguest.zip
All these warnings and errors are OK as I wrote in comment #7.

The suggested fixes were applied.

Package is ACCEPTed.
Comment 10 Tomas Mraz 2007-12-18 03:56:49 EST
One more suggestion - when the namespace.conf file is modified I'd suggest to add
a comment line before and after the xguest configuration so it will be easy to
remove it as soon as the configuration is moved to an extra file in the future
namespace.d. And in the %preun you can already remove these lines between the
comment lines.
Comment 11 Daniel Walsh 2007-12-18 13:26:21 EST
SRPM URL: 
http://people.fedoraproject.org/~dwalsh/xguest/xguest-1.0.6-2.fc9.src.rpm

Has been updated with all your suggested fixes.

Spec file now deletes the namespace.conf entry on removal.

Comment 12 Tom "spot" Callaway 2007-12-19 15:38:20 EST
Dan, the CVS process is here:

http://fedoraproject.org/wiki/PackageMaintainers/CVSAdminProcedure

Please append a New Package CVS Request to this ticket and flip the flag back to ?.
Comment 13 Daniel Walsh 2007-12-20 13:49:35 EST
New Package CVS Request
=======================
Package Name: dwalsh
Short Description: SELinux Kiosk User account setup program
Owners: dwalsh@redhat.com
Branches: F-8 Rawhide
InitialCC: sgrubb@redhat.com
Cvsextras Commits: Yes
Comment 14 Daniel Walsh 2007-12-20 13:51:16 EST
New Package CVS Request
=======================
Package Name: dwalsh
Short Description: SELinux Kiosk User account setup program
Owners: dwalsh
Branches: F-8 Rawhide
InitialCC: sgrubb
Cvsextras Commits: No
Comment 15 Tom "spot" Callaway 2007-12-20 14:05:31 EST
Umm, Dan? Is the package name really dwalsh?

I almost committed that to cvs. :)

Also, you don't need to list Rawhide as a branch, you always get that one.

cvs is done.
Comment 16 Daniel Walsh 2007-12-21 02:52:29 EST
Package Name: xguest
Comment 17 Matt Domsch 2008-09-22 23:30:47 EDT
package is in the repos now, closing.

Note You need to log in before you can comment on or make changes to this bug.