From 27c5236aab76d63a3eb5c8f1abaa1ebb4322dbfc Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Thu, 21 Mar 2019 13:02:12 -0600 Subject: [PATCH] Allow existing users to sign in while READ_ONLY_MODE=1 Currently whenever a user signs in we attempt to update their profile (display name, avatar, email, etc). If we're in read only mode, we can't do that. However, preventing them from signing in entirely seems unfortunate. If we get a read-only error, we will now try to look for an existing user, returning that user if found (or any errors that occurred loading it), and continue to return `ReadOnlyMode` if we couldn't find an existing user. A user will have to sign out and back in again after maintenance has completed if they want to update their information. Ideally we'd display a warning informing them of this fact, but we don't currently have code in place to support that, and with < 3 hours remaining until maintenance time, there's not enough time to write it. I'd also love to add some explicit tests for this, but none of our existing tests handle GitHub oauth, and again, there's not enough time for me to figure out what our recordings need to look like for that. If we're not comfortable shipping this in this imperfect state (no warning, no tests), then we should just close this, and folks won't be able to log in during the maintenance period. That's not the end of the world. However, I've tested this locally to make sure it behaves properly (with #1689 as well to make sure we display the right error), so I'd like to ship this to minimize disruption during the DB maintenance --- src/controllers/user/session.rs | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/controllers/user/session.rs b/src/controllers/user/session.rs index daa5fca50e0..0f0760d8113 100644 --- a/src/controllers/user/session.rs +++ b/src/controllers/user/session.rs @@ -6,6 +6,8 @@ use rand::distributions::Alphanumeric; use rand::{thread_rng, Rng}; use crate::models::{NewUser, User}; +use crate::schema::users; +use crate::util::errors::{CargoError, ReadOnlyMode}; /// Handles the `GET /authorize_url` route. /// @@ -111,8 +113,8 @@ struct GithubUser { } impl GithubUser { - fn save_to_database(&self, access_token: &str, conn: &PgConnection) -> QueryResult { - Ok(NewUser::new( + fn save_to_database(&self, access_token: &str, conn: &PgConnection) -> CargoResult { + NewUser::new( self.id, &self.login, self.email.as_ref().map(|s| &s[..]), @@ -120,7 +122,21 @@ impl GithubUser { self.avatar_url.as_ref().map(|s| &s[..]), access_token, ) - .create_or_update(conn)?) + .create_or_update(conn) + .map_err(Into::into) + .or_else(|e: Box| { + // If we're in read only mode, we can't update their details + // just look for an existing user + if e.is::() { + users::table + .filter(users::gh_id.eq(self.id)) + .first(conn) + .optional()? + .ok_or(e) + } else { + Err(e) + } + }) } }