Skip to content

Commit 8d41d18

Browse files
committed
fix: [torrust#976] do not allow invalid tracker keys
1 parent 1f5de8c commit 8d41d18

File tree

2 files changed

+48
-11
lines changed

2 files changed

+48
-11
lines changed

src/core/auth.rs

+44-7
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,37 @@ impl ExpiringKey {
131131
}
132132
}
133133

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

142+
impl Key {
143+
/// # Errors
144+
///
145+
/// Will return an error is the string represents an invalid key.
146+
/// Valid keys can only contain 32 chars including 0-9, a-z and A-Z.
147+
pub fn new(value: &str) -> Result<Self, ParseKeyError> {
148+
if value.len() != AUTH_KEY_LENGTH {
149+
return Err(ParseKeyError);
150+
}
151+
152+
if !value.chars().all(|c| c.is_ascii_alphanumeric()) {
153+
return Err(ParseKeyError);
154+
}
155+
156+
Ok(Self(value.to_owned()))
157+
}
158+
159+
#[must_use]
160+
pub fn value(&self) -> &str {
161+
&self.0
162+
}
163+
}
164+
141165
/// Error returned when a key cannot be parsed from a string.
142166
///
143167
/// ```rust,no_run
@@ -159,10 +183,7 @@ impl FromStr for Key {
159183
type Err = ParseKeyError;
160184

161185
fn from_str(s: &str) -> Result<Self, Self::Err> {
162-
if s.len() != AUTH_KEY_LENGTH {
163-
return Err(ParseKeyError);
164-
}
165-
186+
Key::new(s)?;
166187
Ok(Self(s.to_string()))
167188
}
168189
}
@@ -209,6 +230,22 @@ mod tests {
209230
assert!(key.is_ok());
210231
assert_eq!(key.unwrap().to_string(), key_string);
211232
}
233+
234+
#[test]
235+
fn length_should_be_32() {
236+
let key = Key::new("");
237+
assert!(key.is_err());
238+
239+
let string_longer_than_32 = "012345678901234567890123456789012"; // DevSkim: ignore DS173237
240+
let key = Key::new(string_longer_than_32);
241+
assert!(key.is_err());
242+
}
243+
244+
#[test]
245+
fn should_only_include_alphanumeric_chars() {
246+
let key = Key::new("%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%");
247+
assert!(key.is_err());
248+
}
212249
}
213250

214251
mod expiring_auth_key {

tests/servers/api/v1/contract/context/auth_key.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,10 @@ async fn should_fail_generating_a_new_auth_key_when_the_provided_key_is_invalid(
136136
let invalid_keys = [
137137
// "", it returns 404
138138
// " ", it returns 404
139-
"-1", // Not a string
140-
"invalid", // Invalid string
141-
"GQEs2ZNcCm9cwEV9dBpcPB5OwNFWFiR", // Not a 32-char string
142-
// "%QEs2ZNcCm9cwEV9dBpcPB5OwNFWFiRd", // Invalid char. todo: this doesn't fail
139+
"-1", // Not a string
140+
"invalid", // Invalid string
141+
"GQEs2ZNcCm9cwEV9dBpcPB5OwNFWFiR", // Not a 32-char string
142+
"%QEs2ZNcCm9cwEV9dBpcPB5OwNFWFiRd", // Invalid char.
143143
];
144144

145145
for invalid_key in invalid_keys {

0 commit comments

Comments
 (0)