From ae4eb84a350994daee72a21f380ea04bb2ca9f63 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Wed, 29 Apr 2020 15:37:32 -0400 Subject: [PATCH] 16349: Set time zone to UTC at db connection level. This covers the trash time comparisons in materialized_permission_view as well as explicit uses of CURRENT_TIMESTAMP in Rails queries. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- .../api/app/controllers/application_controller.rb | 4 ++-- .../api/app/models/api_client_authorization.rb | 4 ++-- services/api/config/initializers/time_zone.rb | 15 +++++++++++++++ services/api/lib/create_superuser_token.rb | 2 +- services/api/test/unit/container_test.rb | 4 ---- services/api/test/unit/time_zone_test.rb | 15 +++++++++++++++ 6 files changed, 35 insertions(+), 9 deletions(-) create mode 100644 services/api/config/initializers/time_zone.rb create mode 100644 services/api/test/unit/time_zone_test.rb diff --git a/services/api/app/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb index c7d7ad8148..83a233cd54 100644 --- a/services/api/app/controllers/application_controller.rb +++ b/services/api/app/controllers/application_controller.rb @@ -386,8 +386,8 @@ class ApplicationController < ActionController::Base @read_auths += ApiClientAuthorization .includes(:user) .where('api_token IN (?) AND - (expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', - secrets, 'UTC') + (expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP)', + secrets) .to_a end @read_auths.select! { |auth| auth.scopes_allow_request? request } diff --git a/services/api/app/models/api_client_authorization.rb b/services/api/app/models/api_client_authorization.rb index a808653480..5386cb119a 100644 --- a/services/api/app/models/api_client_authorization.rb +++ b/services/api/app/models/api_client_authorization.rb @@ -158,7 +158,7 @@ class ApiClientAuthorization < ArvadosModel # fast path: look up the token in the local database auth = ApiClientAuthorization. includes(:user, :api_client). - where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', token_uuid, 'UTC'). + where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', token_uuid). first if auth && auth.user && (secret == auth.api_token || @@ -280,7 +280,7 @@ class ApiClientAuthorization < ArvadosModel # token is not a 'v2' token auth = ApiClientAuthorization. includes(:user, :api_client). - where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', token, 'UTC'). + where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP)', token). first if auth && auth.user return auth diff --git a/services/api/config/initializers/time_zone.rb b/services/api/config/initializers/time_zone.rb new file mode 100644 index 0000000000..cedd8f3e4a --- /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/create_superuser_token.rb b/services/api/lib/create_superuser_token.rb index c1530162e9..57eac048a9 100755 --- a/services/api/lib/create_superuser_token.rb +++ b/services/api/lib/create_superuser_token.rb @@ -40,7 +40,7 @@ module CreateSuperUserToken where(user_id: system_user.id). where(api_client_id: apiClient.id). where_serialized(:scopes, ['all']). - where('(expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', 'UTC'). + where('(expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP)'). first # none exist; create one with the supplied token diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb index 0bbbc17b0c..98e60e0579 100644 --- a/services/api/test/unit/container_test.rb +++ b/services/api/test/unit/container_test.rb @@ -605,10 +605,6 @@ class ContainerTest < ActiveSupport::TestCase end test "Lock and unlock" do - # The "token is expired" check (at the end of this test case) - # requires a west-of-UTC time zone in order to be effective. - ActiveRecord::Base.connection.select_value("SET TIME ZONE '-4'") - set_user_from_auth :active c, cr = minimal_new priority: 0 diff --git a/services/api/test/unit/time_zone_test.rb b/services/api/test/unit/time_zone_test.rb new file mode 100644 index 0000000000..60ca6b50b6 --- /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 -- 2.30.2