From: Ian Jackson <ijackson@chiark.greenend.org.uk>
Date: Fri, 13 Nov 2020 18:57:08 +0000 (+0000)
Subject: sort out some rawtoken permission check
X-Git-Tag: otter-0.2.0~529
X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=31181029025f792dc6701863effc699125bd685f;p=otter.git

sort out some rawtoken permission check

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---

diff --git a/src/accounts.rs b/src/accounts.rs
index d0039d7a..b9083c89 100644
--- a/src/accounts.rs
+++ b/src/accounts.rs
@@ -32,7 +32,7 @@ pub struct AccountName {
 }
 
 /// Record of acess for a player.  Newtype prevents mutable access
-/// without invalidating old tokens.
+/// without invalidating old tokens and permissions check.
 #[derive(Serialize,Deserialize,Debug)]
 #[serde(transparent)]
 pub struct AccessRecord (Arc<dyn PlayerAccessSpec>);
@@ -221,11 +221,14 @@ impl Deref for AccessRecord {
 
 impl AccessRecord {
   pub fn new_unset() -> Self{ Self( Arc::new(PlayerAccessUnset) ) }
-}
 
-impl From<Box<dyn PlayerAccessSpec>> for AccessRecord {
-  // xxx get rid of this From impl and do a server permission check
-  fn from(spec: Box<dyn PlayerAccessSpec>) -> Self { Self(spec.into()) }
+  #[throws(MgmtError)]
+  pub fn from_spec(spec: Box<dyn PlayerAccessSpec>,
+                   superuser: Option<AuthorisationSuperuser>)
+                   -> AccessRecord {
+    spec.check_spec_permission(superuser)?;
+    AccessRecord(spec.into())
+  }
 }
 
 //---------- AccountsGuard and lookup ----------
diff --git a/src/cmdlistener.rs b/src/cmdlistener.rs
index 30a54b6c..4e778455 100644
--- a/src/cmdlistener.rs
+++ b/src/cmdlistener.rs
@@ -34,6 +34,8 @@ const CREATE_PIECES_MAX : u32 = 300;
 const DEFAULT_POS_START : Pos = PosC([20,20]);
 const DEFAULT_POS_DELTA : Pos = PosC([5,5]);
 
+pub type AuthorisationSuperuser = Authorisation<authproofs::Global>;
+
 pub struct CommandListener {
   listener : UnixListener,
 }
@@ -48,7 +50,7 @@ struct CommandStream<'d> {
   euid : Result<Uid, ConnectionEuidDiscoverEerror>,
   desc : &'d str,
   account : Option<AccountSpecified>,
-  superuser: Option<Authorisation<authproofs::Global>>,
+  superuser: Option<AuthorisationSuperuser>,
   chan : MgmtChannel,
   who: Who,
 }
@@ -92,7 +94,7 @@ fn execute(cs: &mut CommandStream, cmd: MgmtCommand) -> MgmtResponse {
     CreateAccont(AccountDetails { account, nick, timezone, access }) => {
       let mut ag = AccountsGuard::lock();
       let auth = authorise_for_account(cs, &ag, &account)?;
-      let access = access.map(Into::into)
+      let access = cs.accountrecord_from_spec(access)?
         .unwrap_or_else(|| AccessRecord::new_unset());
       let nick = nick.unwrap_or_else(|| account.to_string());
       let account = account.to_owned().into();
@@ -922,6 +924,15 @@ impl CommandStream<'_> {
     let auth = get_auth(self, ag, ig, how, p.into())?;
     (ig.by_mut(auth), auth)
   }
+
+  #[throws(MgmtError)]
+  fn accountrecord_from_spec(&self, spec: Option<Box<dyn PlayerAccessSpec>>)
+                             -> Option<AccessRecord> {
+    spec
+      .map(|spec| AccessRecord::from_spec(spec, self.superuser))
+      .transpose()?
+  }
+
 }
 
 #[throws(MgmtError)]
diff --git a/src/commands.rs b/src/commands.rs
index bd79d444..65e96bc4 100644
--- a/src/commands.rs
+++ b/src/commands.rs
@@ -152,6 +152,7 @@ pub enum MgmtGameUpdateMode {
 pub enum MgmtError {
   ParseFailed(String),
   AuthorisationError,
+  SuperuserAuthorisationRequired,
   ParameterMissing,
   SpecifyAccount,
   AlreadyExists,
diff --git a/src/spec.rs b/src/spec.rs
index a545a9ae..b1395f63 100644
--- a/src/spec.rs
+++ b/src/spec.rs
@@ -18,6 +18,8 @@ use num_derive::{ToPrimitive, FromPrimitive};
 
 pub use implementation::PlayerAccessSpec;
 
+type ME = crate::commands::MgmtError;
+
 //---------- common types ----------
 
 pub type Coord = isize;
@@ -316,6 +318,9 @@ pub mod implementation {
     fn override_token(&self) -> Option<&RawToken> {
       None
     }
+    #[throws(MgmtError)]
+    fn check_spec_permission(&self, _: Option<AuthorisationSuperuser>) {
+    }
     fn server_deliver<'t>(&self,
                           _gpl: &GPlayerState,
                           _ipl: &IPlayerState,
@@ -341,6 +346,10 @@ pub mod implementation {
 
   #[typetag::serde]
   impl PlayerAccessSpec for FixedToken {
+    #[throws(MgmtError)]
+    fn check_spec_permission(&self, auth: Option<AuthorisationSuperuser>) {
+      auth.ok_or(ME::SuperuserAuthorisationRequired)?
+    }
     fn override_token(&self) -> Option<&RawToken> {
       Some(&self.token)
     }