Move jwt secret from config to database (fixes #1728)

Cargo.lock

@@ -1628,6 +1628,7 @@ dependencies = [
  "actix-web",
  "chrono",
  "diesel",
+ "jsonwebtoken",
  "lemmy_db_queries",
  "lemmy_db_schema",
  "lemmy_db_views",
@@ -1907,7 +1908,6 @@ dependencies = [
  "futures",
  "http",
  "itertools",
- "jsonwebtoken",
  "lazy_static",
  "lettre",
  "log",

ansible/templates/config.hjson

@@ -16,8 +16,6 @@
   hostname: "{{ domain }}"
   # the port where lemmy should listen for incoming requests
   port: 8536
-  # json web token for authorization between server and client
-  jwt_secret: "{{ jwt_password }}"
   # whether tls is required for activitypub. only disable this for debugging, never for producion.
   tls_enabled: true
   # address where pictrs is available

config/config.hjson

@@ -33,8 +33,6 @@
   port: 8536
   # whether tls is required for activitypub. only disable this for debugging, never for producion.
   tls_enabled: true
-  # json web token for authorization between server and client
-  jwt_secret: "changeme"
   # address where pictrs is available
   pictrs_url: "http://pictrs:8080"
   # maximum length of local community and user names

crates/api/src/lib.rs

@@ -187,17 +187,30 @@ pub(crate) fn captcha_as_wav_base64(captcha: &Captcha) -> String {
 
 #[cfg(test)]
 mod tests {
-  use lemmy_api_common::check_validator_time;
+  use diesel::{
-  use lemmy_db_queries::{establish_unpooled_connection, source::local_user::LocalUser_, Crud};
+    r2d2::{ConnectionManager, Pool},
+    PgConnection,
+  };
+  use lemmy_api_common::{check_validator_time, claims::Claims};
+  use lemmy_db_queries::{
+    establish_unpooled_connection,
+    get_database_url_from_env,
+    source::local_user::LocalUser_,
+    Crud,
+  };
   use lemmy_db_schema::source::{
     local_user::{LocalUser, LocalUserForm},
     person::{Person, PersonForm},
   };
-  use lemmy_utils::claims::Claims;
+  use lemmy_utils::settings::structs::Settings;
 
-  #[test]
+  #[actix_rt::test]
-  fn test_should_not_validate_user_token_after_password_change() {
+  async fn test_should_not_validate_user_token_after_password_change() {
     let conn = establish_unpooled_connection();
+    let db_url = get_database_url_from_env().unwrap_or(Settings::get().get_database_url());
+    let pool = Pool::builder()
+      .build(ConnectionManager::<PgConnection>::new(&db_url))
+      .unwrap();
 
     let new_person = PersonForm {
       name: "Gerry9812".into(),
@@ -214,8 +227,8 @@ mod tests {
 
     let inserted_local_user = LocalUser::create(&conn, &local_user_form).unwrap();
 
-    let jwt = Claims::jwt(inserted_local_user.id.0).unwrap();
+    let jwt = Claims::jwt(inserted_local_user.id.0, &pool).await.unwrap();
-    let claims = Claims::decode(&jwt).unwrap().claims;
+    let claims = Claims::decode(&jwt, &pool).await.unwrap().claims;
     let check = check_validator_time(&inserted_local_user.validator_time, &claims);
     assert!(check.is_ok());
 

crates/api/src/local_user.rs

@@ -6,6 +6,7 @@ use captcha::{gen, Difficulty};
 use chrono::Duration;
 use lemmy_api_common::{
   blocking,
+  claims::Claims,
   collect_moderated_communities,
   get_local_user_view_from_jwt,
   is_admin,
@@ -58,7 +59,6 @@ use lemmy_db_views_actor::{
   person_view::PersonViewSafe,
 };
 use lemmy_utils::{
-  claims::Claims,
   email::send_email,
   location_info,
   settings::structs::Settings,
@@ -104,7 +104,7 @@ impl Perform for Login {
 
     // Return the jwt
     Ok(LoginResponse {
-      jwt: Claims::jwt(local_user_view.local_user.id.0)?,
+      jwt: Claims::jwt(local_user_view.local_user.id.0, context.pool()).await?,
     })
   }
 }
@@ -269,7 +269,7 @@ impl Perform for SaveUserSettings {
 
     // Return the jwt
     Ok(LoginResponse {
-      jwt: Claims::jwt(updated_local_user.id.0)?,
+      jwt: Claims::jwt(updated_local_user.id.0, context.pool()).await?,
     })
   }
 }
@@ -312,7 +312,7 @@ impl Perform for ChangePassword {
 
     // Return the jwt
     Ok(LoginResponse {
-      jwt: Claims::jwt(updated_local_user.id.0)?,
+      jwt: Claims::jwt(updated_local_user.id.0, context.pool()).await?,
     })
   }
 }
@@ -771,7 +771,7 @@ impl Perform for PasswordChange {
 
     // Return the jwt
     Ok(LoginResponse {
-      jwt: Claims::jwt(updated_local_user.id.0)?,
+      jwt: Claims::jwt(updated_local_user.id.0, context.pool()).await?,
     })
   }
 }

crates/api_common/Cargo.toml

@@ -24,3 +24,4 @@ actix-web = { version = "4.0.0-beta.8", default-features = false, features = ["c
 chrono = { version = "0.4.19", features = ["serde"] }
 serde_json = { version = "1.0.66", features = ["preserve_order"] }
 url = "2.2.2"
+jsonwebtoken = "7.2.0"

crates/api_common/src/claims.rs

@@ -0,0 +1,48 @@
+use crate::blocking;
+use chrono::Utc;
+use jsonwebtoken::{decode, encode, DecodingKey, EncodingKey, Header, TokenData, Validation};
+use lemmy_db_queries::{source::secrets::Secrets_, DbPool};
+use lemmy_db_schema::source::secrets::Secrets;
+use lemmy_utils::{settings::structs::Settings, LemmyError};
+use serde::{Deserialize, Serialize};
+
+type Jwt = String;
+
+#[derive(Debug, Serialize, Deserialize)]
+pub struct Claims {
+  /// local_user_id, standard claim by RFC 7519.
+  pub sub: i32,
+  pub iss: String,
+  /// Time when this token was issued as UNIX-timestamp in seconds
+  pub iat: i64,
+}
+
+impl Claims {
+  pub async fn decode(jwt: &str, pool: &DbPool) -> Result<TokenData<Claims>, LemmyError> {
+    let v = Validation {
+      validate_exp: false,
+      ..Validation::default()
+    };
+    let secret = get_jwt_secret(pool).await?;
+    let key = DecodingKey::from_secret(secret.as_ref());
+    Ok(decode::<Claims>(jwt, &key, &v)?)
+  }
+
+  pub async fn jwt(local_user_id: i32, pool: &DbPool) -> Result<Jwt, LemmyError> {
+    let my_claims = Claims {
+      sub: local_user_id,
+      iss: Settings::get().hostname,
+      iat: Utc::now().timestamp(),
+    };
+    let key = EncodingKey::from_secret(get_jwt_secret(pool).await?.as_ref());
+    Ok(encode(&Header::default(), &my_claims, &key)?)
+  }
+}
+
+/// TODO: would be good if we could store the jwt secret in memory, so we dont have to run db
+///       queries all the time (which probably affects performance). but its tricky, we cant use a
+///       static because it requires a db connection to initialize.
+async fn get_jwt_secret(pool: &DbPool) -> Result<String, LemmyError> {
+  let jwt_secret = blocking(pool, move |conn| Secrets::read(conn)).await??;
+  Ok(jwt_secret)
+}

crates/api_common/src/lib.rs

@@ -1,3 +1,4 @@
+pub mod claims;
 pub mod comment;
 pub mod community;
 pub mod person;
@@ -5,7 +6,7 @@ pub mod post;
 pub mod site;
 pub mod websocket;
 
-use crate::site::FederatedInstances;
+use crate::{claims::Claims, site::FederatedInstances};
 use diesel::PgConnection;
 use lemmy_db_queries::{
   source::{
@@ -38,7 +39,6 @@ use lemmy_db_views_actor::{
   community_view::CommunityView,
 };
 use lemmy_utils::{
-  claims::Claims,
   email::send_email,
   settings::structs::Settings,
   utils::MentionData,
@@ -245,7 +245,8 @@ pub async fn get_local_user_view_from_jwt(
   jwt: &str,
   pool: &DbPool,
 ) -> Result<LocalUserView, LemmyError> {
-  let claims = Claims::decode(jwt)
+  let claims = Claims::decode(jwt, pool)
+    .await
     .map_err(|_| ApiError::err("not_logged_in"))?
     .claims;
   let local_user_id = LocalUserId(claims.sub);
@@ -293,7 +294,8 @@ pub async fn get_local_user_settings_view_from_jwt(
   jwt: &str,
   pool: &DbPool,
 ) -> Result<LocalUserSettingsView, LemmyError> {
-  let claims = Claims::decode(jwt)
+  let claims = Claims::decode(jwt, pool)
+    .await
     .map_err(|_| ApiError::err("not_logged_in"))?
     .claims;
   let local_user_id = LocalUserId(claims.sub);

crates/api_crud/src/user/create.rs

@@ -1,6 +1,6 @@
 use crate::PerformCrud;
 use actix_web::web::Data;
-use lemmy_api_common::{blocking, password_length_check, person::*};
+use lemmy_api_common::{blocking, claims::Claims, password_length_check, person::*};
 use lemmy_apub::{
   generate_apub_endpoint,
   generate_followers_url,
@@ -28,7 +28,6 @@ use lemmy_db_schema::{
 use lemmy_db_views_actor::person_view::PersonViewSafe;
 use lemmy_utils::{
   apub::generate_actor_keypair,
-  claims::Claims,
   settings::structs::Settings,
   utils::{check_slurs, is_valid_actor_name},
   ApiError,
@@ -219,7 +218,7 @@ impl PerformCrud for Register {
 
     // Return the jwt
     Ok(LoginResponse {
-      jwt: Claims::jwt(inserted_local_user.id.0)?,
+      jwt: Claims::jwt(inserted_local_user.id.0, context.pool()).await?,
     })
   }
 }

crates/db_queries/src/source/mod.rs

@@ -12,4 +12,5 @@ pub mod person_mention;
 pub mod post;
 pub mod post_report;
 pub mod private_message;
+pub mod secrets;
 pub mod site;

crates/db_queries/src/source/secrets.rs

@@ -0,0 +1,13 @@
+use diesel::{result::Error, *};
+use lemmy_db_schema::source::secrets::Secrets;
+
+pub trait Secrets_ {
+  fn read(conn: &PgConnection) -> Result<String, Error>;
+}
+
+impl Secrets_ for Secrets {
+  fn read(conn: &PgConnection) -> Result<String, Error> {
+    use lemmy_db_schema::schema::secrets::dsl::*;
+    secrets.first::<Self>(conn).map(|s| s.jwt_secret)
+  }
+}

crates/db_schema/src/schema.rs

@@ -551,6 +551,13 @@ table! {
     }
 }
 
+table! {
+  secrets(id) {
+    id -> Int4,
+    jwt_secret -> Varchar,
+  }
+}
+
 joinable!(comment_alias_1 -> person_alias_1 (creator_id));
 joinable!(comment -> comment_alias_1 (parent_id));
 joinable!(person_mention -> person_alias_1 (recipient_id));

crates/db_schema/src/source/mod.rs

@@ -12,4 +12,5 @@ pub mod person_mention;
 pub mod post;
 pub mod post_report;
 pub mod private_message;
+pub mod secrets;
 pub mod site;

crates/db_schema/src/source/secrets.rs

@@ -0,0 +1,8 @@
+use crate::schema::secrets;
+
+#[derive(Queryable, Identifiable)]
+#[table_name = "secrets"]
+pub struct Secrets {
+  pub id: i32,
+  pub jwt_secret: String,
+}

crates/routes/src/feeds.rs

@@ -1,11 +1,11 @@
 use actix_web::{error::ErrorBadRequest, *};
 use anyhow::anyhow;
 use chrono::{DateTime, NaiveDateTime, Utc};
-use diesel::PgConnection;
+use lemmy_api_common::{blocking, claims::Claims};
-use lemmy_api_common::blocking;
 use lemmy_db_queries::{
   source::{community::Community_, person::Person_},
   Crud,
+  DbPool,
   ListingType,
   SortType,
 };
@@ -19,12 +19,7 @@ use lemmy_db_views::{
   site_view::SiteView,
 };
 use lemmy_db_views_actor::person_mention_view::{PersonMentionQueryBuilder, PersonMentionView};
-use lemmy_utils::{
+use lemmy_utils::{settings::structs::Settings, utils::markdown_to_html, LemmyError};
-  claims::Claims,
-  settings::structs::Settings,
-  utils::markdown_to_html,
-  LemmyError,
-};
 use lemmy_websocket::LemmyContext;
 use rss::{
   extension::dublincore::DublinCoreExtensionBuilder,
@@ -141,13 +136,12 @@ async fn get_feed(
     _ => return Err(ErrorBadRequest(LemmyError::from(anyhow!("wrong_type")))),
   };
 
-  let builder = blocking(context.pool(), move |conn| match request_type {
+  let builder = match request_type {
-    RequestType::User => get_feed_user(conn, &sort_type, param),
+    RequestType::User => get_feed_user(context.pool(), sort_type, param).await,
-    RequestType::Community => get_feed_community(conn, &sort_type, param),
+    RequestType::Community => get_feed_community(context.pool(), sort_type, param).await,
-    RequestType::Front => get_feed_front(conn, &sort_type, param),
+    RequestType::Front => get_feed_front(context.pool(), sort_type, param).await,
-    RequestType::Inbox => get_feed_inbox(conn, param),
+    RequestType::Inbox => get_feed_inbox(context.pool(), param).await,
-  })
+  }
-  .await?
   .map_err(ErrorBadRequest)?;
 
   let rss = builder.build().map_err(ErrorBadRequest)?.to_string();
@@ -167,19 +161,23 @@ fn get_sort_type(info: web::Query<Params>) -> Result<SortType, ParseError> {
   SortType::from_str(&sort_query)
 }
 
-fn get_feed_user(
+async fn get_feed_user(
-  conn: &PgConnection,
+  pool: &DbPool,
-  sort_type: &SortType,
+  sort_type: SortType,
   user_name: String,
 ) -> Result<ChannelBuilder, LemmyError> {
-  let site_view = SiteView::read(conn)?;
+  let site_view = blocking(pool, move |conn| SiteView::read(conn)).await??;
-  let person = Person::find_by_name(conn, &user_name)?;
+  let person = blocking(pool, move |conn| Person::find_by_name(conn, &user_name)).await??;
 
-  let posts = PostQueryBuilder::create(conn)
+  let person_id = person.id;
+  let posts = blocking(pool, move |conn| {
+    PostQueryBuilder::create(conn)
       .listing_type(ListingType::All)
-    .sort(*sort_type)
+      .sort(sort_type)
-    .creator_id(person.id)
+      .creator_id(person_id)
-    .list()?;
+      .list()
+  })
+  .await??;
 
   let items = create_post_items(posts)?;
 
@@ -193,19 +191,26 @@ fn get_feed_user(
   Ok(channel_builder)
 }
 
-fn get_feed_community(
+async fn get_feed_community(
-  conn: &PgConnection,
+  pool: &DbPool,
-  sort_type: &SortType,
+  sort_type: SortType,
   community_name: String,
 ) -> Result<ChannelBuilder, LemmyError> {
-  let site_view = SiteView::read(conn)?;
+  let site_view = blocking(pool, move |conn| SiteView::read(conn)).await??;
-  let community = Community::read_from_name(conn, &community_name)?;
+  let community = blocking(pool, move |conn| {
+    Community::read_from_name(conn, &community_name)
+  })
+  .await??;
 
-  let posts = PostQueryBuilder::create(conn)
+  let community_id = community.id;
+  let posts = blocking(pool, move |conn| {
+    PostQueryBuilder::create(conn)
       .listing_type(ListingType::All)
-    .sort(*sort_type)
+      .sort(sort_type)
-    .community_id(community.id)
+      .community_id(community_id)
-    .list()?;
+      .list()
+  })
+  .await??;
 
   let items = create_post_items(posts)?;
 
@@ -223,25 +228,25 @@ fn get_feed_community(
   Ok(channel_builder)
 }
 
-fn get_feed_front(
+async fn get_feed_front(
-  conn: &PgConnection,
+  pool: &DbPool,
-  sort_type: &SortType,
+  sort_type: SortType,
   jwt: String,
 ) -> Result<ChannelBuilder, LemmyError> {
-  let site_view = SiteView::read(conn)?;
+  let site_view = blocking(pool, move |conn| SiteView::read(conn)).await??;
-  let local_user_id = LocalUserId(Claims::decode(&jwt)?.claims.sub);
+  let local_user_id = LocalUserId(Claims::decode(&jwt, pool).await?.claims.sub);
-  let local_user = LocalUser::read(conn, local_user_id)?;
-  let person_id = local_user.person_id;
-  let show_bot_accounts = local_user.show_bot_accounts;
-  let show_read_posts = local_user.show_read_posts;
 
-  let posts = PostQueryBuilder::create(conn)
+  let posts = blocking(pool, move |conn| {
+    let local_user = LocalUser::read(conn, local_user_id)?;
+    PostQueryBuilder::create(conn)
       .listing_type(ListingType::Subscribed)
-    .my_person_id(person_id)
+      .my_person_id(local_user.person_id)
-    .show_bot_accounts(show_bot_accounts)
+      .show_bot_accounts(local_user.show_bot_accounts)
-    .show_read_posts(show_read_posts)
+      .show_read_posts(local_user.show_read_posts)
-    .sort(*sort_type)
+      .sort(sort_type)
-    .list()?;
+      .list()
+  })
+  .await??;
 
   let items = create_post_items(posts)?;
 
@@ -259,27 +264,33 @@ fn get_feed_front(
   Ok(channel_builder)
 }
 
-fn get_feed_inbox(conn: &PgConnection, jwt: String) -> Result<ChannelBuilder, LemmyError> {
+async fn get_feed_inbox(pool: &DbPool, jwt: String) -> Result<ChannelBuilder, LemmyError> {
-  let site_view = SiteView::read(conn)?;
+  let site_view = blocking(pool, move |conn| SiteView::read(conn)).await??;
-  let local_user_id = LocalUserId(Claims::decode(&jwt)?.claims.sub);
+  let local_user_id = LocalUserId(Claims::decode(&jwt, pool).await?.claims.sub);
-  let local_user = LocalUser::read(conn, local_user_id)?;
+  let local_user = blocking(pool, move |conn| LocalUser::read(conn, local_user_id)).await??;
   let person_id = local_user.person_id;
   let show_bot_accounts = local_user.show_bot_accounts;
 
   let sort = SortType::New;
 
-  let replies = CommentQueryBuilder::create(conn)
+  let replies = blocking(pool, move |conn| {
+    CommentQueryBuilder::create(conn)
       .recipient_id(person_id)
       .my_person_id(person_id)
       .show_bot_accounts(show_bot_accounts)
       .sort(sort)
-    .list()?;
+      .list()
+  })
+  .await??;
 
-  let mentions = PersonMentionQueryBuilder::create(conn)
+  let mentions = blocking(pool, move |conn| {
+    PersonMentionQueryBuilder::create(conn)
       .recipient_id(person_id)
       .my_person_id(person_id)
       .sort(sort)
-    .list()?;
+      .list()
+  })
+  .await??;
 
   let items = create_reply_and_mention_items(replies, mentions)?;
 

crates/routes/src/images.rs

@@ -2,7 +2,9 @@ use actix_http::http::header::ACCEPT_ENCODING;
 use actix_web::{body::BodyStream, http::StatusCode, web::Data, *};
 use anyhow::anyhow;
 use awc::Client;
-use lemmy_utils::{claims::Claims, rate_limit::RateLimit, settings::structs::Settings, LemmyError};
+use lemmy_api_common::claims::Claims;
+use lemmy_utils::{rate_limit::RateLimit, settings::structs::Settings, LemmyError};
+use lemmy_websocket::LemmyContext;
 use serde::{Deserialize, Serialize};
 use std::time::Duration;
 
@@ -46,13 +48,14 @@ async fn upload(
   req: HttpRequest,
   body: web::Payload,
   client: web::Data<Client>,
+  context: web::Data<LemmyContext>,
 ) -> Result<HttpResponse, Error> {
   // TODO: check rate limit here
   let jwt = req
     .cookie("jwt")
     .expect("No auth header for picture upload");
 
-  if Claims::decode(jwt.value()).is_err() {
+  if Claims::decode(jwt.value(), context.pool()).await.is_err() {
     return Ok(HttpResponse::Unauthorized().finish());
   };
 

crates/utils/Cargo.toml

@@ -35,7 +35,6 @@ strum_macros = "0.21.1"
 futures = "0.3.16"
 diesel = "1.4.7"
 http = "0.2.4"
-jsonwebtoken = "7.2.0"
 deser-hjson = "1.0.2"
 smart-default = "0.6.0"
 webpage = { version = "1.3.0", default-features = false, features = ["serde"] }

crates/utils/src/claims.rs

@@ -1,42 +0,0 @@
-use crate::settings::structs::Settings;
-use chrono::Utc;
-use jsonwebtoken::{decode, encode, DecodingKey, EncodingKey, Header, TokenData, Validation};
-use serde::{Deserialize, Serialize};
-
-type Jwt = String;
-
-#[derive(Debug, Serialize, Deserialize)]
-pub struct Claims {
-  /// local_user_id, standard claim by RFC 7519.
-  pub sub: i32,
-  pub iss: String,
-  /// Time when this token was issued as UNIX-timestamp in seconds
-  pub iat: i64,
-}
-
-impl Claims {
-  pub fn decode(jwt: &str) -> Result<TokenData<Claims>, jsonwebtoken::errors::Error> {
-    let v = Validation {
-      validate_exp: false,
-      ..Validation::default()
-    };
-    decode::<Claims>(
-      jwt,
-      &DecodingKey::from_secret(Settings::get().jwt_secret.as_ref()),
-      &v,
-    )
-  }
-
-  pub fn jwt(local_user_id: i32) -> Result<Jwt, jsonwebtoken::errors::Error> {
-    let my_claims = Claims {
-      sub: local_user_id,
-      iss: Settings::get().hostname,
-      iat: Utc::now().timestamp(),
-    };
-    encode(
-      &Header::default(),
-      &my_claims,
-      &EncodingKey::from_secret(Settings::get().jwt_secret.as_ref()),
-    )
-  }
-}

crates/utils/src/lib.rs

@@ -6,7 +6,6 @@ extern crate strum_macros;
 extern crate smart_default;
 
 pub mod apub;
-pub mod claims;
 pub mod email;
 pub mod rate_limit;
 pub mod request;

crates/utils/src/settings/structs.rs

@@ -24,8 +24,6 @@ pub struct Settings {
   pub port: u16,
   #[default(true)]
   pub tls_enabled: bool,
-  #[default("changeme")]
-  pub jwt_secret: String,
   #[default(None)]
   pub pictrs_url: Option<String>,
   #[default(None)]

migrations/2021-09-20-112945_jwt-secret/down.sql

@@ -0,0 +1 @@
+drop table secrets;

migrations/2021-09-20-112945_jwt-secret/up.sql

@@ -0,0 +1,10 @@
+-- generate a jwt secret with 62 possible characters and length 43.
+-- this gives an entropy of 256 bits
+-- log2(62^43) = 256
+
+create table secrets(
+  id serial primary key,
+  jwt_secret varchar(43) not null
+);
+-- TODO: generate a random string from A-Za-z0-9, length 43, and insert
+insert into secrets(jwt_secret) values('123');