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.
This commit is contained in:
Claude 2026-08-13 09:36:41 +00:00 committed by Jonas Rabenstein
commit 64f5e0b725

View file

@ -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<SessionAthlete> {
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<SyncQuery>,
jar: axum_extra::extract::cookie::CookieJar,
) -> Result<Html<String>, (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<axum::response::Response, (axum::http::StatusCode, String)> {
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<ImportVisibleForm>,
) -> Result<Html<String>, (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<Option<i64>> {
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<DateTime<Utc>> {
let value = activity
.get("start_date")