[ARVADOS] created: 1.3.0-2518-gf98bf4450
Git user
git at public.arvados.org
Tue Apr 28 20:26:08 UTC 2020
at f98bf44508fd976cd073ba89909f70253705a5e9 (commit)
commit f98bf44508fd976cd073ba89909f70253705a5e9
Author: Tom Clegg <tom at tomclegg.ca>
Date: Mon Apr 27 16:57:55 2020 -0400
16349: Skip converting DB timestamps to & from localtime.
The previous code already returned the correct time. This change
merely avoids converting the time to the local time zone in
PostgreSQL, and back to UTC again in Rails.
This might also avoid rare problems in unusual edge cases where Rails
and PostgreSQL session use different time zones, like a configuration
mishap or a race during a daylight savings time transition.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/services/api/lib/db_current_time.rb b/services/api/lib/db_current_time.rb
index 80516521b..5e1634ecb 100644
--- a/services/api/lib/db_current_time.rb
+++ b/services/api/lib/db_current_time.rb
@@ -3,13 +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')).to_time
+ Time.parse(ActiveRecord::Base.connection.select_value("SELECT current_timestamp AT TIME ZONE 'UTC'") + " +0000")
end
end
commit f4991e436219e135db96b8a9d064fdee3d39b2cd
Author: Tom Clegg <tom at tomclegg.ca>
Date: Mon Apr 27 16:37:49 2020 -0400
16349: Fix TZ-sensitive comparison in token expiry checks.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/lib/controller/federation.go b/lib/controller/federation.go
index 674183dcc..5bdc96528 100644
--- a/lib/controller/federation.go
+++ b/lib/controller/federation.go
@@ -164,7 +164,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 {
return nil, false, nil
} else if err != nil {
@@ -207,9 +207,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/controllers/application_controller.rb b/services/api/app/controllers/application_controller.rb
index 83a233cd5..c7d7ad814 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)',
- secrets)
+ (expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)',
+ secrets, 'UTC')
.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 5386cb119..a80865348 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)', token_uuid).
+ where('uuid=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', token_uuid, 'UTC').
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)', token).
+ where('api_token=? and (expires_at is null or expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', token, 'UTC').
first
if auth && auth.user
return auth
diff --git a/services/api/lib/create_superuser_token.rb b/services/api/lib/create_superuser_token.rb
index 57eac048a..c1530162e 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)').
+ where('(expires_at IS NULL OR expires_at > CURRENT_TIMESTAMP AT TIME ZONE ?)', 'UTC').
first
# none exist; create one with the supplied token
commit 20fd0e67ddcbba8c4bc1e5e9c5e2def6eccd9d85
Author: Tom Clegg <tom at tomclegg.ca>
Date: Mon Apr 27 16:37:19 2020 -0400
16349: Test token expiry using actual token validation func.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/services/api/app/models/container.rb b/services/api/app/models/container.rb
index 376be55ff..912a801a6 100644
--- 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/lib/db_current_time.rb b/services/api/lib/db_current_time.rb
index fdb664152..80516521b 100644
--- a/services/api/lib/db_current_time.rb
+++ b/services/api/lib/db_current_time.rb
@@ -8,4 +8,8 @@ module DbCurrentTime
def db_current_time
Time.parse(ActiveRecord::Base.connection.select_value(CURRENT_TIME_SQL)).to_time
end
+
+ def db_transaction_time
+ Time.parse(ActiveRecord::Base.connection.select_value('SELECT current_timestamp')).to_time
+ end
end
diff --git a/services/api/test/unit/container_test.rb b/services/api/test/unit/container_test.rb
index 5f17efc44..0bbbc17b0 100644
--- a/services/api/test/unit/container_test.rb
+++ b/services/api/test/unit/container_test.rb
@@ -605,6 +605,10 @@ 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
@@ -663,6 +667,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
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list