Skip to content

Commit def94b3

Browse files
committed
Auto merge of #2167 - integer32llc:fix-auth, r=jtgeibel
Fix auth Set the TrustedUserId in the extensions when logging in Fixes #2166. In #2077 (d0b9bcc), we changed the type of the authentication data stored in the extensions from `User` to `TrustedUserId`, but we missed this spot :( We're allowed to insert anything we want into the extensions, but if we use a different type than what we use when looking up data in the extensions, it won't find what we inserted. I checked all the other locations of `mut_extensions` and `extensions` and I think they're good, but overall this system feels a bit loosey-goosey to me. Not sure how to improve it, though. r? @jtgeibel
2 parents ac60f98 + 949d294 commit def94b3

File tree

2 files changed

+8
-14
lines changed

2 files changed

+8
-14
lines changed

src/controllers/user/me.rs

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,6 @@ use crate::views::{EncodableMe, EncodableVersion, OwnedCrate};
1313

1414
/// Handles the `GET /me` route.
1515
pub fn me(req: &mut dyn Request) -> AppResult<Response> {
16-
// Changed to getting User information from database because in
17-
// src/tests/user.rs, when testing put and get on updating email,
18-
// request seems to be somehow 'cached'. When we try to get a
19-
// request from the /me route with the just updated user (call
20-
// this function) the user is the same as the initial GET request
21-
// and does not seem to get the updated user information from the
22-
// database
23-
// This change is not preferable, we'd rather fix the request,
24-
// perhaps adding `req.mut_extensions().insert(user)` to the
25-
// update_user route, however this somehow does not seem to work
26-
2716
let conn = req.db_conn()?;
2817
let user_id = req.authenticate(&conn)?.user_id();
2918

src/controllers/user/session.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use conduit_cookie::RequestSession;
55
use failure::Fail;
66
use oauth2::{prelude::*, AuthorizationCode, TokenResponse};
77

8+
use crate::middleware::current_user::TrustedUserId;
89
use crate::models::{NewUser, User};
910
use crate::schema::users;
1011
use crate::util::errors::ReadOnlyMode;
@@ -88,8 +89,7 @@ pub fn authorize(req: &mut dyn Request) -> AppResult<Response> {
8889
}
8990
}
9091

91-
// Fetch the access token from github using the code we just got
92-
92+
// Fetch the access token from GitHub using the code we just got
9393
let code = AuthorizationCode::new(code);
9494
let token = req
9595
.app()
@@ -98,11 +98,16 @@ pub fn authorize(req: &mut dyn Request) -> AppResult<Response> {
9898
.map_err(|e| e.compat())
9999
.chain_error(|| server_error("Error obtaining token"))?;
100100
let token = token.access_token();
101+
102+
// Fetch the user info from GitHub using the access token we just got and create a user record
101103
let ghuser = github::github_api::<GithubUser>(req.app(), "/user", token)?;
102104
let user = ghuser.save_to_database(&token.secret(), &*req.db_conn()?)?;
105+
106+
// Log in by setting a cookie and the middleware authentication
103107
req.session()
104108
.insert("user_id".to_string(), user.id.to_string());
105-
req.mut_extensions().insert(user);
109+
req.mut_extensions().insert(TrustedUserId(user.id));
110+
106111
super::me::me(req)
107112
}
108113

0 commit comments

Comments
 (0)