Skip to content

Commit 5e4f286

Browse files
committed
Only authenticate on search endpoint when necessary
I believe we previously had to do authentication at the start of the endpoint because the database connection was obtained early and there are lifetime issues around using the `req` value while holding a database connection. This avoids authenticating the user (and possibly updating last_used_at for API tokens) unless the search query requires it.
1 parent 85ec456 commit 5e4f286

File tree

2 files changed

+2
-6
lines changed

2 files changed

+2
-6
lines changed

src/controllers/krate/search.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use diesel_full_text_search::*;
55

66
use crate::controllers::cargo_prelude::*;
77
use crate::controllers::helpers::Paginate;
8-
use crate::controllers::util::AuthenticatedUser;
98
use crate::models::{
109
Crate, CrateBadge, CrateOwner, CrateVersions, OwnerKind, TopVersions, Version,
1110
};
@@ -40,9 +39,6 @@ use crate::models::krate::{canon_crate_name, ALL_COLUMNS};
4039
pub fn search(req: &mut dyn RequestExt) -> EndpointResult {
4140
use diesel::sql_types::{Bool, Text};
4241

43-
// Don't require that authentication succeed, because it's only necessary
44-
// if the "following" param is set.
45-
let authenticated_user: AppResult<AuthenticatedUser> = req.authenticate();
4642
let params = req.query();
4743
let sort = params.get("sort").map(|s| &**s);
4844
let include_yanked = params
@@ -160,7 +156,7 @@ pub fn search(req: &mut dyn RequestExt) -> EndpointResult {
160156
),
161157
);
162158
} else if params.get("following").is_some() {
163-
let user_id = authenticated_user?.user_id();
159+
let user_id = req.authenticate()?.user_id();
164160
query = query.filter(
165161
crates::id.eq_any(
166162
follows::table

src/tests/token.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ fn using_token_updates_last_used_at() {
282282
assert_none!(token.as_model().last_used_at);
283283

284284
// Use the token once
285-
token.search("");
285+
token.search("following=1");
286286

287287
let token: ApiToken =
288288
app.db(|conn| assert_ok!(ApiToken::belonging_to(user.as_model()).first(conn)));

0 commit comments

Comments
 (0)