From f3192742ec0b3e99155622ef505c55c260b3bdb0 Mon Sep 17 00:00:00 2001
From: Bryan Newbold <bnewbold@robocracy.org>
Date: Tue, 9 Apr 2019 11:42:35 -0700
Subject: rust: fix macaroon expiry check

There were two bugs with this code: the expiry timestamps were getting
enclosed in double-quotes (which caused parse bugs), and the actual
caveat check itself was backwards (expires < now instead of expires >
now).

An underlying issue was that these caveats weren't actually getting
checked in the tests.

Should fix a bug where users don't get auth'd correctly when logging in
via mechanisms other than tokens.
---
 rust/src/auth.rs        | 29 +++++++++++++++++++----------
 rust/tests/test_auth.rs |  7 +++++++
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/rust/src/auth.rs b/rust/src/auth.rs
index 7e9b945a..a62c2f58 100644
--- a/rust/src/auth.rs
+++ b/rust/src/auth.rs
@@ -229,7 +229,7 @@ impl AuthConfectionary {
         if let Some(duration) = duration {
             let expires = now_utc + duration;
             mac.add_first_party_caveat(&format!(
-                "time < {:?}",
+                "time < {}",
                 &expires.to_rfc3339_opts(SecondsFormat::Secs, true)
             ));
         };
@@ -291,12 +291,15 @@ impl AuthConfectionary {
         let mut created: Option<DateTime<Utc>> = None;
         for caveat in mac.first_party_caveats() {
             if caveat.predicate().starts_with("time > ") {
-                created = Some(
+                let ts: chrono::ParseResult<DateTime<Utc>> =
                     DateTime::parse_from_rfc3339(caveat.predicate().get(7..).unwrap())
-                        .unwrap()
-                        .with_timezone(&Utc),
-                );
-                break;
+                        .map(|x| x.with_timezone(&Utc));
+                if let Ok(ts) = ts {
+                    created = Some(ts);
+                    break;
+                } else {
+                    info!("couldn't parse macaroon time constraint: {}", caveat.predicate());
+                }
             }
         }
         let created = match created {
@@ -337,10 +340,16 @@ impl AuthConfectionary {
         verifier.satisfy_general(|p: &str| -> bool {
             // not expired (based on time)
             if p.starts_with("time < ") {
-                let expires: DateTime<Utc> = DateTime::parse_from_rfc3339(p.get(7..).unwrap())
-                    .unwrap()
-                    .with_timezone(&Utc);
-                expires < Utc::now()
+                let expires: chrono::ParseResult<DateTime<Utc>> =
+                    DateTime::parse_from_rfc3339(p.get(7..).unwrap())
+                        .map(|x| x.with_timezone(&Utc));
+                if let Ok(when) = expires {
+                    //info!("checking time constraint: {} < {}", Utc::now(), when);
+                    Utc::now() < when
+                } else {
+                    info!("couldn't parse macaroon time constraint: {}", p);
+                    false
+                }
             } else {
                 false
             }
diff --git a/rust/tests/test_auth.rs b/rust/tests/test_auth.rs
index c0d81753..2faf78ec 100644
--- a/rust/tests/test_auth.rs
+++ b/rust/tests/test_auth.rs
@@ -34,6 +34,13 @@ fn test_auth_db() {
     let editor_row = c.parse_macaroon_token(&conn, &token, None).unwrap();
     assert_eq!(editor_row.id, editor_id.to_uuid());
 
+    // create token w/ expiration
+    let token = c.create_token(editor_id, Some(chrono::Duration::days(1))).unwrap();
+
+    // verify token w/ expiration
+    let editor_row = c.parse_macaroon_token(&conn, &token, None).unwrap();
+    assert_eq!(editor_row.id, editor_id.to_uuid());
+
     // revoke token
     auth::revoke_tokens(&conn, editor_id).unwrap();
 
-- 
cgit v1.2.3