misc: remove session (#1596)

* refactor: remove session from auth * refactor: remove session calls from gw and deployer * refactor: cargo fmt * refactor: remove `Option` from key * refactor: remove references to cookie * refactor: remove references to login * refactor: undo hyper dependency
shuttle-hq · Jan 30, 2024 · 4dd7971 · 4dd7971
1 parent 69864f8
commit 4dd7971
Show file tree

Hide file tree

Showing 12 changed files with 82 additions and 457 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -43,7 +43,6 @@ shuttle-service = { path = "service", version = "0.37.0" }
 anyhow = "1.0.66"
 async-trait = "0.1.58"
 axum = { version = "0.6.13", default-features = false }
-axum-sessions = "0.5.0"
 base64 = "0.21.5"
 bollard = "0.15.0"
 bytes = "1.3.0"

diff --git a/DEVELOPING.md b/DEVELOPING.md
@@ -51,7 +51,7 @@ graph BT
 
 - `cargo-shuttle` is the CLI used by users to initialize, deploy and manage their projects and services on Shuttle.
 - `gateway` starts and manages instances of `deployer`. It proxies commands from the user sent via the CLI on port 8001 and traffic on port 8000 to the correct instance of `deployer`.
-- `auth` is an authentication service that creates and manages users. In addition to that, requests to the `gateway` that contain an api-key or cookie will be proxied to the `auth` service where it will be converted to a JWT for authorization between internal services (like a `deployer` requesting a database from
+- `auth` is an authentication service that creates and manages users. In addition to that, requests to the `gateway` that contain an api-key will be proxied to the `auth` service where it will be converted to a JWT for authorization between internal services (like a `deployer` requesting a database from
 `provisioner`).
 - `deployer` is a service that runs in its own docker container, one per user project. It manages a project's deployments and state.
 - `provisioner` is a service used for requesting databases and other resources, using a gRPC API.

diff --git a/auth/Cargo.toml b/auth/Cargo.toml
@@ -15,16 +15,14 @@ shuttle-common = { workspace = true, features = [
 anyhow = { workspace = true }
 async-stripe = { version = "0.25.1", default-features = false, features = ["checkout", "runtime-tokio-hyper-rustls"] }
 async-trait = { workspace = true }
-axum = { workspace = true, features = ["headers"] }
-axum-sessions = { workspace = true }
+axum = { workspace = true, features = ["headers", "tokio"] }
 base64 = { workspace = true }
 chrono = { workspace = true }
 clap = { workspace = true }
 http = { workspace = true }
 jsonwebtoken = { workspace = true }
 opentelemetry = { workspace = true }
 pem = "2"
-rand = { workspace = true }
 ring = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 sqlx = { workspace = true, features = ["postgres", "json", "migrate", "chrono"] }
@@ -36,7 +34,7 @@ tracing-opentelemetry = { workspace = true }
 tracing-subscriber = { workspace = true }
 
 [dev-dependencies]
-axum-extra = { version = "0.7.1", features = ["cookie"] }
+axum-extra = { version = "0.7.1" }
 ctor = { workspace = true }
 hyper = { workspace = true }
 once_cell = { workspace = true }

diff --git a/auth/src/api/builder.rs b/auth/src/api/builder.rs
@@ -6,8 +6,6 @@ use axum::{
     routing::{get, post, put},
     Router, Server,
 };
-use axum_sessions::{async_session::MemoryStore, SessionLayer};
-use rand::RngCore;
 use shuttle_common::{
     backends::metrics::{Metrics, TraceLayer},
     request_span,
@@ -18,12 +16,11 @@ use tracing::field;
 use crate::{
     secrets::{EdDsaManager, KeyManager},
     user::{UserManagement, UserManager},
-    COOKIE_EXPIRATION,
 };
 
 use super::handlers::{
-    convert_cookie, convert_key, get_public_key, get_user, health_check, logout, post_user,
-    put_user_reset_key, refresh_token, update_user_tier,
+    convert_key, get_public_key, get_user, health_check, post_user, put_user_reset_key,
+    refresh_token, update_user_tier,
 };
 
 pub type UserManagerState = Arc<Box<dyn UserManagement>>;
@@ -52,7 +49,6 @@ impl FromRef<RouterState> for KeyManagerState {
 pub struct ApiBuilder {
     router: Router<RouterState>,
     pool: Option<PgPool>,
-    session_layer: Option<SessionLayer<MemoryStore>>,
     stripe_client: Option<stripe::Client>,
     jwt_signing_private_key: Option<String>,
 }
@@ -67,8 +63,6 @@ impl ApiBuilder {
     pub fn new() -> Self {
         let router = Router::new()
             .route("/", get(health_check))
-            .route("/logout", post(logout))
-            .route("/auth/session", get(convert_cookie))
             .route("/auth/key", get(convert_key))
             .route("/auth/refresh", post(refresh_token))
             .route("/public-key", get(get_public_key))
@@ -94,7 +88,6 @@ impl ApiBuilder {
         Self {
             router,
             pool: None,
-            session_layer: None,
             stripe_client: None,
             jwt_signing_private_key: None,
         }
@@ -105,20 +98,6 @@ impl ApiBuilder {
         self
     }
 
-    pub fn with_sessions(mut self) -> Self {
-        let store = MemoryStore::new();
-        let mut secret = [0u8; 128];
-        rand::thread_rng().fill_bytes(&mut secret[..]);
-        self.session_layer = Some(
-            SessionLayer::new(store, &secret)
-                .with_cookie_name("shuttle.sid")
-                .with_session_ttl(Some(COOKIE_EXPIRATION))
-                .with_secure(true),
-        );
-
-        self
-    }
-
     pub fn with_stripe_client(mut self, stripe_client: stripe::Client) -> Self {
         self.stripe_client = Some(stripe_client);
         self
@@ -131,7 +110,6 @@ impl ApiBuilder {
 
     pub fn into_router(self) -> Router {
         let pool = self.pool.expect("an sqlite pool is required");
-        let session_layer = self.session_layer.expect("a session layer is required");
         let stripe_client = self.stripe_client.expect("a stripe client is required");
         let jwt_signing_private_key = self
             .jwt_signing_private_key
@@ -147,7 +125,7 @@ impl ApiBuilder {
             key_manager: Arc::new(Box::new(key_manager)),
         };
 
-        self.router.layer(session_layer).with_state(state)
+        self.router.with_state(state)
     }
 }
 

diff --git a/auth/src/api/handlers.rs b/auth/src/api/handlers.rs
@@ -6,9 +6,7 @@ use axum::{
     extract::{Path, State},
     Json,
 };
-use axum_sessions::extractors::{ReadableSession, WritableSession};
 use http::StatusCode;
-use serde::{Deserialize, Serialize};
 use shuttle_common::{
     claims::{AccountTier, Claim},
     models::user,
@@ -69,56 +67,19 @@ pub(crate) async fn update_user_tier(
 }
 
 pub(crate) async fn put_user_reset_key(
-    session: ReadableSession,
     State(user_manager): State<UserManagerState>,
-    key: Option<Key>,
+    key: Key,
 ) -> Result<(), Error> {
-    let account_name = match session.get::<String>("account_name") {
-        Some(account_name) => account_name.into(),
-        None => match key {
-            Some(key) => user_manager.get_user_by_key(key.into()).await?.name,
-            None => return Err(Error::Unauthorized),
-        },
-    };
+    let account_name = user_manager.get_user_by_key(key.into()).await?.name;
 
     user_manager.reset_key(account_name).await
 }
 
-pub(crate) async fn logout(mut session: WritableSession) {
-    session.destroy();
-}
-
 // Dummy health-check returning 200 if the auth server is up.
 pub(crate) async fn health_check() -> Result<(), Error> {
     Ok(())
 }
 
-pub(crate) async fn convert_cookie(
-    session: ReadableSession,
-    State(key_manager): State<KeyManagerState>,
-) -> Result<Json<shuttle_common::backends::auth::ConvertResponse>, StatusCode> {
-    let account_name = session
-        .get::<String>("account_name")
-        .ok_or(StatusCode::UNAUTHORIZED)?;
-
-    let account_tier = session
-        .get::<AccountTier>("account_tier")
-        .ok_or(StatusCode::UNAUTHORIZED)?;
-
-    let claim = Claim::new(
-        account_name,
-        account_tier.into(),
-        account_tier,
-        account_tier,
-    );
-
-    let token = claim.into_token(key_manager.private_key())?;
-
-    let response = shuttle_common::backends::auth::ConvertResponse { token };
-
-    Ok(Json(response))
-}
-
 /// Convert a valid API-key bearer token to a JWT.
 pub(crate) async fn convert_key(
     _: Admin,
@@ -154,8 +115,3 @@ pub(crate) async fn refresh_token() {}
 pub(crate) async fn get_public_key(State(key_manager): State<KeyManagerState>) -> Vec<u8> {
     key_manager.public_key().to_vec()
 }
-
-#[derive(Deserialize, Serialize)]
-pub struct LoginRequest {
-    account_name: AccountName,
-}
diff --git a/auth/src/lib.rs b/auth/src/lib.rs
@@ -4,7 +4,7 @@ mod error;
 mod secrets;
 mod user;
 
-use std::{io, time::Duration};
+use std::io;
 
 use args::StartArgs;
 use shuttle_common::{claims::AccountTier, ApiKey};
@@ -15,14 +15,11 @@ use crate::api::serve;
 pub use api::ApiBuilder;
 pub use args::{Args, Commands, InitArgs};
 
-pub const COOKIE_EXPIRATION: Duration = Duration::from_secs(60 * 60 * 24); // One day
-
 pub static MIGRATIONS: Migrator = sqlx::migrate!("./migrations");
 
 pub async fn start(pool: PgPool, args: StartArgs) -> io::Result<()> {
     let router = api::ApiBuilder::new()
         .with_pg_pool(pool)
-        .with_sessions()
         .with_stripe_client(stripe::Client::new(args.stripe_secret_key))
         .with_jwt_signing_private_key(args.jwt_signing_private_key)
         .into_router();

diff --git a/auth/tests/api/helpers.rs b/auth/tests/api/helpers.rs
@@ -49,7 +49,6 @@ pub(crate) async fn app() -> TestApp {
 
     let router = ApiBuilder::new()
         .with_pg_pool(pg_pool)
-        .with_sessions()
         .with_stripe_client(stripe::Client::from_url(
             mock_server.uri().as_str(),
             STRIPE_TEST_KEY,

diff --git a/auth/tests/api/main.rs b/auth/tests/api/main.rs
@@ -1,5 +1,4 @@
 mod auth;
 mod helpers;
-mod session;
 mod stripe;
 mod users;
diff --git a/auth/tests/api/session.rs b/auth/tests/api/session.rs
diff --git a/deployer/src/handlers/local.rs b/deployer/src/handlers/local.rs
@@ -1,7 +1,7 @@
 use std::net::Ipv4Addr;
 
 use axum::{
-    headers::{authorization::Bearer, Authorization, Cookie, Header, HeaderMapExt},
+    headers::{authorization::Bearer, Authorization, Header, HeaderMapExt},
     http::Request,
     middleware::Next,
     response::Response,
@@ -32,17 +32,9 @@ pub async fn set_jwt_bearer<B>(
     mut request: Request<B>,
     next: Next<B>,
 ) -> Result<Response, StatusCode> {
-    let mut auth_details = None;
-
     if let Some(bearer) = request.headers().typed_get::<Authorization<Bearer>>() {
-        auth_details = Some(make_token_request("/auth/key", bearer));
-    }
-
-    if let Some(cookie) = request.headers().typed_get::<Cookie>() {
-        auth_details = Some(make_token_request("/auth/session", cookie));
-    }
+        let token_request = make_token_request("/auth/key", bearer);
 
-    if let Some(token_request) = auth_details {
         let response = PROXY_CLIENT
             .call(
                 Ipv4Addr::LOCALHOST.into(),
@@ -68,7 +60,7 @@ pub async fn set_jwt_bearer<B>(
 
         Ok(response)
     } else {
-        error!("No api-key bearer token or cookie found, make sure you are logged in.");
+        error!("No api-key bearer token found, make sure you are logged in.");
         Err(StatusCode::UNAUTHORIZED)
     }
 }