From eb5dc014179415a0e5476e986519ac96c36221f9 Mon Sep 17 00:00:00 2001 Message-Id: From: Mark Wooding Date: Fri, 21 Dec 2007 12:06:20 +0000 Subject: [PATCH] rights now apply to commands; docs catch up a bit Organization: Straylight/Edgeware From: Richard Kettlewell --- clients/disorder.c | 10 ++ doc/disorder_config.5.in | 138 ++++++++++------- doc/disorder_protocol.5.in | 83 ++++++++--- lib/cookies.c | 13 +- lib/cookies.h | 2 +- lib/rights.c | 3 +- lib/rights.h | 12 +- lib/trackdb.c | 5 - python/disorder.py.in | 2 +- scripts/completion.bash | 2 +- server/server.c | 297 ++++++++++++++++++++++--------------- tests/dtest.py | 6 +- tests/user.py | 2 + 13 files changed, 366 insertions(+), 209 deletions(-) diff --git a/clients/disorder.c b/clients/disorder.c index 9402340..215ec0c 100644 --- a/clients/disorder.c +++ b/clients/disorder.c @@ -427,6 +427,14 @@ static void cf_adduser(disorder_client *c, exit(EXIT_FAILURE); } +static void cf_edituser(disorder_client *c, + char **argv) { + if(disorder_edituser(c, argv[0], argv[1], argv[2])) + exit(EXIT_FAILURE); +} + +/* TODO: userinfo */ + static const struct command { const char *name; int min, max; @@ -448,6 +456,8 @@ static const struct command { "Disable play" }, { "disable-random", 0, 0, cf_random_disable, 0, "", "Disable random play" }, + { "edituser", 3, 3, cf_edituser, 0, "USER KEY VALUE", + "Set a property of a user" }, { "enable", 0, 0, cf_enable, 0, "", "Enable play" }, { "enable-random", 0, 0, cf_random_enable, 0, "", diff --git a/doc/disorder_config.5.in b/doc/disorder_config.5.in index 6eb57b8..a90d029 100644 --- a/doc/disorder_config.5.in +++ b/doc/disorder_config.5.in @@ -52,10 +52,70 @@ DisOrder distinguishes between multiple users. This is for access control and reporting, not to provide different views of the world: i.e. preferences and so on are global. .PP -It's possible to restrict a small number of operations to a specific subset of -users. However, it is assumed that every user is supposed to be able to do -most operations - since the users are all sharing the same audio environment -they are expected to cooperate with each other. +Each user has an associated set of rights which contorl which commands they may +execute. Normally you would give all users most rights, and expect them to +cooperate (they are after all presumed to be in a shared sound environment). +.PP +The full set of rights are: +.TP +.B read +User can perform read-only operations +.TP +.B play +User can add tracks to the queue +.TP +.B "move any" +User can move any track +.TP +.B "move mine" +User can move their own tracks +.TP +.B "move random" +User can move randomly chosen tracks +.TP +.B "remove any" +User can remove any track +.TP +.B "remove mine" +User can remove their own tracks +.TP +.B "remove random" +User can remove randomly chosen tracks +.TP +.B "scratch any" +User can scratch any track +.TP +.B "scratch mine" +User can scratch their own tracks +.TP +.B "scratch random" +User can scratch randomly chosen tracks +.TP +.B volume +User can change the volume +.TP +.B admin +User can perform admin operations +.TP +.B rescan +User can initiate a rescan +.TP +.B register +User can register new users. Normally only the +.B guest +user would have this right. +.TP +.B userinfo +User can edit their own userinfo +.TP +.B prefs +User can modify track preferences +.TP +.B "global prefs" +User can modify global preferences +.TP +.B pause +User can pause/resume .PP Access control is entirely used-based. If you configure DisOrder to listen for TCP/IP connections then it will accept a connection from anywhere provided the @@ -133,6 +193,24 @@ it re-read it. If there is anything wrong with it the daemon will record a log message and ignore the new config file. (You should fix it before next terminating and restarting the daemon, as it cannot start up without a valid config file.) +.SS "Configuration Files" +Configuration files are read in the following order: +.TP +.I pkgconfdir/config +.TP +.I pkgconfdir/config.private +Should be readable only by the jukebox group. Not really useful any more and +may be abolished in future. +.TP +.I pkgconfdir/config.\fRUSER +Per-user system-controlled client configuration. Optional but if it +exists must be readable only by the relevant user. Would normally +contain a \fBpassword\fR directive. +.TP +.I ~\fRUSER\fI/.disorder/passwd +Per-user client configuration. Optional but if it exists must be +readable only by the relevant user. Would normally contain a +\fBpassword\fR directive. .SS "Global Configuration" .TP .B home \fIDIRECTORY\fR @@ -383,20 +461,6 @@ to 3600, i.e. one hour. The target size of the queue. If random play is enabled then randomly picked tracks will be added until the queue is at least this big. The default is 10. .TP -.B restrict \fR[\fBscratch\fR] [\fBremove\fR] [\fBmove\fR] -Determine which operations are restricted to the submitter of a -track. By default, no operations are restricted, i.e. anyone can -scratch or remove anything. -.IP -If \fBrestrict scratch\fR or \fBrestrict remove\fR are set then only the user -that submitted a track can scratch or remove it, respectively. -.IP -If \fBrestrict move\fR is set then only trusted users can move tracks around in -the queue. -.IP -If \fBrestrict\fR is used more than once then only the final use has any -effect. -.TP .B sample_format \fIBITS\fB/\fIRATE\fB/\fICHANNELS Describes the sample format expected by the \fBspeaker_command\fR (below). The components of the format specification are as follows: @@ -505,6 +569,10 @@ Specifies the module used to calculate the length of files matching .IP If \fBtracklength\fR is used without arguments then the list of modules is cleared. +.TP +.B user \fIUSER\fR +Specifies the user to run as. Only makes sense if invoked as root (or +the target user). .SS "Client Configuration" .TP .B connect \fIHOST SERVICE\fR @@ -564,45 +632,15 @@ generated web pages. This must be the full URL, e.g. \fBhttp://myhost/cgi-bin/jukebox\fR and not \fB/cgi-bin/jukebox\fR. .SS "Authentication Configuration" +These options would normally be used in \fI~\fRUSER\fI/.disorder/passwd\fR or +\fIpkgconfdir/config.\fRUSER. .TP .B password \fIPASSWORD\fR Specify password. .TP -.B trust \fIUSERNAME\fR -Allow \fIUSERNAME\fR to perform privileged operations such as shutting -down or reconfiguring the daemon, or becoming another user. -.IP -If \fBtrust\fR is used without arguments then the list of trusted users is -cleared. -.TP -.B user \fIUSER\fR -Specifies the user to run as. Only makes sense if invoked as root (or -the target user). -.TP .B username \fIUSERNAME\fR Specify username. The default is taken from the environment variable \fBLOGNAME\fR. -.PP -Configuration files are read in the following order: -.TP -.I pkgconfdir/config -.TP -.I pkgconfdir/config.private -Should be readable only by the jukebox group, and contain \fBallow\fR -commands for authorised users. -.IP -If this file does not exist at startup then the server will create it with a -randomly chosen password for the root user. -.TP -.I pkgconfdir/config.\fRUSER -Per-user system-controlled client configuration. Optional but if it -exists must be readable only by the relevant user. Would normally -contain a \fBpassword\fR directive. -.TP -.I ~\fRUSER\fI/.disorder/passwd -Per-user client configuration. Optional but if it exists must be -readable only by the relevant user. Would normally contain a -\fBpassword\fR directive. .SH "GLOBAL PREFERENCES" These are the values set with \fBset-global\fR. .TP diff --git a/doc/disorder_protocol.5.in b/doc/disorder_protocol.5.in index a9d973d..7fdb819 100644 --- a/doc/disorder_protocol.5.in +++ b/doc/disorder_protocol.5.in @@ -45,17 +45,25 @@ always have a 3-digit response code as the first field. See below for more details about this field. .PP All commands require the connection to have been already authenticated unless -stated otherwise. +stated otherwise. If not stated otherwise, the \fBread\fR right is sufficient +to execute the command. .PP Neither commands nor responses have a body unless stated otherwise. .TP +.B adduser \fIUSERNAME PASSWORD +Creates a new user with the given username and password. Requires the +\fBadmin\fR right, and only works on local connections. +.TP .B allfiles \fIDIRECTORY\fR [\fIREGEXP\fR] Lists all the files and directories in \fIDIRECTORY\fR in a response body. If \fIREGEXP\fR is present only matching files and directories are returned. .TP -.B become \fIUSER\fR -Instructs the server to treat the connection as if \fIUSER\fR had -authenticated it. Only trusted users may issue this command. +.B cookie \fICOOKIE +Log a user back in using a cookie created with \fBmake-cookie\fR. +.TP +.B deluser \fIUSERNAME +Deletes the named user. Requires the \fBadmin\fR right, and only works on +local connections. .TP .B dirs \fIDIRECTORY\fR [\fIREGEXP\fR] Lists all the directories in \fIDIRECTORY\fR in a response body. @@ -63,10 +71,16 @@ If \fIREGEXP\fR is present only matching directories are returned. .TP .B disable \fR[\fBnow\fR] Disables further playing. If the optional \fBnow\fR argument is present then -the current track is stopped. +the current track is stopped. Requires the \fBglobal prefs\fR right. +.TP +.B edituser \fIUSERNAME PROPERTY VALUE +Sets a user property. With the \fBadmin\fR right any username and property may +be specified. Otherwise the \fBuserinfo\fR right is required and only the +\fBemail\fR and \fBpassword\fR properties may be set. .TP .B enable -Re-enables further playing, and is the opposite of \fBdisable\fR. +Re-enables further playing, and is the opposite of \fBdisable\fR. Requires the +\fBglobal prefs\fR right. .TP .B enabled Reports whether playing is enabled. The second field of the response line will @@ -102,11 +116,18 @@ not accumulate in a buffer somewhere). .IP See \fBEVENT LOG\fR below for more details. .TP +.B make-cookie +Returns an opaque string that can be used by the \fBcookie\fR command to log +this user back in on another connection (until the cookie expires). +.TP .B move \fITRACK\fR \fIDELTA\fR Move a track in the queue. The track may be identified by ID (preferred) or name (which might cause confusion if it's there twice). \fIDELTA\fR should be an negative or positive integer and indicates how many steps towards the head of the queue the track should be moved. +.IP +Requires one of the \fBmove mine\fR, \fBmove random\fR or \fBmove any\fR rights +depending on how the track came to be added to the queue. .TP .B moveafter \fITARGET\fR \fIID\fR ... Move all the tracks in the \fIID\fR list after ID \fITARGET\fR. If @@ -114,6 +135,9 @@ Move all the tracks in the \fIID\fR list after ID \fITARGET\fR. If the queue. If \fITARGET\fR is listed in the ID list then the tracks are moved to just after the first non-listed track before it, or to the head if there is no such track. +.IP +Requires one of the \fBmove mine\fR, \fBmove random\fR or \fBmove any\fR rights +depending on how the tracks came to be added to the queue. .TP .B new \fR[\fIMAX\fR] Sends the most recently added \fIMAX\fR tracks in a response body. If the @@ -122,7 +146,7 @@ argument is ommitted, all recently added tracks are listed. .B nop Do nothing. Used by .BR disobedience (1) -as a keepalive measure. +as a keepalive measure. This command does not require authentication. .TP .B part \fITRACK\fR \fICONTEXT\fI \fIPART\fR Get a track name part. Returns an empty string if a name part cannot be @@ -142,10 +166,11 @@ or .BR title . .TP .B pause -Pause the current track. +Pause the current track. Requires the \fBpause\R right. .TP .B play \fITRACK\fR Add a track to the queue. The response contains the queue ID of the track. +Requires the \fBplay\fR right. .TP .B playing Reports what track is playing. @@ -166,10 +191,11 @@ at the head of the queue (i.e. next to be be played) first. See below for the track information syntax. .TP .B random-disable -Disable random play (but don't stop the current track). +Disable random play (but don't stop the current track). Requires the \fBglobal +prefs\fR right. .TP .B random-enable -Enable random play. +Enable random play. Requires the \fBglobal prefs\fR right. .TP .B random-enabled Reports whether random play is enabled. The second field of the response line @@ -181,31 +207,37 @@ line, the track most recently played last. See below for the track information syntax. .TP .B reconfigure -Request that DisOrder reconfigure itself. Only trusted users may issue this +Request that DisOrder reconfigure itself. Requires the \fBadmin\fR right. command. .TP .B remove \fIID\fR -Remove the track identified by \fIID\fR. If \fBrestrict remove\fR is enabled -in the server's configuration then only the user that submitted the track may -remove it. +Remove the track identified by \fIID\fR. Requires one of the \fBremove +mine\fR, \fBremove random\fR or \fBremove any\fR rights depending on how the +track came to be added to the queue. .TP .B rescan -Rescan all roots for new or obsolete tracks. +Rescan all roots for new or obsolete tracks. Requires the \fBrescan\fR right. .TP .B resolve \fITRACK\fR Resolve a track name, i.e. if this is an alias then return the real track name. .TP .B resume -Resume the current track after a \fBpause\fR command. +Resume the current track after a \fBpause\fR command. Requires the \fBpause\fR +right. +.TP +.B revoke \fBcookie\fR +Revokes a cookie previously created with \fBmake-cookie\fR. It will not be +possible to use this cookie in the future. .TP .B rtp-address Reports the RTP broadcast (or multicast) address, in the form \fIADDRESS -PORT\fR. +PORT\fR. This command does not require authentication. .TP .B scratch \fR[\fIID\fR] Remove the track identified by \fIID\fR, or the currently playing track if no -\fIID\fR is specified. If \fBrestrict scratch\fR is enabled in the server's -configuration then only the user that submitted the track may scratch it. +\fIID\fR is specified. Requires one of the \fBscratch mine\fR, \fBscratch +random\fR or \fBscratch any\fR rights depending on how the track came to be +added to the queue. .TP .B search \fITERMS\fR Search for tracks matching the search terms. The results are put in a response @@ -224,10 +256,10 @@ Spaces in terms don't currently make sense, but may one day be interpreted to allow searching for phrases. .TP .B \fBset\fR \fITRACK\fR \fIPREF\fR \fIVALUE\fR -Set a preference. +Set a preference. Requires the \fBprefs\fR right. .TP .B set-global \fIKEY\fR \fIVALUE\fR -Set a global preference. +Set a global preference. Requires the \fBglobal prefs\fR right. .TP .B stats Send server statistics in plain text in a response body. @@ -236,16 +268,19 @@ Send server statistics in plain text in a response body. Send the list of currently known tags in a response body. .TP .B \fBunset\fR \fITRACK\fR \fIPREF\fR -Unset a preference. +Unset a preference. Requires the \fBprefs\fR right. .TP .B \fBunset-global\fR \fIKEY\fR -Unset a global preference. +Unset a global preference. Requires the \fBglobal prefs\fR right. .TP .B user \fIUSER\fR \fIRESPONSE\fR Authenticate as \fIUSER\fR. See .B AUTHENTICATION below. .TP +.B users +Sends the list of currently known users in a response body. +.TP .B version Send back a response with the server version as the second field. .TP @@ -256,7 +291,7 @@ With zero parameters just gets the volume and reports the left and right sides as the 2nd and 3rd fields of the response. .IP With one parameter sets both sides to the same value. With two parameters sets -each side independently. +each side independently. Setting the volume requires the \fBvolume\fR right. .SH RESPONSES Responses are three-digit codes. The first digit distinguishes errors from succesful responses: diff --git a/lib/cookies.c b/lib/cookies.c index cce6b1b..585fd7e 100644 --- a/lib/cookies.c +++ b/lib/cookies.c @@ -32,6 +32,7 @@ #include #include +#include "rights.h" #include "cookies.h" #include "hash.h" #include "mem.h" @@ -148,14 +149,16 @@ char *make_cookie(const char *user) { /** @brief Verify a cookie * @param cookie Cookie to verify + * @param rights Where to store rights value * @return Verified user or NULL */ -char *verify_cookie(const char *cookie) { +char *verify_cookie(const char *cookie, rights_type *rights) { char *c1, *c2; intmax_t t; time_t now; char *user, *bp, *sig; const char *password; + struct kvp *k; /* check the revocation list */ if(revoked && hash_find(revoked, cookie)) { @@ -188,11 +191,15 @@ char *verify_cookie(const char *cookie) { return 0; } /* look up the password */ - password = trackdb_get_password(user); - if(!password) { + k = trackdb_getuserinfo(user); + if(!k) { error(0, "verify_cookie for nonexistent user"); return 0; } + password = kvp_get(k, "password"); + if(!password) password = ""; + if(parse_rights(kvp_get(k, "rights"), rights)) + return 0; /* construct the expected subject. We re-encode the timestamp and the * password. */ byte_xasprintf(&bp, "%jx;%s;%s", t, urlencodestring(user), password); diff --git a/lib/cookies.h b/lib/cookies.h index f7e3a3a..f02013d 100644 --- a/lib/cookies.h +++ b/lib/cookies.h @@ -25,7 +25,7 @@ #define COOKIES_H char *make_cookie(const char *user); -char *verify_cookie(const char *cookie); +char *verify_cookie(const char *cookie, rights_type *rights); void revoke_cookie(const char *cookie); #endif /* COOKIES_H */ diff --git a/lib/rights.c b/lib/rights.c index 8d84743..5a9935b 100644 --- a/lib/rights.c +++ b/lib/rights.c @@ -53,7 +53,8 @@ static const struct { { RIGHT_REGISTER, "register" }, { RIGHT_USERINFO, "userinfo" }, { RIGHT_PREFS, "prefs" }, - { RIGHT_GLOBAL_PREFS, "global prefs" } + { RIGHT_GLOBAL_PREFS, "global prefs" }, + { RIGHT_PAUSE, "pause" } }; #define NRIGHTS (sizeof rights_names / sizeof *rights_names) diff --git a/lib/rights.h b/lib/rights.h index f4bf363..6192772 100644 --- a/lib/rights.h +++ b/lib/rights.h @@ -84,8 +84,18 @@ /** @brief User can modify global preferences */ #define RIGHT_GLOBAL_PREFS 0x00020000 +/** @brief User can pause/resume */ +#define RIGHT_PAUSE 0x00040000 + /** @brief Current rights mask */ -#define RIGHTS__MASK 0x0003ffff +#define RIGHTS__MASK 0x0007ffff + +/** @brief Connection is local + * + * This isn't a rights bit, it's used in @file server.c to limit + * certain commands to local connections. + */ +#define RIGHT__LOCAL 0x80000000 /** @brief Unsigned type big enough for rights */ typedef uint32_t rights_type; diff --git a/lib/trackdb.c b/lib/trackdb.c index c57dab2..2aed9a4 100644 --- a/lib/trackdb.c +++ b/lib/trackdb.c @@ -2514,9 +2514,6 @@ void trackdb_create_root(void) { * Only works if running as a user that can read the database! * * If the user exists but has no password, "" is returned. - * - * If the user was created with 'register' and has not yet been confirmed then - * NULL is still returned. */ const char *trackdb_get_password(const char *user) { int e; @@ -2526,8 +2523,6 @@ const char *trackdb_get_password(const char *user) { WITH_TRANSACTION(trackdb_getdata(trackdb_usersdb, user, &k, tid)); if(e) return 0; - if(kvp_get(k, "confirmation")) - return 0; password = kvp_get(k, "password"); return password ? password : ""; } diff --git a/python/disorder.py.in b/python/disorder.py.in index 9a6b385..4e9b8cd 100644 --- a/python/disorder.py.in +++ b/python/disorder.py.in @@ -854,7 +854,7 @@ class client: def make_cookie(self): """Create a login cookie""" ret, details = self._simple("make-cookie") - return details + return _split(details)[0] def revoke(self): """Revoke a login cookie""" diff --git a/scripts/completion.bash b/scripts/completion.bash index 437165c..f9b823a 100644 --- a/scripts/completion.bash +++ b/scripts/completion.bash @@ -31,7 +31,7 @@ complete -o default \ random-enable recent reconfigure remove rescan scratch search set set-volume shutdown stats unset version resolve part pause resume scratch-id get-global set-global unset-global - tags new rtp-address adduser users + tags new rtp-address adduser users edituser -h --help -H --help-commands --version -V --config -c --length --debug -d" \ disorder diff --git a/server/server.c b/server/server.c index 6392e89..ef32151 100644 --- a/server/server.c +++ b/server/server.c @@ -111,6 +111,8 @@ struct conn { const struct listener *l; /** @brief Login cookie or NULL */ char *cookie; + /** @brief Connection rights */ + rights_type rights; }; static int reader_callback(ev_source *ev, @@ -169,16 +171,6 @@ static int reader_error(ev_source attribute((unused)) *ev, return 0; } -/** @brief Return true if we are talking to a trusted user */ -static int trusted(struct conn *c) { - int n; - - for(n = 0; (n < config->trust.n - && strcmp(config->trust.s[n], c->who)); ++n) - ; - return n < config->trust.n; -} - static int c_disable(struct conn *c, char **vec, int nvec) { if(nvec == 0) disable_playing(c->who); @@ -240,22 +232,28 @@ static int c_play(struct conn *c, char **vec, static int c_remove(struct conn *c, char **vec, int attribute((unused)) nvec) { struct queue_entry *q; + rights_type r; if(!(q = queue_find(vec[0]))) { sink_writes(ev_writer_sink(c->w), "550 no such track on the queue\n"); return 1; } - if(config->restrictions & RESTRICT_REMOVE) { - /* can only remove tracks that you submitted */ - if(!q->submitter || strcmp(q->submitter, c->who)) { - sink_writes(ev_writer_sink(c->w), "550 you didn't submit that track!\n"); - return 1; - } + if(q->submitter) + if(!strcmp(q->submitter, c->who)) + r = RIGHT_REMOVE_MINE; + else + r = RIGHT_REMOVE_ANY; + else + r = RIGHT_REMOVE_RANDOM; + if(!(c->rights & r)) { + sink_writes(ev_writer_sink(c->w), + "550 Not authorized to remove that track\n"); + return 1; } queue_remove(q, c->who); /* De-prepare the track. */ abandon(c->ev, q); - /* If we removed the random track then add another one. */ + /* If we removed a random track then add another one. */ if(q->state == playing_random) add_random_track(); /* Prepare whatever the next head track is. */ @@ -269,16 +267,26 @@ static int c_remove(struct conn *c, char **vec, static int c_scratch(struct conn *c, char **vec, int nvec) { + rights_type r; + if(!playing) { sink_writes(ev_writer_sink(c->w), "250 nothing is playing\n"); return 1; /* completed */ } - if(config->restrictions & RESTRICT_SCRATCH) { - /* can only scratch tracks you submitted and randomly selected ones */ - if(playing->submitter && strcmp(playing->submitter, c->who)) { - sink_writes(ev_writer_sink(c->w), "550 you didn't submit that track!\n"); - return 1; - } + /* TODO there is a bug here: if we specify an ID but it's not the currently + * playing track then you will get 550 if you weren't authorized to scratch + * the currently playing track. */ + if(playing->submitter) + if(!strcmp(playing->submitter, c->who)) + r = RIGHT_SCRATCH_MINE; + else + r = RIGHT_SCRATCH_ANY; + else + r = RIGHT_SCRATCH_RANDOM; + if(!(c->rights & r)) { + sink_writes(ev_writer_sink(c->w), + "550 Not authorized to scratch that track\n"); + return 1; } scratch(c->who, nvec == 1 ? vec[0] : 0); /* If you scratch an unpaused track then it is automatically unpaused */ @@ -366,14 +374,6 @@ static int c_playing(struct conn *c, return 1; /* completed */ } -static int c_become(struct conn *c, - char **vec, - int attribute((unused)) nvec) { - c->who = vec[0]; - sink_writes(ev_writer_sink(c->w), "230 OK\n"); - return 1; -} - static const char *connection_host(struct conn *c) { union { struct sockaddr sa; @@ -404,7 +404,9 @@ static const char *connection_host(struct conn *c) { static int c_user(struct conn *c, char **vec, int attribute((unused)) nvec) { + struct kvp *k; const char *res, *host, *password; + rights_type rights; if(c->who) { sink_writes(ev_writer_sink(c->w), "530 already authenticated\n"); @@ -416,10 +418,23 @@ static int c_user(struct conn *c, return 1; } /* find the user */ - password = trackdb_get_password(vec[0]); + k = trackdb_getuserinfo(vec[0]); /* reject nonexistent users */ - if(!password) { - info("S%x unknown user '%s' from %s", c->tag, vec[0], host); + if(!k) { + error(0, "S%x unknown user '%s' from %s", c->tag, vec[0], host); + sink_writes(ev_writer_sink(c->w), "530 authentication failed\n"); + return 1; + } + /* reject unconfirmed users */ + if(kvp_get(k, "confirmation")) { + error(0, "S%x unconfirmed user '%s' from %s", c->tag, vec[0], host); + sink_writes(ev_writer_sink(c->w), "530 authentication failed\n"); + return 1; + } + password = kvp_get(k, "password"); + if(!password) password = ""; + if(parse_rights(kvp_get(k, "rights"), &rights)) { + error(0, "error parsing rights for %s", vec[0]); sink_writes(ev_writer_sink(c->w), "530 authentication failed\n"); return 1; } @@ -428,9 +443,12 @@ static int c_user(struct conn *c, config->authorization_algorithm); if(wideopen || (res && !strcmp(res, vec[1]))) { c->who = vec[0]; + c->rights = rights; /* currently we only bother logging remote connections */ - if(strcmp(host, "local")) + if(strcmp(host, "local")) { info("S%x %s connected from %s", c->tag, vec[0], host); + c->rights |= RIGHT__LOCAL; + } sink_writes(ev_writer_sink(c->w), "230 OK\n"); return 1; } @@ -715,6 +733,7 @@ static int c_volume(struct conn *c, int nvec) { int l, r, set; char lb[32], rb[32]; + rights_type rights; switch(nvec) { case 0: @@ -732,6 +751,11 @@ static int c_volume(struct conn *c, default: abort(); } + rights = set ? RIGHT_VOLUME : RIGHT_READ; + if(!(c->rights & rights)) { + sink_writes(ev_writer_sink(c->w), "530 Prohibited\n"); + return 1; + } if(mixer_control(&l, &r, set)) sink_writes(ev_writer_sink(c->w), "550 error accessing mixer\n"); else { @@ -817,23 +841,44 @@ static int c_log(struct conn *c, return 0; } +/** @brief Test whether a move is allowed + * @param c Connection + * @param qs List of IDs on queue + * @param nqs Number of IDs + * @return 0 if move is prohibited, non-0 if it is allowed + */ +static int has_move_rights(struct conn *c, struct queue_entry **qs, int nqs) { + rights_type r = 0; + + for(; nqs > 0; ++qs, --nqs) { + struct queue_entry *const q = *qs; + + if(q->submitter) + if(!strcmp(q->submitter, c->who)) + r |= RIGHT_MOVE_MINE; + else + r |= RIGHT_MOVE_ANY; + else + r |= RIGHT_MOVE_RANDOM; + } + return (c->rights & r) == r; +} + static int c_move(struct conn *c, char **vec, int attribute((unused)) nvec) { struct queue_entry *q; int n; - if(config->restrictions & RESTRICT_MOVE) { - if(!trusted(c)) { - sink_writes(ev_writer_sink(c->w), - "550 only trusted users can move tracks\n"); - return 1; - } - } if(!(q = queue_find(vec[0]))) { sink_writes(ev_writer_sink(c->w), "550 no such track on the queue\n"); return 1; } + if(!has_move_rights(c, &q, 1)) { + sink_writes(ev_writer_sink(c->w), + "550 Not authorized to move that track\n"); + return 1; + } n = queue_move(q, atoi(vec[1]), c->who); sink_printf(ev_writer_sink(c->w), "252 %d\n", n); /* If we've moved to the head of the queue then prepare the track. */ @@ -848,13 +893,6 @@ static int c_moveafter(struct conn *c, struct queue_entry *q, **qs; int n; - if(config->restrictions & RESTRICT_MOVE) { - if(!trusted(c)) { - sink_writes(ev_writer_sink(c->w), - "550 only trusted users can move tracks\n"); - return 1; - } - } if(vec[0][0]) { if(!(q = queue_find(vec[0]))) { sink_writes(ev_writer_sink(c->w), "550 no such track on the queue\n"); @@ -870,6 +908,11 @@ static int c_moveafter(struct conn *c, sink_writes(ev_writer_sink(c->w), "550 no such track on the queue\n"); return 1; } + if(!has_move_rights(c, qs, nvec)) { + sink_writes(ev_writer_sink(c->w), + "550 Not authorized to move those tracks\n"); + return 1; + } queue_moveafter(q, nvec, qs, c->who); sink_printf(ev_writer_sink(c->w), "250 Moved tracks\n"); /* If we've moved to the head of the queue then prepare the track. */ @@ -978,6 +1021,7 @@ static int c_cookie(struct conn *c, int attribute((unused)) nvec) { const char *host; char *user; + rights_type rights; /* Can't log in twice on the same connection */ if(c->who) { @@ -990,16 +1034,19 @@ static int c_cookie(struct conn *c, return 1; } /* Check the cookie */ - user = verify_cookie(vec[0]); + user = verify_cookie(vec[0], &rights); if(!user) { sink_writes(ev_writer_sink(c->w), "530 authentication failure\n"); return 1; } /* Log in */ - c->who = user; + c->who = vec[0]; c->cookie = vec[0]; - if(strcmp(host, "local")) + c->rights = rights; + if(strcmp(host, "local")) { info("S%x %s connected with cookie from %s", c->tag, user, host); + c->rights |= RIGHT__LOCAL; + } sink_writes(ev_writer_sink(c->w), "230 OK\n"); return 1; } @@ -1010,7 +1057,7 @@ static int c_make_cookie(struct conn *c, const char *cookie = make_cookie(c->who); if(cookie) - sink_printf(ev_writer_sink(c->w), "252 %s\n", cookie); + sink_printf(ev_writer_sink(c->w), "252 %s\n", quoteutf8(cookie)); else sink_writes(ev_writer_sink(c->w), "550 Cannot create cookie\n"); return 1; @@ -1030,7 +1077,6 @@ static int c_revoke(struct conn *c, static int c_adduser(struct conn *c, char **vec, int attribute((unused)) nvec) { - /* TODO local only */ if(trackdb_adduser(vec[0], vec[1], default_rights(), 0)) sink_writes(ev_writer_sink(c->w), "550 Cannot create user\n"); else @@ -1041,7 +1087,6 @@ static int c_adduser(struct conn *c, static int c_deluser(struct conn *c, char **vec, int attribute((unused)) nvec) { - /* TODO local only */ if(trackdb_deluser(vec[0])) sink_writes(ev_writer_sink(c->w), "550 Cannot deleted user\n"); else @@ -1052,8 +1097,9 @@ static int c_deluser(struct conn *c, static int c_edituser(struct conn *c, char **vec, int attribute((unused)) nvec) { - /* TODO local only */ - if(trusted(c) + /* RIGHT_ADMIN can do anything; otherwise you can only set your own email + * address and password. */ + if((c->rights & RIGHT_ADMIN) || (!strcmp(c->who, vec[0]) && (!strcmp(vec[1], "email") || !strcmp(vec[1], "password")))) { @@ -1071,9 +1117,10 @@ static int c_userinfo(struct conn *c, int attribute((unused)) nvec) { struct kvp *k; const char *value; - - /* TODO local only */ - if(trusted(c) + + /* RIGHT_ADMIN allows anything; otherwise you can only get your own email + * address and righst list. */ + if((c->rights & RIGHT_ADMIN) || (!strcmp(c->who, vec[0]) && (!strcmp(vec[1], "email") || !strcmp(vec[1], "rights")))) { @@ -1105,67 +1152,77 @@ static int c_users(struct conn *c, return 1; /* completed */ } -#define C_AUTH 0001 /* must be authenticated */ -#define C_TRUSTED 0002 /* must be trusted user */ - static const struct command { + /** @brief Command name */ const char *name; - int minargs, maxargs; + + /** @brief Minimum number of arguments */ + int minargs; + + /** @brief Maximum number of arguments */ + int maxargs; + + /** @brief Function to process command */ int (*fn)(struct conn *, char **, int); - unsigned flags; + + /** @brief Rights required to execute command + * + * 0 means that the command can be issued without logging in. If multiple + * bits are listed here any of those rights will do. + */ + rights_type rights; } commands[] = { - { "adduser", 2, 2, c_adduser, C_AUTH|C_TRUSTED }, - { "allfiles", 0, 2, c_allfiles, C_AUTH }, - { "become", 1, 1, c_become, C_AUTH|C_TRUSTED }, + { "adduser", 2, 2, c_adduser, RIGHT_ADMIN|RIGHT__LOCAL }, + { "allfiles", 0, 2, c_allfiles, RIGHT_READ }, { "cookie", 1, 1, c_cookie, 0 }, - { "deluser", 1, 1, c_deluser, C_AUTH|C_TRUSTED }, - { "dirs", 0, 2, c_dirs, C_AUTH }, - { "disable", 0, 1, c_disable, C_AUTH }, - { "edituser", 3, 3, c_edituser, C_AUTH }, - { "enable", 0, 0, c_enable, C_AUTH }, - { "enabled", 0, 0, c_enabled, C_AUTH }, - { "exists", 1, 1, c_exists, C_AUTH }, - { "files", 0, 2, c_files, C_AUTH }, - { "get", 2, 2, c_get, C_AUTH }, - { "get-global", 1, 1, c_get_global, C_AUTH }, - { "length", 1, 1, c_length, C_AUTH }, - { "log", 0, 0, c_log, C_AUTH }, - { "make-cookie", 0, 0, c_make_cookie, C_AUTH }, - { "move", 2, 2, c_move, C_AUTH }, - { "moveafter", 1, INT_MAX, c_moveafter, C_AUTH }, - { "new", 0, 1, c_new, C_AUTH }, - { "nop", 0, 0, c_nop, C_AUTH }, - { "part", 3, 3, c_part, C_AUTH }, - { "pause", 0, 0, c_pause, C_AUTH }, - { "play", 1, 1, c_play, C_AUTH }, - { "playing", 0, 0, c_playing, C_AUTH }, - { "prefs", 1, 1, c_prefs, C_AUTH }, - { "queue", 0, 0, c_queue, C_AUTH }, - { "random-disable", 0, 0, c_random_disable, C_AUTH }, - { "random-enable", 0, 0, c_random_enable, C_AUTH }, - { "random-enabled", 0, 0, c_random_enabled, C_AUTH }, - { "recent", 0, 0, c_recent, C_AUTH }, - { "reconfigure", 0, 0, c_reconfigure, C_AUTH|C_TRUSTED }, - { "remove", 1, 1, c_remove, C_AUTH }, - { "rescan", 0, 0, c_rescan, C_AUTH|C_TRUSTED }, - { "resolve", 1, 1, c_resolve, C_AUTH }, - { "resume", 0, 0, c_resume, C_AUTH }, - { "revoke", 0, 0, c_revoke, C_AUTH }, - { "rtp-address", 0, 0, c_rtp_address, C_AUTH }, - { "scratch", 0, 1, c_scratch, C_AUTH }, - { "search", 1, 1, c_search, C_AUTH }, - { "set", 3, 3, c_set, C_AUTH, }, - { "set-global", 2, 2, c_set_global, C_AUTH }, - { "shutdown", 0, 0, c_shutdown, C_AUTH|C_TRUSTED }, - { "stats", 0, 0, c_stats, C_AUTH }, - { "tags", 0, 0, c_tags, C_AUTH }, - { "unset", 2, 2, c_set, C_AUTH }, - { "unset-global", 1, 1, c_set_global, C_AUTH }, + { "deluser", 1, 1, c_deluser, RIGHT_ADMIN|RIGHT__LOCAL }, + { "dirs", 0, 2, c_dirs, RIGHT_READ }, + { "disable", 0, 1, c_disable, RIGHT_GLOBAL_PREFS }, + { "edituser", 3, 3, c_edituser, RIGHT_ADMIN|RIGHT_USERINFO }, + { "enable", 0, 0, c_enable, RIGHT_GLOBAL_PREFS }, + { "enabled", 0, 0, c_enabled, RIGHT_READ }, + { "exists", 1, 1, c_exists, RIGHT_READ }, + { "files", 0, 2, c_files, RIGHT_READ }, + { "get", 2, 2, c_get, RIGHT_READ }, + { "get-global", 1, 1, c_get_global, RIGHT_READ }, + { "length", 1, 1, c_length, RIGHT_READ }, + { "log", 0, 0, c_log, RIGHT_READ }, + { "make-cookie", 0, 0, c_make_cookie, RIGHT_READ }, + { "move", 2, 2, c_move, RIGHT_MOVE__MASK }, + { "moveafter", 1, INT_MAX, c_moveafter, RIGHT_MOVE__MASK }, + { "new", 0, 1, c_new, RIGHT_READ }, + { "nop", 0, 0, c_nop, 0 }, + { "part", 3, 3, c_part, RIGHT_READ }, + { "pause", 0, 0, c_pause, RIGHT_PAUSE }, + { "play", 1, 1, c_play, RIGHT_PLAY }, + { "playing", 0, 0, c_playing, RIGHT_READ }, + { "prefs", 1, 1, c_prefs, RIGHT_READ }, + { "queue", 0, 0, c_queue, RIGHT_READ }, + { "random-disable", 0, 0, c_random_disable, RIGHT_GLOBAL_PREFS }, + { "random-enable", 0, 0, c_random_enable, RIGHT_GLOBAL_PREFS }, + { "random-enabled", 0, 0, c_random_enabled, RIGHT_READ }, + { "recent", 0, 0, c_recent, RIGHT_READ }, + { "reconfigure", 0, 0, c_reconfigure, RIGHT_ADMIN }, + { "remove", 1, 1, c_remove, RIGHT_REMOVE__MASK }, + { "rescan", 0, 0, c_rescan, RIGHT_RESCAN }, + { "resolve", 1, 1, c_resolve, RIGHT_READ }, + { "resume", 0, 0, c_resume, RIGHT_PAUSE }, + { "revoke", 0, 0, c_revoke, RIGHT_READ }, + { "rtp-address", 0, 0, c_rtp_address, 0 }, + { "scratch", 0, 1, c_scratch, RIGHT_SCRATCH__MASK }, + { "search", 1, 1, c_search, RIGHT_READ }, + { "set", 3, 3, c_set, RIGHT_PREFS, }, + { "set-global", 2, 2, c_set_global, RIGHT_GLOBAL_PREFS }, + { "shutdown", 0, 0, c_shutdown, RIGHT_ADMIN }, + { "stats", 0, 0, c_stats, RIGHT_READ }, + { "tags", 0, 0, c_tags, RIGHT_READ }, + { "unset", 2, 2, c_set, RIGHT_PREFS }, + { "unset-global", 1, 1, c_set_global, RIGHT_GLOBAL_PREFS }, { "user", 2, 2, c_user, 0 }, - { "userinfo", 2, 2, c_userinfo, C_AUTH }, - { "users", 0, 0, c_users, C_AUTH }, - { "version", 0, 0, c_version, C_AUTH }, - { "volume", 0, 2, c_volume, C_AUTH } + { "userinfo", 2, 2, c_userinfo, RIGHT_READ }, + { "users", 0, 0, c_users, RIGHT_READ }, + { "version", 0, 0, c_version, RIGHT_READ }, + { "volume", 0, 2, c_volume, RIGHT_READ|RIGHT_VOLUME } }; static void command_error(const char *msg, void *u) { @@ -1196,12 +1253,9 @@ static int command(struct conn *c, char *line) { if((n = TABLE_FIND(commands, struct command, name, vec[0])) < 0) sink_writes(ev_writer_sink(c->w), "500 unknown command\n"); else { - if((commands[n].flags & C_AUTH) && !c->who) { - sink_writes(ev_writer_sink(c->w), "530 not authenticated\n"); - return 1; - } - if((commands[n].flags & C_TRUSTED) && !trusted(c)) { - sink_writes(ev_writer_sink(c->w), "530 insufficient privilege\n"); + if(commands[n].rights + && !(c->rights & commands[n].rights)) { + sink_writes(ev_writer_sink(c->w), "530 Prohibited\n"); return 1; } ++vec; @@ -1296,6 +1350,7 @@ static int listen_callback(ev_source *ev, c->fd = fd; c->reader = reader_callback; c->l = l; + c->rights = 0; gcry_randomize(c->nonce, sizeof c->nonce, GCRY_STRONG_RANDOM); if(!strcmp(config->authorization_algorithm, "sha1") || !strcmp(config->authorization_algorithm, "SHA1")) { diff --git a/tests/dtest.py b/tests/dtest.py index 5ab56ed..edf793b 100644 --- a/tests/dtest.py +++ b/tests/dtest.py @@ -259,11 +259,15 @@ Start the daemon.""" def create_user(username="fred", password="fredpass"): """create_user(USERNAME, PASSWORD) - Create a user, abusing direct database access to do so.""" + Create a user, abusing direct database access to do so. Gives the + user rights 'all', allowing them to do anything.""" print " creating user %s" % username command(["disorder", "--config", disorder._configfile, "--no-per-user-config", "--user", "root", "adduser", username, password]) + command(["disorder", + "--config", disorder._configfile, "--no-per-user-config", + "--user", "root", "edituser", username, "rights", "all"]) def stop_daemon(): """stop_daemon() diff --git a/tests/user.py b/tests/user.py index 345fdef..b04e2a2 100755 --- a/tests/user.py +++ b/tests/user.py @@ -30,6 +30,8 @@ def test(): users = c.users() assert dtest.lists_have_same_contents(users, ["fred", "bob", "root"]) + rights = c.userinfo("bob", "rights") + print " new user rights are: %s" % rights print " checking new user can log in" c = disorder.client(user="bob", password="bobpass") c.version() -- [mdw]