Skip to content

Implement token-based sessions. #4690

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
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
13 changes: 12 additions & 1 deletion Cargo.lock

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

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ cookie = { version = "=0.16.0", features = ["secure"] }
dashmap = { version = "=5.2.0", features = ["raw-api"] }
derive_deref = "=1.1.1"
dialoguer = "=0.10.1"
diesel = { version = "=1.4.8", features = ["postgres", "serde_json", "chrono", "r2d2"] }
diesel = { version = "=1.4.8", features = ["postgres", "serde_json", "chrono", "r2d2", "network-address"] }
diesel_full_text_search = "=1.0.1"
diesel_migrations = { version = "=1.4.0", features = ["postgres"] }
dotenv = "=0.15.0"
Expand Down
1 change: 1 addition & 0 deletions migrations/2022-02-21-211645_create_sessions/down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
DROP TABLE persistent_sessions;
12 changes: 12 additions & 0 deletions migrations/2022-02-21-211645_create_sessions/up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
CREATE TABLE persistent_sessions
(
id BIGSERIAL
CONSTRAINT persistent_sessions_pk
PRIMARY KEY,
user_id INTEGER NOT NULL,
hashed_token bytea NOT NULL,
created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP NOT NULL,
revoked BOOLEAN DEFAULT FALSE NOT NULL
);

COMMENT ON TABLE persistent_sessions IS 'This table contains the hashed tokens for all of the cookie-based persistent sessions';
46 changes: 43 additions & 3 deletions src/controllers/user/session.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
use crate::controllers::frontend_prelude::*;

use conduit_cookie::RequestSession;
use conduit_cookie::{RequestCookies, RequestSession};
use cookie::Cookie;
use oauth2::reqwest::http_client;
use oauth2::{AuthorizationCode, Scope, TokenResponse};
use thiserror::Error;

use crate::email::Emails;
use crate::github::GithubUser;
use crate::models::{NewUser, User};
use crate::models::persistent_session::ParseSessionCookieError;
use crate::models::persistent_session::SessionCookie;
use crate::models::{NewUser, PersistentSession, User};
use crate::schema::users;
use crate::util::errors::ReadOnlyMode;
use crate::Env;

/// Handles the `GET /api/private/session/begin` route.
///
Expand Down Expand Up @@ -102,7 +107,15 @@ pub fn authorize(req: &mut dyn RequestExt) -> EndpointResult {
&*req.db_write()?,
)?;

// Log in by setting a cookie and the middleware authentication
// Setup a persistent session for the newly logged in user.
let (_session, cookie) = PersistentSession::create(user.id).insert(&*req.db_write()?)?;

// Setup persistent session cookie.
let secure = req.app().config.env() == Env::Production;
req.cookies_mut().add(cookie.build(secure));

// TODO(adsnaider): Remove as part of https://github.com/rust-lang/crates.io/issues/2630.
// Log in by setting a cookie and the middleware authentication.
req.session_mut()
.insert("user_id".to_string(), user.id.to_string());

Expand Down Expand Up @@ -139,9 +152,36 @@ fn save_user_to_database(
})
}

#[derive(Error, Debug, PartialEq)]
pub enum LogoutError {
#[error("No session cookie found.")]
MissingSessionCookie,
#[error("Session cookie had an unexpected format.")]
SessionCookieMalformatted(#[from] ParseSessionCookieError),
#[error("Session is not in the database.")]
SessionNotInDB,
}

/// Handles the `DELETE /api/private/session` route.
pub fn logout(req: &mut dyn RequestExt) -> EndpointResult {
// TODO(adsnaider): Remove as part of https://github.com/rust-lang/crates.io/issues/2630.
req.session_mut().remove(&"user_id".to_string());

// Remove persistent session from database.
let session_cookie = req
.cookies()
.get(SessionCookie::SESSION_COOKIE_NAME)
.ok_or(LogoutError::MissingSessionCookie)?
.value()
.parse::<SessionCookie>()?;

req.cookies_mut()
.remove(Cookie::named(SessionCookie::SESSION_COOKIE_NAME));

let conn = req.db_write()?;
let mut session = PersistentSession::find(session_cookie.session_id(), &conn)?
.ok_or(LogoutError::SessionNotInDB)?;
session.revoke().update(&conn)?;
Ok(req.json(&true))
}

Expand Down
29 changes: 26 additions & 3 deletions src/controllers/util.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
use chrono::Utc;
use conduit_cookie::RequestSession;
use conduit_cookie::RequestCookies;

use super::prelude::*;

use crate::middleware::log_request;
use crate::models::{ApiToken, User};
use crate::models::persistent_session::SessionCookie;
use crate::models::{ApiToken, PersistentSession, User};
use crate::util::errors::{
account_locked, forbidden, internal, AppError, AppResult, InsecurelyGeneratedTokenRevoked,
};
use conduit_cookie::RequestSession;

#[derive(Debug)]
pub struct AuthenticatedUser {
Expand Down Expand Up @@ -67,6 +68,8 @@ fn verify_origin(req: &dyn RequestExt) -> AppResult<()> {
fn authenticate_user(req: &dyn RequestExt) -> AppResult<AuthenticatedUser> {
let conn = req.db_write()?;

// TODO(adsnaider): Remove as part of https://github.com/rust-lang/crates.io/issues/2630.
// Log in with the session id.
let session = req.session();
let user_id_from_session = session.get("user_id").and_then(|s| s.parse::<i32>().ok());

Expand All @@ -80,6 +83,26 @@ fn authenticate_user(req: &dyn RequestExt) -> AppResult<AuthenticatedUser> {
});
}

// Log in with persistent session token.
if let Some(Ok(session_cookie)) = req
.cookies()
.get(SessionCookie::SESSION_COOKIE_NAME)
.map(|cookie| cookie.value())
.map(|cookie| cookie.parse::<SessionCookie>())
{
if let Some(session) = PersistentSession::find(session_cookie.session_id(), &conn)? {
if session.is_authorized(session_cookie.token()) {
let user = User::find(&conn, session.user_id).map_err(|e| {
e.chain(internal("user_id from session not found in the database"))
})?;
return Ok(AuthenticatedUser {
user,
token_id: None,
});
}
}
}

// Otherwise, look for an `Authorization` header on the request
let maybe_authorization = req
.headers()
Expand Down
2 changes: 2 additions & 0 deletions src/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub use self::follow::Follow;
pub use self::keyword::{CrateKeyword, Keyword};
pub use self::krate::{Crate, CrateVersions, NewCrate, RecentCrateDownloads};
pub use self::owner::{CrateOwner, Owner, OwnerKind};
pub use self::persistent_session::{NewPersistentSession, PersistentSession};
pub use self::rights::Rights;
pub use self::team::{NewTeam, Team};
pub use self::token::{ApiToken, CreatedApiToken};
Expand All @@ -28,6 +29,7 @@ mod follow;
mod keyword;
pub mod krate;
mod owner;
pub mod persistent_session;
mod rights;
mod team;
mod token;
Expand Down
177 changes: 177 additions & 0 deletions src/models/persistent_session.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
use chrono::NaiveDateTime;
use cookie::{Cookie, SameSite};
use diesel::prelude::*;
use std::num::ParseIntError;
use std::str::FromStr;
use thiserror::Error;

use crate::schema::persistent_sessions;
use crate::util::token::NewSecureToken;
use crate::util::token::SecureToken;
use crate::util::token::SecureTokenKind;

/// A persistent session model (as is stored in the database).
///
/// The sessions table works by maintaining a `hashed_token`. In order for a user to securely
/// demonstrate authenticity, the user provides us with the token (stored as part of a cookie). We
/// hash the token and search in the database for matches. If we find one and the token hasn't
/// been revoked, then we update the session with the latest values and authorize the user.
#[derive(Clone, Debug, PartialEq, Eq, Identifiable, Queryable)]
#[table_name = "persistent_sessions"]
pub struct PersistentSession {
/// The id of this session.
pub id: i64,
/// The user id associated with this session.
pub user_id: i32,
/// The token (hashed) that identifies the session.
pub hashed_token: SecureToken,
/// Datetime the session was created.
pub created_at: NaiveDateTime,
/// Whether the session is revoked.
pub revoked: bool,
}

impl PersistentSession {
/// Creates a `NewPersistentSession` for the `user_id` and the token associated with it.
pub fn create(user_id: i32) -> NewPersistentSession {
let token = SecureToken::generate(SecureTokenKind::Session);
NewPersistentSession { user_id, token }
}

/// Finds the session with the ID.
///
/// # Returns
///
/// * `Ok(Some(...))` if a session matches the id.
/// * `Ok(None)` if no session matches the id.
/// * `Err(...)` for other errors..
pub fn find(id: i64, conn: &PgConnection) -> Result<Option<Self>, diesel::result::Error> {
persistent_sessions::table
.find(id)
.get_result(conn)
.optional()
}

/// Updates the session in the database.
pub fn update(&self, conn: &PgConnection) -> Result<(), diesel::result::Error> {
diesel::update(persistent_sessions::table.find(self.id))
.set((
persistent_sessions::user_id.eq(&self.user_id),
persistent_sessions::hashed_token.eq(&self.hashed_token),
persistent_sessions::revoked.eq(&self.revoked),
))
.get_result::<Self>(conn)
.map(|_| ())
}

pub fn is_authorized(&self, token: &str) -> bool {
if let Some(hashed_token) = SecureToken::parse(SecureTokenKind::Session, token) {
!self.revoked && self.hashed_token == hashed_token
} else {
false
}
}

/// Revokes the session (needs update).
pub fn revoke(&mut self) -> &mut Self {
self.revoked = true;
self
}
}

/// A new, insertable persistent session.
pub struct NewPersistentSession {
user_id: i32,
token: NewSecureToken,
}

impl NewPersistentSession {
/// Inserts the session into the database.
///
/// # Returns
///
/// The
pub fn insert(
self,
conn: &PgConnection,
) -> Result<(PersistentSession, SessionCookie), diesel::result::Error> {
let session: PersistentSession = diesel::insert_into(persistent_sessions::table)
.values((
persistent_sessions::user_id.eq(&self.user_id),
persistent_sessions::hashed_token.eq(&*self.token),
))
.get_result(conn)?;
let id = session.id;
Ok((
session,
SessionCookie::new(id, self.token.plaintext().to_string()),
))
}
}

/// Holds the information needed for the session cookie.
#[derive(Debug, PartialEq, Eq)]
pub struct SessionCookie {
/// The session ID in the database.
id: i64,
/// The token
token: String,
}

impl SessionCookie {
/// Name of the cookie used for session-based authentication.
pub const SESSION_COOKIE_NAME: &'static str = "__Host-auth";

/// Creates a new `SessionCookie`.
pub fn new(id: i64, token: String) -> Self {
Self { id, token }
}

/// Returns the `[Cookie]`.
pub fn build(&self, secure: bool) -> Cookie<'static> {
Cookie::build(
Self::SESSION_COOKIE_NAME,
format!("{}:{}", self.id, &self.token),
)
.http_only(true)
.secure(secure)
.same_site(SameSite::Strict)
.path("/")
.finish()
}

pub fn session_id(&self) -> i64 {
self.id
}

pub fn token(&self) -> &str {
&self.token
}
}

/// Error returned when the session cookie couldn't be parsed.
#[derive(Error, Debug, PartialEq)]
pub enum ParseSessionCookieError {
#[error("The session id wasn't in the cookie.")]
MissingSessionId,
#[error("The session id couldn't be parsed from the cookie.")]
IdParseError(#[from] ParseIntError),
#[error("The session token wasn't in the cookie.")]
MissingToken,
}

impl FromStr for SessionCookie {
type Err = ParseSessionCookieError;
fn from_str(s: &str) -> Result<Self, Self::Err> {
let mut id_and_token = s.split(':');
let id: i64 = id_and_token
.next()
.ok_or(ParseSessionCookieError::MissingSessionId)?
.parse()?;
let token = id_and_token
.next()
.ok_or(ParseSessionCookieError::MissingToken)?;

Ok(Self::new(id, token.to_string()))
}
}
Loading