on second thought, add hard delete by token

Author: david-crespo

nexus/db-queries/src/db/datastore/console_session.rs

@@ -175,4 +175,25 @@ impl DataStore {
                 ))
             })
     }
+
+    pub async fn session_hard_delete_by_token(
+        &self,
+        opctx: &OpContext,
+        token: String,
+    ) -> DeleteResult {
+        // we don't do an authz check here because the possession of
+        // the token is the check
+        use nexus_db_schema::schema::console_session;
+        diesel::delete(console_session::table)
+            .filter(console_session::token.eq(token))
+            .execute_async(&*self.pool_connection_authorized(opctx).await?)
+            .await
+            .map(|_rows_deleted| ())
+            .map_err(|e| {
+                Error::internal_error(&format!(
+                    "error deleting session by token: {:?}",
+                    e
+                ))
+            })
+    }
 }

nexus/db-queries/src/db/datastore/mod.rs

@@ -700,6 +700,10 @@ mod test {
             datastore.session_hard_delete(&opctx, &authz_session).await;
         assert_eq!(delete_again, Ok(()));
 
+        let delete_again =
+            datastore.session_hard_delete_by_token(&opctx, token.clone()).await;
+        assert_eq!(delete_again, Ok(()));
+
         db.terminate().await;
         logctx.cleanup_successful();
     }

nexus/src/app/session.rs

@@ -94,6 +94,14 @@ impl super::Nexus {
         self.db_datastore.session_hard_delete(opctx, &authz_session).await
     }
 
+    pub(crate) async fn session_hard_delete_by_token(
+        &self,
+        opctx: &OpContext,
+        token: String,
+    ) -> DeleteResult {
+        self.db_datastore.session_hard_delete_by_token(opctx, token).await
+    }
+
     pub(crate) async fn lookup_silo_for_authn(
         &self,
         opctx: &OpContext,

nexus/src/external_api/http_entrypoints.rs

@@ -7563,33 +7563,17 @@ impl NexusExternalApi for NexusExternalApiImpl {
         let apictx = rqctx.context();
         let handler = async {
             let nexus = &apictx.context.nexus;
-            // this is unique among the hundreds of calls to this function in
-            // that we are not using ? to return early on error
-            let opctx =
-                crate::context::op_context_for_external_api(&rqctx).await;
+            // this is kind of a weird one, but we're only doing things here
+            // that are authorized directly by the possession of the token,
+            // which makes it somewhat like a login
+            let opctx = nexus.opctx_external_authn();
             let session_cookie =
                 cookies.get(session_cookie::SESSION_COOKIE_COOKIE_NAME);
 
-            // Look up session and delete it if present. Noop on any errors.
-            // This is the ONE spot where we do the hard delete by token and we
-            // haven't already looked up the session by token. Looking up the
-            // token first works, but it would be nice to avoid it.
-            if let Ok(opctx) = opctx {
-                if let Some(cookie) = session_cookie {
-                    let token = cookie.value().to_string();
-                    match nexus.session_fetch(&opctx, token).await {
-                        Ok(session) => {
-                            let id = session.console_session.id();
-                            // ? here because if this fails, we did not delete the
-                            // session when we meant to
-                            nexus.session_hard_delete(&opctx, id).await?;
-                        }
-                        // blow up only on errors other than not found, because not
-                        // found is fine: nothing to delete
-                        Err(Error::ObjectNotFound { .. }) => {} // noop
-                        Err(e) => return Err(e.into()),
-                    };
-                }
+            // Look up session and delete it if present
+            if let Some(cookie) = session_cookie {
+                let token = cookie.value().to_string();
+                nexus.session_hard_delete_by_token(&opctx, token).await?;
             }
 
             // If user's session was already expired, they fail auth and their

nexus/tests/integration_tests/console_api.rs

@@ -137,6 +137,19 @@ async fn test_sessions(cptestctx: &ControlPlaneTestContext) {
         .await
         .expect("failed to log out");
 
+    // logout with a nonexistent session token does the same thing: clears it out
+    RequestBuilder::new(&testctx, Method::POST, "/v1/logout")
+        .header(header::COOKIE, "session=abc")
+        .expect_status(Some(StatusCode::NO_CONTENT))
+        // logout also clears the cookie client-side
+        .expect_response_header(
+            header::SET_COOKIE,
+            "session=; Path=/; HttpOnly; SameSite=Lax; Max-Age=0",
+        )
+        .execute()
+        .await
+        .expect("failed to log out");
+
     // now the same requests with the same session cookie should 401/302 because
     // logout also deletes the session server-side
     RequestBuilder::new(&testctx, Method::POST, "/v1/projects")