diff --git a/Cargo.lock b/Cargo.lock index 61df94d89d5..4ace57b2d36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -268,6 +268,7 @@ dependencies = [ "conduit-test", "cookie", "ctrlc", + "derive_builder", "derive_deref", "dialoguer", "diesel", @@ -285,6 +286,7 @@ dependencies = [ "hyper", "hyper-tls", "indexmap", + "ipnetwork", "jemallocator", "lazy_static", "lettre", @@ -403,7 +405,7 @@ dependencies = [ "indexmap", "lazy_static", "os_str_bytes", - "strsim", + "strsim 0.10.0", "termcolor", "textwrap", "unicode-width", @@ -664,6 +666,41 @@ dependencies = [ "winapi", ] +[[package]] +name = "darling" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0d706e75d87e35569db781a9b5e2416cff1236a47ed380831f959382ccd5f858" +dependencies = [ + "darling_core", + "darling_macro", +] + +[[package]] +name = "darling_core" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f0c960ae2da4de88a91b2d920c2a7233b400bc33cb28453a2987822d8392519b" +dependencies = [ + "fnv", + "ident_case", + "proc-macro2", + "quote", + "strsim 0.9.3", + "syn", +] + +[[package]] +name = "darling_macro" +version = "0.10.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d9b5a2f4ac4969822c62224815d069952656cadc7084fdca9751e6d959189b72" +dependencies = [ + "darling_core", + "quote", + "syn", +] + [[package]] name = "debugid" version = "0.7.2" @@ -674,6 +711,31 @@ dependencies = [ "uuid", ] +[[package]] +name = "derive_builder" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a2658621297f2cf68762a6f7dc0bb7e1ff2cfd6583daef8ee0fed6f7ec468ec0" +dependencies = [ + "darling", + "derive_builder_core", + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "derive_builder_core" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2791ea3e372c8495c0bc2033991d76b512cd799d07491fbd6890124db9458bef" +dependencies = [ + "darling", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "derive_deref" version = "1.1.1" @@ -707,6 +769,8 @@ dependencies = [ "byteorder", "chrono", "diesel_derives", + "ipnetwork", + "libc", "pq-sys", "r2d2", "serde_json", @@ -1242,6 +1306,12 @@ dependencies = [ "unicase", ] +[[package]] +name = "ident_case" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" + [[package]] name = "idna" version = "0.2.1" @@ -1300,6 +1370,15 @@ version = "2.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "47be2f14c678be2fdcab04ab1171db51b2762ce6f0a8ee87c8dd4a04ed216135" +[[package]] +name = "ipnetwork" +version = "0.16.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b8eca9f51da27bc908ef3dd85c21e1bbba794edaf94d7841e37356275b82d31e" +dependencies = [ + "serde", +] + [[package]] name = "itoa" version = "0.4.7" @@ -2684,6 +2763,12 @@ dependencies = [ "quote", ] +[[package]] +name = "strsim" +version = "0.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6446ced80d6c486436db5c078dde11a9f73d42b57fb273121e160b84f63d894c" + [[package]] name = "strsim" version = "0.10.0" diff --git a/Cargo.toml b/Cargo.toml index 78ba486a16c..43b9b523f2a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,9 +49,10 @@ conduit-static = "0.9.0-alpha.3" cookie = { version = "0.14", features = ["secure"] } ctrlc = { version = "3.0", features = ["termination"] } +derive_builder = "0.9.0" derive_deref = "1.1.1" dialoguer = "0.7.1" -diesel = { version = "1.4.0", features = ["postgres", "serde_json", "chrono", "r2d2"] } +diesel = { version = "1.4.0", features = ["postgres", "serde_json", "chrono", "r2d2", "network-address"] } diesel_full_text_search = "1.0.0" dotenv = "0.15" flate2 = "1.0" @@ -64,6 +65,7 @@ htmlescape = "0.3.1" http = "0.2" hyper = { version = "0.14", features = ["client", "http1"] } indexmap = "1.0.2" +ipnetwork = "0.16.0" jemallocator = { version = "0.3", features = ['unprefixed_malloc_on_supported_platforms', 'profiling'] } lettre = { version = "0.10.0-alpha.1", default-features = false, features = ["file-transport", "smtp-transport", "native-tls", "hostname", "builder"] } license-exprs = "^1.4" diff --git a/migrations/2021-02-14-201950_session-table/down.sql b/migrations/2021-02-14-201950_session-table/down.sql new file mode 100644 index 00000000000..180974db46e --- /dev/null +++ b/migrations/2021-02-14-201950_session-table/down.sql @@ -0,0 +1 @@ +DROP TABLE sessions; diff --git a/migrations/2021-02-14-201950_session-table/up.sql b/migrations/2021-02-14-201950_session-table/up.sql new file mode 100644 index 00000000000..02f3d6918e5 --- /dev/null +++ b/migrations/2021-02-14-201950_session-table/up.sql @@ -0,0 +1,24 @@ +CREATE TABLE sessions +( + id SERIAL + CONSTRAINT sessions_pk + PRIMARY KEY, + user_id INTEGER NOT NULL + CONSTRAINT sessions_users_id_fk + REFERENCES users + ON UPDATE CASCADE ON DELETE CASCADE, + hashed_token bytea NOT NULL, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP NOT NULL, + last_used_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP NOT NULL, + revoked BOOLEAN DEFAULT FALSE NOT NULL, + last_ip_address inet NOT NULL, + last_user_agent VARCHAR NOT NULL +); + +COMMENT ON TABLE sessions IS 'This table contains the tokens for all of the cookie-based sessions'; + +CREATE INDEX sessions_user_id_index + ON sessions (user_id); + +CREATE UNIQUE INDEX sessions_token_uindex + ON sessions (hashed_token); diff --git a/src/controllers.rs b/src/controllers.rs index 48f0c39da89..53dab074a0c 100644 --- a/src/controllers.rs +++ b/src/controllers.rs @@ -82,7 +82,7 @@ mod prelude { } pub mod helpers; -mod util; +pub mod util; pub mod category; pub mod crate_owner_invitation; diff --git a/src/controllers/user/session.rs b/src/controllers/user/session.rs index 36106dd9da8..b39c81991b4 100644 --- a/src/controllers/user/session.rs +++ b/src/controllers/user/session.rs @@ -1,13 +1,15 @@ use crate::controllers::frontend_prelude::*; -use conduit_cookie::RequestSession; +use conduit_cookie::{RequestCookies, RequestSession}; use oauth2::reqwest::http_client; use oauth2::{AuthorizationCode, Scope, TokenResponse}; +use crate::controllers::util::{auth_cookie, AUTH_COOKIE_NAME}; use crate::github::GithubUser; -use crate::models::{NewUser, User}; +use crate::models::{NewUser, Session, User}; use crate::schema::users; use crate::util::errors::ReadOnlyMode; +use crate::Env; /// Handles the `GET /api/private/session/begin` route. /// @@ -104,9 +106,32 @@ pub fn authorize(req: &mut dyn RequestExt) -> EndpointResult { let ghuser = req.app().github.current_user(token)?; let user = save_user_to_database(&ghuser, &token.secret(), &*req.db_conn()?)?; - // Log in by setting a cookie and the middleware authentication - req.session_mut() - .insert("user_id".to_string(), user.id.to_string()); + // Create a new `Session` + let token = Session::generate_token(); + + let ip_addr = req.remote_addr().ip(); + + let user_agent = req + .headers() + .get(header::USER_AGENT) + .and_then(|value| value.to_str().ok()) + .map(|value| value.to_string()) + .unwrap_or_default(); + + Session::new() + .user_id(user.id) + .token(&token) + .last_ip_address(ip_addr) + .last_user_agent(user_agent) + .build() + .map_err(|_err| server_error("Error obtaining token"))? + .insert(&*req.db_conn()?)?; + + // Log in by setting an auth cookie + let app = req.app(); + let secure = app.config.env == Env::Production; + + req.cookies_mut().add(auth_cookie(token, secure)); super::me::me(req) } @@ -143,6 +168,28 @@ fn save_user_to_database( /// Handles the `DELETE /api/private/session` route. pub fn logout(req: &mut dyn RequestExt) -> EndpointResult { req.session_mut().remove(&"user_id".to_string()); + + // read the current session token, if it exists + let session_token = req + .cookies() + .get(AUTH_COOKIE_NAME) + .map(|cookie| cookie.value().to_string()); + + if let Some(token) = session_token { + let app = req.app(); + let secure = app.config.env == Env::Production; + + // remove the `cargo_auth` cookie + req.cookies_mut().remove(auth_cookie("", secure)); + + // try to revoke the session in the database, but explicitly + // ignore failures + let _result: Result<_, Box> = req + .db_conn() + .map_err(Into::into) + .and_then(|conn| Session::revoke_by_token(&conn, &token).map_err(Into::into)); + } + Ok(req.json(&true)) } diff --git a/src/controllers/util.rs b/src/controllers/util.rs index e0f943216be..c8638d4f4a1 100644 --- a/src/controllers/util.rs +++ b/src/controllers/util.rs @@ -1,14 +1,18 @@ use chrono::Utc; -use conduit_cookie::RequestSession; +use conduit_cookie::{RequestCookies, RequestSession}; +use cookie::{Cookie, SameSite}; use super::prelude::*; use crate::middleware::log_request; -use crate::models::{ApiToken, User}; +use crate::models::{ApiToken, Session, User}; use crate::util::errors::{ account_locked, forbidden, internal, AppError, AppResult, ChainError, InsecurelyGeneratedTokenRevoked, }; +use std::borrow::Cow; + +pub const AUTH_COOKIE_NAME: &str = "cargo_auth"; #[derive(Debug)] pub struct AuthenticatedUser { @@ -60,44 +64,85 @@ fn verify_origin(req: &dyn RequestExt) -> AppResult<()> { Ok(()) } +pub fn auth_cookie<'c, T>(token: T, secure: bool) -> Cookie<'c> +where + T: Into>, +{ + Cookie::build(AUTH_COOKIE_NAME, token) + .http_only(true) + .secure(secure) + .same_site(SameSite::Strict) + .path("/") + .finish() +} + fn authenticate_user(req: &dyn RequestExt) -> AppResult { let conn = req.db_conn()?; + let cookies = req.cookies(); + let session_token = cookies.get(AUTH_COOKIE_NAME).map(|cookie| cookie.value()); + if let Some(session_token) = session_token { + let ip_addr = req.remote_addr().ip(); + + let user_agent = req + .headers() + .get(header::USER_AGENT) + .and_then(|value| value.to_str().ok()) + .unwrap_or_default(); + + let session = Session::find_by_token_and_update(&conn, session_token, ip_addr, user_agent)?; + if let Some(session) = session { + let user = User::find(&conn, session.user_id) + .chain_error(|| internal("user_id from auth cookie not found in database"))?; + + return Ok(AuthenticatedUser { + user, + token_id: None, + }); + } + + return Err(internal("invalid session token")).chain_error(forbidden); + } + let session = req.session(); let user_id_from_session = session.get("user_id").and_then(|s| s.parse::().ok()); - let (user_id, token_id) = if let Some(id) = user_id_from_session { - (id, None) - } else { - // Otherwise, look for an `Authorization` header on the request - let maybe_authorization: Option = { - req.headers() - .get(header::AUTHORIZATION) - .and_then(|h| h.to_str().ok()) - .map(|h| h.to_string()) - }; - if let Some(header_value) = maybe_authorization { - let (user_id, token_id) = ApiToken::find_by_api_token(&conn, &header_value) - .map(|token| (token.user_id, Some(token.id))) - .map_err(|e| { - if e.is::() { - e - } else { - e.chain(internal("invalid token")).chain(forbidden()) - } - })?; - - (user_id, token_id) - } else { - // Unable to authenticate the user - return Err(internal("no cookie session or auth header found")).chain_error(forbidden); - } - }; + if let Some(id) = user_id_from_session { + let user = User::find(&conn, id) + .chain_error(|| internal("user_id from cookie not found in database"))?; - let user = User::find(&conn, user_id) - .chain_error(|| internal("user_id from cookie or token not found in database"))?; + return Ok(AuthenticatedUser { + user, + token_id: None, + }); + } + + // Otherwise, look for an `Authorization` header on the request + let maybe_authorization = req + .headers() + .get(header::AUTHORIZATION) + .and_then(|h| h.to_str().ok()); + + if let Some(header_value) = maybe_authorization { + let token = ApiToken::find_by_api_token(&conn, header_value).map_err(|e| { + if e.is::() { + e + } else { + e.chain(internal("invalid token")).chain(forbidden()) + } + })?; + + let user = User::find(&conn, token.user_id) + .chain_error(|| internal("user_id from token not found in database"))?; + + return Ok(AuthenticatedUser { + user, + token_id: Some(token.id), + }); + } - Ok(AuthenticatedUser { user, token_id }) + // Unable to authenticate the user + return Err(internal("no cookie session or auth header found")).chain_error(forbidden); } impl<'a> UserAuthenticationExt for dyn RequestExt + 'a { diff --git a/src/lib.rs b/src/lib.rs index 850472e9424..7cdeaf20059 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,6 +11,8 @@ #[macro_use] extern crate claim; #[macro_use] +extern crate derive_builder; +#[macro_use] extern crate derive_deref; #[macro_use] extern crate diesel; diff --git a/src/models.rs b/src/models.rs index f038460015b..a34e0c163c3 100644 --- a/src/models.rs +++ b/src/models.rs @@ -10,6 +10,7 @@ pub use self::keyword::{CrateKeyword, Keyword}; pub use self::krate::{Crate, CrateVersions, NewCrate, RecentCrateDownloads}; pub use self::owner::{CrateOwner, Owner, OwnerKind}; pub use self::rights::Rights; +pub use self::session::Session; pub use self::team::{NewTeam, Team}; pub use self::token::{ApiToken, CreatedApiToken}; pub use self::user::{NewUser, User}; @@ -29,6 +30,7 @@ mod keyword; pub mod krate; mod owner; mod rights; +mod session; mod team; mod token; pub mod user; diff --git a/src/models/session.rs b/src/models/session.rs new file mode 100644 index 00000000000..efc1186d80b --- /dev/null +++ b/src/models/session.rs @@ -0,0 +1,290 @@ +use crate::schema::sessions; +use chrono::NaiveDateTime; +use diesel::prelude::*; +use ipnetwork::IpNetwork; +use sha2::digest::consts::U32; +use sha2::digest::generic_array::GenericArray; +use sha2::{Digest, Sha256}; +use std::borrow::Cow; +use std::net::IpAddr; + +const TOKEN_LENGTH: usize = 32; + +#[derive(Clone, Debug, PartialEq, Eq, Identifiable, Queryable)] +#[table_name = "sessions"] +pub struct Session { + pub id: i32, + pub user_id: i32, + hashed_token: Vec, + pub created_at: NaiveDateTime, + pub last_used_at: NaiveDateTime, + pub revoked: bool, + last_ip_address: IpNetwork, + pub last_user_agent: String, +} + +impl Session { + /// Creates a new session builder that can be used to insert new session + /// into the database. + /// + /// ``` + /// let session = Session::new() + /// .user_id(user.id) + /// .token(&token) + /// .last_ip_address(ip_addr) + /// .last_user_agent(user_agent) + /// .build()? + /// .insert(&conn)?; + /// ``` + /// + /// New tokens can be generated by using `Session::generate_token()`. + pub fn new() -> NewSessionBuilder<'static> { + NewSessionBuilder::default() + } + + /// Looks for an unrevoked session with the given `token` in the database + /// and returns `Some(Session)` if successful. + /// + /// If the session exists then the `last_used_at`, `last_ip_address` and + /// `last_user_agent` fields will be updated in the same step. + pub fn find_by_token_and_update( + conn: &PgConnection, + token: &str, + ip_address: IpAddr, + user_agent: &str, + ) -> Result, diesel::result::Error> { + let hashed_token = Self::hash_token(token); + + let sessions = sessions::table + .filter(sessions::revoked.eq(false)) + .filter(sessions::hashed_token.eq(hashed_token.as_slice())); + + // If the database is in read only mode, we can't update these fields. + // Try updating in a new transaction, if that fails, fall back to reading + conn.transaction(|| { + diesel::update(sessions) + .set(( + sessions::last_used_at.eq(diesel::dsl::now), + sessions::last_ip_address.eq(IpNetwork::from(ip_address)), + sessions::last_user_agent.eq(user_agent), + )) + .get_result(conn) + .optional() + }) + .or_else(|_| sessions.first(conn).optional()) + } + + /// Looks for a unrevoked sessions with the given `user_id` in the database + /// and returns a list if successful. + pub fn find_by_user_id( + conn: &PgConnection, + user_id: i32, + ) -> Result, diesel::result::Error> { + sessions::table + .filter(sessions::revoked.eq(false)) + .filter(sessions::user_id.eq(user_id)) + .get_results(conn) + } + + /// Looks for an unrevoked session with the given `id` and `user_id` and + /// revokes it, if it exists. The `bool` return value indicates whether a + /// corresponding session was found or not. + pub fn revoke( + conn: &PgConnection, + user_id: i32, + session_id: i32, + ) -> Result { + let sessions = sessions::table + .filter(sessions::id.eq(session_id)) + .filter(sessions::user_id.eq(user_id)) + .filter(sessions::revoked.eq(false)); + + diesel::update(sessions) + .set(sessions::revoked.eq(true)) + .execute(conn) + .map(|changed_rows| changed_rows == 1) + } + + /// Looks for an unrevoked session with a `hashed_token` that matches the + /// given `token` and revokes it, if it exists. The `bool` return value + /// indicates whether a corresponding session was found or not. + pub fn revoke_by_token( + conn: &PgConnection, + token: &str, + ) -> Result { + let hashed_token = Self::hash_token(token); + + let sessions = sessions::table + .filter(sessions::hashed_token.eq(hashed_token.as_slice())) + .filter(sessions::revoked.eq(false)); + + diesel::update(sessions) + .set(sessions::revoked.eq(true)) + .execute(conn) + .map(|changed_rows| changed_rows == 1) + } + + /// Generates a new plaintext token + /// + /// Note that this needs to be hashed before saving it in the database! + pub fn generate_token() -> String { + crate::util::generate_secure_alphanumeric_string(TOKEN_LENGTH) + } + + /// Calculates the SHA256 hash of the given `token` so that it can safely + /// be stored in the database. + fn hash_token(token: &str) -> GenericArray { + Sha256::digest(token.as_bytes()) + } + + /// Returns the `IpAddr` of the last time that this session was used. + pub fn last_ip_address(&self) -> IpAddr { + self.last_ip_address.ip() + } +} + +#[derive(Builder, Clone, Debug, PartialEq, Eq, Insertable)] +#[table_name = "sessions"] +pub struct NewSession<'a> { + user_id: i32, + #[builder(private)] + hashed_token: Vec, + #[builder(private, setter(name = "_last_ip_address"))] + last_ip_address: IpNetwork, + #[builder(setter(into))] + last_user_agent: Cow<'a, str>, +} + +impl NewSession<'_> { + /// Inserts this new session record into the `sessions` database table and + /// returns a corresponding `Session` struct if successful. + pub fn insert(self, conn: &PgConnection) -> Result { + diesel::insert_into(sessions::table) + .values(self) + .get_result(conn) + } +} + +impl<'a> NewSessionBuilder<'a> { + /// Calculates the hash of the given token and sets the `hashed_token` + /// field accordingly. + pub fn token(&mut self, token: &str) -> &mut Self { + let hashed_token = Session::hash_token(token); + self.hashed_token(hashed_token.to_vec()) + } + + /// Converts the `IpAddr` to an `IpNetwork` and updates the + /// corresponding field. + pub fn last_ip_address(&mut self, ip_address: IpAddr) -> &mut Self { + self._last_ip_address(IpNetwork::from(ip_address)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::models::{NewUser, User}; + use crate::schema::users; + use crate::test_util::pg_connection; + use std::net::IpAddr; + use std::str::FromStr; + + #[test] + fn test_session() { + let conn = pg_connection(); + + // insert a new user so that the foreign key works + let new_user = NewUser::new(42, "johndoe", None, None, "secret123"); + + let user: User = assert_ok!(diesel::insert_into(users::table) + .values(new_user) + .get_result(&conn)); + + // insert a new session + let ip_addr = "192.168.0.42"; + let user_agent = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36"; + + let token = Session::generate_token(); + let new_session = assert_ok!(Session::new() + .user_id(user.id) + .token(&token) + .last_ip_address(IpAddr::from_str(ip_addr).unwrap()) + .last_user_agent(user_agent) + .build()); + + let session = assert_ok!(new_session.insert(&conn)); + assert_eq!(session.user_id, user.id); + assert_eq!(session.hashed_token, Session::hash_token(&token).to_vec()); + assert_eq!(session.revoked, false); + assert_eq!( + session.last_ip_address(), + IpAddr::from_str(ip_addr).unwrap() + ); + assert_eq!(session.last_user_agent, user_agent); + + // query the session by `token` + let ip_addr = "192.168.0.1"; + let user_agent = + "Mozilla/5.0 (Macintosh; Intel Mac OS X x.y; rv:42.0) Gecko/20100101 Firefox/42.0"; + + let query_result = assert_ok!(Session::find_by_token_and_update( + &conn, + &token, + IpAddr::from_str(ip_addr).unwrap(), + user_agent + )); + let session = assert_some!(query_result); + assert_eq!(session.user_id, user.id); + assert_eq!(session.hashed_token, Session::hash_token(&token).to_vec()); + assert_eq!(session.revoked, false); + assert_eq!( + session.last_ip_address(), + IpAddr::from_str(ip_addr).unwrap() + ); + assert_eq!(session.last_user_agent, user_agent); + + // query the session by an unknown `token` + let query_result = assert_ok!(Session::find_by_token_and_update( + &conn, + "some-other-token", + IpAddr::from_str(ip_addr).unwrap(), + user_agent + )); + assert_none!(query_result); + + // find all session by `user_id` + let query_result = assert_ok!(Session::find_by_user_id(&conn, user.id)); + assert_eq!(query_result.len(), 1); + + // revoke the session + assert_eq!( + assert_ok!(Session::revoke(&conn, user.id, session.id)), + true + ); + + // query the revoked session + let query_result = assert_ok!(Session::find_by_token_and_update( + &conn, + &token, + IpAddr::from_str(ip_addr).unwrap(), + user_agent + )); + assert_none!(query_result); + + // try to revoke the session again + assert_eq!( + assert_ok!(Session::revoke(&conn, user.id, session.id)), + false + ); + + // try to revoke a different session + assert_eq!( + assert_ok!(Session::revoke(&conn, user.id, session.id + 42)), + false + ); + + // find all session by `user_id` + let query_result = assert_ok!(Session::find_by_user_id(&conn, user.id)); + assert_eq!(query_result.len(), 0); + } +} diff --git a/src/schema.rs b/src/schema.rs index 1570beccb2b..fccfeeefcff 100644 --- a/src/schema.rs +++ b/src/schema.rs @@ -708,6 +708,65 @@ table! { } } +table! { + use diesel::sql_types::*; + use diesel_full_text_search::{TsVector as Tsvector}; + + /// Representation of the `sessions` table. + /// + /// (Automatically generated by Diesel.) + sessions (id) { + /// The `id` column of the `sessions` table. + /// + /// Its SQL type is `Int4`. + /// + /// (Automatically generated by Diesel.) + id -> Int4, + /// The `user_id` column of the `sessions` table. + /// + /// Its SQL type is `Int4`. + /// + /// (Automatically generated by Diesel.) + user_id -> Int4, + /// The `hashed_token` column of the `sessions` table. + /// + /// Its SQL type is `Bytea`. + /// + /// (Automatically generated by Diesel.) + hashed_token -> Bytea, + /// The `created_at` column of the `sessions` table. + /// + /// Its SQL type is `Timestamp`. + /// + /// (Automatically generated by Diesel.) + created_at -> Timestamp, + /// The `last_used_at` column of the `sessions` table. + /// + /// Its SQL type is `Timestamp`. + /// + /// (Automatically generated by Diesel.) + last_used_at -> Timestamp, + /// The `revoked` column of the `sessions` table. + /// + /// Its SQL type is `Bool`. + /// + /// (Automatically generated by Diesel.) + revoked -> Bool, + /// The `last_ip_address` column of the `sessions` table. + /// + /// Its SQL type is `Inet`. + /// + /// (Automatically generated by Diesel.) + last_ip_address -> Inet, + /// The `last_user_agent` column of the `sessions` table. + /// + /// Its SQL type is `Varchar`. + /// + /// (Automatically generated by Diesel.) + last_user_agent -> Varchar, + } +} + table! { use diesel::sql_types::*; use diesel_full_text_search::{TsVector as Tsvector}; @@ -1050,6 +1109,7 @@ joinable!(publish_limit_buckets -> users (user_id)); joinable!(publish_rate_overrides -> users (user_id)); joinable!(readme_renderings -> versions (version_id)); joinable!(recent_crate_downloads -> crates (crate_id)); +joinable!(sessions -> users (user_id)); joinable!(version_authors -> versions (version_id)); joinable!(version_downloads -> versions (version_id)); joinable!(version_owner_actions -> api_tokens (api_token_id)); @@ -1079,6 +1139,7 @@ allow_tables_to_appear_in_same_query!( readme_renderings, recent_crate_downloads, reserved_crate_names, + sessions, teams, users, version_authors, diff --git a/src/tasks/dump_db/dump-db.toml b/src/tasks/dump_db/dump-db.toml index 8658adc7b5e..016e577b9d5 100644 --- a/src/tasks/dump_db/dump-db.toml +++ b/src/tasks/dump_db/dump-db.toml @@ -152,6 +152,16 @@ rendered_at = "private" [reserved_crate_names.columns] name = "public" +[sessions.columns] +id = "private" +user_id = "private" +hashed_token = "private" +created_at = "private" +last_used_at = "private" +revoked = "private" +last_ip_address = "private" +last_user_agent = "private" + [teams.columns] id = "public" login = "public" diff --git a/src/tests/authentication.rs b/src/tests/authentication.rs index 23a0340bff0..ed4e04f4a2e 100644 --- a/src/tests/authentication.rs +++ b/src/tests/authentication.rs @@ -1,45 +1,90 @@ use crate::{util::RequestHelper, TestApp}; use crate::util::encode_session_header; +use cargo_registry::controllers::util::auth_cookie; use conduit::{header, Handler, HandlerResult, Method, StatusCode}; use conduit_test::MockRequest; +use serde_json::Value; static URL: &str = "/api/v1/me/updates"; -static MUST_LOGIN: &[u8] = - b"{\"errors\":[{\"detail\":\"must be logged in to perform that action\"}]}"; static INTERNAL_ERROR_NO_USER: &str = - "user_id from cookie or token not found in database caused by NotFound"; + "user_id from cookie not found in database caused by NotFound"; fn call(app: &TestApp, mut request: MockRequest) -> HandlerResult { app.as_middleware().call(&mut request) } -fn into_parts(response: HandlerResult) -> (StatusCode, std::borrow::Cow<'static, [u8]>) { - use conduit_test::ResponseExt; +#[test] +fn session_user() { + let token = "some-random-token"; + + let (app, _) = TestApp::init().empty(); + let session_user = app.db_new_user("user1").with_session(token); + let request = session_user.request_builder(Method::GET, URL); + + let response = session_user.run::(request); + assert_eq!(response.status(), StatusCode::OK); +} + +#[test] +fn cookie_user() { + let (_app, _, cookie_user) = TestApp::init().with_user(); + let request = cookie_user.request_builder(Method::GET, URL); + + let response = cookie_user.run::(request); + assert_eq!(response.status(), StatusCode::OK); +} + +#[test] +fn token_user() { + let (_app, _, _, token_user) = TestApp::init().with_token(); + let request = token_user.request_builder(Method::GET, URL); - let response = response.unwrap(); - (response.status(), response.into_cow()) + let response = token_user.run::(request); + assert_eq!(response.status(), StatusCode::OK); } #[test] fn anonymous_user_unauthorized() { - let (app, anon) = TestApp::init().empty(); + let (_app, anon) = TestApp::init().empty(); let request = anon.request_builder(Method::GET, URL); - let (status, body) = into_parts(call(&app, request)); - assert_eq!(status, StatusCode::FORBIDDEN); - assert_eq!(body, MUST_LOGIN); + let response = anon.run::(request); + assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "must be logged in to perform that action" }] }) + ); +} + +#[test] +fn session_auth_cannot_find_token() { + let cookie = auth_cookie("some-unknown-token", false).to_string(); + + let (_app, anon) = TestApp::init().empty(); + let mut request = anon.request_builder(Method::GET, URL); + request.header(header::COOKIE, &cookie); + + let response = anon.run::(request); + assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "must be logged in to perform that action" }] }) + ); } #[test] fn token_auth_cannot_find_token() { - let (app, anon) = TestApp::init().empty(); + let (_app, anon) = TestApp::init().empty(); let mut request = anon.request_builder(Method::GET, URL); request.header(header::AUTHORIZATION, "cio1tkfake-token"); - let (status, body) = into_parts(call(&app, request)); - assert_eq!(status, StatusCode::FORBIDDEN); - assert_eq!(body, MUST_LOGIN); + let response = anon.run::(request); + assert_eq!(response.status(), StatusCode::FORBIDDEN); + assert_eq!( + response.json(), + json!({ "errors": [{ "detail": "must be logged in to perform that action" }] }) + ); } // Ensure that an unexpected authentication error is available for logging. The user would see diff --git a/src/tests/util.rs b/src/tests/util.rs index 33c875397b3..6fb7a1a2e0d 100644 --- a/src/tests/util.rs +++ b/src/tests/util.rs @@ -27,7 +27,7 @@ use cargo_registry::{ background_jobs::Environment, db::DieselPool, git::{Credentials, RepositoryConfig}, - models::{ApiToken, CreatedApiToken, User}, + models::{ApiToken, CreatedApiToken, Session, User}, util::AppResponse, App, Config, }; @@ -45,9 +45,12 @@ use git2::Repository as UpstreamRepository; use url::Url; +use cargo_registry::controllers::util::auth_cookie; pub use conduit::{header, StatusCode}; use cookie::Cookie; use std::collections::HashMap; +use std::net::IpAddr; +use std::str::FromStr; pub fn init_logger() { let _ = tracing_subscriber::fmt() @@ -535,6 +538,47 @@ impl MockCookieUser { token, } } + + pub fn with_session(&self, token: &str) -> MockSessionUser { + let ip_addr = "192.168.0.42"; + let user_agent = "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36"; + + self.app.db(|conn| { + Session::new() + .user_id(self.user.id) + .token(&token) + .last_ip_address(IpAddr::from_str(ip_addr).unwrap()) + .last_user_agent(user_agent) + .build() + .unwrap() + .insert(&conn) + .unwrap() + }); + + MockSessionUser { + app: TestApp(Rc::clone(&self.app.0)), + token: token.to_string(), + } + } +} + +pub struct MockSessionUser { + app: TestApp, + token: String, +} + +impl RequestHelper for MockSessionUser { + fn request_builder(&self, method: Method, path: &str) -> MockRequest { + let cookie = auth_cookie(&self.token, false).to_string(); + + let mut request = req(method, path); + request.header(header::COOKIE, &cookie); + request + } + + fn app(&self) -> &TestApp { + &self.app + } } /// A type that can generate token authenticated requests