From: Ian Jackson Date: Thu, 29 Oct 2020 18:59:16 +0000 (+0000) Subject: wip new accounts lock X-Git-Tag: otter-0.2.0~582 X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?a=commitdiff_plain;h=3104c799c39f619c3b7362574444806ca5a3ddf5;p=otter.git wip new accounts lock Signed-off-by: Ian Jackson --- diff --git a/src/accounts.rs b/src/accounts.rs index b7c60a0a..8edf278e 100644 --- a/src/accounts.rs +++ b/src/accounts.rs @@ -148,7 +148,7 @@ pub struct AccountRecord { #[derive(Serialize,Deserialize,Debug)] #[serde(transparent)] -struct AccessRecord (Arc); +pub struct AccessRecord (Arc); #[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> for AccessRecord { + fn from(spec: Box) -> 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( + &self, + key: K + ) -> Option { + let accounts = self.as_ref()?; + let acctid = key.initial_lookup(accounts)?; + let _record = accounts.records.get(acctid)?; + Some(acctid) + } + pub fn bulk_check( &self, keys: &[K] ) -> Vec> { - 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( @@ -299,13 +313,12 @@ impl AccountsGuard { #[throws(MgmtError)] pub fn insert_entry(&mut self, - account: AccountName, - _auth: Authorisation, - new_record: AccountRecord) + new_record: AccountRecord, + _auth: Authorisation) { 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 90ab3a5b..2d39e9ff 100644 --- a/src/api.rs +++ b/src/api.rs @@ -185,8 +185,8 @@ fn api_grab(form : Json>) 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>) 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>) 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>) -> 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>) -> 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>) -> 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 3a5c498a..daa596f5 100644 --- 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(spec: Option, 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 { if let Some(y) = cs.is_superuser() { return y } diff --git a/src/global.rs b/src/global.rs index c27e6707..c585586e 100644 --- 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 33de1c4b..50895941 100644 --- a/src/session.rs +++ b/src/session.rs @@ -95,8 +95,8 @@ fn session(form : Json) -> Result { } 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 e99e7195..32d7e2d6 100644 --- 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;