Skip to content
Open
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
1 change: 1 addition & 0 deletions rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion rust/auth-impls/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@
use api::auth::{AuthResponse, Authorizer};
use api::error::VssError;
use async_trait::async_trait;
use jsonwebtoken::{decode, Algorithm, DecodingKey, Validation};
use jsonwebtoken::{decode, Algorithm, Validation};
use serde::{Deserialize, Serialize};
use std::collections::HashMap;

pub use jsonwebtoken::DecodingKey;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Hmm, leaking this auth-depdenent type to main.rs is a bit strange. Wouldn't it rather make sense to have JWTAuthorizer::new be fallible and handle the teh DecodingKey::from_rsa_pem step?


/// A JWT based authorizer, only allows requests with verified 'JsonWebToken' signed by the given
/// issuer key.
///
Expand Down
1 change: 1 addition & 0 deletions rust/server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ edition = "2021"

[dependencies]
api = { path = "../api" }
auth-impls = { path = "../auth-impls" }
impls = { path = "../impls" }

hyper = { version = "1", default-features = false, features = ["server", "http1"] }
Expand Down
55 changes: 39 additions & 16 deletions rust/server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@ use hyper_util::rt::TokioIo;
use crate::vss_service::VssService;
use api::auth::{Authorizer, NoopAuthorizer};
use api::kv_store::KvStore;
use auth_impls::{DecodingKey, JWTAuthorizer};
use impls::postgres_store::{Certificate, PostgresPlaintextBackend, PostgresTlsBackend};
use std::sync::Arc;

pub(crate) mod util;
pub(crate) mod vss_service;
mod util;
mod vss_service;

use util::config::{Config, ServerConfig};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This likely should import be added in the above group.


fn main() {
let args: Vec<String> = std::env::args().collect();
Expand All @@ -33,22 +36,21 @@ fn main() {
std::process::exit(1);
}

let config = match util::config::load_config(&args[1]) {
Ok(cfg) => cfg,
Err(e) => {
eprintln!("Failed to load configuration: {}", e);
std::process::exit(1);
},
};

let addr: SocketAddr =
match format!("{}:{}", config.server_config.host, config.server_config.port).parse() {
Ok(addr) => addr,
let Config { server_config: ServerConfig { host, port, rsa_pub_file_path }, postgresql_config } =
match util::config::load_config(&args[1]) {
Ok(cfg) => cfg,
Err(e) => {
eprintln!("Invalid host/port configuration: {}", e);
eprintln!("Failed to load configuration: {}", e);
std::process::exit(1);
},
};
let addr: SocketAddr = match format!("{}:{}", host, port).parse() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot going on here. Can we declutter this? Btw, since we're already here, might make sense to switch this to the From<(I, u16)> for SocketAddr implementation rather than allocating a string and then parsing it again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is personal preference, but in the follow-up PR I consolidate the host and port setting into a single address setting, string type, ie "127.0.0.1:54234". We would then allocate the string upfront. WDYT ? Overall looking to reduce the number of lines in the config files, and the number of different settings. The address the server binds to is a single setting in my head :)

Ok(addr) => addr,
Err(e) => {
eprintln!("Invalid host/port configuration: {}", e);
std::process::exit(1);
},
};

let runtime = match tokio::runtime::Builder::new_multi_thread().enable_all().build() {
Ok(runtime) => Arc::new(runtime),
Expand All @@ -66,9 +68,29 @@ fn main() {
std::process::exit(-1);
},
};
let authorizer: Arc<dyn Authorizer> = Arc::new(NoopAuthorizer {});

let authorizer: Arc<dyn Authorizer> = if let Some(file_path) = rsa_pub_file_path {
let rsa_pub_file = match std::fs::read(file_path) {
Ok(pem) => pem,
Err(e) => {
println!("Failed to read RSA public key file: {}", e);
std::process::exit(-1);
},
};
let rsa_public_key = match DecodingKey::from_rsa_pem(&rsa_pub_file) {
Ok(pem) => pem,
Err(e) => {
println!("Failed to parse RSA public key file: {}", e);
std::process::exit(-1);
},
};
Arc::new(JWTAuthorizer::new(rsa_public_key).await)
} else {
Arc::new(NoopAuthorizer {})
};

let postgresql_config =
config.postgresql_config.expect("PostgreSQLConfig must be defined in config file.");
postgresql_config.expect("PostgreSQLConfig must be defined in config file.");
let endpoint = postgresql_config.to_postgresql_endpoint();
let db_name = postgresql_config.database;
let store: Arc<dyn KvStore> = if let Some(tls_config) = postgresql_config.tls {
Expand Down Expand Up @@ -109,6 +131,7 @@ fn main() {
Arc::new(postgres_plaintext_backend)
};
println!("Connected to PostgreSQL backend with DSN: {}/{}", endpoint, db_name);

let rest_svc_listener =
TcpListener::bind(&addr).await.expect("Failed to bind listening port");
println!("Listening for incoming connections on {}", addr);
Expand Down
1 change: 1 addition & 0 deletions rust/server/src/util/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub(crate) struct Config {
pub(crate) struct ServerConfig {
pub(crate) host: String,
pub(crate) port: u16,
pub(crate) rsa_pub_file_path: Option<String>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't that rather be part of a new optional JwtAuthConfig section?

}

#[derive(Deserialize)]
Expand Down
1 change: 1 addition & 0 deletions rust/server/vss-server-config.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
[server_config]
host = "127.0.0.1"
port = 8080
# rsa_pub_file_path = "rsa_public_key.pem" # Uncomment to verify JWT tokens in the HTTP Authorization header

[postgresql_config]
username = "postgres" # Optional in TOML, can be overridden by env var `VSS_POSTGRESQL_USERNAME`
Expand Down