From 64f5e0b725f0a33edc64a4f1a288aaac83d53c2d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 09:36:41 +0000 Subject: [PATCH] sync: allow syncing athletes other than the current session require_athlete_access rejected any /sync/{athlete_id} request where athlete_id didn't match the session's own Intervals.icu athlete id. That's a local, unconditional block that has nothing to do with whether the session's access token actually has access to that athlete's data on Intervals.icu -- for example a coach account whose API key can see multiple athletes' activities. require_athlete_access is replaced by require_authenticated_session, which only checks that the request carries a valid session; the session's own access token is then used to call Intervals.icu for whichever athlete_id was requested, and Intervals.icu's own API is what actually authorizes (or 401s/403s) that per-athlete access. Import status and the 'last synced' default were previously looked up using the *session's* local athlete id even when browsing a different athlete_id -- i.e. a coach browsing an athlete's activities would see their own import history instead of that athlete's. Both /sync/{athlete_id} and its import-visible handler now resolve the local athlete row for the athlete_id being browsed (local_athlete_id) and use that instead. If that athlete has never logged in locally, everything is treated as not-yet-imported; actually importing then fails with the existing 'no local athlete for Intervals.icu athlete' error rather than silently doing the wrong thing. process_activity_with_client (session-derived client) is now used instead of process_activity (which looks up the target athlete's own stored token) for both single-activity and bulk import, since the token doing the request is the session's, not necessarily the target athlete's own. --- src/main.rs | 119 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 90 insertions(+), 29 deletions(-) diff --git a/src/main.rs b/src/main.rs index 639de19..6826f69 100644 --- a/src/main.rs +++ b/src/main.rs @@ -139,21 +139,22 @@ async fn current_session_athlete( .context("session is invalid or expired") } -async fn require_athlete_access( +/// Requires a valid session but does not restrict it to the athlete_id +/// being acted on. +/// +/// Access to another athlete's Intervals.icu data (e.g. a coach account) +/// is not something this application can determine locally -- it depends +/// entirely on what the session's access token is allowed to see on +/// Intervals.icu's side. So instead of guessing here and blocking +/// legitimate requests, callers use the session's own access token to +/// talk to Intervals.icu for the requested athlete_id, and let +/// Intervals.icu's API return its own 401/403 if that token has no +/// access to that athlete's data. +async fn require_authenticated_session( state: &AppState, jar: &axum_extra::extract::cookie::CookieJar, - athlete_id: &str, ) -> Result { - let session = current_session_athlete(state, jar).await?; - - if session.intervals_athlete_id != athlete_id { - return Err(anyhow!( - "current authentication token has no access to athlete {}", - athlete_id - )); - } - - Ok(session) + current_session_athlete(state, jar).await } async fn sync_index( @@ -241,7 +242,18 @@ async fn sync_athlete( Query(query): Query, jar: axum_extra::extract::cookie::CookieJar, ) -> Result, (axum::http::StatusCode, String)> { - let session = require_athlete_access(&state, &jar, &athlete_id) + let session = require_authenticated_session(&state, &jar) + .await + .map_err(http_error)?; + + /* + * athlete_id is whichever athlete the session's access token is + * pointed at, which is not necessarily the session's own athlete + * (see require_authenticated_session). Import status and the default + * "oldest" bound are tracked per local athlete row, so both are + * resolved against athlete_id, not the session. + */ + let target_athlete_id = local_athlete_id(&state.db, &athlete_id) .await .map_err(http_error)?; @@ -253,10 +265,13 @@ async fn sync_athlete( * * An explicit query value overrides the default. */ - let default_oldest = db::last_synced_activity_time(&state.db, session.id) - .await - .map_err(http_error)? - .unwrap_or_else(|| Utc::now() - Duration::days(30)); + let default_oldest = match target_athlete_id { + Some(id) => db::last_synced_activity_time(&state.db, id) + .await + .map_err(http_error)? + .unwrap_or_else(|| Utc::now() - Duration::days(30)), + None => Utc::now() - Duration::days(30), + }; let oldest = parse_datetime_local(query.oldest.as_deref()).unwrap_or(default_oldest); @@ -284,9 +299,12 @@ async fn sync_athlete( continue; }; - let imported = activity_exists(&state.db, session.id, activity_id) - .await - .map_err(http_error)?; + let imported = match target_athlete_id { + Some(id) => activity_exists(&state.db, id, activity_id) + .await + .map_err(http_error)?, + None => false, + }; if imported { continue; @@ -672,13 +690,22 @@ async fn sync_import_activity( jar: axum_extra::extract::cookie::CookieJar, headers: HeaderMap, ) -> Result { - require_athlete_access(&state, &jar, &athlete_id) + let session = require_authenticated_session(&state, &jar) .await .map_err(http_error)?; let wants_json = wants_json_response(&headers); - match process_activity(state.clone(), athlete_id.clone(), activity_id.clone()).await { + let client = IntervalsClient::with_api_key(state.config.clone(), session.access_token.clone()); + + match process_activity_with_client( + state.clone(), + athlete_id.clone(), + activity_id.clone(), + &client, + ) + .await + { Ok(crossings) => { if wants_json { Ok(Json(serde_json::json!({ @@ -758,7 +785,7 @@ async fn sync_import_visible( jar: axum_extra::extract::cookie::CookieJar, axum::Form(form): axum::Form, ) -> Result, (axum::http::StatusCode, String)> { - let session = require_athlete_access(&state, &jar, &athlete_id) + let session = require_authenticated_session(&state, &jar) .await .map_err(http_error)?; @@ -769,6 +796,10 @@ async fn sync_import_visible( let client = IntervalsClient::with_api_key(state.config.clone(), session.access_token.clone()); + let target_athlete_id = local_athlete_id(&state.db, &athlete_id) + .await + .map_err(http_error)?; + let activities = client .activities( &athlete_id, @@ -787,9 +818,12 @@ async fn sync_import_visible( continue; }; - let imported = activity_exists(&state.db, session.id, activity_id) - .await - .map_err(http_error)?; + let imported = match target_athlete_id { + Some(id) => activity_exists(&state.db, id, activity_id) + .await + .map_err(http_error)?, + None => false, + }; if imported { continue; @@ -808,9 +842,14 @@ async fn sync_import_visible( continue; } - process_activity(state.clone(), athlete_id.clone(), activity_id.to_string()) - .await - .map_err(http_error)?; + process_activity_with_client( + state.clone(), + athlete_id.clone(), + activity_id.to_string(), + &client, + ) + .await + .map_err(http_error)?; } Ok(Html(format!( @@ -859,6 +898,28 @@ async fn activity_exists(db: &PgPool, athlete_id: i64, activity_id: &str) -> Res Ok(row.try_get("exists")?) } +/// Resolves the local database id for an Intervals.icu athlete id, if that +/// athlete has ever logged in locally (via OAuth or an API key). Used when +/// browsing/importing an athlete other than the current session, since +/// import status and "last synced" bookkeeping is tracked per local +/// athlete row, not per session. +async fn local_athlete_id(db: &PgPool, intervals_athlete_id: &str) -> Result> { + let row = sqlx::query( + r#" + SELECT id + FROM athletes + WHERE intervals_athlete_id = $1 + "#, + ) + .bind(intervals_athlete_id) + .fetch_optional(db) + .await?; + + use sqlx::Row; + + Ok(row.map(|row| row.get("id"))) +} + fn activity_start_time(activity: &Value) -> Option> { let value = activity .get("start_date")