[ARVADOS] created: 1.3.0-2518-gf84adca24
Git user
git at public.arvados.org
Mon Apr 27 21:03:04 UTC 2020
at f84adca24070357d3da8092a2104a979efee7c76 (commit)
commit f84adca24070357d3da8092a2104a979efee7c76
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 59ca7503ea290442f45ba32ed7327a9df5b17c52
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 baf86998988b0fae1808ecc39d0bacdf0e1359ea
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..98e60e057 100644
--- 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
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list