Remove generics from authentication code (#9504)

This prepares for moving some of the implementation into axum extractors, which only get access to `Parts`.

Author: Turbo87

src/auth.rs

@@ -11,6 +11,7 @@ use crate::util::errors::{
 use crate::util::token::HashedToken;
 use chrono::Utc;
 use http::header;
+use http::request::Parts;
 
 #[derive(Debug, Clone)]
 pub struct AuthCheck {
@@ -57,18 +58,14 @@ impl AuthCheck {
     }
 
     #[instrument(name = "auth.check", skip_all)]
-    pub fn check<T: RequestPartsExt>(
-        &self,
-        request: &T,
-        conn: &mut impl Conn,
-    ) -> AppResult<Authentication> {
-        let auth = authenticate(request, conn)?;
+    pub fn check(&self, parts: &Parts, conn: &mut impl Conn) -> AppResult<Authentication> {
+        let auth = authenticate(parts, conn)?;
 
         if let Some(token) = auth.api_token() {
             if !self.allow_token {
                 let error_message =
                     "API Token authentication was explicitly disallowed for this API";
-                request.request_log().add("cause", error_message);
+                parts.request_log().add("cause", error_message);
 
                 return Err(forbidden(
                     "this action can only be performed on the crates.io website",
@@ -77,7 +74,7 @@ impl AuthCheck {
 
             if !self.endpoint_scope_matches(token.endpoint_scopes.as_ref()) {
                 let error_message = "Endpoint scope mismatch";
-                request.request_log().add("cause", error_message);
+                parts.request_log().add("cause", error_message);
 
                 return Err(forbidden(
                     "this token does not have the required permissions to perform this action",
@@ -86,7 +83,7 @@ impl AuthCheck {
 
             if !self.crate_scope_matches(token.crate_scopes.as_ref()) {
                 let error_message = "Crate scope mismatch";
-                request.request_log().add("cause", error_message);
+                parts.request_log().add("cause", error_message);
 
                 return Err(forbidden(
                     "this token does not have the required permissions to perform this action",
@@ -171,11 +168,11 @@ impl Authentication {
 }
 
 #[instrument(skip_all)]
-fn authenticate_via_cookie<T: RequestPartsExt>(
-    req: &T,
+fn authenticate_via_cookie(
+    parts: &Parts,
     conn: &mut impl Conn,
 ) -> AppResult<Option<CookieAuthentication>> {
-    let user_id_from_session = req
+    let user_id_from_session = parts
         .session()
         .get("user_id")
         .and_then(|s| s.parse::<i32>().ok());
@@ -185,23 +182,23 @@ fn authenticate_via_cookie<T: RequestPartsExt>(
     };
 
     let user = User::find(conn, id).map_err(|err| {
-        req.request_log().add("cause", err);
+        parts.request_log().add("cause", err);
         internal("user_id from cookie not found in database")
     })?;
 
     ensure_not_locked(&user)?;
 
-    req.request_log().add("uid", id);
+    parts.request_log().add("uid", id);
 
     Ok(Some(CookieAuthentication { user }))
 }
 
 #[instrument(skip_all)]
-fn authenticate_via_token<T: RequestPartsExt>(
-    req: &T,
+fn authenticate_via_token(
+    parts: &Parts,
     conn: &mut impl Conn,
 ) -> AppResult<Option<TokenAuthentication>> {
-    let maybe_authorization = req
+    let maybe_authorization = parts
         .headers()
         .get(header::AUTHORIZATION)
         .and_then(|h| h.to_str().ok());
@@ -215,43 +212,43 @@ fn authenticate_via_token<T: RequestPartsExt>(
 
     let token = ApiToken::find_by_api_token(conn, &token).map_err(|e| {
         let cause = format!("invalid token caused by {e}");
-        req.request_log().add("cause", cause);
+        parts.request_log().add("cause", cause);
 
         forbidden("authentication failed")
     })?;
 
     let user = User::find(conn, token.user_id).map_err(|err| {
-        req.request_log().add("cause", err);
+        parts.request_log().add("cause", err);
         internal("user_id from token not found in database")
     })?;
 
     ensure_not_locked(&user)?;
 
-    req.request_log().add("uid", token.user_id);
-    req.request_log().add("tokenid", token.id);
+    parts.request_log().add("uid", token.user_id);
+    parts.request_log().add("tokenid", token.id);
 
     Ok(Some(TokenAuthentication { user, token }))
 }
 
 #[instrument(skip_all)]
-fn authenticate<T: RequestPartsExt>(req: &T, conn: &mut impl Conn) -> AppResult<Authentication> {
-    controllers::util::verify_origin(req)?;
+fn authenticate(parts: &Parts, conn: &mut impl Conn) -> AppResult<Authentication> {
+    controllers::util::verify_origin(parts)?;
 
-    match authenticate_via_cookie(req, conn) {
+    match authenticate_via_cookie(parts, conn) {
         Ok(None) => {}
         Ok(Some(auth)) => return Ok(Authentication::Cookie(auth)),
         Err(err) => return Err(err),
     }
 
-    match authenticate_via_token(req, conn) {
+    match authenticate_via_token(parts, conn) {
         Ok(None) => {}
         Ok(Some(auth)) => return Ok(Authentication::Token(auth)),
         Err(err) => return Err(err),
     }
 
     // Unable to authenticate the user
     let cause = "no cookie session or auth header found";
-    req.request_log().add("cause", cause);
+    parts.request_log().add("cause", cause);
 
     return Err(forbidden("this action requires authentication"));
 }

src/controllers/crate_owner_invitation.rs

@@ -275,16 +275,18 @@ struct OwnerInvitation {
 
 /// Handles the `PUT /api/v1/me/crate_owner_invitations/:crate_id` route.
 pub async fn handle_invite(state: AppState, req: BytesRequest) -> AppResult<Json<Value>> {
+    let (parts, body) = req.0.into_parts();
+
     let crate_invite: OwnerInvitation =
-        serde_json::from_slice(req.body()).map_err(|_| bad_request("invalid json request"))?;
+        serde_json::from_slice(&body).map_err(|_| bad_request("invalid json request"))?;
 
     let crate_invite = crate_invite.crate_owner_invite;
 
     let conn = state.db_write().await?;
     spawn_blocking(move || {
         let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into();
 
-        let auth = AuthCheck::default().check(&req, conn)?;
+        let auth = AuthCheck::default().check(&parts, conn)?;
         let user_id = auth.user_id();
 
         let config = &state.config;

src/controllers/token.rs

@@ -67,6 +67,8 @@ pub async fn list(
 
 /// Handles the `PUT /me/tokens` route.
 pub async fn new(app: AppState, req: BytesRequest) -> AppResult<Json<Value>> {
+    let (parts, body) = req.0.into_parts();
+
     let conn = app.db_write().await?;
     spawn_blocking(move || {
         let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into();
@@ -87,15 +89,15 @@ pub async fn new(app: AppState, req: BytesRequest) -> AppResult<Json<Value>> {
             api_token: NewApiToken,
         }
 
-        let new: NewApiTokenRequest = json::from_slice(req.body())
+        let new: NewApiTokenRequest = json::from_slice(&body)
             .map_err(|e| bad_request(format!("invalid new token request: {e:?}")))?;
 
         let name = &new.api_token.name;
         if name.is_empty() {
             return Err(bad_request("name must have a value"));
         }
 
-        let auth = AuthCheck::default().check(&req, conn)?;
+        let auth = AuthCheck::default().check(&parts, conn)?;
         if auth.api_token_id().is_some() {
             return Err(bad_request(
                 "cannot use an API token to create a new API token",

src/controllers/user/me.rs

@@ -129,26 +129,27 @@ pub async fn confirm_user_email(state: AppState, Path(token): Path<String>) -> A
 
 /// Handles `PUT /me/email_notifications` route
 pub async fn update_email_notifications(app: AppState, req: BytesRequest) -> AppResult<Response> {
+    let (parts, body) = req.0.into_parts();
+
     #[derive(Deserialize)]
     struct CrateEmailNotifications {
         id: i32,
         email_notifications: bool,
     }
 
-    let updates: HashMap<i32, bool> =
-        serde_json::from_slice::<Vec<CrateEmailNotifications>>(req.body())
-            .map_err(|_| bad_request("invalid json request"))?
-            .iter()
-            .map(|c| (c.id, c.email_notifications))
-            .collect();
+    let updates: HashMap<i32, bool> = serde_json::from_slice::<Vec<CrateEmailNotifications>>(&body)
+        .map_err(|_| bad_request("invalid json request"))?
+        .iter()
+        .map(|c| (c.id, c.email_notifications))
+        .collect();
 
     let conn = app.db_write().await?;
     spawn_blocking(move || {
         let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into();
 
         use diesel::pg::upsert::excluded;
 
-        let user_id = AuthCheck::default().check(&req, conn)?.user_id();
+        let user_id = AuthCheck::default().check(&parts, conn)?.user_id();
 
         // Build inserts from existing crates belonging to the current user
         let to_insert = CrateOwner::by_owner_kind(OwnerKind::User)

src/controllers/util.rs

@@ -10,9 +10,9 @@ use http::{header, Extensions, HeaderMap, HeaderValue, Method, Request, Uri, Ver
 /// We don't want to accept authenticated requests that originated from other sites, so this
 /// function returns an error if the Origin header doesn't match what we expect "this site" to
 /// be: <https://crates.io> in production, or <http://localhost:port/> in development.
-pub fn verify_origin<T: RequestPartsExt>(req: &T) -> AppResult<()> {
-    let headers = req.headers();
-    let allowed_origins = &req.app().config.allowed_origins;
+pub fn verify_origin(parts: &Parts) -> AppResult<()> {
+    let headers = parts.headers();
+    let allowed_origins = &parts.app().config.allowed_origins;
 
     let bad_origin = headers
         .get_all(header::ORIGIN)
@@ -23,7 +23,7 @@ pub fn verify_origin<T: RequestPartsExt>(req: &T) -> AppResult<()> {
         let error_message =
             format!("only same-origin requests can be authenticated. got {bad_origin:?}");
 
-        req.request_log().add("cause", error_message);
+        parts.request_log().add("cause", error_message);
 
         return Err(forbidden("invalid origin header"));
     }