Merge branch '16349-non-utc-timestamps'

fixes #16349

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@tomclegg.ca>

diff --git a/lib/controller/federation.go b/lib/controller/federation.go
index c0d127284ce35a21d5618814dd2792984809f5f2..ac239fb9b23f5c4c106034e2a910fb441b9c2218 100644 (file)
--- a/lib/controller/federation.go
+++ b/lib/controller/federation.go
@@ -166,7 +166,7 @@ func (h *Handler) validateAPItoken(req *http.Request, token string) (*CurrentUse
        }
        user.Authorization.APIToken = token
        var scopes string
-       err = db.QueryRowContext(req.Context(), `SELECT api_client_authorizations.uuid, api_client_authorizations.scopes, users.uuid FROM api_client_authorizations JOIN users on api_client_authorizations.user_id=users.id WHERE api_token=$1 AND (expires_at IS NULL OR expires_at > current_timestamp) LIMIT 1`, token).Scan(&user.Authorization.UUID, &scopes, &user.UUID)
+       err = db.QueryRowContext(req.Context(), `SELECT api_client_authorizations.uuid, api_client_authorizations.scopes, users.uuid FROM api_client_authorizations JOIN users on api_client_authorizations.user_id=users.id WHERE api_token=$1 AND (expires_at IS NULL OR expires_at > current_timestamp AT TIME ZONE 'UTC') LIMIT 1`, token).Scan(&user.Authorization.UUID, &scopes, &user.UUID)
        if err == sql.ErrNoRows {
                ctxlog.FromContext(req.Context()).Debugf("validateAPItoken(%s): not found in database", token)
                return nil, false, nil
@@ -214,9 +214,9 @@ func (h *Handler) createAPItoken(req *http.Request, userUUID string, scopes []st
 (uuid, api_token, expires_at, scopes,
 user_id,
 api_client_id, created_at, updated_at)
-VALUES ($1, $2, CURRENT_TIMESTAMP + INTERVAL '2 weeks', $3,
+VALUES ($1, $2, CURRENT_TIMESTAMP AT TIME ZONE 'UTC' + INTERVAL '2 weeks', $3,
 (SELECT id FROM users WHERE users.uuid=$4 LIMIT 1),
-0, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`,
+0, CURRENT_TIMESTAMP AT TIME ZONE 'UTC', CURRENT_TIMESTAMP AT TIME ZONE 'UTC')`,
                uuid, token, string(scopesjson), userUUID)
 
        if err != nil {

diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 376be55ffbf1a762ae81c2ed3fbbf76292b87883..912a801a6fb1820724489216f0ec38d99bd80210 100644 (file)
--- a/services/api/app/models/container.rb
+++ b/services/api/app/models/container.rb
@@ -570,8 +570,13 @@ class Container < ArvadosModel
          return errors.add :auth_uuid, 'is readonly'
     end
     if not [Locked, Running].include? self.state
-      # don't need one
-      self.auth.andand.update_attributes(expires_at: db_current_time)
+      # Don't need one. If auth already exists, expire it.
+      #
+      # We use db_transaction_time here (not db_current_time) to
+      # ensure the token doesn't validate later in the same
+      # transaction (e.g., in a test case) by satisfying expires_at >
+      # transaction timestamp.
+      self.auth.andand.update_attributes(expires_at: db_transaction_time)
       self.auth = nil
       return
     elsif self.auth

diff --git a/services/api/config/initializers/time_zone.rb b/services/api/config/initializers/time_zone.rb
new file mode 100644 (file)
index 0000000..cedd8f3
--- /dev/null
+++ b/services/api/config/initializers/time_zone.rb
@@ -0,0 +1,15 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+ActiveRecord::Base.connection.class.set_callback :checkout, :after do
+  # If the database connection is in a time zone other than UTC,
+  # "timestamp" values don't behave as desired.
+  #
+  # For example, ['select now() > ?', Time.now] returns true in time
+  # zones +0100 and UTC (which makes sense since Time.now is evaluated
+  # before now()), but false in time zone -0100 (now() returns an
+  # earlier clock time, and its time zone is dropped when comparing to
+  # a "timestamp without time zone").
+  raw_connection.sync_exec("SET TIME ZONE 'UTC'")
+end

diff --git a/services/api/lib/db_current_time.rb b/services/api/lib/db_current_time.rb
index fdb66415210aed9b95338f6dafc6e377de45ec50..5e1634ecb96f17661002afc3eb62dea44100adad 100644 (file)
--- a/services/api/lib/db_current_time.rb
+++ b/services/api/lib/db_current_time.rb
@@ -3,9 +3,13 @@
 # SPDX-License-Identifier: AGPL-3.0
 
 module DbCurrentTime
-  CURRENT_TIME_SQL = "SELECT clock_timestamp()"
+  CURRENT_TIME_SQL = "SELECT clock_timestamp() AT TIME ZONE 'UTC'"
 
   def db_current_time
-    Time.parse(ActiveRecord::Base.connection.select_value(CURRENT_TIME_SQL)).to_time
+    Time.parse(ActiveRecord::Base.connection.select_value(CURRENT_TIME_SQL) + " +0000")
+  end
+
+  def db_transaction_time
+    Time.parse(ActiveRecord::Base.connection.select_value("SELECT current_timestamp AT TIME ZONE 'UTC'") + " +0000")
   end
 end

diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 5f17efc4452c3ac24e5e53f9d532da1ce3b9d673..98e60e057910f034194ad9f47289627a245f97e4 100644 (file)
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -663,6 +663,8 @@ class ContainerTest < ActiveSupport::TestCase
 
     auth_exp = ApiClientAuthorization.find_by_uuid(auth_uuid_was).expires_at
     assert_operator auth_exp, :<, db_current_time
+
+    assert_nil ApiClientAuthorization.validate(token: ApiClientAuthorization.find_by_uuid(auth_uuid_was).token)
   end
 
   test "Exceed maximum lock-unlock cycles" do

diff --git a/services/api/test/unit/time_zone_test.rb b/services/api/test/unit/time_zone_test.rb
new file mode 100644 (file)
index 0000000..60ca6b5
--- /dev/null
+++ b/services/api/test/unit/time_zone_test.rb
@@ -0,0 +1,15 @@
+# Copyright (C) The Arvados Authors. All rights reserved.
+#
+# SPDX-License-Identifier: AGPL-3.0
+
+require 'test_helper'
+
+class TimeZoneTest < ActiveSupport::TestCase
+  test "Database connection time zone" do
+    # This is pointless if the testing host is already using the UTC
+    # time zone.  But if not, the test confirms that
+    # config/initializers/time_zone.rb has successfully changed the
+    # database connection time zone to UTC.
+    assert_equal('UTC', ActiveRecord::Base.connection.select_value("show timezone"))
+  end
+end