Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not allow invalid tracker keys #980

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 55 additions & 12 deletions src/core/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,13 +131,37 @@
}
}

/// A randomly generated token used for authentication.
/// A token used for authentication.
///
/// It contains lower and uppercase letters and numbers.
/// It's a 32-char string.
/// - It contains only ascii alphanumeric chars: lower and uppercase letters and
/// numbers.
/// - It's a 32-char string.
#[derive(Serialize, Deserialize, Debug, Eq, PartialEq, Clone, Display, Hash)]
pub struct Key(String);

impl Key {
/// # Errors
///
/// Will return an error is the string represents an invalid key.
/// Valid keys can only contain 32 chars including 0-9, a-z and A-Z.
pub fn new(value: &str) -> Result<Self, ParseKeyError> {
if value.len() != AUTH_KEY_LENGTH {
return Err(ParseKeyError::InvalidKeyLength);
}

if !value.chars().all(|c| c.is_ascii_alphanumeric()) {
return Err(ParseKeyError::InvalidChars);
}

Ok(Self(value.to_owned()))
}

#[must_use]
pub fn value(&self) -> &str {
&self.0
}

Check warning on line 162 in src/core/auth.rs

View check run for this annotation

Codecov / codecov/patch

src/core/auth.rs#L160-L162

Added lines #L160 - L162 were not covered by tests
}

/// Error returned when a key cannot be parsed from a string.
///
/// ```rust,no_run
Expand All @@ -151,24 +175,27 @@
/// assert_eq!(key.unwrap().to_string(), key_string);
/// ```
///
/// If the string does not contains a valid key, the parser function will return this error.
#[derive(Debug, PartialEq, Eq, Display)]
pub struct ParseKeyError;
/// If the string does not contains a valid key, the parser function will return
/// this error.
#[derive(Debug, Error)]
pub enum ParseKeyError {
#[error("Invalid key length. Key must be have 32 chars")]
InvalidKeyLength,
#[error("Invalid chars for key. Key can only alphanumeric chars (0-9, a-z, A-Z)")]
InvalidChars,
}

impl FromStr for Key {
type Err = ParseKeyError;

fn from_str(s: &str) -> Result<Self, Self::Err> {
if s.len() != AUTH_KEY_LENGTH {
return Err(ParseKeyError);
}

Key::new(s)?;
Ok(Self(s.to_string()))
}
}

/// Verification error. Error returned when an [`ExpiringKey`] cannot be verified with the [`verify(...)`](crate::core::auth::verify) function.
///
/// Verification error. Error returned when an [`ExpiringKey`] cannot be
/// verified with the [`verify(...)`](crate::core::auth::verify) function.
#[derive(Debug, Error)]
#[allow(dead_code)]
pub enum Error {
Expand Down Expand Up @@ -209,6 +236,22 @@
assert!(key.is_ok());
assert_eq!(key.unwrap().to_string(), key_string);
}

#[test]
fn length_should_be_32() {
let key = Key::new("");
assert!(key.is_err());

let string_longer_than_32 = "012345678901234567890123456789012"; // DevSkim: ignore DS173237
let key = Key::new(string_longer_than_32);
assert!(key.is_err());
}

#[test]
fn should_only_include_alphanumeric_chars() {
let key = Key::new("%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%");
assert!(key.is_err());
}
}

mod expiring_auth_key {
Expand Down
21 changes: 8 additions & 13 deletions tests/servers/api/v1/asserts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ pub async fn assert_bad_request(response: Response, body: &str) {
assert_eq!(response.text().await.unwrap(), body);
}

pub async fn assert_bad_request_with_text(response: Response, text: &str) {
assert_eq!(response.status(), 400);
assert_eq!(response.headers().get("content-type").unwrap(), "text/plain; charset=utf-8");
assert!(response.text().await.unwrap().contains(text));
}

pub async fn assert_unprocessable_content(response: Response, text: &str) {
assert_eq!(response.status(), 422);
assert_eq!(response.headers().get("content-type").unwrap(), "text/plain; charset=utf-8");
Expand Down Expand Up @@ -93,20 +99,9 @@ pub async fn assert_invalid_auth_key_get_param(response: Response, invalid_auth_
}

pub async fn assert_invalid_auth_key_post_param(response: Response, invalid_auth_key: &str) {
assert_bad_request(
assert_bad_request_with_text(
response,
&format!(
"Invalid URL: invalid auth key: string \"{}\", ParseKeyError",
&invalid_auth_key
),
)
.await;
}

pub async fn _assert_unprocessable_auth_key_param(response: Response, _invalid_value: &str) {
assert_unprocessable_content(
response,
"Failed to deserialize the JSON body into the target type: seconds_valid: invalid type",
&format!("Invalid URL: invalid auth key: string \"{}\"", &invalid_auth_key),
)
.await;
}
Expand Down
8 changes: 4 additions & 4 deletions tests/servers/api/v1/contract/context/auth_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,10 +136,10 @@ async fn should_fail_generating_a_new_auth_key_when_the_provided_key_is_invalid(
let invalid_keys = [
// "", it returns 404
// " ", it returns 404
"-1", // Not a string
"invalid", // Invalid string
"GQEs2ZNcCm9cwEV9dBpcPB5OwNFWFiR", // Not a 32-char string
// "%QEs2ZNcCm9cwEV9dBpcPB5OwNFWFiRd", // Invalid char. todo: this doesn't fail
"-1", // Not a string
"invalid", // Invalid string
"GQEs2ZNcCm9cwEV9dBpcPB5OwNFWFiR", // Not a 32-char string
"%QEs2ZNcCm9cwEV9dBpcPB5OwNFWFiRd", // Invalid char.
];

for invalid_key in invalid_keys {
Expand Down
Loading