Post-review changes

4 files changed, 30 insertions, 15 deletions

diff --git a/mod/jwt.cxx b/mod/jwt.cxx
index 43b8629..962ed1e 100644
--- a/mod/jwt.cxx
+++ b/mod/jwt.cxx
@@ -2,7 +2,6 @@
 
 #include <libbutl/base64.hxx>
 #include <libbutl/openssl.hxx>
-#include <libbutl/timestamp.hxx>
 #include <libbutl/json/serializer.hxx>
 
 using namespace std;
@@ -56,7 +55,8 @@ string brep::
 gen_jwt (const options::openssl_options& o,
          const path& pk,
          const string& iss,
-         const std::chrono::minutes& vp)
+         const chrono::minutes& vp,
+         const chrono::seconds& bd)
 {
   // Create the header.
   //
@@ -81,15 +81,15 @@ gen_jwt (const options::openssl_options& o,
 
     // "Issued at" time.
     //
-    seconds iat (
-        duration_cast<seconds> (system_clock::now ().time_since_epoch ()));
+    seconds iat (duration_cast<seconds> (
+        system_clock::now ().time_since_epoch () - bd));
 
     // Expiration time.
     //
     seconds exp (iat + vp);
 
     vector<char> b;
-    json::buffer_serializer s (b);
+    json::buffer_serializer s (b, 0 /* indentation */);
 
     s.begin_object ();
     s.member ("iss", iss);
@@ -116,7 +116,7 @@ gen_jwt (const options::openssl_options& o,
     // Note that RSA is indicated by the contents of the private key.
     //
     // Note that here we assume both output and diagnostics will fit into pipe
-    // buffers and don't both with fdselect().
+    // buffers and don't poll both with fdselect().
     //
     openssl os (path ("-"), // Read message from openssl::out.
                 path ("-"), // Write output to openssl::in.
diff --git a/mod/jwt.hxx b/mod/jwt.hxx
index 25e9c21..4b5a54f 100644
--- a/mod/jwt.hxx
+++ b/mod/jwt.hxx
@@ -22,16 +22,19 @@ namespace brep
   //
   // The backdate argument specifies the number of seconds to subtract from
   // the "issued at" time in order to combat potential clock drift (which can
-  // casue the token to be not valid yet).
+  // cause the token to be not valid yet).
   //
-  // Return the token or std::system_error in case if an error.
+  //   @@ TODO Is there a standard term? "drift" or "drift value" seems to be
+  //           used quite often.
+  //
+  // Return the token or throw std::system_error in case of an error.
   //
   string
   gen_jwt (const options::openssl_options&,
            const path& private_key,
            const string& issuer,
            const std::chrono::minutes& validity_period,
-           const std::chrono::seconds& backdate = 60);
+           const std::chrono::seconds& backdate = std::chrono::seconds (60));
 }
 
 #endif
diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index 86f52b9..53ec9a7 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -235,13 +235,15 @@ handle (request& rq, response& rs)
 
       try
       {
-        // Use the maximum validity period allowed by GitHub (10 minutes).
-        // @@ Let's make configurable.
+        // Set token's "issued at" time 60 seconds in the past to combat clock
+        // drift (as recommended by GitHub).
         //
-        string jwt (gen_jwt (*options_,
-                             options_->ci_github_app_private_key (),
-                             to_string (options_->ci_github_app_id ()),
-                             chrono::minutes (10)));
+        string jwt (gen_jwt (
+            *options_,
+            options_->ci_github_app_private_key (),
+            to_string (options_->ci_github_app_id ()),
+            chrono::minutes (options_->ci_github_jwt_validity_period ()),
+            chrono::seconds (60)));
 
         if (jwt.empty ())
           fail << "unable to generate JWT: " << options_->openssl ()
@@ -275,6 +277,9 @@ handle (request& rq, response& rs)
     //
     // Note: these exceptions end up in the apache error log.
     //
+    //  @@ TMP Actually I was wrong, these do not end up in any logs. Pretty
+    //         sure I saw them go there but they're definitely not anymore.
+    //
     throw invalid_request (400, "malformed JSON in request body");
   }
 }
diff --git a/mod/module.cli b/mod/module.cli
index c61ce8a..68d1d35 100644
--- a/mod/module.cli
+++ b/mod/module.cli
@@ -835,6 +835,13 @@ namespace brep
         "The private key used during GitHub API authentication. Created in
          the GitHub app's settings."
       }
+
+      uint16_t ci-github-jwt-validity-period = 10
+      {
+        "<minutes>",
+        "The number of minutes a JWT (authentication token) should be valid for.
+         The maximum allowed by GitHub is 10 minutes."
+      }
     };
 
     class upload: build, build_db, build_upload, repository_email, handler