Skip to content

Commit

Permalink
Merge #234: Make sure user exist in upload torrent endpoint
Browse files Browse the repository at this point in the history
a534e38 fix: [#230] make sure user exist in upload torrent endpoint (Jose Celano)
fe25778 chore: udpate rust toolchain 1.72.0-nightly (Jose Celano)

Pull request description:

  We get the user ID from the Json Web Token, but that does not mean the user actually exists. The session could be valid, but the user could have been removed from the database.

Top commit has no ACKs.

Tree-SHA512: 48052dcb59c3eaeb31777c1831966b9a239489648b193fd25e3eab8a0a24f2d79e7c29f57e41dee55cf1584e68da9086f39b8c8f785112c12dc0141eccdfb3bc
  • Loading branch information
josecelano committed Jul 3, 2023
2 parents 96f17c7 + a534e38 commit 277a905
Show file tree
Hide file tree
Showing 24 changed files with 476 additions and 392 deletions.
713 changes: 338 additions & 375 deletions Cargo.lock

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ pub struct Running {
pub tracker_data_importer_handle: tokio::task::JoinHandle<()>,
}

/// Runs the application.
///
/// # Panics
///
/// It panics if there is an error connecting to the database.
#[allow(clippy::too_many_lines)]
pub async fn run(configuration: Configuration, api_version: &Version) -> Running {
let log_level = configuration.settings.read().await.log_level.clone();
Expand Down
10 changes: 10 additions & 0 deletions src/cache/image/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ pub enum Error {

type UserQuotas = HashMap<i64, ImageCacheQuota>;

/// Returns the current time in seconds.
///
/// # Panics
///
/// This function will panic if the current time is before the UNIX EPOCH.
#[must_use]
pub fn now_in_secs() -> u64 {
SystemTime::now()
Expand Down Expand Up @@ -87,6 +92,11 @@ pub struct ImageCacheService {
}

impl ImageCacheService {
/// Create a new image cache service.
///
/// # Panics
///
/// This function will panic if the image cache could not be created.
pub async fn new(cfg: Arc<Configuration>) -> Self {
let settings = cfg.settings.read().await;

Expand Down
2 changes: 1 addition & 1 deletion src/cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl BytesCache {
pub fn total_size(&self) -> usize {
let mut size: usize = 0;

for (_, entry) in self.bytes_table.iter() {
for (_, entry) in &self.bytes_table {
size += entry.bytes.len();
}

Expand Down
4 changes: 4 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,10 @@ impl Configuration {
}

/// Returns the save to file of this [`Configuration`].
///
/// # Panics
///
/// This function will panic if it can't write to the file.
pub async fn save_to_file(&self, config_path: &str) {
let settings = self.settings.read().await;

Expand Down
5 changes: 5 additions & 0 deletions src/console/commands/import_tracker_statistics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ fn print_usage() {
);
}

/// Import Tracker Statistics Command
///
/// # Panics
///
/// Panics if arguments cannot be parsed.
pub async fn run_importer() {
parse_args().expect("unable to parse command arguments");
import().await;
Expand Down
6 changes: 3 additions & 3 deletions src/databases/mysql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ impl Database for Mysql {
let category_filter_query = if let Some(c) = categories {
let mut i = 0;
let mut category_filters = String::new();
for category in c.iter() {
for category in c {
// don't take user input in the db query
if let Ok(sanitized_category) = self.get_category_from_name(category).await {
let mut str = format!("tc.name = '{}'", sanitized_category.name);
Expand All @@ -352,7 +352,7 @@ impl Database for Mysql {
let tag_filter_query = if let Some(t) = tags {
let mut i = 0;
let mut tag_filters = String::new();
for tag in t.iter() {
for tag in t {
// don't take user input in the db query
if let Ok(sanitized_tag) = self.get_tag_from_name(tag).await {
let mut str = format!("tl.tag_id = '{}'", sanitized_tag.tag_id);
Expand Down Expand Up @@ -479,7 +479,7 @@ impl Database for Mysql {
} else {
let files = torrent.info.files.as_ref().unwrap();

for file in files.iter() {
for file in files {
let path = file.path.join("/");

let _ = query("INSERT INTO torrust_torrent_files (md5sum, torrent_id, length, path) VALUES (?, ?, ?, ?)")
Expand Down
6 changes: 3 additions & 3 deletions src/databases/sqlite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ impl Database for Sqlite {
let category_filter_query = if let Some(c) = categories {
let mut i = 0;
let mut category_filters = String::new();
for category in c.iter() {
for category in c {
// don't take user input in the db query
if let Ok(sanitized_category) = self.get_category_from_name(category).await {
let mut str = format!("tc.name = '{}'", sanitized_category.name);
Expand All @@ -342,7 +342,7 @@ impl Database for Sqlite {
let tag_filter_query = if let Some(t) = tags {
let mut i = 0;
let mut tag_filters = String::new();
for tag in t.iter() {
for tag in t {
// don't take user input in the db query
if let Ok(sanitized_tag) = self.get_tag_from_name(tag).await {
let mut str = format!("tl.tag_id = '{}'", sanitized_tag.tag_id);
Expand Down Expand Up @@ -469,7 +469,7 @@ impl Database for Sqlite {
} else {
let files = torrent.info.files.as_ref().unwrap();

for file in files.iter() {
for file in files {
let path = file.path.join("/");

let _ = query("INSERT INTO torrust_torrent_files (md5sum, torrent_id, length, path) VALUES (?, ?, ?, ?)")
Expand Down
6 changes: 5 additions & 1 deletion src/mailer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,15 @@ impl Service {
}
}

/// Send Verification Email
/// Send Verification Email.
///
/// # Errors
///
/// This function will return an error if unable to send an email.
///
/// # Panics
///
/// This function will panic if the multipart builder had an error.
pub async fn send_verification_mail(
&self,
to: &str,
Expand Down
23 changes: 22 additions & 1 deletion src/models/torrent_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ impl TorrentInfo {
}
}

/// It returns the root hash as a `i64` value.
///
/// # Panics
///
/// This function will panic if the root hash cannot be converted into a
/// `i64` value.
#[must_use]
pub fn get_root_hash_as_i64(&self) -> i64 {
match &self.root_hash {
Expand Down Expand Up @@ -96,6 +102,11 @@ pub struct Torrent {
}

impl Torrent {
/// It hydrates a `Torrent` struct from the database data.
///
/// # Panics
///
/// This function will panic if the `torrent_info.pieces` is not a valid hex string.
#[must_use]
pub fn from_db_info_files_and_announce_urls(
torrent_info: DbTorrentInfo,
Expand Down Expand Up @@ -180,6 +191,11 @@ impl Torrent {
}
}

/// It calculates the info hash of the torrent file.
///
/// # Panics
///
/// This function will panic if the `info` part of the torrent file cannot be serialized.
#[must_use]
pub fn calculate_info_hash_as_bytes(&self) -> [u8; 20] {
let info_bencoded = ser::to_bytes(&self.info).expect("variable `info` was not able to be serialized.");
Expand All @@ -204,7 +220,7 @@ impl Torrent {
None => 0,
Some(files) => {
let mut file_size = 0;
for file in files.iter() {
for file in files {
file_size += file.length;
}
file_size
Expand All @@ -213,6 +229,11 @@ impl Torrent {
}
}

/// It returns the announce urls of the torrent file.
///
/// # Panics
///
/// This function will panic if both the `announce_list` and the `announce` are `None`.
#[must_use]
pub fn announce_urls(&self) -> Vec<String> {
match &self.announce_list {
Expand Down
5 changes: 5 additions & 0 deletions src/services/authentication.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,11 @@ impl JsonWebToken {
}

/// Create Json Web Token.
///
/// # Panics
///
/// This function will panic if the default encoding algorithm does not ç
/// match the encoding key.
pub async fn sign(&self, user: UserCompact) -> String {
let settings = self.cfg.settings.read().await;

Expand Down
2 changes: 2 additions & 0 deletions src/services/torrent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ impl Index {
/// * Unable to insert the torrent into the database.
/// * Unable to add the torrent to the whitelist.
pub async fn add_torrent(&self, mut torrent_request: AddTorrentRequest, user_id: UserId) -> Result<TorrentId, ServiceError> {
let _user = self.user_repository.get_compact(&user_id).await?;

torrent_request.torrent.set_announce_urls(&self.configuration).await;

let category = self
Expand Down
9 changes: 9 additions & 0 deletions src/services/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ impl RegistrationService {
/// * `ServiceError::FailedToSendVerificationEmail` if unable to send the required verification email.
/// * An error if unable to successfully hash the password.
/// * An error if unable to insert user into the database.
///
/// # Panics
///
/// This function will panic if the email is required, but missing.
pub async fn register_user(&self, registration_form: &RegistrationForm, api_base_url: &str) -> Result<UserId, ServiceError> {
info!("registering user: {}", registration_form.username);

Expand Down Expand Up @@ -328,6 +332,11 @@ impl DbBannedUserList {
/// # Errors
///
/// It returns an error if there is a database error.
///
/// # Panics
///
/// It panics if the expiration date cannot be parsed. It should never
/// happen as the date is hardcoded for now.
pub async fn add(&self, user_id: &UserId) -> Result<(), Error> {
// todo: add reason and `date_expiry` parameters to request.

Expand Down
5 changes: 5 additions & 0 deletions src/upgrades/from_v1_0_0_to_v2_0_0/databases/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ pub async fn migrate_target_database(target_database: Arc<SqliteDatabaseV2_0_0>)
target_database.migrate().await;
}

/// It truncates all tables in the target database.
///
/// # Panics
///
/// It panics if it cannot truncate the tables.
pub async fn truncate_target_database(target_database: Arc<SqliteDatabaseV2_0_0>) {
println!("Truncating all tables in target database ...");
target_database
Expand Down
5 changes: 5 additions & 0 deletions src/upgrades/from_v1_0_0_to_v2_0_0/databases/sqlite_v1_0_0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ pub struct SqliteDatabaseV1_0_0 {
}

impl SqliteDatabaseV1_0_0 {
/// It creates a new instance of the `SqliteDatabaseV1_0_0`.
///
/// # Panics
///
/// This function will panic if it is unable to create the database pool.
pub async fn new(database_url: &str) -> Self {
let db = SqlitePoolOptions::new()
.connect(database_url)
Expand Down
15 changes: 15 additions & 0 deletions src/upgrades/from_v1_0_0_to_v2_0_0/databases/sqlite_v2_0_0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ impl TorrentRecordV2 {
}
}

/// It converts a timestamp in seconds to a datetime string.
///
/// # Panics
///
/// It panics if the timestamp is too big and it overflows i64. Very future!
#[must_use]
pub fn convert_timestamp_to_datetime(timestamp: i64) -> String {
// The expected format in database is: 2022-11-04 09:53:57
Expand All @@ -66,6 +71,11 @@ pub struct SqliteDatabaseV2_0_0 {
}

impl SqliteDatabaseV2_0_0 {
/// Creates a new instance of the database.
///
/// # Panics
///
/// It panics if it cannot create the database pool.
pub async fn new(database_url: &str) -> Self {
let db = SqlitePoolOptions::new()
.connect(database_url)
Expand All @@ -74,6 +84,11 @@ impl SqliteDatabaseV2_0_0 {
Self { pool: db }
}

/// It migrates the database to the latest version.
///
/// # Panics
///
/// It panics if it cannot run the migrations.
pub async fn migrate(&self) {
sqlx::migrate!("migrations/sqlite3")
.run(&self.pool)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pub async fn transfer_torrents(
// Multiple files are being shared
let files = torrent_from_file.info.files.as_ref().unwrap();

for file in files.iter() {
for file in files {
println!(
"[v2][torrust_torrent_files][multiple-file-torrent] adding torrent file: {:?} ...",
&file
Expand Down
6 changes: 6 additions & 0 deletions src/utils/clock.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
/// Returns the current timestamp in seconds.
///
/// # Panics
///
/// This function should never panic unless the current timestamp from the
/// time library is negative, which should never happen.
#[must_use]
pub fn now() -> u64 {
u64::try_from(chrono::prelude::Utc::now().timestamp()).expect("timestamp should be positive")
Expand Down
5 changes: 5 additions & 0 deletions src/utils/regex.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
use regex::Regex;

/// Validates an email address.
///
/// # Panics
///
/// It panics if the regex fails to compile.
#[must_use]
pub fn validate_email_address(email_address_to_be_checked: &str) -> bool {
let email_regex = Regex::new(r"^([a-z\d_+]([a-z\d_+.]*[a-z\d_+])?)@([a-z\d]+([\-.][a-z\d]+)*\.[a-z]{2,6})")
Expand Down
4 changes: 4 additions & 0 deletions src/web/api/v1/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ impl Authentication {
}

/// Parses the token from the `Authorization` header.
///
/// # Panics
///
/// This function will panic if the `Authorization` header is not a valid `String`.
pub fn parse_token(authorization: &HeaderValue) -> String {
let split: Vec<&str> = authorization
.to_str()
Expand Down
4 changes: 3 additions & 1 deletion src/web/api/v1/contexts/proxy/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ pub async fn get_proxy_image_handler(
return png_image(map_error_to_image(&Error::Unauthenticated));
}

let Ok(user_id) = app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await else { return png_image(map_error_to_image(&Error::Unauthenticated)) };
let Ok(user_id) = app_data.auth.get_user_id_from_bearer_token(&maybe_bearer_token).await else {
return png_image(map_error_to_image(&Error::Unauthenticated));
};

// code-review: Handling status codes in the frontend other tan OK is quite a pain.
// Return OK for now.
Expand Down
Loading

0 comments on commit 277a905

Please sign in to comment.