From 2da8a836e3e7b1265babff493c9f5ecde48d6828 Mon Sep 17 00:00:00 2001 From: Zoe Spellman Date: Thu, 7 Mar 2024 22:31:42 -0800 Subject: [PATCH] refactor: refactor out JWT authentication for REST REST is going to require admin authentication, instead of the JWT token signing we do now just for valid client hashes. So we need to move the authentication to a more reusable form. This has a couple incidental changes like introducing `secrecy` which lets us shred secrets in memory when they go out of scope. Minor benefit, but I added it for the password secrets so may as well use it for JWT too. --- Cargo.lock | 44 ++++++ Cargo.toml | 2 + src/app/interfaces/authentication/jwt.rs | 121 ++++++++++++++++ src/app/interfaces/authentication/mod.rs | 136 ++++++++++++++++++ src/app/interfaces/mod.rs | 1 + .../interfaces/servers/grpc/authentication.rs | 63 -------- src/app/interfaces/servers/grpc/mod.rs | 90 +++++++----- src/app/interfaces/servers/mod.rs | 7 +- src/app/mod.rs | 3 +- src/app/run.rs | 5 +- src/features/chart/service/mod.rs | 3 + src/features/rating/service/mod.rs | 3 + src/features/user/service/grpc.rs | 2 +- src/features/user/service/mod.rs | 6 + src/utils/config.rs | 3 +- src/utils/infrastructure.rs | 15 +- src/utils/jwt.rs | 71 ++++----- tests/authentication.rs | 4 +- tests/helpers/assert.rs | 10 +- 19 files changed, 429 insertions(+), 160 deletions(-) create mode 100644 src/app/interfaces/authentication/jwt.rs create mode 100644 src/app/interfaces/authentication/mod.rs delete mode 100644 src/app/interfaces/servers/grpc/authentication.rs diff --git a/Cargo.lock b/Cargo.lock index 23332d87..af62eb62 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -114,6 +114,18 @@ version = "1.0.80" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5ad32ce52e4161730f7098c077cd2ed6229b5804ccf99e5366be1ab72a98b4e1" +[[package]] +name = "argon2" +version = "0.5.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c3610892ee6e0cbce8ae2700349fcf8f98adb0dbfbee85aec3c9179d29cc072" +dependencies = [ + "base64ct", + "blake2", + "cpufeatures", + "password-hash", +] + [[package]] name = "async-stream" version = "0.3.5" @@ -263,6 +275,15 @@ dependencies = [ "serde", ] +[[package]] +name = "blake2" +version = "0.10.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46502ad458c9a52b69d4d4d32775c788b7a1b85e8bc9d482d92250fc0e3f8efe" +dependencies = [ + "digest", +] + [[package]] name = "block-buffer" version = "0.10.4" @@ -1726,6 +1747,17 @@ dependencies = [ "windows-targets 0.48.5", ] +[[package]] +name = "password-hash" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "346f04948ba92c43e8469c1ee6736c7563d71012b17d40745260fe106aac2166" +dependencies = [ + "base64ct", + "rand_core", + "subtle", +] + [[package]] name = "paste" version = "1.0.14" @@ -2028,6 +2060,7 @@ dependencies = [ name = "ratings" version = "0.0.3" dependencies = [ + "argon2", "axum", "chrono", "cucumber", @@ -2047,6 +2080,7 @@ dependencies = [ "rand", "regex", "reqwest", + "secrecy", "serde", "serde_json", "sha2", @@ -2306,6 +2340,16 @@ dependencies = [ "syn 2.0.52", ] +[[package]] +name = "secrecy" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9bd1c54ea06cfd2f6b63219704de0b9b4f72dcc2b8fdef820be6cd799780e91e" +dependencies = [ + "serde", + "zeroize", +] + [[package]] name = "security-framework" version = "2.9.2" diff --git a/Cargo.toml b/Cargo.toml index f2ad5fcf..f2c4afe3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,7 @@ version = "0.0.3" edition = "2021" [dependencies] +argon2 = "0.5.3" axum = "0.6.20" # this *must* be pinned because 0.7.x relies on hyper 1.x causing a ton of type conversion issues chrono = { version = "0.4.34", default-features = false, features = [ "std", @@ -26,6 +27,7 @@ prost = "0.12" prost-types = "0.12" rand = "0.8" reqwest = "0.11" +secrecy = { version = "0.8.0", features = ["serde"] } serde = { version = "1.0", features = ["derive"] } sha2 = "0.10" snapd = { git = "https://github.com/ZoopOTheGoop/snapd-rs", branch = "framework" } diff --git a/src/app/interfaces/authentication/jwt.rs b/src/app/interfaces/authentication/jwt.rs new file mode 100644 index 00000000..3bd5da9e --- /dev/null +++ b/src/app/interfaces/authentication/jwt.rs @@ -0,0 +1,121 @@ +//! JSON Web Tokens infrastructure and utlities. +use jsonwebtoken::{DecodingKey, Validation}; +use secrecy::{ExposeSecret, SecretString}; +use serde::Deserialize; +use thiserror::Error; +use tracing::error; + +use crate::utils::{jwt::Claims, Config}; + +use super::CredentialVerifier; + +/// An error for things that can go wrong with JWT verification +#[derive(Error, Debug)] +#[allow(clippy::missing_docs_in_private_items, missing_docs)] +pub enum JwtVerifierError { + #[error("jwt: invalid shape")] + InvalidShape, + #[error("jwt: could not retrieve secret fron environment: {0}")] + EnvError(#[from] envy::Error), + #[error("jwt: error decoding secret: {0}")] + DecodeSecretError(#[from] jsonwebtoken::errors::Error), + #[error("jwt: invalid authz token")] + InvalidHeader, +} + +impl From for tonic::Status { + fn from(value: JwtVerifierError) -> Self { + match value { + JwtVerifierError::InvalidShape | JwtVerifierError::EnvError(_) => { + tonic::Status::internal("Internal Server Error") + } + JwtVerifierError::DecodeSecretError(_) => { + tonic::Status::unauthenticated("invalid JWT token") + } + JwtVerifierError::InvalidHeader => { + tonic::Status::unauthenticated("invalid authz header") + } + } + } +} + +#[derive(Clone)] +/// A JWT verification agent that allows verifying assigned tokens are valid +pub struct JwtVerifier { + /// A decoding key for receipt + decoding_key: DecodingKey, +} + +impl JwtVerifier { + /// Attempts to create a new verifier from the invoker's environment. + pub fn from_env() -> Result { + let config = JwtConfig::from_env()?; + + Self::from_secret(&config.jwt_secret) + } + + /// Creates a new verifier from the given secret. + pub fn from_secret(secret: &SecretString) -> Result { + let decoding_key = DecodingKey::from_base64_secret(secret.expose_secret())?; + + Ok(Self { decoding_key }) + } + + /// Loads this verifier from the secret enclosed in [`Config`]. + #[allow(dead_code)] + pub fn from_config(cfg: &Config) -> Result { + Self::from_secret(&cfg.jwt_secret) + } + + /// Decodes a given token received + pub fn decode(&self, token: &str) -> Result { + jsonwebtoken::decode::(token, &self.decoding_key, &Validation::default()) + .map(|t| t.claims) + .map_err(|e| { + error!("{e:?}"); + JwtVerifierError::InvalidShape + }) + } +} + +impl CredentialVerifier for JwtVerifier { + type Extension = Claims; + + type Error = JwtVerifierError; + + fn verify( + &self, + credential: &hyper::header::HeaderValue, + ) -> Result, Self::Error> { + let raw: Vec<&str> = credential + .to_str() + .unwrap_or("") + .split_whitespace() + .collect(); + + if raw.len() != 2 { + error!("{}", JwtVerifierError::InvalidHeader); + return Err(JwtVerifierError::InvalidHeader); + } + + let token = raw[1]; + self.decode(token).map(Some) + } +} + +/// A configuration only containing a JWT secret, just used for fast +/// on-the-fly construction with `from_env`` +#[derive(Deserialize)] +#[allow(clippy::missing_docs_in_private_items)] +struct JwtConfig { + jwt_secret: SecretString, +} + +impl JwtConfig { + /// Loads the secret from the environment + fn from_env() -> Result { + dotenvy::dotenv().ok(); + + envy::prefixed("APP_").from_env::() + } +} diff --git a/src/app/interfaces/authentication/mod.rs b/src/app/interfaces/authentication/mod.rs new file mode 100644 index 00000000..f4f1036c --- /dev/null +++ b/src/app/interfaces/authentication/mod.rs @@ -0,0 +1,136 @@ +//! Contains a generic authenticator implementation for use in different backends + +use hyper::header::{self, HeaderValue}; +use tonic::Status; +use tracing::error; + +pub mod jwt; + +/// Verifies the given credentials, this is only really ever used by [`Authenticator`] +/// and unless you're adding a new auth method or endpoint, is probably useless to you. +pub trait CredentialVerifier { + /// Any extensions that will be added to the request's extensions field before being + /// passed down to the handler, should authentication succeed. + type Extension: Send + Sync; + /// Any errors that can be encountered during the verification procedure. These must + /// be convertable to [`tonic::Status`] values, especially so anything sensitive can be + /// erased before sending the error back to the client. + type Error: Into + std::error::Error; + + /// Verifies the passed in header has the authentication values necessary. This does + /// NOT need to verify paths, nor that the header is actually an authorization header, + /// the [`Authenticator`] does that already. However, it should validate things like + /// header length, authentication type (Basic, etc) on its own. + fn verify(&self, credential: &HeaderValue) -> Result, Self::Error>; +} + +/// A utility meant to verify requests. +#[derive(Debug, Clone)] +pub struct Authenticator { + /// The paths that can be accessed without verifying whether a user is authorized + public_paths: Vec, + /// A [`CredentialVerifier`] that will check if the auth header is valid for non-public paths. + verifier: V, +} + +impl> Authenticator +where + V: CredentialVerifier, + V::Extension: 'static, + A: AsRef, +{ + /// Attempts to authenticate the request's Authorization Header with the underlying [`CredentialVerifier`], + /// unless the URL path was designated as public during construction. + pub fn authenticate(&self, req: &mut hyper::Request) -> Result<(), tonic::Status> { + let uri = req.uri().to_string(); + + if self.public_paths.iter().any(|s| uri.ends_with(s.as_ref())) { + return Ok(()); + } + + let Some(header) = req.headers().get(header::AUTHORIZATION.as_str()) else { + let error = Status::unauthenticated("missing authz header"); + error!("{error}"); + return Err(error); + }; + + let extension = self + .verifier + .verify(header) + .inspect_err(|err| error!("{err}")) + .map_err(Into::into)?; + + if let Some(extension) = extension { + req.extensions_mut().insert(extension); + } + Ok(()) + } +} + +/// Builder pattern for [`Authenticator`], this is mostly used for iteratively adding +/// new public paths. +#[allow(clippy::missing_docs_in_private_items)] +pub struct AuthenticatorBuilder { + verifier: V, + public_paths: Vec, +} + +impl AuthenticatorBuilder { + /// Constructs a new in-progress authenticator from the given [`CredentialVerifier`]. + pub fn new(verifier: V) -> Self { + Self { + verifier, + public_paths: Vec::new(), + } + } + + /// Overrides an existing [`CredentialVerifier`] with a new one. + #[allow(dead_code)] + pub fn override_verifier( + self, + verifier: V2, + ) -> AuthenticatorBuilder { + AuthenticatorBuilder { + verifier, + public_paths: self.public_paths, + } + } +} + +impl> AuthenticatorBuilder { + /// Adds a new path that the constructed [`Authenticator`] will consider public, + /// and thus pass without attempting to authenticate the passed header. + pub fn with_public_path(mut self, path: A) -> Self { + self.public_paths.push(path); + self + } + + /// Like [AuthenticatorBuilder::with_public_path], but adds multiple at once. This + /// is slightly more efficient (and cleaner) than doing it yourself in a loop. + pub fn with_public_paths>(mut self, paths: I) -> Self { + self.public_paths.extend(paths); + self + } +} + +impl> AuthenticatorBuilder { + /// Renders this builder into an [`Authenticator`], ready to be used. + pub fn build(self) -> Authenticator { + Authenticator { + verifier: self.verifier, + public_paths: self.public_paths, + } + } +} + +impl Default for AuthenticatorBuilder +where + V: Default, +{ + fn default() -> Self { + Self { + verifier: Default::default(), + public_paths: Default::default(), + } + } +} diff --git a/src/app/interfaces/mod.rs b/src/app/interfaces/mod.rs index 0e5ea5cd..6b363780 100644 --- a/src/app/interfaces/mod.rs +++ b/src/app/interfaces/mod.rs @@ -1,5 +1,6 @@ //! Contains submodules with various middleware and utility layers for the app. +pub mod authentication; pub mod middleware; pub mod servers; diff --git a/src/app/interfaces/servers/grpc/authentication.rs b/src/app/interfaces/servers/grpc/authentication.rs deleted file mode 100644 index e73aff71..00000000 --- a/src/app/interfaces/servers/grpc/authentication.rs +++ /dev/null @@ -1,63 +0,0 @@ -//! Contains the utilities for authorizing user requests - -use http::header; -use hyper::Request; -use tonic::Status; -use tracing::error; - -use crate::app::context::AppContext; - -/// Authentication for a GRPC Server, it validates any passed in Grpc JWT client token -#[derive(Default, Debug, Copy, Clone)] -pub struct GrpcAuthenticator; - -impl GrpcAuthenticator { - /// Our public API paths, in the future, could refactor into a `Vec` - /// if needed, but this should remain relatively constant. - const PUBLIC_PATHS: [&'static str; 3] = [ - "ratings.features.user.User/Register", - "ratings.features.user.User/Authenticate", - "grpc.reflection.v1alpha.ServerReflection/ServerReflectionInfo", - ]; - - /// Authenticates the given HTTP Request header, on success, this modifies the underlying - /// request with its JWT [`Claims`] - /// - /// [`Claims`]: crate::utils::jwt::Claims - pub fn authenticate(&self, req: &mut Request) -> Result<(), Status> { - let app_ctx = req.extensions().get::().unwrap().clone(); - - let uri = req.uri().to_string(); - - if Self::PUBLIC_PATHS.iter().any(|&s| uri.contains(s)) { - return Ok(()); - } - - let Some(header) = req.headers().get(header::AUTHORIZATION.as_str()) else { - let error = Err(Status::unauthenticated("missing authz header")); - error!("{error:?}"); - return error; - }; - - let raw: Vec<&str> = header.to_str().unwrap_or("").split_whitespace().collect(); - - if raw.len() != 2 { - let error = Err(Status::unauthenticated("invalid authz token")); - error!("{error:?}"); - return error; - } - - let token = raw[1]; - let infra = app_ctx.infrastructure(); - match infra.jwt.decode(token) { - Ok(claim) => { - req.extensions_mut().insert(claim); - Ok(()) - } - Err(error) => { - error!("{error:?}"); - Err(Status::unauthenticated("Failed to decode token.")) - } - } - } -} diff --git a/src/app/interfaces/servers/grpc/mod.rs b/src/app/interfaces/servers/grpc/mod.rs index 463b6652..58b45a5d 100644 --- a/src/app/interfaces/servers/grpc/mod.rs +++ b/src/app/interfaces/servers/grpc/mod.rs @@ -10,11 +10,13 @@ use tonic::{ }; use tower::Service; -use crate::features::{chart::ChartService, rating::RatingService, user::UserService}; - -mod authentication; - -use authentication::GrpcAuthenticator; +use crate::{ + app::interfaces::authentication::{ + jwt::{JwtVerifier, JwtVerifierError}, + Authenticator, AuthenticatorBuilder, + }, + features::{chart::ChartService, rating::RatingService, user::UserService}, +}; /// An error deriving from the GRPC Endpoints #[derive(Error, Debug)] @@ -42,12 +44,12 @@ const FILE_DESCRIPTOR_SET: &[u8] = tonic::include_file_descriptor_set!("ratings_ /// The GRPC Service endpoint for the program, you probably want to build this with /// [`GrpcServiceBuilder`] instead of using this directly. -#[derive(Default, Debug, Clone)] +#[derive(Clone)] pub struct GrpcService { /// The router that automatically sends requests to the proper underlying service routes: Routes, /// The authentication routine we use for validating input - authenticator: GrpcAuthenticator, + authenticator: Authenticator, } /// A type definition which is simply a future that's in a pinned location in the heap. @@ -79,38 +81,72 @@ impl Service> for GrpcService { } } +/// The path of the reflection server, since we do this internally we don't +/// construct it in the same was as the servers under [`features`](crate::features). +const REFLECTION_SERVER_PATH: &str = + "grpc.reflection.v1alpha.ServerReflection/ServerReflectionInfo"; + +/// Errors that can occur while constructing our GRPC service +#[derive(Error, Debug)] +#[allow(clippy::missing_docs_in_private_items, missing_docs)] +pub enum GrpcServerBuildError { + #[error("grpc builder: error creating JWT authentication: {0}")] + JwtDecodeError(#[from] JwtVerifierError), +} + /// A builder for the ratings GRPC backend pub struct GrpcServiceBuilder { /// The builder for the service's route dispatcher builder: RoutesBuilder, /// The authenticator we want to use - authenticator: GrpcAuthenticator, + authenticator: AuthenticatorBuilder, } impl GrpcServiceBuilder { /// Creates a new builder for our GrpcService - pub fn new() -> GrpcServiceBuilder { - GrpcServiceBuilder { + pub fn from_env() -> Result { + Ok(GrpcServiceBuilder { builder: RoutesBuilder::default(), - authenticator: GrpcAuthenticator, + authenticator: AuthenticatorBuilder::new(JwtVerifier::from_env()?), + }) + } + + /// Creates a new builder with the given [`AuthenticatorBuilder`], should + /// it be constructed elsewhere. + #[allow(dead_code)] + pub fn from_authenticator_builder( + authenticator: AuthenticatorBuilder, + ) -> Self { + Self { + authenticator, + builder: Default::default(), } } /// Adds the [`ChartService`] to the [`GrpcService`] pub fn with_charts(mut self) -> Self { self.builder.add_service(ChartService.to_server()); + self.authenticator = self + .authenticator + .with_public_paths(ChartService::PUBLIC_PATHS.into_iter()); self } /// Adds the [`RatingService`] to the [`GrpcService`] pub fn with_ratings(mut self) -> Self { self.builder.add_service(RatingService.to_server()); + self.authenticator = self + .authenticator + .with_public_paths(RatingService::PUBLIC_PATHS.into_iter()); self } /// Adds the [`UserService`] to the [`GrpcService`] pub fn with_user(mut self) -> Self { self.builder.add_service(UserService.to_server()); + self.authenticator = self + .authenticator + .with_public_paths(UserService::PUBLIC_PATHS.into_iter()); self } @@ -124,38 +160,24 @@ impl GrpcServiceBuilder { .build() .unwrap(), ); - self - } - /// Adds the given [`GrpcAuthenticator`] to the [`GrpcService`], - /// this is largely pointless because at the moment it has no configuration, - /// but is here for posterity. - #[allow(dead_code)] - pub fn with_authenticator(mut self, authenticator: GrpcAuthenticator) -> Self { - self.authenticator = authenticator; + self.authenticator = self.authenticator.with_public_path(REFLECTION_SERVER_PATH); self } - /// Converts the builder into its underlying routes with the given client - pub fn routes(self) -> Routes { - self.builder.routes() + /// Constructs this with the default routes expected of our GRPC client. + pub fn with_default_routes(self) -> Self { + self.with_charts() + .with_ratings() + .with_user() + .with_reflection() } /// Builds this into the GrpcService pub fn build(self) -> GrpcService { GrpcService { - authenticator: self.authenticator, - routes: self.routes(), + routes: self.builder.routes(), + authenticator: self.authenticator.build(), } } } - -impl Default for GrpcServiceBuilder { - fn default() -> Self { - Self::new() - .with_charts() - .with_ratings() - .with_user() - .with_reflection() - } -} diff --git a/src/app/interfaces/servers/mod.rs b/src/app/interfaces/servers/mod.rs index 928ea15f..566ff748 100644 --- a/src/app/interfaces/servers/mod.rs +++ b/src/app/interfaces/servers/mod.rs @@ -31,7 +31,7 @@ pub enum AppCenterRatingsError { } /// The general service for our app, containing all our endpoints -#[derive(Clone, Debug)] +#[derive(Clone)] #[allow(clippy::missing_docs_in_private_items)] pub struct AppCenterRatingsService { grpc_service: GrpcService, @@ -44,7 +44,10 @@ impl AppCenterRatingsService { /// Constructs the service with all the default service endpoints for REST and GRPC pub fn with_default_routes() -> AppCenterRatingsService { Self { - grpc_service: GrpcServiceBuilder::default().build(), + grpc_service: GrpcServiceBuilder::from_env() + .expect("could not create GRPC service from environment") + .with_default_routes() + .build(), grpc_ready: false, rest_service: RestServiceBuilder::default().build(), rest_ready: false, diff --git a/src/app/mod.rs b/src/app/mod.rs index 20c3484c..83a5bb40 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -3,6 +3,7 @@ pub use context::AppContext; pub use run::run; +pub mod interfaces; + mod context; -mod interfaces; mod run; diff --git a/src/app/run.rs b/src/app/run.rs index 6a51712e..a9fb2038 100644 --- a/src/app/run.rs +++ b/src/app/run.rs @@ -21,6 +21,10 @@ pub async fn run(config: Config) -> Result<(), Box> { info!("{} infrastructure initialized", config.name); + let socket: SocketAddr = config.socket().parse()?; + // Shred the secrets in `config` + drop(config); + let service = ServiceBuilder::new() .timeout(Duration::from_secs(30)) .layer(ContextMiddlewareLayer::new(app_ctx)) @@ -28,7 +32,6 @@ pub async fn run(config: Config) -> Result<(), Box> { let shared = tower::make::Shared::new(service); - let socket: SocketAddr = config.socket().parse()?; info!("Binding to {socket}"); hyper::Server::bind(&socket).serve(shared).await?; Ok(()) diff --git a/src/features/chart/service/mod.rs b/src/features/chart/service/mod.rs index 00c28b81..70537b8a 100644 --- a/src/features/chart/service/mod.rs +++ b/src/features/chart/service/mod.rs @@ -11,6 +11,9 @@ mod grpc; pub struct ChartService; impl ChartService { + /// The paths which are accessible without authentication, if any + pub const PUBLIC_PATHS: [&'static str; 0] = []; + /// Converts this service into its corresponding server pub fn to_server(self) -> ChartServer { self.into() diff --git a/src/features/rating/service/mod.rs b/src/features/rating/service/mod.rs index f0a86b37..b815e308 100644 --- a/src/features/rating/service/mod.rs +++ b/src/features/rating/service/mod.rs @@ -8,6 +8,9 @@ mod grpc; pub struct RatingService; impl RatingService { + /// The paths which are accessible without authentication, if any + pub const PUBLIC_PATHS: [&'static str; 0] = []; + /// Converts this service into its corresponding server pub fn to_server(self) -> AppServer { self.into() diff --git a/src/features/user/service/grpc.rs b/src/features/user/service/grpc.rs index 3062e36a..419681b0 100644 --- a/src/features/user/service/grpc.rs +++ b/src/features/user/service/grpc.rs @@ -35,7 +35,7 @@ impl User for UserService { match use_cases::authenticate(&app_ctx, &id).await { Ok(user) => app_ctx .infrastructure() - .jwt + .jwt_encoder .encode(user.client_hash) .map(|token| AuthenticateResponse { token }) .map(Response::new) diff --git a/src/features/user/service/mod.rs b/src/features/user/service/mod.rs index 1217b2ca..091c1885 100644 --- a/src/features/user/service/mod.rs +++ b/src/features/user/service/mod.rs @@ -10,6 +10,12 @@ mod grpc; pub struct UserService; impl UserService { + /// The paths which are accessible without authentication, if any + pub const PUBLIC_PATHS: [&'static str; 2] = [ + "ratings.features.user.User/Register", + "ratings.features.user.User/Authenticate", + ]; + /// Converts this service into its corresponding server pub fn to_server(self) -> UserServer { self.into() diff --git a/src/utils/config.rs b/src/utils/config.rs index b148d775..0345447a 100644 --- a/src/utils/config.rs +++ b/src/utils/config.rs @@ -1,5 +1,6 @@ //! Utility functions and definitions for configuring the service. use dotenvy::dotenv; +use secrecy::SecretString; use serde::Deserialize; /// Configuration for the general app center ratings backend service. @@ -10,7 +11,7 @@ pub struct Config { /// The host configuration pub host: String, /// The JWT secret value - pub jwt_secret: String, + pub jwt_secret: SecretString, /// Log level to use pub log_level: String, /// The service name diff --git a/src/utils/infrastructure.rs b/src/utils/infrastructure.rs index e21a6a71..e76c7d46 100644 --- a/src/utils/infrastructure.rs +++ b/src/utils/infrastructure.rs @@ -11,9 +11,8 @@ use tokio::sync::OnceCell; use tracing::level_filters::LevelFilter; use tracing_subscriber::{reload::Handle, Registry}; -use crate::utils::{config::Config, jwt::Jwt}; - -use super::log_util; +use super::{jwt::JwtEncoder, log_util}; +use crate::utils::config::Config; /// The global reload handle, since [`tracing_subscriber`] is we have to be too because it panics /// if you call init twice, which makes it so tests can't initialize [`Infrastructure`] more than once. @@ -26,10 +25,10 @@ pub struct Infrastructure { pub postgres: Arc, /// The client for making snapd requests pub snapd_client: SnapdClient, - /// The JWT instance - pub jwt: Arc, /// The reload handle for the logger pub log_reload_handle: &'static Handle, + /// The utility which lets us encode user tokens with our JWT credentials + pub jwt_encoder: Arc, } impl Infrastructure { @@ -41,8 +40,8 @@ impl Infrastructure { let postgres = PgPoolOptions::new().max_connections(5).connect(uri).await?; let postgres = Arc::new(postgres); - let jwt = Jwt::new(&config.jwt_secret)?; - let jwt = Arc::new(jwt); + let jwt_encoder = JwtEncoder::from_config(config)?; + let jwt_encoder = Arc::new(jwt_encoder); let reload_handle = RELOAD_HANDLE .get_or_try_init(|| async move { log_util::init_logging(&config.log_level) }) @@ -50,7 +49,7 @@ impl Infrastructure { Ok(Infrastructure { postgres, - jwt, + jwt_encoder, snapd_client: Default::default(), log_reload_handle: reload_handle, }) diff --git a/src/utils/jwt.rs b/src/utils/jwt.rs index bdcc8a9e..cd816548 100644 --- a/src/utils/jwt.rs +++ b/src/utils/jwt.rs @@ -1,26 +1,17 @@ -//! JSON Web Tokens infrastructure and utlities. -use std::ops::Add; +//! Definitions meant to help with JWT handling throughout the app. -use jsonwebtoken::{DecodingKey, EncodingKey, Header, Validation}; +use jsonwebtoken::{EncodingKey, Header}; +use secrecy::{ExposeSecret, SecretString}; use serde::{Deserialize, Serialize}; use thiserror::Error; use time::{Duration, OffsetDateTime}; use tracing::error; +use super::Config; + /// How many days until JWT info expires static JWT_EXPIRY_IN_DAYS: i64 = 1; -/// An error for things that can go wrong with JWT handling -#[derive(Error, Debug)] -pub enum JwtError { - /// The shape of the data is invalid - #[error("jwt: invalid shape")] - InvalidShape, - /// Anything else that can go wrong - #[error("jwt: unknown error")] - Unknown, -} - /// Information representating a claim on a specific subject at a specific time #[derive(Debug, Serialize, Deserialize, Clone)] pub struct Claims { @@ -33,50 +24,48 @@ pub struct Claims { impl Claims { /// Creates a new claim with the current datetime for the subject given by `sub`. pub fn new(sub: String) -> Self { - let exp = OffsetDateTime::now_utc().add(Duration::days(JWT_EXPIRY_IN_DAYS)); + let exp = OffsetDateTime::now_utc() + Duration::days(JWT_EXPIRY_IN_DAYS); let exp = exp.unix_timestamp() as usize; Self { sub, exp } } } -/// A JWT transaction representation -pub struct Jwt { +/// Errors that can happen while encoding and signing tokens with JWT. +#[derive(Error, Debug)] +#[allow(missing_docs)] +pub enum JwtEncoderError { + #[error("jwt: error decoding secret: {0}")] + DecodeSecretError(#[from] jsonwebtoken::errors::Error), + #[error("jwt: an error occurred, but the reason was erased for security reasons")] + Erased, +} + +/// An encoder which allows converting user hashes into valid JWT tokens +pub struct JwtEncoder { /// An encoding key for transfer encoding_key: EncodingKey, - /// A decoding key for receipt - decoding_key: DecodingKey, } -impl Jwt { - /// Attempts to create a new JWT representation from a given secret - pub fn new(secret: &str) -> Result { - let encoding_key = EncodingKey::from_base64_secret(secret)?; - let decoding_key = DecodingKey::from_base64_secret(secret)?; +impl JwtEncoder { + /// Loads the encoder from the given JWT secret + pub fn from_secret(secret: &SecretString) -> Result { + let encoding_key = EncodingKey::from_base64_secret(secret.expose_secret())?; + Ok(Self { encoding_key }) + } - Ok(Self { - encoding_key, - decoding_key, - }) + /// Loads our encoder from the secret enclosed in [`Config`] + pub fn from_config(config: &Config) -> Result { + Self::from_secret(&config.jwt_secret) } /// Encodes a token for use - pub fn encode(&self, sub: String) -> Result { + pub fn encode(&self, sub: String) -> Result { let claims = Claims::new(sub); jsonwebtoken::encode(&Header::default(), &claims, &self.encoding_key).map_err(|e| { - error!("{e:?}"); - JwtError::Unknown + error!("{e}"); + JwtEncoderError::Erased }) } - - /// Decodes a given token received - pub fn decode(&self, token: &str) -> Result { - jsonwebtoken::decode::(token, &self.decoding_key, &Validation::default()) - .map(|t| t.claims) - .map_err(|e| { - error!("{e:?}"); - JwtError::InvalidShape - }) - } } diff --git a/tests/authentication.rs b/tests/authentication.rs index 236f5f08..c4a44d6e 100644 --- a/tests/authentication.rs +++ b/tests/authentication.rs @@ -63,10 +63,8 @@ fn verify_token(world: &mut AuthenticationWorld) { world.auth_error ); - let config = Config::load().expect("Could not load config"); - for token in world.tokens.iter() { - helpers::assert::assert_token_is_valid(token, &config.jwt_secret); + helpers::assert::assert_token_is_valid(token); } } diff --git a/tests/helpers/assert.rs b/tests/helpers/assert.rs index 78fcc791..00d95db1 100644 --- a/tests/helpers/assert.rs +++ b/tests/helpers/assert.rs @@ -1,8 +1,8 @@ -use ratings::utils::jwt::Jwt; +use ratings::app::interfaces::authentication::jwt::JwtVerifier; #[allow(dead_code)] -pub fn assert_token_is_valid(value: &str, jwt_secret: &str) { - let jwt = Jwt::new(jwt_secret); +pub fn assert_token_is_valid(value: &str) { + let jwt = JwtVerifier::from_env(); assert!( jwt.unwrap().decode(value).is_ok(), "value should be a valid jwt" @@ -10,7 +10,7 @@ pub fn assert_token_is_valid(value: &str, jwt_secret: &str) { } #[allow(dead_code)] -pub fn assert_token_is_not_valid(value: &str, jwt_secret: &str) { - let jwt = Jwt::new(jwt_secret); +pub fn assert_token_is_not_valid(value: &str) { + let jwt = JwtVerifier::from_env(); assert!(jwt.unwrap().decode(value).is_err(), "expected invalid jwt"); }