Use a single query to revoke and return the token when responding to an alert from GitHub and add test for false positive feedback.

Author: Dan Gardner

src/controllers/github/secret_scanning.rs

@@ -156,14 +156,13 @@ fn alert_revoke_token(
     alert: &GitHubSecretAlert,
 ) -> Result<(), Box<dyn AppError>> {
     let conn = req.db_write()?;
+
     // not using ApiToken::find_by_api_token in order to preserve last_used_at
-    let token = api_tokens::table
+    // the token field has a uniqueness constraint so get_result() should be safe to use
+    let token: ApiToken = diesel::update(api_tokens::table)
         .filter(api_tokens::token.eq(alert.token.as_bytes()))
-        .first::<ApiToken>(&*conn)?;
-
-    diesel::update(&token)
         .set(api_tokens::revoked.eq(true))
-        .execute(&*conn)?;
+        .get_result::<ApiToken>(&*conn)?;
 
     // send email notification to the token owner
     let user = User::find(&conn, token.user_id)?;

src/tests/github_secret_scanning.rs

@@ -46,7 +46,7 @@ fn github_secret_alert_revokes_token() {
     assert_eq!(response.status(), StatusCode::OK);
 
     // Ensure feedback is a true positive
-    let feedback: Vec<GitHubSecretAlertFeedback> = response.good();
+    let feedback = response.good();
     assert_eq!(feedback.len(), 1);
     assert_eq!(feedback[0].token_raw, "some_token");
     assert_eq!(feedback[0].token_type, "some_type");
@@ -88,9 +88,16 @@ fn github_secret_alert_for_unknown_token() {
     request.with_body(GITHUB_ALERT);
     request.header("GITHUB-PUBLIC-KEY-IDENTIFIER", GITHUB_PUBLIC_KEY_IDENTIFIER);
     request.header("GITHUB-PUBLIC-KEY-SIGNATURE", GITHUB_PUBLIC_KEY_SIGNATURE);
-    let response = anon.run::<()>(request);
+    let response = anon.run::<Vec<GitHubSecretAlertFeedback>>(request);
     assert_eq!(response.status(), StatusCode::OK);
 
+    // Ensure feedback is a false positive
+    let feedback = response.good();
+    assert_eq!(feedback.len(), 1);
+    assert_eq!(feedback[0].token_raw, "some_token");
+    assert_eq!(feedback[0].token_type, "some_type");
+    assert_eq!(feedback[0].label, "false_positive");
+
     // Ensure that the token was not revoked
     app.db(|conn| {
         let tokens: Vec<ApiToken> = assert_ok!(ApiToken::belonging_to(user.as_model())