Skip to content

Commit ead3008

Browse files
committed
Use SecretString for access tokens
1 parent 2771ba7 commit ead3008

File tree

12 files changed

+78
-46
lines changed

12 files changed

+78
-46
lines changed

services/headless-lms/Cargo.lock

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

services/headless-lms/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,5 @@ sqlx = { version = "0.8.6", features = [
5555
"chrono",
5656
"json",
5757
] }
58+
59+
secrecy = { version = "0.10.3", features = ["serde"] }

services/headless-lms/models/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ anyhow = "1.0.100"
5959
# HTTP client for Rust
6060
reqwest = { version = "0.12.23", features = ["json"] }
6161
# Secret value wrapper to avoid leaking secrets in logs and memory
62-
secrecy = "0.10.3"
62+
secrecy.workspace = true
6363
# Password hashing using Argon2id
6464
argon2 = "0.5.3"
6565

services/headless-lms/server/Cargo.toml

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,14 @@ blake3 = "1.8.2"
6060
# Recursively walk a directory.
6161
walkdir = "2.5.0"
6262
# higher level HTTP client library
63-
reqwest = { version = "0.12.23", features = ["brotli", "gzip", "json", "http2", "rustls-tls", "stream"] }
63+
reqwest = { version = "0.12.23", features = [
64+
"brotli",
65+
"gzip",
66+
"json",
67+
"http2",
68+
"rustls-tls",
69+
"stream",
70+
] }
6471
# Sessions for Actix web
6572
actix-session = { version = "0.11.0", features = ["cookie-session"] }
6673
# An extensible, strongly-typed implementation of OAuth2
@@ -130,7 +137,7 @@ tokio-util = "0.7.16"
130137
# Asynchronous streams using async & await notation
131138
async-stream = "0.3.6"
132139
# Secret value wrapper to avoid leaking secrets in logs and memory
133-
secrecy = { version = "0.10.3", features = ["serde"] }
140+
secrecy.workspace = true
134141
# Password hashing using Argon2id
135142
argon2 = "0.5.3"
136143

services/headless-lms/server/src/controllers/tmc_server/users_by_upstream_id.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ Handlers for HTTP requests to `/api/v0/tmc-server/users-by-upstream-id`.
44
These endpoints are used by the TMC server so that it can integrate with this system.
55
*/
66

7-
use std::env;
8-
97
use crate::{
108
domain::authorization::{
119
authorize_access_from_tmc_server_to_course_mooc_fi,
@@ -30,12 +28,8 @@ pub async fn get_user_by_upstream_id(
3028
) -> ControllerResult<web::Json<User>> {
3129
let mut conn = pool.acquire().await?;
3230
let token = authorize_access_from_tmc_server_to_course_mooc_fi(&request).await?;
33-
let tmc_access_token = env::var("TMC_ACCESS_TOKEN").expect("TMC_ACCESS_TOKEN must be defined");
3431
let tmc_user = tmc_client
35-
.get_user_from_tmc_mooc_fi_by_tmc_access_token_and_upstream_id(
36-
&tmc_access_token,
37-
&upstream_id,
38-
)
32+
.get_user_from_tmc_mooc_fi_by_tmc_access_token_and_upstream_id(&upstream_id)
3933
.await?;
4034

4135
debug!(

services/headless-lms/server/src/domain/authorization.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use oauth2::ResourceOwnerUsername;
2222
use oauth2::StandardTokenResponse;
2323
use oauth2::TokenResponse;
2424
use oauth2::basic::BasicTokenType;
25+
use secrecy::SecretString;
2526
use serde::{Deserialize, Serialize};
2627
use sqlx::PgConnection;
2728
use std::env;
@@ -722,7 +723,7 @@ pub async fn authenticate_moocfi_user(
722723
email: String,
723724
password: String,
724725
tmc_client: &TmcClient,
725-
) -> anyhow::Result<Option<(User, LoginToken)>> {
726+
) -> anyhow::Result<Option<(User, SecretString)>> {
726727
info!("Attempting to authenticate user with TMC");
727728
let token = match exchange_password_with_tmc(client, email.clone(), password).await? {
728729
Some(token) => token,
@@ -731,7 +732,7 @@ pub async fn authenticate_moocfi_user(
731732
debug!("Successfully obtained OAuth token from TMC");
732733

733734
let tmc_user = tmc_client
734-
.get_user_from_tmc_mooc_fi_by_tmc_access_token(token.access_token().secret())
735+
.get_user_from_tmc_mooc_fi_by_tmc_access_token(token.clone())
735736
.await?;
736737
debug!(
737738
"Creating or fetching user with TMC id {} and mooc.fi UUID {}",
@@ -766,7 +767,7 @@ pub async fn exchange_password_with_tmc(
766767
client: &OAuthClient,
767768
email: String,
768769
password: String,
769-
) -> anyhow::Result<Option<LoginToken>> {
770+
) -> anyhow::Result<Option<SecretString>> {
770771
let token_result = client
771772
.exchange_password(
772773
&ResourceOwnerUsername::new(email),
@@ -775,7 +776,9 @@ pub async fn exchange_password_with_tmc(
775776
.request_async(&async_http_client_with_headers)
776777
.await;
777778
match token_result {
778-
Ok(token) => Ok(Some(token)),
779+
Ok(token) => Ok(Some(SecretString::new(
780+
token.access_token().secret().to_owned().into(),
781+
))),
779782
Err(RequestTokenError::ServerResponse(server_response)) => {
780783
let error = server_response.error();
781784
let error_description = server_response.error_description();
@@ -912,12 +915,13 @@ pub async fn authenticate_test_user(
912915
// Only used for testing, not to use in production.
913916
pub async fn authenticate_test_token(
914917
conn: &mut PgConnection,
915-
token: &str,
918+
_token: &SecretString,
916919
application_configuration: &ApplicationConfiguration,
917920
) -> anyhow::Result<User> {
918921
// Sanity check to ensure this is not called outside of test mode. The whole application configuration is passed to this function instead of just the boolean to make mistakes harder.
919922
assert!(application_configuration.test_mode);
920-
let user = models::users::get_by_email(conn, token).await?;
923+
// TODO: this has never worked
924+
let user = models::users::get_by_email(conn, "TODO").await?;
921925
Ok(user)
922926
}
923927

services/headless-lms/server/src/domain/langs/token.rs

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
use crate::{
2-
domain::authorization::{self, LoginToken, get_or_create_user_from_tmc_mooc_fi_response},
2+
domain::authorization::{self, get_or_create_user_from_tmc_mooc_fi_response},
33
prelude::*,
44
};
55
use actix_web::{FromRequest, http::header};
66
use futures_util::{FutureExt, future::LocalBoxFuture};
77
use headless_lms_utils::{cache::Cache, tmc::TmcClient};
88
use models::users::User;
9-
use oauth2::TokenResponse;
9+
use secrecy::{ExposeSecret, SecretString};
1010
use std::ops::{Deref, DerefMut};
1111
use std::time::Duration;
1212

@@ -48,7 +48,8 @@ impl FromRequest for UserFromTMCAccessToken {
4848
.headers()
4949
.get(header::AUTHORIZATION)
5050
.map(|hv| String::from_utf8_lossy(hv.as_bytes()))
51-
.and_then(|h| h.strip_prefix("Bearer ").map(str::to_string));
51+
.and_then(|h| h.strip_prefix("Bearer ").map(str::to_string))
52+
.map(|o| SecretString::new(o.into()));
5253

5354
let tmc_client: web::Data<TmcClient> = req
5455
.app_data::<web::Data<TmcClient>>()
@@ -79,15 +80,8 @@ impl FromRequest for UserFromTMCAccessToken {
7980
match load_user(&cache, &token).await {
8081
Some(user) => user,
8182
None => {
82-
let token = LoginToken::new(
83-
oauth2::AccessToken::new(token),
84-
oauth2::basic::BasicTokenType::Bearer,
85-
oauth2::EmptyExtraTokenFields {},
86-
);
8783
let tmc_user = tmc_client
88-
.get_user_from_tmc_mooc_fi_by_tmc_access_token(
89-
token.access_token().secret(),
90-
)
84+
.get_user_from_tmc_mooc_fi_by_tmc_access_token(token.clone())
9185
.await?;
9286

9387
debug!(
@@ -126,16 +120,22 @@ struct TmcUser {
126120
administrator: bool,
127121
}
128122

129-
pub async fn cache_user(cache: &Cache, token: &LoginToken, user: &User) {
123+
fn token_to_cache_key(token: &SecretString) -> String {
124+
let mut hasher = blake3::Hasher::new();
125+
hasher.update(token.expose_secret().as_bytes());
126+
format!("user:{}", hasher.finalize().to_hex())
127+
}
128+
129+
pub async fn cache_user(cache: &Cache, token: &SecretString, user: &User) {
130130
cache
131131
.cache_json(
132-
token.access_token().secret(),
132+
token_to_cache_key(token),
133133
user,
134134
Duration::from_secs(60 * 60),
135135
)
136136
.await;
137137
}
138138

139-
pub async fn load_user(cache: &Cache, token: &str) -> Option<User> {
140-
cache.get_json(token).await
139+
pub async fn load_user(cache: &Cache, token: &SecretString) -> Option<User> {
140+
cache.get_json(token_to_cache_key(token)).await
141141
}

services/headless-lms/server/src/test_helper.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::{
66
use headless_lms_utils::{
77
ApplicationConfiguration, file_store::local_file_store::LocalFileStore, tmc::TmcClient,
88
};
9+
use secrecy::SecretString;
910
use sqlx::{Connection, PgConnection, Postgres, Transaction};
1011
use std::{env, sync::Arc};
1112
use tokio::sync::Mutex;
@@ -31,6 +32,7 @@ postgres://headless-lms:only-for-local-development-intentionally-public@postgres
3132
azure_configuration: None,
3233
test_chatbot: false,
3334
tmc_account_creation_origin: None,
35+
tmc_admin_access_token: SecretString::new("mock-access-token".to_string().into()),
3436
},
3537
redis_url: "redis://example.com".to_string(),
3638
jwt_password: "sMG87WlKnNZoITzvL2+jczriTR7JRsCtGu/bSKaSIvw=asdfjklasd***FSDfsdASDFDS"

services/headless-lms/server/tests/integration_test.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ use headless_lms_server::{
1515
use headless_lms_utils::{
1616
ApplicationConfiguration, file_store::local_file_store::LocalFileStore, tmc::TmcClient,
1717
};
18+
use secrecy::SecretString;
1819
use sqlx::{Connection, PgConnection, PgPool, Postgres, migrate::MigrateDatabase};
1920
use tokio::sync::Mutex;
2021
use uuid::Uuid;
@@ -81,6 +82,7 @@ pub async fn test_config() -> ServerConfig {
8182
azure_configuration: None,
8283
test_chatbot: false,
8384
tmc_account_creation_origin: None,
85+
tmc_admin_access_token: SecretString::new("mock-access-token".to_string().into()),
8486
},
8587
redis_url: "redis://example.com".to_string(),
8688
jwt_password: "sMG87WlKnNZoITzvL2+jczriTR7JRsCtGu/bSKaSIvw=asdfjklasd***FSDfsdASDFDS"

services/headless-lms/utils/Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ redis = { version = "0.32.6", features = [
8888
"tokio-comp",
8989
"connection-manager",
9090
] }
91+
# Secret value wrapper to avoid leaking secrets in logs and memory
92+
secrecy.workspace = true
9193

9294
[dev-dependencies]
9395
# Overwrite `assert_eq!` and `assert_ne!` with drop-in replacements, adding colorful diffs.

0 commit comments

Comments
 (0)