From bbcb7acb95915008d5e636c80fe82ed8e930de65 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Tue, 5 Dec 2023 01:30:27 +0530 Subject: [PATCH] Add sysctl alter user We will now also only output server logs to the terminal in CI, iff there is an error. Otherwise, it plagues output. --- CHANGELOG.md | 9 ++- harness/src/main.rs | 7 +- harness/src/test/mod.rs | 1 + harness/src/test/svc.rs | 77 +++++++++++++++++-- server/src/engine/core/dcl.rs | 31 ++++++-- server/src/engine/fractal/sys_store.rs | 17 ++++ server/src/engine/ql/dcl.rs | 61 +++++++-------- server/src/engine/ql/tests/dcl.rs | 26 +++++-- .../engine/tests/client_misc/sec/dcl_sec.rs | 7 +- sky-macros/src/dbtest.rs | 10 +-- 10 files changed, 175 insertions(+), 71 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc77a7d0..93d1ac18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,15 +14,15 @@ All changes in this project will be noted in this file. - `space`s are the equivalent of the `keyspace` from previous versions - `model`s are the equivalent of `table`s from previous version - The following queries were added: - - `CREATE SPACE ...` - - `CREATE MODEL ...` + - `CREATE SPACE [IF NOT EXISTS] ...` + - `CREATE MODEL [IF NOT EXISTS] ...` - Nested lists are now supported - Type definitions are now supported - Multiple fields are now supported - `ALTER SPACE ...` - `ALTER MODEL ...` - - `DROP SPACE ...` - - `DROP MODEL ...` + - `DROP SPACE [IF EXISTS] ...` + - `DROP MODEL [IF EXISTS] ...` - `USE `: - works just like SQL - **does not work with DDL queries**: the reason it works in this way is to prevent accidental deletes @@ -56,6 +56,7 @@ All changes in this project will be noted in this file. - `DELETE FROM . WHERE = ` - DCL: - `SYSCTL CREATE USER WITH { password: }` + - `SYSCTL ALTER USER WITH { password: }` - `SYSCTL DROP USER ` #### Fractal engine diff --git a/harness/src/main.rs b/harness/src/main.rs index 70011fce..77e1087e 100644 --- a/harness/src/main.rs +++ b/harness/src/main.rs @@ -52,10 +52,13 @@ fn main() { Builder::new() .parse_filters(&env::var("SKYHARNESS_LOG").unwrap_or_else(|_| "info".to_owned())) .init(); - // avoid verbose logging - env::set_var("SKY_LOG", "error"); + env::set_var("SKY_LOG", "trace"); if let Err(e) = runner() { error!("harness failed with: {}", e); + error!("fetching logs from server processes"); + for ret in test::get_children() { + ret.print_logs(); + } process::exit(0x01); } } diff --git a/harness/src/test/mod.rs b/harness/src/test/mod.rs index 2695e109..46c2a0f1 100644 --- a/harness/src/test/mod.rs +++ b/harness/src/test/mod.rs @@ -46,6 +46,7 @@ use { std::{fs, io::Write}, }; mod svc; +pub use svc::get_children; /// Run the test suite pub fn run_test() -> HarnessResult<()> { diff --git a/harness/src/test/svc.rs b/harness/src/test/svc.rs index 1203f81a..55d5f57b 100644 --- a/harness/src/test/svc.rs +++ b/harness/src/test/svc.rs @@ -33,12 +33,73 @@ use { HarnessError, HarnessResult, ROOT_DIR, }, std::{ + cell::RefCell, io::ErrorKind, path::Path, - process::{Child, Command}, + process::{Child, Command, Output, Stdio}, }, }; +thread_local! { + static CHILDREN: RefCell> = RefCell::default(); +} + +pub struct ChildStatus { + id: &'static str, + stdout: String, + stderr: String, + exit_code: i32, +} + +impl ChildStatus { + pub fn new(id: &'static str, stdout: String, stderr: String, exit_code: i32) -> Self { + Self { + id, + stdout, + stderr, + exit_code, + } + } + pub fn print_logs(&self) { + println!( + "######################### LOGS FROM {} #########################", + self.id + ); + println!("-> exit code: `{}`", self.exit_code); + if !self.stdout.is_empty() { + println!("+++++++++++++++++++++ STDOUT +++++++++++++++++++++"); + println!("{}", self.stdout); + println!("++++++++++++++++++++++++++++++++++++++++++++++++++"); + } + if !self.stderr.is_empty() { + println!("+++++++++++++++++++++ STDERR +++++++++++++++++++++"); + println!("{}", self.stderr); + println!("++++++++++++++++++++++++++++++++++++++++++++++++++"); + } + println!("######################### ############ #########################"); + } +} + +pub fn get_children() -> Vec { + CHILDREN.with(|c| { + let mut ret = vec![]; + for (name, child) in c.borrow_mut().drain(..) { + let Output { + status, + stdout, + stderr, + } = child.wait_with_output().unwrap(); + ret.push(ChildStatus::new( + name, + String::from_utf8(stdout).unwrap(), + String::from_utf8(stderr).unwrap(), + status.code().unwrap(), + )) + } + ret + }) +} + #[cfg(windows)] /// The powershell script hack to send CTRL+C using kernel32 const POWERSHELL_SCRIPT: &str = include_str!("../../../ci/windows/stop.ps1"); @@ -69,6 +130,8 @@ pub fn get_run_server_cmd(server_id: &'static str, target_folder: impl AsRef HarnessResult<()> { } /// Start the servers returning handles to the child processes -fn start_servers(target_folder: impl AsRef) -> HarnessResult> { - let mut ret = Vec::with_capacity(SERVERS.len()); +fn start_servers(target_folder: impl AsRef) -> HarnessResult<()> { for (server_id, _ports) in SERVERS { let cmd = get_run_server_cmd(server_id, target_folder.as_ref()); info!("Starting {server_id} ..."); - ret.push(util::get_child(format!("start {server_id}"), cmd)?); + let child = util::get_child(format!("start {server_id}"), cmd)?; + CHILDREN.with(|c| c.borrow_mut().push((server_id, child))); } wait_for_startup()?; - Ok(ret) + Ok(()) } pub(super) fn run_with_servers( @@ -191,14 +254,12 @@ pub(super) fn run_with_servers( run_what: impl FnOnce() -> HarnessResult<()>, ) -> HarnessResult<()> { info!("Starting servers ..."); - let children = start_servers(target_folder.as_ref())?; + start_servers(target_folder.as_ref())?; run_what()?; if kill_servers_when_done { kill_servers()?; wait_for_shutdown()?; } - // just use this to avoid ignoring the children vector - assert_eq!(children.len(), SERVERS.len()); Ok(()) } diff --git a/server/src/engine/core/dcl.rs b/server/src/engine/core/dcl.rs index cfb3356f..d3e3476e 100644 --- a/server/src/engine/core/dcl.rs +++ b/server/src/engine/core/dcl.rs @@ -29,7 +29,7 @@ use crate::engine::{ error::{QueryError, QueryResult}, fractal::GlobalInstanceLike, net::protocol::ClientLocalState, - ql::dcl::{SysctlCommand, UserAdd, UserDel}, + ql::dcl::{SysctlCommand, UserDecl, UserDel}, }; const KEY_PASSWORD: &str = "password"; @@ -45,22 +45,41 @@ pub fn exec( match cmd { SysctlCommand::CreateUser(new) => create_user(&g, new), SysctlCommand::DropUser(drop) => drop_user(&g, current_user, drop), + SysctlCommand::AlterUser(usermod) => alter_user(&g, current_user, usermod), SysctlCommand::ReportStatus => Ok(()), } } -fn create_user(global: &impl GlobalInstanceLike, mut user_add: UserAdd<'_>) -> QueryResult<()> { - let username = user_add.username().to_owned(); - let password = match user_add.options_mut().remove(KEY_PASSWORD) { +fn alter_user( + global: &impl GlobalInstanceLike, + cstate: &ClientLocalState, + user: UserDecl, +) -> QueryResult<()> { + if cstate.is_root() { + // the root password can only be changed by shutting down the server + return Err(QueryError::SysAuthError); + } + let (username, password) = get_user_data(user)?; + global.sys_store().alter_user(username, password) +} + +fn create_user(global: &impl GlobalInstanceLike, user: UserDecl) -> QueryResult<()> { + let (username, password) = get_user_data(user)?; + global.sys_store().create_new_user(username, password) +} + +fn get_user_data(mut user: UserDecl) -> Result<(String, String), QueryError> { + let username = user.username().to_owned(); + let password = match user.options_mut().remove(KEY_PASSWORD) { Some(DictEntryGeneric::Data(d)) - if d.kind() == TagClass::Str && user_add.options().is_empty() => + if d.kind() == TagClass::Str && user.options().is_empty() => unsafe { d.into_str().unwrap_unchecked() }, None | Some(_) => { // invalid properties return Err(QueryError::QExecDdlInvalidProperties); } }; - global.sys_store().create_new_user(username, password) + Ok((username, password)) } fn drop_user( diff --git a/server/src/engine/fractal/sys_store.rs b/server/src/engine/fractal/sys_store.rs index 5dd7ac44..f20820fa 100644 --- a/server/src/engine/fractal/sys_store.rs +++ b/server/src/engine/fractal/sys_store.rs @@ -183,6 +183,23 @@ impl SystemStore { Entry::Occupied(_) => Err(QueryError::SysAuthError), } } + pub fn alter_user(&self, username: String, password: String) -> QueryResult<()> { + let mut auth = self.system_store().auth_data().write(); + match auth.users.get_mut(username.as_str()) { + Some(user) => { + let last_pass_hash = core::mem::replace( + &mut user.key, + rcrypt::hash(password, rcrypt::DEFAULT_COST) + .unwrap() + .into_boxed_slice(), + ); + self._try_sync_or(&mut auth, |auth| { + auth.users.get_mut(username.as_str()).unwrap().key = last_pass_hash; + }) + } + None => Err(QueryError::SysAuthError), + } + } pub fn drop_user(&self, username: &str) -> QueryResult<()> { let mut auth = self.system_store().auth_data().write(); if username == SysAuthUser::USER_ROOT { diff --git a/server/src/engine/ql/dcl.rs b/server/src/engine/ql/dcl.rs index b04737c3..a8ddcba2 100644 --- a/server/src/engine/ql/dcl.rs +++ b/server/src/engine/ql/dcl.rs @@ -25,23 +25,23 @@ */ use crate::engine::{ - data::{ - tag::{DataTag, TagClass}, - DictGeneric, - }, + data::DictGeneric, error::{QueryError, QueryResult}, ql::{ ast::{traits, QueryData, State}, ddl::syn, + lex::Ident, }, }; #[derive(Debug, PartialEq)] pub enum SysctlCommand<'a> { /// `sysctl create user ...` - CreateUser(UserAdd<'a>), + CreateUser(UserDecl<'a>), /// `sysctl drop user ...` DropUser(UserDel<'a>), + /// `systcl alter user ...` + AlterUser(UserDecl<'a>), /// `sysctl status` ReportStatus, } @@ -62,16 +62,19 @@ impl<'a> traits::ASTNode<'a> for SysctlCommand<'a> { return Err(QueryError::QLUnexpectedEndOfStatement); } let (a, b) = (state.fw_read(), state.fw_read()); + let alter = Token![alter].eq(a) & b.ident_eq("user"); let create = Token![create].eq(a) & b.ident_eq("user"); let drop = Token![drop].eq(a) & b.ident_eq("user"); let status = a.ident_eq("report") & b.ident_eq("status"); - if !(create | drop | status) { + if !(create | drop | status | alter) { return Err(QueryError::QLUnknownStatement); } if create { - UserAdd::parse(state).map(SysctlCommand::CreateUser) + UserDecl::parse(state).map(SysctlCommand::CreateUser) } else if drop { UserDel::parse(state).map(SysctlCommand::DropUser) + } else if alter { + UserDecl::parse(state).map(SysctlCommand::AlterUser) } else { Ok(SysctlCommand::ReportStatus) } @@ -89,7 +92,7 @@ fn parse<'a, Qd: QueryData<'a>>(state: &mut State<'a, Qd>) -> QueryResult>(state: &mut State<'a, Qd>) -> QueryResult { - username: &'a str, + username: Ident<'a>, options: DictGeneric, } #[derive(Debug, PartialEq)] -pub struct UserAdd<'a> { - username: &'a str, +pub struct UserDecl<'a> { + username: Ident<'a>, options: DictGeneric, } -impl<'a> UserAdd<'a> { - pub(in crate::engine::ql) fn new(username: &'a str, options: DictGeneric) -> Self { +impl<'a> UserDecl<'a> { + pub(in crate::engine::ql) fn new(username: Ident<'a>, options: DictGeneric) -> Self { Self { username, options } } - /// Parse a `user add` DCL command - /// - /// MUSTENDSTREAM: YES pub fn parse>(state: &mut State<'a, Qd>) -> QueryResult { parse(state).map(|UserMeta { username, options }: UserMeta| Self::new(username, options)) } pub fn username(&self) -> &str { - self.username + self.username.as_str() } pub fn options_mut(&mut self) -> &mut DictGeneric { &mut self.options @@ -150,33 +146,28 @@ impl<'a> UserAdd<'a> { #[derive(Debug, PartialEq)] pub struct UserDel<'a> { - username: &'a str, + username: Ident<'a>, } impl<'a> UserDel<'a> { - pub(in crate::engine::ql) fn new(username: &'a str) -> Self { + pub(in crate::engine::ql) fn new(username: Ident<'a>) -> Self { Self { username } } /// Parse a `user del` DCL command /// /// MUSTENDSTREAM: YES pub fn parse>(state: &mut State<'a, Qd>) -> QueryResult { - if state.can_read_lit_rounded() & (state.remaining() == 1) { - let lit = unsafe { + if state.cursor_has_ident_rounded() & (state.remaining() == 1) { + let username = unsafe { // UNSAFE(@ohsayan): +boundck - state.read_cursor_lit_unchecked() + state.read().uck_read_ident() }; state.cursor_ahead(); - if lit.kind().tag_class() == TagClass::Str { - return Ok(Self::new(unsafe { - // UNSAFE(@ohsayan): +tagck - lit.str() - })); - } + return Ok(Self::new(username)); } Err(QueryError::QLInvalidSyntax) } pub fn username(&self) -> &str { - self.username + self.username.as_str() } } diff --git a/server/src/engine/ql/tests/dcl.rs b/server/src/engine/ql/tests/dcl.rs index 44596d99..d7c8a169 100644 --- a/server/src/engine/ql/tests/dcl.rs +++ b/server/src/engine/ql/tests/dcl.rs @@ -39,12 +39,25 @@ fn report_status_simple() { #[test] fn create_user_simple() { - let query = lex_insecure(b"sysctl create user 'sayan' with { password: 'mypass123' }").unwrap(); + let query = lex_insecure(b"sysctl create user sayan with { password: 'mypass123' }").unwrap(); let q = ast::parse_ast_node_full::(&query[1..]).unwrap(); assert_eq!( q, - SysctlCommand::CreateUser(dcl::UserAdd::new( - "sayan", + SysctlCommand::CreateUser(dcl::UserDecl::new( + "sayan".into(), + into_dict!("password" => lit!("mypass123")) + )) + ) +} + +#[test] +fn alter_user_simple() { + let query = lex_insecure(b"sysctl alter user sayan with { password: 'mypass123' }").unwrap(); + let q = ast::parse_ast_node_full::(&query[1..]).unwrap(); + assert_eq!( + q, + SysctlCommand::AlterUser(dcl::UserDecl::new( + "sayan".into(), into_dict!("password" => lit!("mypass123")) )) ) @@ -52,7 +65,10 @@ fn create_user_simple() { #[test] fn delete_user_simple() { - let query = lex_insecure(b"sysctl drop user 'monster'").unwrap(); + let query = lex_insecure(b"sysctl drop user monster").unwrap(); let q = ast::parse_ast_node_full::(&query[1..]).unwrap(); - assert_eq!(q, SysctlCommand::DropUser(dcl::UserDel::new("monster"))); + assert_eq!( + q, + SysctlCommand::DropUser(dcl::UserDel::new("monster".into())) + ); } diff --git a/server/src/engine/tests/client_misc/sec/dcl_sec.rs b/server/src/engine/tests/client_misc/sec/dcl_sec.rs index 9db90b34..961203a3 100644 --- a/server/src/engine/tests/client_misc/sec/dcl_sec.rs +++ b/server/src/engine/tests/client_misc/sec/dcl_sec.rs @@ -57,12 +57,9 @@ fn ensure_sysctl_status_end_of_tokens() { #[dbtest] fn ensure_sysctl_create_user() { let mut db = db!(); + let query = format!("sysctl create user myuser with {{ password: ? }} blah"); assert_err_eq!( - db.query_parse::<()>(&query!( - "sysctl create user ? with { password: ? } blah", - "myuser", - "mypass" - )), + db.query_parse::<()>(&query!(query, "mypass")), Error::ServerError(INVALID_SYNTAX_ERR) ); } diff --git a/sky-macros/src/dbtest.rs b/sky-macros/src/dbtest.rs index 6f81f23b..52c882a7 100644 --- a/sky-macros/src/dbtest.rs +++ b/sky-macros/src/dbtest.rs @@ -230,10 +230,9 @@ pub fn dbtest(attrs: TokenStream, item: TokenStream) -> TokenStream { /// password set by [`sky_macros::dbtest`] (relogin) const __DBTEST_PASS: &str = #new_password; { + let query_assembled = format!("sysctl create user {} with {{ password: ? }}", #new_username); let mut db = skytable::Config::new(#host, #port, #login_username, #login_password).connect().unwrap(); - db.query_parse::<()>( - &skytable::query!("sysctl create user ? with { password: ? }", #new_username, #new_password) - ).unwrap(); + db.query_parse::<()>(&skytable::query!(query_assembled, #new_password)).unwrap(); } }; } @@ -272,10 +271,9 @@ pub fn dbtest(attrs: TokenStream, item: TokenStream) -> TokenStream { ret_block = quote! { #ret_block { + let query_assembled = format!("sysctl drop user {}", #new_username); let mut db = skytable::Config::new(#host, #port, #login_username, #login_password).connect().unwrap(); - db.query_parse::<()>( - &skytable::query!("sysctl drop user ?", #new_username) - ).unwrap(); + db.query_parse::<()>(&skytable::query!(query_assembled)).unwrap(); } }; }