From dd6656f09a79f11f629e964589b7a2d98159c21e Mon Sep 17 00:00:00 2001 From: Mikail Bagishov Date: Thu, 19 Sep 2019 21:54:38 +0300 Subject: [PATCH] Basic access control --- Cargo.lock | 1 + cfg/src/lib.rs | 7 ++ frontend-engine/Cargo.toml | 1 + frontend-engine/src/gql_server.rs | 4 + frontend-engine/src/gql_server/auth.rs | 3 +- frontend-engine/src/gql_server/context.rs | 34 ++++++- frontend-engine/src/gql_server/runs.rs | 13 ++- frontend-engine/src/security.rs | 14 ++- frontend-engine/src/security/access_ck.rs | 49 ++++++++++ frontend-engine/src/security/token.rs | 42 ++++++--- frontend-engine/src/security/token_mgr.rs | 42 +++++++++ frontend-engine/tests/common/mod.rs | 104 +++++++++++++++------- frontend-engine/tests/gql.rs | 62 +++++++++---- frontend-engine/tests/snapshots/run.yaml | 44 --------- 14 files changed, 305 insertions(+), 115 deletions(-) create mode 100644 frontend-engine/src/security/access_ck.rs create mode 100644 frontend-engine/src/security/token_mgr.rs delete mode 100644 frontend-engine/tests/snapshots/run.yaml diff --git a/Cargo.lock b/Cargo.lock index 54b98a0e..182b543d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -866,6 +866,7 @@ dependencies = [ "serde_yaml 0.8.9 (registry+https://github.com/rust-lang/crates.io-index)", "sha3 0.8.2 (registry+https://github.com/rust-lang/crates.io-index)", "slog 2.5.2 (registry+https://github.com/rust-lang/crates.io-index)", + "snafu 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", "toml 0.5.3 (registry+https://github.com/rust-lang/crates.io-index)", "uuid 0.7.4 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/cfg/src/lib.rs b/cfg/src/lib.rs index 8ce1f181..fda12c29 100644 --- a/cfg/src/lib.rs +++ b/cfg/src/lib.rs @@ -191,6 +191,13 @@ impl Config { pub fn find_problem(&self, name: &str) -> Option<&Problem> { self.problems.get(name) } + + pub fn find_contest(&self, name: &str) -> Option<&Contest> { + match name { + "TODO" => Some(&self.contests[0]), + _ => None, + } + } } pub fn parse_file(path: PathBuf) -> Config { diff --git a/frontend-engine/Cargo.toml b/frontend-engine/Cargo.toml index 6d368a5b..87fe7a61 100644 --- a/frontend-engine/Cargo.toml +++ b/frontend-engine/Cargo.toml @@ -23,6 +23,7 @@ serde_json = "1.0.40" branca = "0.9.0" juniper_rocket = "0.4.1" backtrace = "0.3.35" +snafu = "0.5.0" [dev-dependencies] diff --git a/frontend-engine/src/gql_server.rs b/frontend-engine/src/gql_server.rs index dfca493d..4e06aa1d 100644 --- a/frontend-engine/src/gql_server.rs +++ b/frontend-engine/src/gql_server.rs @@ -64,6 +64,10 @@ impl ApiError { s.dev_backtrace(); s } + + pub fn access_denied(ctx: &Context) -> Self { + Self::new(ctx, "AccessDenied") + } } mod impl_display { diff --git a/frontend-engine/src/gql_server/auth.rs b/frontend-engine/src/gql_server/auth.rs index c83dee7f..67c69a38 100644 --- a/frontend-engine/src/gql_server/auth.rs +++ b/frontend-engine/src/gql_server/auth.rs @@ -1,5 +1,4 @@ use super::prelude::*; -use crate::security::Token; pub(super) fn simple( ctx: &Context, @@ -17,7 +16,7 @@ pub(super) fn simple( reject_reason = "UnknownUser"; } if success { - let token = Token::issue_for_user(&login); + let token = ctx.token_mgr.create_token(&login).internal(ctx)?; let buf = token.serialize(&ctx.secret_key); let sess = schema::SessionToken { data: buf, diff --git a/frontend-engine/src/gql_server/context.rs b/frontend-engine/src/gql_server/context.rs index 9174fbe7..0f0b2384 100644 --- a/frontend-engine/src/gql_server/context.rs +++ b/frontend-engine/src/gql_server/context.rs @@ -1,4 +1,4 @@ -use crate::security::TokenFromRequestError; +use crate::security::{AccessChecker, Token, TokenFromRequestError, TokenMgr}; use std::sync::Arc; pub(crate) type DbPool = Arc; @@ -10,6 +10,17 @@ pub(crate) struct ContextData { pub(crate) secret_key: Arc<[u8]>, pub(crate) env: crate::config::Env, pub(crate) logger: slog::Logger, + pub(crate) token_mgr: TokenMgr, + pub(crate) token: Token, +} + +impl ContextData { + pub(crate) fn access(&self) -> AccessChecker { + AccessChecker { + token: &self.token, + cfg: &self.cfg, + } + } } #[derive(Clone)] @@ -41,12 +52,31 @@ impl<'a, 'r> rocket::request::FromRequest<'a, 'r> for ContextData { .guard::>() .expect("State missing"); + let token = request + .headers() + .get("X-Jjs-Auth") + .next() + .ok_or(TokenFromRequestError::Missing); + + let token = token + .and_then(|header| Token::deserialize(&*secret_key, header.as_bytes(), env.is_dev())); + let token = match token { + Ok(tok) => Ok(tok), + Err(e) => match e { + TokenFromRequestError::Missing => Ok(Token::new_guest()), + _ => Err(e), + }, + }; + let token = token.map_err(|e| Err((rocket::http::Status::BadRequest, e)))?; + rocket::Outcome::Success(ContextData { db: factory.pool.clone(), cfg: factory.cfg.clone(), env: *env, secret_key: Arc::clone(&(*secret_key).0), logger: factory.logger.clone(), + token_mgr: TokenMgr::new(factory.pool.clone()), + token, }) } } @@ -78,6 +108,8 @@ impl ContextFactory { secret_key: Arc::new([]), env: crate::config::Env::Dev, logger: self.logger.clone(), + token_mgr: TokenMgr::new(self.pool.clone()), + token: Token::new_root(), } } } diff --git a/frontend-engine/src/gql_server/runs.rs b/frontend-engine/src/gql_server/runs.rs index b2ffd8cc..36686d01 100644 --- a/frontend-engine/src/gql_server/runs.rs +++ b/frontend-engine/src/gql_server/runs.rs @@ -39,12 +39,14 @@ pub(super) fn submit_simple( let toolchain = ctx.cfg.toolchains.iter().find(|t| t.name == toolchain); let toolchain = match toolchain { Some(tc) => tc.clone(), - None => return "unknown toolchain".report(ctx), + None => return Err(ApiError::new(ctx, "ToolchainUnknown")), }; if contest != "TODO" { - return "unknown contest".report(ctx); + return Err(ApiError::new(ctx, "ContestUnknown")); + } + if !ctx.access().user_can_submit(&contest).internal(ctx)? { + return Err(ApiError::access_denied(ctx)); } - let problem = ctx.cfg.contests[0] .problems .iter() @@ -52,7 +54,7 @@ pub(super) fn submit_simple( .cloned(); let problem = match problem { Some(p) => p, - None => return "unknown problem".report(ctx), + None => return Err(ApiError::new(ctx, "ProblemUnknown")), }; let prob_name = problem.name.clone(); @@ -96,6 +98,9 @@ pub(super) fn modify( rejudge: Option, delete: Option, ) -> ApiResult<()> { + if !ctx.access().user_can_modify_run(id).internal(ctx)? { + return Err(ApiError::access_denied(ctx)); + } let should_delete = delete.unwrap_or(false); if should_delete { if status.is_some() || rejudge.is_some() { diff --git a/frontend-engine/src/security.rs b/frontend-engine/src/security.rs index bc1a9c54..40a3a0fd 100644 --- a/frontend-engine/src/security.rs +++ b/frontend-engine/src/security.rs @@ -1,7 +1,19 @@ +mod access_ck; mod token; +mod token_mgr; +pub(crate) use access_ck::AccessChecker; use std::sync::Arc; -pub use token::{Token, TokenFromRequestError}; +pub(crate) use token::{Token, TokenFromRequestError}; +pub(crate) use token_mgr::TokenMgr; #[derive(Clone)] pub struct SecretKey(pub Arc<[u8]>); + +impl std::ops::Deref for SecretKey { + type Target = [u8]; + + fn deref(&self) -> &Self::Target { + &*(self.0) + } +} diff --git a/frontend-engine/src/security/access_ck.rs b/frontend-engine/src/security/access_ck.rs new file mode 100644 index 00000000..77e060bb --- /dev/null +++ b/frontend-engine/src/security/access_ck.rs @@ -0,0 +1,49 @@ +use crate::security::Token; +use snafu::Snafu; + +/// Access check service +pub(crate) struct AccessChecker<'a> { + pub(crate) token: &'a Token, + pub(crate) cfg: &'a cfg::Config, + //TODO: pub(crate) db: &'a dyn db::DbConn +} + +#[derive(Debug, Snafu)] +pub(crate) enum AccessCheckError { + NotFound, +} + +pub(crate) type AccessResult = Result; + +impl AccessChecker<'_> { + pub(crate) fn user_can_submit(&self, contest_id: &str) -> AccessResult { + let contest = self + .cfg + .find_contest(contest_id) + .ok_or(AccessCheckError::NotFound)?; + if self.user_is_contest_sudo(contest_id)? { + return Ok(true); + } + for registered_group in &contest.group { + if self.token.user_info.groups.contains(registered_group) { + return Ok(true); + } + } + Ok(false) + } + + fn is_sudo(&self) -> AccessResult { + // When namespaces are introduced, this function will account for that + Ok(self.token.user_info.name == "Global/Root") + } + + fn user_is_contest_sudo(&self, _contest_id: &str) -> AccessResult { + // TODO + self.is_sudo() + } + + pub(crate) fn user_can_modify_run(&self, _run_id: i32) -> AccessResult { + // TODO + Ok(true) + } +} diff --git a/frontend-engine/src/security/token.rs b/frontend-engine/src/security/token.rs index 92678183..5f0d1dc6 100644 --- a/frontend-engine/src/security/token.rs +++ b/frontend-engine/src/security/token.rs @@ -1,27 +1,19 @@ use serde::{Deserialize, Serialize}; -// TODO #[derive(Serialize, Deserialize, Debug, Clone)] pub struct UserInfo { - name: String, - groups: Vec, + /// TODO: name should have hierarchical type + pub(super) name: String, + pub(super) groups: Vec, } +/// Struct representing API session #[derive(Serialize, Deserialize, Debug, Clone)] pub struct Token { - user_info: UserInfo, + pub(super) user_info: UserInfo, } impl Token { - pub fn issue_for_user(user_name: &str) -> Token { - Token { - user_info: UserInfo { - name: user_name.to_string(), - groups: Vec::new(), - }, - } - } - pub fn issue_for_virtual_user(name: String, groups: Vec) -> Token { Token { user_info: UserInfo { name, groups }, @@ -54,6 +46,30 @@ impl Token { rand_gen.fill(&mut nonce); branca::encode(&ser, key, 0).expect("Token encoding error") } + + pub fn deserialize( + key: &[u8], + data: &[u8], + allow_dev: bool, + ) -> Result { + let data = match std::str::from_utf8(data) { + Ok(d) => d, + Err(_) => return Err(TokenFromRequestError::BadFormat), + }; + if allow_dev && data.starts_with("dev_") { + let data = data.trim_start_matches("dev_"); + if data == "root" { + return Ok(Token::new_root()); + } + return Err(TokenFromRequestError::BadFormat); + } + let token_data = match branca::decode(data, key, 0) { + Ok(s) => s, + Err(err) => return Err(TokenFromRequestError::Branca(err)), + }; + let res = serde_json::from_str(&token_data).expect("Token decoding error"); + Ok(res) + } } #[derive(Debug)] diff --git a/frontend-engine/src/security/token_mgr.rs b/frontend-engine/src/security/token_mgr.rs new file mode 100644 index 00000000..2ff7b469 --- /dev/null +++ b/frontend-engine/src/security/token_mgr.rs @@ -0,0 +1,42 @@ +use crate::security::{token::UserInfo, Token}; +use snafu::Snafu; +use std::sync::Arc; + +/// Token Manager - entity manipulating tokens +pub(crate) struct TokenMgr { + db: Arc, +} + +#[derive(Debug, Snafu)] +pub enum TokenMgrError { + #[snafu(display("db error: {}", source))] + Db { source: db::Error }, + #[snafu(display("user not exists"))] + UserMissing, +} + +impl From for TokenMgrError { + fn from(source: db::Error) -> Self { + Self::Db { source } + } +} + +impl TokenMgr { + pub fn new(db: Arc) -> Self { + Self { db } + } + + // TODO: use custom errors + pub fn create_token(&self, username: &str) -> Result { + let user_data = self + .db + .user_try_load_by_login(username)? + .ok_or(TokenMgrError::UserMissing)?; + Ok(Token { + user_info: UserInfo { + name: user_data.username, + groups: user_data.groups, + }, + }) + } +} diff --git a/frontend-engine/tests/common/mod.rs b/frontend-engine/tests/common/mod.rs index dd105dfa..d8730d9e 100644 --- a/frontend-engine/tests/common/mod.rs +++ b/frontend-engine/tests/common/mod.rs @@ -79,39 +79,82 @@ pub struct Env { client: rocket::local::Client, } -impl Env { - pub fn new(name: &str) -> Self { - EnvBuilder::new().build(name) +pub struct RequestBuilder<'a> { + vars: Option, + auth_token: Option, + operation: Option, + client: &'a rocket::local::Client, +} + +impl RequestBuilder<'_> { + pub fn vars(&mut self, v: &serde_json::Value) -> &mut Self { + assert!(v.is_object()); + self.vars = Some(v.clone()); + self + } + + pub fn operation(&mut self, op: &str) -> &mut Self { + self.operation = Some(op.to_string()); + self } - pub fn exec(&self, operation: &str, vars: &serde_json::Value) -> serde_json::Value { + pub fn exec(&self) -> Response { let obj = serde_json::json!({ - "query": operation, - "variables": vars, + "query": self.operation.as_ref().unwrap(), + "variables": self.vars.clone().unwrap_or_else(||serde_json::Value::Null), }); let body = serde_json::to_string(&obj).unwrap(); let request = self .client .post("/graphql") .body(body) + .header(rocket::http::Header::new( + "X-Jjs-Auth", + self.auth_token + .clone() + .unwrap_or_else(|| "dev_root".to_string()) + .to_string(), + )) .header(rocket::http::ContentType::JSON); let mut response = request.dispatch(); - assert_eq!(response.status(), rocket::http::Status::Ok); + if response.status() != rocket::http::Status::Ok { + eprintln!("Frontend returned non-200: {:?}", response.status()); + eprintln!("Response: {}", response.body_string().unwrap_or_default()); + panic!() + } assert_eq!( response.content_type(), Some("application/json".parse().unwrap()) ); let body = response.body_string().unwrap(); let body: serde_json::Value = serde_json::from_str(&body).unwrap(); - body + Response(body) + } +} + +#[derive(Debug, Clone)] +pub struct Response(serde_json::Value); + +impl std::ops::Deref for Response { + type Target = serde_json::Value; + + fn deref(&self) -> &Self::Target { + &self.0 } +} - fn check_ok(res: serde_json::Value) -> serde_json::Value { - if util::is_ok(&res) { - res.get("data").unwrap().clone() +impl Response { + pub fn is_ok(&self) -> bool { + self.0.get("errors").is_none() + } + + pub fn unwrap_ok(self) -> serde_json::Value { + if self.is_ok() { + self.0.get("data").unwrap().clone() } else { - let errs = res + let errs = self + .0 .get("errors") .expect("errors missing on failed request") .as_array() @@ -129,36 +172,35 @@ impl Env { } } - pub fn exec_ok(&self, req: &str) -> serde_json::Value { - let res = self.exec(req, &serde_json::Value::Object(Default::default())); - Self::check_ok(res) - } - - pub fn exec_ok_with_vars(&self, req: &str, vars: &serde_json::Value) -> serde_json::Value { - assert!(vars.is_object()); - let res = self.exec(req, vars); - Self::check_ok(res) - } - - pub fn exec_err(&self, req: &str) -> Vec { - let res = self.exec(req, &serde_json::Value::Object(Default::default())); - if util::is_ok(&res) { + pub fn unwrap_errs(self) -> Vec { + if self.is_ok() { eprintln!("Error: query with fail=true succeeded"); - eprintln!("Response: \n{:?}", res); + eprintln!("Response: \n{:?}", self.0); panic!("Operation succeeded unexpectedly"); } else { - let errs = res.get("errors").unwrap().as_array().unwrap(); + let errs = self.0.get("errors").unwrap().as_array().unwrap(); assert!(!errs.is_empty()); errs.clone() } } } -pub mod util { - pub fn is_ok(res: &serde_json::Value) -> bool { - res.get("errors").is_none() +impl Env { + pub fn new(name: &str) -> Self { + EnvBuilder::new().build(name) } + pub fn req(&self) -> RequestBuilder { + RequestBuilder { + vars: None, + auth_token: None, + operation: None, + client: &self.client, + } + } +} + +pub mod util { pub fn print_error(err: &serde_json::Value) { let mut err = err.as_object().unwrap().clone(); let ext = err.remove("extensions"); diff --git a/frontend-engine/tests/gql.rs b/frontend-engine/tests/gql.rs index 0068ceee..cb337660 100644 --- a/frontend-engine/tests/gql.rs +++ b/frontend-engine/tests/gql.rs @@ -8,13 +8,17 @@ use serde_json::json; #[test] fn test_smoke_ops() { let env = common::Env::new("SmokeOps"); - let res = env.exec_ok( - " + let res = env + .req() + .operation( + " query GetApiVersion { apiVersion } ", - ); + ) + .exec() + .unwrap_ok(); assert_eq!( res, json!({ @@ -27,15 +31,19 @@ query GetApiVersion { #[test] fn test_user_ops() { let env = common::Env::new("UserOps"); - let res = env.exec_ok( - r#" + let res = env + .req() + .operation( + r#" mutation CreateAUser { createUser(login: "JonSnow", password: "VerySecretPass", groups: []) { login } } "#, - ); + ) + .exec() + .unwrap_ok(); assert_eq!( res, json!({ @@ -45,15 +53,19 @@ mutation CreateAUser { }) ); - let res = env.exec_err( - r#" + let res = env + .req() + .operation( + r#" mutation CreateSameUserAgain { createUser(login: "JonSnow", password: "VerySecretPass", groups: []) { login } } "#, - ); + ) + .exec() + .unwrap_errs(); assert_eq!(res.len(), 1); util::check_error(&res[0], "UserAlreadyExists"); } @@ -73,15 +85,19 @@ fn test_runs_ops() { }) .build("runs_ops"); - let res = env.exec_ok( - r#" + let res = env + .req() + .operation( + r#" query GetNonExistingRun { runs(id: 0) { id } } "#, - ); + ) + .exec() + .unwrap_ok(); assert_eq!( res, json!({ @@ -99,16 +115,20 @@ int main() { "#; let run_encoded = base64::encode(RUN_TEXT); - let res = env.exec_ok_with_vars( - r#" + let res = env + .req() + .operation( + r#" mutation CreateRun($runCode: String!) { submitSimple(toolchain: "cpp", runCode: $runCode, problem: "A", contest: "TODO") { id } } "#, - &json!({ "runCode": &run_encoded }), - ); + ) + .vars(&json!({ "runCode": &run_encoded })) + .exec() + .unwrap_ok(); assert_eq!( res, json!({ @@ -118,15 +138,19 @@ mutation CreateRun($runCode: String!) { }) ); - let res = env.exec_ok( - r#" + let res = env + .req() + .operation( + r#" query GetRun { runs(id: 0) { source } } "#, - ); + ) + .exec() + .unwrap_ok(); let res = res .get("runs") .unwrap() diff --git a/frontend-engine/tests/snapshots/run.yaml b/frontend-engine/tests/snapshots/run.yaml deleted file mode 100644 index 75f01332..00000000 --- a/frontend-engine/tests/snapshots/run.yaml +++ /dev/null @@ -1,44 +0,0 @@ -description: | - -env: - toolchains: - cpp: | - title = "C++" - filename = "source.cpp" - - [[build]] - argv = ["not", "matters"] - - [run] - argv = ["not", "matters"] - -steps: - - query: | - query GetNonExistingRun { - runs(id: 0) { - toolchain { - name - } - } - } - res: - runs: [] - - eval: - - var: runCode - fn: base64::encode - data: | - - query: | - - res: - submitSimple: - id: 0 - - query: | - query GetRun { - runs(id: 0) { - source - } - } - res: - runs: - - source: I2luY2x1ZGUgPGNzdGRpbz4KCmludCBtYWluKCkgewogICAgaW50IGEsIGI7CiAgICBzY2FuZigiJWQgJWQiLCAmYSwgJmIpOwogICAgcHJpbnRmKCIlZCIsIGEgKyBiKTsKfQo= \ No newline at end of file