Skip to content

Commit e480bb9

Browse files
authored
fix(auth): scope info/collections expiry exemption to its exact route (#2435)
* fix(auth): scope info/collections expiry exemption to its exact route The Hawk expiry exemption for the legacy read-only /1.5/{uid}/info/collections endpoint was applied to any path ending in "/info/collections", which also matches the storage route /1.5/{uid}/storage/info/collections (collection "info", item "collections"). Match the legacy route exactly (four path segments) so the exemption no longer applies to that writable storage item. Adds a unit test. Closes STOR-620 * ci: install clippy/rustfmt components and python deps on cache miss The clippy, clippy-release, and rust-checks jobs only restored the Rust toolchain cache and ran `rustup default`, which yields a toolchain without the clippy/rustfmt components when the `*-rust-toolchain-${RUST_VERSION}` cache is absent (e.g. after a RUST_VERSION bump) or holds a component-less toolchain, causing "cargo-clippy is not installed" failures. These jobs now install the components idempotently. Likewise, python-checks now runs `poetry install` so ruff/mypy/etc. are present when the virtualenv cache misses.
1 parent a22fc76 commit e480bb9

2 files changed

Lines changed: 41 additions & 5 deletions

File tree

.github/workflows/main-workflow.yml

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,9 @@ jobs:
118118
- name: Install Poetry
119119
run: pip3 install poetry
120120

121+
- name: Install Python dependencies
122+
run: poetry install --with tokenserver-unit-tests,dev --no-interaction --no-ansi
123+
121124
- name: Python Format Check
122125
run: poetry run ruff format --diff tools
123126

@@ -152,7 +155,7 @@ jobs:
152155
key: ${{ runner.os }}-rust-toolchain-${{ env.RUST_VERSION }}
153156

154157
- name: Set Rust toolchain
155-
run: rustup default ${RUST_VERSION}
158+
run: rustup toolchain install ${RUST_VERSION} --component rustfmt --component clippy --no-self-update && rustup default ${RUST_VERSION}
156159

157160
- name: Cache cargo-audit
158161
id: cache-cargo-audit
@@ -209,7 +212,7 @@ jobs:
209212
key: ${{ runner.os }}-rust-toolchain-${{ env.RUST_VERSION }}
210213

211214
- name: Set Rust toolchain
212-
run: rustup default ${RUST_VERSION}
215+
run: rustup toolchain install ${RUST_VERSION} --component rustfmt --component clippy --no-self-update && rustup default ${RUST_VERSION}
213216

214217
- name: Rust Clippy ${{ matrix.target }}
215218
run: make clippy_${{ matrix.target }}
@@ -248,7 +251,7 @@ jobs:
248251
${{ runner.os }}-cargo-release-${{ matrix.target }}-
249252
250253
- name: Set Rust toolchain
251-
run: rustup default ${RUST_VERSION}
254+
run: rustup toolchain install ${RUST_VERSION} --component rustfmt --component clippy --no-self-update && rustup default ${RUST_VERSION}
252255

253256
- name: Rust Clippy release ${{ matrix.target }}
254257
run: make clippy_release_${{ matrix.target }}

syncserver/src/web/auth.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ impl HawkPayload {
187187
80
188188
};
189189
let path = uri.path_and_query().ok_or(HawkErrorKind::MissingPath)?;
190-
let expiry = if path.path().ends_with("/info/collections") {
190+
let expiry = if is_info_collections_path(path.path()) {
191191
0
192192
} else {
193193
Utc::now().timestamp() as u64
@@ -197,6 +197,25 @@ impl HawkPayload {
197197
}
198198
}
199199

200+
/// Matches only the legacy read-only `/1.5/{uid}/info/collections` endpoint,
201+
/// requiring exactly four path segments. An exact match is needed because a
202+
/// plain `/info/collections` suffix also matches the storage route
203+
/// `/1.5/{uid}/storage/info/collections` (collection `info`, item
204+
/// `collections`), which must not receive the same handling.
205+
fn is_info_collections_path(path: &str) -> bool {
206+
let mut segments = path.trim_start_matches('/').split('/');
207+
matches!(
208+
(
209+
segments.next(),
210+
segments.next(),
211+
segments.next(),
212+
segments.next(),
213+
segments.next(),
214+
),
215+
(Some("1.5"), Some(uid), Some("info"), Some("collections"), None) if !uid.is_empty()
216+
)
217+
}
218+
200219
/// Helper function for [HMAC](https://tools.ietf.org/html/rfc2104) verification.
201220
fn verify_hmac(info: &[u8], key: &[u8], expected: &[u8]) -> ApiResult<()> {
202221
let mut hmac = Hmac::<Sha256>::new_from_slice(key)?;
@@ -208,7 +227,21 @@ fn verify_hmac(info: &[u8], key: &[u8], expected: &[u8]) -> ApiResult<()> {
208227
mod tests {
209228
use std::fmt::{self, Display, Formatter};
210229

211-
use super::{HawkPayload, Secrets};
230+
use super::{HawkPayload, Secrets, is_info_collections_path};
231+
232+
#[test]
233+
fn info_collections_exemption_matches_only_exact_route() {
234+
// The legacy read-only endpoint is exempt.
235+
assert!(is_info_collections_path("/1.5/123/info/collections"));
236+
// The writable storage route shares the same suffix but must not match.
237+
assert!(!is_info_collections_path(
238+
"/1.5/123/storage/info/collections"
239+
));
240+
// Other storage items must not match.
241+
assert!(!is_info_collections_path("/1.5/123/storage/bookmarks/abc"));
242+
// A trailing slash or extra segment must not match.
243+
assert!(!is_info_collections_path("/1.5/123/info/collections/"));
244+
}
212245

213246
#[test]
214247
fn valid_header() {

0 commit comments

Comments
 (0)