Merge pull request #135 from cole-h/token-use-indexmap

token: switch to using IndexMap for consistent ordering of entries

Cargo.lock

@@ -351,6 +351,7 @@ dependencies = [
  "base64 0.21.5",
  "chrono",
  "displaydoc",
+ "indexmap 2.2.6",
  "jwt-simple",
  "lazy_static",
  "regex",
@@ -2204,9 +2205,9 @@ dependencies = [
 
 [[package]]
 name = "indexmap"
-version = "2.1.0"
+version = "2.2.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d530e1a18b1cb4c484e6e34556a0d948706958449fca0cab753d649f2bce3d1f"
+checksum = "168fb715dda47215e360912c096649d23d58bf392ac62f73919e831745e40f26"
 dependencies = [
  "equivalent",
  "hashbrown 0.14.2",
@@ -3680,7 +3681,7 @@ dependencies = [
  "chrono",
  "hex",
  "indexmap 1.9.3",
- "indexmap 2.1.0",
+ "indexmap 2.2.6",
  "serde",
  "serde_json",
  "serde_with_macros",
@@ -3705,7 +3706,7 @@ version = "0.9.27"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "3cc7a1570e38322cfe4154732e5110f887ea57e22b76f4bfd32b5bdd3368666c"
 dependencies = [
- "indexmap 2.1.0",
+ "indexmap 2.2.6",
  "itoa",
  "ryu",
  "serde",
@@ -3912,7 +3913,7 @@ dependencies = [
  "futures-util",
  "hashlink",
  "hex",
- "indexmap 2.1.0",
+ "indexmap 2.2.6",
  "log",
  "memchr",
  "once_cell",
@@ -4404,7 +4405,7 @@ version = "0.21.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "d34d383cd00a163b4a5b85053df514d45bc330f6de7737edfe0a93311d1eaa03"
 dependencies = [
- "indexmap 2.1.0",
+ "indexmap 2.2.6",
  "serde",
  "serde_spanned",
  "toml_datetime",

token/Cargo.toml

@@ -11,6 +11,7 @@ attic = { path = "../attic", default-features = false }
 base64 = "0.21.2"
 chrono = "0.4.24"
 displaydoc = "0.2.4"
+indexmap = { version = "2.2.6", features = ["serde"] }
 jwt-simple = "0.11.5"
 lazy_static = "1.4.0"
 regex = "1.8.3"

token/src/lib.rs

@@ -83,12 +83,12 @@ pub mod util;
 #[cfg(test)]
 mod tests;
 
-use std::collections::HashMap;
 use std::error::Error as StdError;
 
 use base64::{engine::general_purpose::STANDARD as BASE64_STANDARD, Engine};
 use chrono::{DateTime, Utc};
 use displaydoc::Display;
+use indexmap::IndexMap;
 pub use jwt_simple::{
     algorithms::{HS256Key, MACLike},
     claims::{Claims, JWTClaims},
@@ -146,7 +146,7 @@ pub struct AtticAccess {
     /// Cache permissions.
     ///
     /// Keys here may include wildcards.
-    caches: HashMap<CacheNamePattern, CachePermission>,
+    caches: IndexMap<CacheNamePattern, CachePermission>,
 }
 
 /// Permission to a single cache.
@@ -274,7 +274,7 @@ impl Token {
         &mut self,
         pattern: CacheNamePattern,
     ) -> &mut CachePermission {
-        use std::collections::hash_map::Entry;
+        use indexmap::map::Entry;
 
         let access = self.attic_access_mut();
         match access.caches.entry(pattern) {

token/src/tests.rs

@@ -21,6 +21,8 @@ fn test_basic() {
         "exp": 4102324986,
         "https://jwt.attic.rs/v1": {
           "caches": {
+            "all-*": {"r":1},
+            "all-ci-*": {"w":1},
             "cache-rw": {"r":1,"w":1},
             "cache-ro": {"r":1},
             "team-*": {"r":1,"w":1,"cc":1}
@@ -29,7 +31,29 @@ fn test_basic() {
       }
     */
 
-    let token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiJtZW93IiwiZXhwIjo0MTAyMzI0OTg2LCJodHRwczovL2p3dC5hdHRpYy5ycy92MSI6eyJjYWNoZXMiOnsiY2FjaGUtcnciOnsiciI6MSwidyI6MX0sImNhY2hlLXJvIjp7InIiOjF9LCJ0ZWFtLSoiOnsiciI6MSwidyI6MSwiY2MiOjF9fX19.UlsIM9bQHr9SXGAcSQcoVPo9No8Zhh6Y5xfX8vCmKmA";
+    let token = "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJleHAiOjQxMDIzMjQ5ODYsImh0dHBzOi8vand0LmF0dGljLnJzL3YxIjp7ImNhY2hlcyI6eyJhbGwtKiI6eyJyIjoxfSwiYWxsLWNpLSoiOnsidyI6MX0sImNhY2hlLXJvIjp7InIiOjF9LCJjYWNoZS1ydyI6eyJyIjoxLCJ3IjoxfSwidGVhbS0qIjp7ImNjIjoxLCJyIjoxLCJ3IjoxfX19LCJpYXQiOjE3MTY2NjA1ODksInN1YiI6Im1lb3cifQ.8vtxp_1OEYdcnkGPM4c9ORXooJZV7DOTS4NRkMKN8mw";
+
+    // NOTE(cole-h): check that we get a consistent iteration order when getting permissions for
+    // caches -- this depends on the order of the fields in the token, but should otherwise be
+    // consistent between iterations
+    let mut was_ever_wrong = false;
+    for _ in 0..=1_000 {
+        // NOTE(cole-h): we construct a new Token every iteration in order to get different "random
+        // state"
+        let decoded = Token::from_jwt(token, &dec_key).unwrap();
+        let perm_all_ci = decoded.get_permission_for_cache(&cache! { "all-ci-abc" });
+
+        // NOTE(cole-h): if the iteration order of the token is inconsistent, the permissions may be
+        // retrieved from the `all-ci-*` pattern (which only allows writing/pushing), even though
+        // the `all-*` pattern (which only allows reading/pulling) is specified first
+        if perm_all_ci.require_pull().is_err() || perm_all_ci.require_push().is_ok() {
+            was_ever_wrong = true;
+        }
+    }
+    assert!(
+        !was_ever_wrong,
+        "Iteration order should be consistent to prevent random auth failures (and successes)"
+    );
 
     let decoded = Token::from_jwt(token, &dec_key).unwrap();