Improved error messagees, implemented delete ciphers, attachments and account, implemented two factor recovery.

Known missing:
 - import ciphers, create ciphers types other than login and card, update ciphers
 - clear and put device_tokens
 - Equivalent domains
 - Organizations
This commit is contained in:
Daniel García 2018-02-15 19:05:57 +01:00
parent 47a116bbee
commit 84a75c871b
15 changed files with 181 additions and 192 deletions

View File

@ -20,7 +20,7 @@ on other systems use their respective package managers.
Then run:
```
cargo run
cargo run --bin bitwarden_rs
# or
cargo build
```

View File

@ -1,10 +0,0 @@
## Docker Compose file, experimental and untested
# Run 'docker compose up' to start the service
version: '3'
services:
web:
build: .
ports:
- "8000:80"
volumes:
- ./data:/data

View File

@ -144,11 +144,23 @@ fn delete_account(data: Json<Value>, headers: Headers, conn: DbConn) -> Result<(
err!("Invalid password")
}
// Delete all ciphers by user_uuid
// Delete all devices by user_uuid
// Delete user
// Delete ciphers and their attachments
for cipher in Cipher::find_by_user(&user.uuid, &conn) {
for a in Attachment::find_by_cipher(&cipher.uuid, &conn) { a.delete(&conn); }
err!("Not implemented")
cipher.delete(&conn);
}
// Delete folders
for f in Folder::find_by_user(&user.uuid, &conn) { f.delete(&conn); }
// Delete devices
for d in Device::find_by_user(&user.uuid, &conn) { d.delete(&conn); }
// Delete user
user.delete(&conn);
Ok(())
}
#[get("/accounts/revision-date")]

View File

@ -258,11 +258,7 @@ fn delete_attachment(uuid: String, attachment_id: String, headers: Headers, conn
err!("Cipher is not owned by user")
}
// Delete file
let file = attachment.get_file_path();
util::delete_file(&file);
// Delete entry in cipher
// Delete attachment
attachment.delete(&conn);
Ok(())
@ -274,13 +270,32 @@ fn post_cipher(uuid: String, headers: Headers, conn: DbConn) -> Result<Json, Bad
}
#[put("/ciphers/<uuid>")]
fn put_cipher(uuid: String, headers: Headers, conn: DbConn) -> Result<Json, BadRequest<Json>> { err!("Not implemented") }
fn put_cipher(uuid: String, headers: Headers, conn: DbConn) -> Result<Json, BadRequest<Json>> {
err!("Not implemented")
}
#[delete("/ciphers/<uuid>")]
fn delete_cipher(uuid: String, headers: Headers, conn: DbConn) -> Result<Json, BadRequest<Json>> { err!("Not implemented") }
fn delete_cipher(uuid: String, headers: Headers, conn: DbConn) -> Result<(), BadRequest<Json>> {
let cipher = match Cipher::find_by_uuid(&uuid, &conn) {
Some(cipher) => cipher,
None => err!("Cipher doesn't exist")
};
if cipher.user_uuid != headers.user.uuid {
err!("Cipher is not owned by user")
}
// Delete attachments
for a in Attachment::find_by_cipher(&cipher.uuid, &conn) { a.delete(&conn); }
// Delete cipher
cipher.delete(&conn);
Ok(())
}
#[post("/ciphers/delete", data = "<data>")]
fn delete_all(data: Json<Value>, headers: Headers, conn: DbConn) -> Result<Json, BadRequest<Json>> {
fn delete_all(data: Json<Value>, headers: Headers, conn: DbConn) -> Result<(), BadRequest<Json>> {
let password_hash = data["masterPasswordHash"].as_str().unwrap();
let user = headers.user;
@ -289,7 +304,15 @@ fn delete_all(data: Json<Value>, headers: Headers, conn: DbConn) -> Result<Json,
err!("Invalid password")
}
// Cipher::delete_from_user(&conn);
// Delete ciphers and their attachments
for cipher in Cipher::find_by_user(&user.uuid, &conn) {
for a in Attachment::find_by_cipher(&cipher.uuid, &conn) { a.delete(&conn); }
err!("Not implemented")
cipher.delete(&conn);
}
// Delete folders
for f in Folder::find_by_user(&user.uuid, &conn) { f.delete(&conn); }
Ok(())
}

View File

@ -43,6 +43,7 @@ pub fn routes() -> Vec<Route> {
get_twofactor,
get_recover,
recover,
generate_authenticator,
activate_authenticator,
disable_authenticator,
@ -107,8 +108,7 @@ fn post_eq_domains(data: Json<EquivDomainData>, headers: Headers, conn: DbConn)
let user = headers.user;
//BODY. "{\"ExcludedGlobalEquivalentDomains\":[2],\"EquivalentDomains\":[[\"uoc.edu\",\"uoc.es\"]]}"
//BODY. "{\"ExcludedGlobalEquivalentDomains\":[2],\"EquivalentDomains\":[[\"example.org\",\"example.net\"]]}"
err!("Not implemented")
}

View File

@ -44,6 +44,39 @@ fn get_recover(data: Json<Value>, headers: Headers) -> Result<Json, BadRequest<J
})))
}
#[post("/two-factor/recover", data = "<data>")]
fn recover(data: Json<Value>, conn: DbConn) -> Result<Json, BadRequest<Json>> {
println!("{:#?}", data);
use db::models::User;
// Get the user
let username = data["email"].as_str().unwrap();
let mut user = match User::find_by_mail(username, &conn) {
Some(user) => user,
None => err!("Username or password is incorrect. Try again.")
};
// Check password
let password = data["masterPasswordHash"].as_str().unwrap();
if !user.check_valid_password(password) {
err!("Username or password is incorrect. Try again.")
}
// Check if recovery code is correct
let recovery_code = data["recoveryCode"].as_str().unwrap();
if !user.check_valid_recovery_code(recovery_code) {
err!("Recovery code is incorrect. Try again.")
}
user.totp_secret = None;
user.totp_recover = None;
user.save(&conn);
Ok(Json(json!({})))
}
#[post("/two-factor/get-authenticator", data = "<data>")]
fn generate_authenticator(data: Json<Value>, headers: Headers) -> Result<Json, BadRequest<Json>> {
let password_hash = data["masterPasswordHash"].as_str().unwrap();
@ -71,8 +104,8 @@ fn activate_authenticator(data: Json<Value>, headers: Headers, conn: DbConn) ->
if !headers.user.check_valid_password(password_hash) {
err!("Invalid password");
}
let token = data["token"].as_str(); // 123456
let key = data["key"].as_str().unwrap(); // YI4SKBIXG32LOA6VFKH2NI25VU3E4QML
let token = data["token"].as_str();
let key = data["key"].as_str().unwrap();
// Validate key as base32 and 20 bytes length
let decoded_key: Vec<u8> = match BASE32.decode(key.as_bytes()) {

View File

@ -71,7 +71,7 @@ fn get_icon_cached(key: &str, url: &str) -> io::Result<Vec<u8>> {
// Save the currently downloaded icon
match File::create(path) {
Ok(mut f) => { f.write_all(&icon); }
Ok(mut f) => { f.write_all(&icon).expect("Error writing icon file"); }
Err(_) => { /* Continue */ }
};

View File

@ -41,61 +41,44 @@ fn login(connect_data: Form<ConnectData>, conn: DbConn) -> Result<Json, BadReque
let username = data.get("username").unwrap();
let user = match User::find_by_mail(username, &conn) {
Some(user) => user,
None => err!("Invalid username or password")
None => err!("Username or password is incorrect. Try again.")
};
// Check password
let password = data.get("password").unwrap();
if !user.check_valid_password(password) {
err!("Invalid username or password")
err!("Username or password is incorrect. Try again.")
}
/*
//TODO: When invalid username or password, return this with a 400 BadRequest:
{
"error": "invalid_grant",
"error_description": "invalid_username_or_password",
"ErrorModel": {
"Message": "Username or password is incorrect. Try again.",
"ValidationErrors": null,
"ExceptionMessage": null,
"ExceptionStackTrace": null,
"InnerExceptionMessage": null,
"Object": "error"
}
}
*/
// Check if totp code is required and the value is correct
let totp_code = util::parse_option_string(data.get("twoFactorToken").map(String::as_ref));
let totp_code = util::parse_option_string(data.get("twoFactorToken"));
if !user.check_totp_code(totp_code) {
// Return error 400
err_json!(json!({
"error" : "invalid_grant",
"error_description" : "Two factor required.",
"TwoFactorProviders" : [ 0 ],
"TwoFactorProviders2" : { "0" : null }
}))
"error" : "invalid_grant",
"error_description" : "Two factor required.",
"TwoFactorProviders" : [ 0 ],
"TwoFactorProviders2" : { "0" : null }
}))
}
// Let's only use the header and ignore the 'devicetype' parameter
// TODO Get header Device-Type
let device_type_num = 0;// headers.device_type;
let (device_id, device_name) = match data.get("client_id").unwrap().as_ref() {
"web" => { (format!("web-{}", user.uuid), String::from("web")) }
"browser" | "mobile" => {
let (device_id, device_name) = match data.is_device {
false => { (format!("web-{}", user.uuid), String::from("web")) }
true => {
(
data.get("deviceidentifier").unwrap().clone(),
data.get("devicename").unwrap().clone(),
)
}
_ => err!("Invalid client id")
};
// Find device or create new
let device = match Device::find_by_uuid(&device_id, &conn) {
match Device::find_by_uuid(&device_id, &conn) {
Some(device) => {
// Check if valid device
if device.user_uuid != user.uuid {
@ -109,10 +92,7 @@ fn login(connect_data: Form<ConnectData>, conn: DbConn) -> Result<Json, BadReque
// Create new device
Device::new(device_id, user.uuid, device_name, device_type_num)
}
};
device
}
}
};
@ -120,7 +100,6 @@ fn login(connect_data: Form<ConnectData>, conn: DbConn) -> Result<Json, BadReque
let (access_token, expires_in) = device.refresh_tokens(&user);
device.save(&conn);
// TODO: when to include :privateKey and :TwoFactorToken?
Ok(Json(json!({
"access_token": access_token,
"expires_in": expires_in,
@ -134,27 +113,22 @@ fn login(connect_data: Form<ConnectData>, conn: DbConn) -> Result<Json, BadReque
#[derive(Debug)]
struct ConnectData {
grant_type: GrantType,
is_device: bool,
data: HashMap<String, String>,
}
#[derive(Debug, Copy, Clone)]
enum GrantType { RefreshToken, Password }
impl ConnectData {
fn get(&self, key: &str) -> Option<&String> {
self.data.get(&key.to_lowercase())
}
}
#[derive(Debug, Copy, Clone)]
enum GrantType { RefreshToken, Password }
const VALUES_REFRESH: [&str; 1] = ["refresh_token"];
const VALUES_PASSWORD: [&str; 5] = ["client_id",
"grant_type", "password", "scope", "username"];
const VALUES_DEVICE: [&str; 3] = ["deviceidentifier",
"devicename", "devicetype"];
const VALUES_PASSWORD: [&str; 5] = ["client_id", "grant_type", "password", "scope", "username"];
const VALUES_DEVICE: [&str; 3] = ["deviceidentifier", "devicename", "devicetype"];
impl<'f> FromForm<'f> for ConnectData {
type Error = String;
@ -164,62 +138,40 @@ impl<'f> FromForm<'f> for ConnectData {
// Insert data into map
for (key, value) in items {
let decoded_key: String = match key.url_decode() {
Ok(decoded) => decoded,
Err(_) => return Err(format!("Error decoding key: {}", value)),
match (key.url_decode(), value.url_decode()) {
(Ok(key), Ok(value)) => data.insert(key.to_lowercase(), value),
_ => return Err(format!("Error decoding key or value")),
};
let decoded_value: String = match value.url_decode() {
Ok(decoded) => decoded,
Err(_) => return Err(format!("Error decoding value: {}", value)),
};
data.insert(decoded_key.to_lowercase(), decoded_value);
}
// Validate needed values
let grant_type =
match data.get("grant_type").map(|s| &s[..]) {
let (grant_type, is_device) =
match data.get("grant_type").map(String::as_ref) {
Some("refresh_token") => {
// Check if refresh token is proviced
if let Err(msg) = check_values(&data, &VALUES_REFRESH) {
return Err(msg);
}
GrantType::RefreshToken
check_values(&data, &VALUES_REFRESH)?;
(GrantType::RefreshToken, false) // Device doesn't matter here
}
Some("password") => {
// Check if basic values are provided
if let Err(msg) = check_values(&data, &VALUES_PASSWORD) {
return Err(msg);
}
check_values(&data, &VALUES_PASSWORD)?;
// Check that device values are present on device
match data.get("client_id").unwrap().as_ref() {
"browser" | "mobile" => {
if let Err(msg) = check_values(&data, &VALUES_DEVICE) {
return Err(msg);
}
}
_ => {}
}
GrantType::Password
let is_device = match data.get("client_id").unwrap().as_ref() {
"browser" | "mobile" => check_values(&data, &VALUES_DEVICE)?,
_ => false
};
(GrantType::Password, is_device)
}
_ => return Err(format!("Grant type not supported"))
};
Ok(ConnectData { grant_type, data })
Ok(ConnectData { grant_type, is_device, data })
}
}
fn check_values(map: &HashMap<String, String>, values: &[&str]) -> Result<(), String> {
fn check_values(map: &HashMap<String, String>, values: &[&str]) -> Result<bool, String> {
for value in values {
if !map.contains_key(*value) {
return Err(format!("{} cannot be blank", value));
}
}
Ok(())
Ok(true)
}

View File

@ -93,7 +93,6 @@ use db::DbConn;
use db::models::{User, Device};
pub struct Headers {
pub device_type: Option<i32>,
pub host: String,
pub device: Device,
pub user: User,
@ -105,29 +104,19 @@ impl<'a, 'r> FromRequest<'a, 'r> for Headers {
fn from_request(request: &'a Request<'r>) -> request::Outcome<Self, Self::Error> {
let headers = request.headers();
// Get device type
let device_type = match headers.get_one("Device-Type")
.map(|s| s.parse::<i32>()) {
Some(Ok(dt)) => Some(dt),// dt,
_ => None // return err_handler!("Device-Type is invalid or missing")
};
// Get host
let host = match headers.get_one("Host") {
Some(host) => format!("http://{}", host), // TODO: Check if HTTPS
_ => String::new() // return err_handler!("Host is invalid or missing")
_ => String::new()
};
// Get access_token
let access_token: &str = match request.headers().get_one("Authorization") {
Some(a) => {
let split: Option<&str> = a.rsplit("Bearer ").next();
if split.is_none() {
err_handler!("No access token provided")
match a.rsplit("Bearer ").next() {
Some(split) => split,
None => err_handler!("No access token provided")
}
split.unwrap()
}
None => err_handler!("No access token provided")
};
@ -135,10 +124,7 @@ impl<'a, 'r> FromRequest<'a, 'r> for Headers {
// Check JWT token is valid and get device and user from it
let claims: JWTClaims = match decode_jwt(access_token) {
Ok(claims) => claims,
Err(msg) => {
println!("Invalid claim: {}", msg);
err_handler!("Invalid claim")
}
Err(msg) => err_handler!("Invalid claim")
};
let device_uuid = claims.device;
@ -163,6 +149,6 @@ impl<'a, 'r> FromRequest<'a, 'r> for Headers {
err_handler!("Invalid security stamp")
}
Outcome::Success(Headers { device_type, host, device, user })
Outcome::Success(Headers { host, device, user })
}
}

View File

@ -63,6 +63,10 @@ impl Attachment {
}
pub fn delete(self, conn: &DbConn) -> bool {
use util;
util::delete_file(&self.get_file_path());
match diesel::delete(attachments::table.filter(
attachments::id.eq(self.id)))
.execute(&**conn) {

View File

@ -113,4 +113,10 @@ impl Device {
.filter(devices::refresh_token.eq(refresh_token))
.first::<Self>(&**conn).ok()
}
pub fn find_by_user(user_uuid: &str, conn: &DbConn) -> Vec<Self> {
devices::table
.filter(devices::user_uuid.eq(user_uuid))
.load::<Self>(&**conn).expect("Error loading devices")
}
}

View File

@ -3,6 +3,7 @@ use serde_json::Value as JsonValue;
use uuid::Uuid;
use crypto;
use CONFIG;
#[derive(Debug, Identifiable, Queryable, Insertable)]
@ -38,8 +39,6 @@ impl User {
let now = Utc::now().naive_utc();
let email = mail.to_lowercase();
use crypto;
let iterations = CONFIG.password_iterations;
let salt = crypto::get_random_64();
let password_hash = crypto::hash_password(password.as_bytes(), &salt, iterations as u32);
@ -70,16 +69,21 @@ impl User {
}
pub fn check_valid_password(&self, password: &str) -> bool {
use crypto;
crypto::verify_password_hash(password.as_bytes(),
&self.salt,
&self.password_hash,
self.password_iterations as u32)
}
pub fn check_valid_recovery_code(&self, recovery_code: &str) -> bool {
if let Some(ref totp_recover) = self.totp_recover {
recovery_code == totp_recover.to_lowercase()
} else {
false
}
}
pub fn set_password(&mut self, password: &str) {
use crypto;
self.password_hash = crypto::hash_password(password.as_bytes(),
&self.salt,
self.password_iterations as u32);
@ -149,6 +153,15 @@ impl User {
}
}
pub fn delete(self, conn: &DbConn) -> bool {
match diesel::delete(users::table.filter(
users::uuid.eq(self.uuid)))
.execute(&**conn) {
Ok(1) => true, // One row deleted
_ => false,
}
}
pub fn find_by_mail(mail: &str, conn: &DbConn) -> Option<Self> {
let lower_mail = mail.to_lowercase();
users::table

View File

@ -53,7 +53,6 @@ fn init_rocket() -> Rocket {
.mount("/identity", api::identity_routes())
.mount("/icons", api::icons_routes())
.manage(db::init_pool())
.attach(DebugFairing)
}
// Embed the migrations from the migrations folder into the application
@ -66,7 +65,7 @@ fn main() {
// Make sure the database is up to date (create if it doesn't exist, or run the migrations)
let connection = db::get_connection().expect("Can't conect to DB");
embedded_migrations::run_with_output(&connection, &mut io::stdout());
embedded_migrations::run_with_output(&connection, &mut io::stdout()).expect("Can't run migrations");
// Validate location of rsa keys
if !util::file_exists(&CONFIG.private_rsa_key) {
@ -114,37 +113,3 @@ impl Config {
}
}
}
struct DebugFairing;
impl Fairing for DebugFairing {
fn info(&self) -> Info {
Info {
name: "Request Debugger",
kind: Kind::Request,
}
}
fn on_request(&self, req: &mut Request, data: &Data) {
let uri_string = req.uri().to_string();
// Ignore web requests
if !uri_string.starts_with("/api") &&
!uri_string.starts_with("/identity") {
return;
}
/*
for header in req.headers().iter() {
println!("DEBUG- {:#?} {:#?}", header.name(), header.value());
}
*/
/*let body_data = data.peek();
if body_data.len() > 0 {
println!("DEBUG- Body Complete: {}", data.peek_complete());
println!("DEBUG- {}", String::from_utf8_lossy(body_data));
}*/
}
}

View File

@ -2,19 +2,9 @@ use super::init_rocket;
use rocket::local::Client;
use rocket::http::Status;
#[test]
fn hello_world() {
let client = Client::new(init_rocket()).expect("valid rocket instance");
let mut response = client.get("/alive").dispatch();
assert_eq!(response.status(), Status::Ok);
// assert_eq!(response.body_string(), Some("Hello, world!".into()));
}
// TODO: For testing, we can use either a test_transaction, or an in-memory database
// TODO: test_transaction http://docs.diesel.rs/diesel/connection/trait.Connection.html#method.begin_test_transaction
// TODO: in-memory database https://github.com/diesel-rs/diesel/issues/419 (basically use ":memory:" as the connection string
// test_transaction: http://docs.diesel.rs/diesel/connection/trait.Connection.html#method.begin_test_transaction
// in-memory database: https://github.com/diesel-rs/diesel/issues/419 (basically use ":memory:" as the connection string
describe! route_tests {
before_each {

View File

@ -3,9 +3,18 @@
///
#[macro_export]
macro_rules! err {
($expr:expr) => {{
err_json!(json!($expr));
}}
($err:expr, $err_desc:expr, $msg:expr) => {
err_json!(json!({
"error": $err,
"error_description": $err_desc,
"ErrorModel": {
"Message": $msg,
"ValidationErrors": null,
"Object": "error"
}
}))
};
($msg:expr) => { err!("default_error", "default_error_description", $msg) }
}
#[macro_export]
@ -49,7 +58,13 @@ pub fn read_file(path: &str) -> Result<Vec<u8>, String> {
}
pub fn delete_file(path: &str) -> bool {
fs::remove_file(path).is_ok()
let res = fs::remove_file(path).is_ok();
if let Some(parent) = Path::new(path).parent() {
fs::remove_dir(parent); // Only removes if the directory is empty
}
res
}
@ -88,8 +103,8 @@ pub fn upcase_first(s: &str) -> String {
}
}
pub fn parse_option_string<S, T>(string: Option<S>) -> Option<T> where S: Into<String>, T: FromStr {
if let Some(Ok(value)) = string.map(|s| s.into().parse::<T>()) {
pub fn parse_option_string<S, T>(string: Option<S>) -> Option<T> where S: AsRef<str>, T: FromStr {
if let Some(Ok(value)) = string.map(|s| s.as_ref().parse::<T>()) {
Some(value)
} else {
None