wip new accounts lock

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

diff --git a/src/accounts.rs b/src/accounts.rs
index b7c60a0ae13851d9e5941ba84f65ea44cbbb3603..8edf278ed562569fda54dfde0491786015ff7036 100644 (file)
--- a/src/accounts.rs
+++ b/src/accounts.rs
@@ -148,7 +148,7 @@ pub struct AccountRecord {
 
 #[derive(Serialize,Deserialize,Debug)]
 #[serde(transparent)]
-struct AccessRecord (Arc<dyn PlayerAccessSpec>);
+pub struct AccessRecord (Arc<dyn PlayerAccessSpec>);
 
 #[derive(Copy,Clone,Debug,Ord,PartialOrd,Eq,PartialEq)]
 #[derive(Serialize,Deserialize)]
@@ -174,6 +174,14 @@ impl Deref for AccessRecord {
   fn deref(&self) -> &Self::Target { return &self.0 }
 }
 
+impl AccessRecord {
+  pub fn new_unset() -> Self{ Self( Arc::new(PlayerAccessUnset) ) }
+}
+
+impl From<Box<dyn PlayerAccessSpec>> for AccessRecord {
+  fn from(spec: Box<dyn PlayerAccessSpec>) -> Self { Self(spec.into()) }
+}
+
 // xxx load, incl reveleation expiry
 // xxx periodic token reveleation expiry
 
@@ -222,17 +230,23 @@ impl LookupHelper {
 }
 
 impl AccountsGuard {
+  pub fn lock() -> Self { Self(ACCOUNTS.lock()) }
+
+  pub fn check<K:AccountNameOrId>(
+    &self,
+    key: K
+  ) -> Option<AccountId> {
+    let accounts = self.as_ref()?;
+    let acctid = key.initial_lookup(accounts)?;
+    let _record = accounts.records.get(acctid)?;
+    Some(acctid)
+  }
+
   pub fn bulk_check<K:AccountNameOrId>(
     &self,
     keys: &[K]
   ) -> Vec<Option<AccountId>> {
-    let accounts = ACCOUNTS.read();
-    keys.iter().map(|&key|{
-      let accounts = accounts.as_ref()?;
-      let acctid = key.initial_lookup(accounts)?;
-      let _record = accounts.records.get(acctid)?;
-      Some(acctid)
-    }).collect()
+    keys.iter().map(|&key| self.check(key)).collect()
   }
 
   pub fn lookup<K: AccountNameOrId>(
@@ -299,13 +313,12 @@ impl AccountsGuard {
 
   #[throws(MgmtError)]
   pub fn insert_entry(&mut self,
-                      account: AccountName,
-                      _auth: Authorisation<AccountName>,
-                      new_record: AccountRecord)
+                      new_record: AccountRecord,
+                      _auth: Authorisation<AccountName>)
   {
     use hash_map::Entry::*;
     let accounts = self.get_or_insert_with(default);
-    let name_entry = accounts.names.entry(account);
+    let name_entry = accounts.names.entry(new_record.account.clone());
     if_chain!{
       if let Occupied(oe) = name_entry;
       let acctid = *oe.get();

diff --git a/src/api.rs b/src/api.rs
index 90ab3a5be52003c37087a04c55c7368afdfc67e3..2d39e9ff725e95d9c45741cf7266a282dd128f4d 100644 (file)
--- a/src/api.rs
+++ b/src/api.rs
@@ -185,8 +185,8 @@ fn api_grab(form : Json<ApiPiece<ApiPieceGrab>>)
 impl ApiPieceOp for ApiPieceGrab {
   #[throws(ApiPieceOpError)]
   fn op(&self, a: ApiPieceOpArgs) -> PieceUpdateFromOp {
-    let ApiPieceOpArgs { gs,player,pst,piece,p,lens, .. } = a;
-    let pl = gs.players.byid(player)?;
+    let ApiPieceOpArgs { gs,player,piece,p,lens, .. } = a;
+    let gpl = gs.players.byid(player)?;
     let pc = gs.pieces.byid_mut(piece)?;
 
     if pc.held.is_some() { throw!(OnlineError::PieceHeld) }
@@ -196,7 +196,7 @@ impl ApiPieceOp for ApiPieceGrab {
 
     let logent = LogEntry {
       html : Html(format!("{} grasped {}",
-                     &htmlescape::encode_minimal(&pst.nick),
+                     &htmlescape::encode_minimal(&gpl.nick),
                      p.describe_pri(&lens.log_pri(piece, pc)).0)),
     };
 
@@ -221,22 +221,22 @@ impl ApiPieceOp for ApiPieceWrest {
 
   #[throws(ApiPieceOpError)]
   fn op(&self, a: ApiPieceOpArgs) -> PieceUpdateFromOp {
-    let ApiPieceOpArgs { gs,player,pst,piece,p,lens,iplayers, .. } = a;
-    let pl = gs.players.byid(player)?;
+    let ApiPieceOpArgs { gs,player,piece,p,lens,iplayers, .. } = a;
+    let gpl = gs.players.byid(player)?;
     let pc = gs.pieces.byid_mut(piece)?;
     let pcs = p.describe_pri(&lens.log_pri(piece, pc)).0;
 
     let was = pc.held;
     pc.held = Some(player);
-    let was = was.and_then(|p| iplayers.get(p));    
+    let was = was.and_then(|p| gs.players.get(p));    
 
     let update = PieceUpdateOp::Modify(());
 
-    let pls = &htmlescape::encode_minimal(&pst.nick);
+    let pls = &htmlescape::encode_minimal(&gpl.nick);
 
     let logent = LogEntry { html : Html(match was {
         Some(was) => format!("{} wrested {} from {}", pls, pcs,
-                             &htmlescape::encode_minimal(&was.pst.nick)),
+                             &htmlescape::encode_minimal(&was.nick)),
         None => format!("{} wrested {}", pls, pcs),
     })};
 
@@ -257,8 +257,8 @@ fn api_ungrab(form : Json<ApiPiece<ApiPieceUngrab>>)
 impl ApiPieceOp for ApiPieceUngrab {
   #[throws(ApiPieceOpError)]
   fn op(&self, a: ApiPieceOpArgs) -> PieceUpdateFromOp {
-    let ApiPieceOpArgs { gs,player,pst,piece,p,lens, .. } = a;
-    let pl = gs.players.byid(player).unwrap();
+    let ApiPieceOpArgs { gs,player,piece,p,lens, .. } = a;
+    let gpl = gs.players.byid(player).unwrap();
     let pc = gs.pieces.byid_mut(piece).unwrap();
 
     if pc.held != Some(player) { throw!(OnlineError::PieceHeld) }
@@ -268,7 +268,7 @@ impl ApiPieceOp for ApiPieceUngrab {
 
     let logent = LogEntry {
       html : Html(format!("{} released {}",
-                     &htmlescape::encode_minimal(&pst.nick),
+                     &htmlescape::encode_minimal(&gpl.nick),
                      p.describe_pri(&lens.log_pri(piece, pc)).0)),
     };
 
@@ -290,7 +290,7 @@ fn api_raise(form : Json<ApiPiece<ApiPieceRaise>>)
 impl ApiPieceOp for ApiPieceRaise {
   #[throws(ApiPieceOpError)]
   fn op(&self, a: ApiPieceOpArgs) -> PieceUpdateFromOp {
-    let ApiPieceOpArgs { gs,player,pst,piece,p,lens, .. } = a;
+    let ApiPieceOpArgs { gs,player,piece,p,lens, .. } = a;
     let pc = gs.pieces.byid_mut(piece).unwrap();
     pc.zlevel = ZLevel { z : self.z.clone(), zg : gs.gen };
     let update = PieceUpdateOp::SetZLevel(());
@@ -309,7 +309,7 @@ fn api_move(form : Json<ApiPiece<ApiPieceMove>>) -> impl response::Responder<'st
 impl ApiPieceOp for ApiPieceMove {
   #[throws(ApiPieceOpError)]
   fn op(&self, a: ApiPieceOpArgs) -> PieceUpdateFromOp {
-    let ApiPieceOpArgs { gs,player,pst,piece,p,lens, .. } = a;
+    let ApiPieceOpArgs { gs,player,piece,p,lens, .. } = a;
     let pc = gs.pieces.byid_mut(piece).unwrap();
     let (pos, clamped) = self.0.clamped(gs.table_size);
     let logents = vec![];
@@ -336,14 +336,14 @@ fn api_pin(form : Json<ApiPiece<ApiPiecePin>>) -> impl response::Responder<'stat
 impl ApiPieceOp for ApiPiecePin {
   #[throws(ApiPieceOpError)]
   fn op(&self, a: ApiPieceOpArgs) -> PieceUpdateFromOp {
-    let ApiPieceOpArgs { gs,player,pst,piece,p,lens, .. } = a;
+    let ApiPieceOpArgs { gs,player,piece,p,lens, .. } = a;
     let pc = gs.pieces.byid_mut(piece).unwrap();
-    let pl = gs.players.byid(player).unwrap();
+    let gpl = gs.players.byid(player).unwrap();
     pc.pinned = self.0;
     let update = PieceUpdateOp::Modify(());
     let logents = vec![ LogEntry { html: Html(format!(
       "{} {} {}",
-      &htmlescape::encode_minimal(&pst.nick),
+      &htmlescape::encode_minimal(&gpl.nick),
       if pc.pinned { "pinned" } else { "unpinned" },
       p.describe_pri(&lens.log_pri(piece, pc)).0
     ))}];
@@ -364,10 +364,10 @@ fn api_uo(form : Json<ApiPiece<ApiPieceUo>>) -> impl response::Responder<'static
 impl ApiPieceOp for ApiPieceUo {
   #[throws(ApiPieceOpError)]
   fn op(&self, a: ApiPieceOpArgs) -> PieceUpdateFromOp {
-    let ApiPieceOpArgs { gs,player,pst,piece,p,lens, .. } = a;
+    let ApiPieceOpArgs { gs,player,piece,p,lens, .. } = a;
     '_normal_global_ops__not_loop: loop {
       let pc = gs.pieces.byid_mut(piece)?;
-      let pl = gs.players.byid(player)?;
+      let gpl = gs.players.byid(player)?;
       let _: Impossible = match (self.opname.as_str(), self.wrc) {
 
         ("flip", wrc@ WRC::UpdateSvg) => {
@@ -378,7 +378,7 @@ impl ApiPieceOp for ApiPieceUo {
             PieceUpdateOp::Modify(()),
             vec![ LogEntry { html: Html(format!(
               "{} flipped {}",
-              &htmlescape::encode_minimal(&pst.nick),
+              &htmlescape::encode_minimal(&gpl.nick),
               p.describe_pri(&lens.log_pri(piece, pc)).0
             )) }])
         },

diff --git a/src/cmdlistener.rs b/src/cmdlistener.rs
index 3a5c498a4f3bf55c8fcaebae85905cbd98eab283..daa596f5ad0c33693cd398eac4e0a6e2111b9f15 100644 (file)
--- a/src/cmdlistener.rs
+++ b/src/cmdlistener.rs
@@ -90,28 +90,31 @@ fn execute(cs: &mut CommandStream, cmd: MgmtCommand) -> MgmtResponse {
     },
 
     CreateAccont(AccountDetails { account, nick, timezone, access }) => {
-      let auth = authorise_for_account(cs, &account)?;
+      let mut ag = AccountsGuard::lock();
+      let auth = authorise_for_account(&ag, cs, &account)?;
       let access = access.map(Into::into)
-        .unwrap_or_else(|| Arc::new(PlayerAccessUnset) as Arc<_>);
+        .unwrap_or_else(|| AccessRecord::new_unset());
       let nick = nick.unwrap_or_else(|| account.to_string());
+      let account = account.to_owned().into();
       let record = AccountRecord {
-        nick, access,
+        account, nick, access,
         timezone: timezone.unwrap_or_default(),
         tokens_revealed: default(),
       };
-      AccountRecord::insert_entry(account, auth, record)?;
+      ag.insert_entry(record, auth)?;
       Fine
     }
 
     UpdateAccont(AccountDetails { account, nick, timezone, access }) => {
-      let auth = authorise_for_account(cs, &account)?;
-      AccountRecord::with_entry_mut(&account, auth, |record, acctid|{
+      let mut ag = AccountsGuard::lock();
+      let auth = authorise_for_account(&ag, cs, &account)?;
+      let access = access.map(Into::into);
+      ag.with_entry_mut(&account, auth, access, |record, acctid|{
         fn update_from<T>(spec: Option<T>, record: &mut T) {
           if let Some(new) = spec { *record = new; }
         }
         update_from(nick,                   &mut record.nick    );
         update_from(timezone,               &mut record.timezone);
-        update_from(access.map(Into::into), &mut record.access  );
         Fine
       })
         ?
@@ -119,15 +122,16 @@ fn execute(cs: &mut CommandStream, cmd: MgmtCommand) -> MgmtResponse {
     }
 
     DeleteAccount(account) => {
-      let auth = authorise_for_account(cs, &account)?;
-      AccountRecord::remove_entry(&account, auth)?;
+      let mut ag = AccountsGuard::lock();
+      let auth = authorise_for_account(&ag, cs, &account)?;
+      ag.remove_entry(&account, auth)?;
       Fine
     }
 
     SetAccount(wanted_account) => {
       let auth = authorise_scope_direct(cs, &wanted_account.scope)?;
       cs.account = Some(AccountSpecified {
-        account: wanted_account,
+        notional_account: wanted_account,
         cooked: wanted_account.to_string(),
         auth: auth.therefore_ok(),
       });
@@ -170,9 +174,10 @@ fn execute(cs: &mut CommandStream, cmd: MgmtCommand) -> MgmtResponse {
         let auth = authorise_scope_direct(cs, &AS::Server)?;
         (None, auth.therefore_ok())
       } else {
-        let AccountSpecified { account, auth, .. } = cs.account.as_ref()
+        let AccountSpecified { notional_account, auth, .. } =
+          cs.account.as_ref()
           .ok_or(ME::SpecifyAccount)?;
-        (Some(account), *auth)
+        (Some(notional_account), *auth)
       };
       let mut games = Instance::list_names(scope, auth);
       games.sort_unstable();
@@ -266,9 +271,12 @@ fn execute_game_insn<'ig>(
     }
 
     Insn::AddPlayer(add) => {
-      // todo some kind of permissions check for player too
+      let ag = AccountsGuard::lock();
+      let player_auth = Authorisation::authorise_any();
+      // ^ todo some kind of permissions check for player too
       let ig = cs.check_acl(ig, PCH::Instance, &[TP::AddPlayer])?.0;
-      let record = AccountRecord::lookup(&add.account)?;
+      let (record, acctid) = ag.lookup(&add.account, player_auth)
+        .ok_or(ME::AccountNotFound)?;
       let nick = add.nick.ok_or(ME::ParameterMissing)?;
       let logentry = LogEntry {
         html: Html(format!("{} added a player: {}", &who,
@@ -284,10 +292,10 @@ fn execute_game_insn<'ig>(
         nick: nick.to_string(),
       };
       let ipl = IPlayerState {
-        account: add.account,
+        acctid,
         tz,
       };
-      let (player, logentry) = ig.player_new(gpl, ipl, tz, logentry)?;
+      let (player, logentry) = ig.player_new(gpl, ipl, logentry)?;
       (U{ pcs: vec![],
           log: vec![ logentry ],
           raw: None },
@@ -319,11 +327,13 @@ fn execute_game_insn<'ig>(
     })?,
 
     RemovePlayer(account) => {
+      let accounts = AccountsGuard::lock();
       let ig = cs.check_acl(ig,
                             PCH::InstanceOrOnlyAffectedAccount(&account),
                             &[TP::RemovePlayer])?.0;
-      let player = ig.gs.players.iter()
-        .filter_map(|(k,v)| if v == &account { Some(k) } else { None })
+      let acctid = accounts.check(&account)?;
+      let player = ig.g.iplayers.iter()
+        .filter_map(|(k,v)| if v.acctid == acctid { Some(k) } else { None })
         .next()
         .ok_or(ME::PlayerNotFound)?;
       let (_, old_state) = ig.player_remove(player)?;
@@ -778,7 +788,8 @@ impl CommandStream<'_> {
 }
 
 #[throws(MgmtError)]
-fn authorise_for_account(cs: &CommandStream, wanted: &AccountName)
+fn authorise_for_account(_accounts: &AccountsGuard,
+                         cs: &CommandStream, wanted: &AccountName)
                         -> Authorisation<AccountName> {
   if let Some(y) = cs.is_superuser() { return y }
 

diff --git a/src/global.rs b/src/global.rs
index c27e6707f900011b6855c4656b610d4b52a6fa01..c585586e965d913879edf21e65668abea96d7598 100644 (file)
--- a/src/global.rs
+++ b/src/global.rs
@@ -1103,7 +1103,7 @@ pub fn process_all_players_for_account<
   for gref in games.values() {
     let c = gref.lock_even_poisoned();
     let remove : Vec<_> = c.g.iplayers.iter().filter_map(|(player,pr)| {
-      if pr.pst.acctid == acctid { Some(player) } else { None }
+      if pr.ipl.acctid == acctid { Some(player) } else { None }
     }).collect();
     let ig = InstanceGuard { gref: gref.clone(), c };
     for player in remove.drain(..) {

diff --git a/src/session.rs b/src/session.rs
index 33de1c4b4ae64cdc97d52ab20323ddec44d651f2..50895941be6aafc66f936f7a14703a0813ded111 100644 (file)
--- a/src/session.rs
+++ b/src/session.rs
@@ -95,8 +95,8 @@ fn session(form : Json<SessionForm>) -> Result<Template,OE> {
     }
 
     let gpl = ig.gs.players.byid_mut(player)?;
-    let ipl = ig.iplayers.byid(player)?;
-    let tz = &ipl.pst.tz;
+    let pr = ig.iplayers.byid(player)?;
+    let tz = &pr.ipl.tz;
     let mut pieces : Vec<_> = ig.gs.pieces.iter().collect();
 
     pieces.sort_by_key(|(_,pr)| &pr.zlevel);

diff --git a/src/sse.rs b/src/sse.rs
index e99e71954765025e83421f2586aab56bdfe6a839..32d7e2d6a376b8ea05de1e7da225741ebb319e31 100644 (file)
--- a/src/sse.rs
+++ b/src/sse.rs
@@ -125,7 +125,7 @@ impl Read for UpdateReader {
         }
         self.overflow = {
           let mut overflow = Vec::with_capacity(next_len);
-          self.wn.write_next(&mut overflow, &iplayer.pst.tz, &next)
+          self.wn.write_next(&mut overflow, &iplayer.ipl.tz, &next)
             .map_err(|e| self.wn.trouble("overflow.write_next",&e))?;
           debug!("overflow {} {}, len={}",
                  &self.wn.player, &self.wn.client, &overflow.len());
@@ -134,7 +134,7 @@ impl Read for UpdateReader {
         continue;
       }
 
-      self.wn.write_next(&mut buf, &iplayer.pst.tz, &next)
+      self.wn.write_next(&mut buf, &iplayer.ipl.tz, &next)
         .map_err(|e| self.wn.trouble("UpdateReader.write_next",&e))?;
 
       let before = next.when - UPDATE_EXPIRE;