-
Notifications
You must be signed in to change notification settings - Fork 645
WIP: Implement sessions
database table
#3307
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
Changes from all commits
617a8b2
657a982
1357225
3cc43e9
3980224
7d66257
4887398
ad00a93
86e7d77
f4614d4
e46a3d8
372d08b
21fb296
24b1f5a
14bba97
36ed665
3036ff7
c2adb0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
DROP TABLE sessions; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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<dyn AppError>> = req | ||
.db_conn() | ||
.map_err(Into::into) | ||
.and_then(|conn| Session::revoke_by_token(&conn, &token).map_err(Into::into)); | ||
Comment on lines
+185
to
+190
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps log or increment a stat in this case? Or have a set of expected errors (like timeout) and pass through unexpected errors (like permission denied or malformed SQL). |
||
} | ||
|
||
Ok(req.json(&true)) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll probably want to delete stale sessions eventually. In a big database, deleting old rows can be relatively expensive. For this, you might want to use partitioning (https://www.postgresql.org/docs/10/ddl-partitioning.html), which lets you drop old, stale partitions cheaply while still treating the partitions as a single table. However, to use partitioning you need to have no foreign keys on the partitioned table. Suggestion: Make user_id not a foreign key. We don't expect that user_ids change, so we don't need
ON UPDATE CASCADE
. And if the users entry is deleted, we can simply treat the session as invalid. So we don't needON DELETE CASCADE
.