storage: fix postgres timezone handling

Dex's Postgres client currently uses the `timestamp` datatype for
storing times. This lops of timezones with no conversion, causing
times to lose locality information.

We could convert all times to UTC before storing them, but this is
a backward incompatible change for upgrades, since the new version
of dex would still be reading times from the database with no
locality.

Because of this intrinsic issue that current Postgres users don't
save any timezone data, we chose to treat any existing installation
as corrupted and change the datatype used for times to `timestamptz`.
This is a breaking change, but it seems hard to offer an
alternative that's both correct and backward compatible.

Additionally, an internal flag has been added to SQL flavors,
`supportsTimezones`. This allows us to handle SQLite3, which doesn't
support timezones, while still storing timezones in other flavors.
Flavors that don't support timezones are explicitly converted to
UTC.
This commit is contained in:
Eric Chiang
2016-12-16 11:03:36 -08:00
parent dd3133072c
commit fd20b213bb
4 changed files with 121 additions and 31 deletions

View File

@@ -48,6 +48,7 @@ func RunTests(t *testing.T, newStorage func() storage.Storage) {
{"PasswordCRUD", testPasswordCRUD},
{"KeysCRUD", testKeysCRUD},
{"GarbageCollection", testGC},
{"TimezoneSupport", testTimezones},
})
}
@@ -370,14 +371,23 @@ func testKeysCRUD(t *testing.T, s storage.Storage) {
}
func testGC(t *testing.T, s storage.Storage) {
n := time.Now().UTC()
est, err := time.LoadLocation("America/New_York")
if err != nil {
t.Fatal(err)
}
pst, err := time.LoadLocation("America/Los_Angeles")
if err != nil {
t.Fatal(err)
}
expiry := time.Now().In(est)
c := storage.AuthCode{
ID: storage.NewID(),
ClientID: "foobar",
RedirectURI: "https://localhost:80/callback",
Nonce: "foobar",
Scopes: []string{"openid", "email"},
Expiry: n.Add(time.Second),
Expiry: expiry,
ConnectorID: "ldap",
ConnectorData: []byte(`{"some":"data"}`),
Claims: storage.Claims{
@@ -393,14 +403,21 @@ func testGC(t *testing.T, s storage.Storage) {
t.Fatalf("failed creating auth code: %v", err)
}
if _, err := s.GarbageCollect(n); err != nil {
t.Errorf("garbage collection failed: %v", err)
}
if _, err := s.GetAuthCode(c.ID); err != nil {
t.Errorf("expected to be able to get auth code after GC: %v", err)
for _, tz := range []*time.Location{time.UTC, est, pst} {
result, err := s.GarbageCollect(expiry.Add(-time.Hour).In(tz))
if err != nil {
t.Errorf("garbage collection failed: %v", err)
} else {
if result.AuthCodes != 0 || result.AuthRequests != 0 {
t.Errorf("expected no garbage collection results, got %#v", result)
}
}
if _, err := s.GetAuthCode(c.ID); err != nil {
t.Errorf("expected to be able to get auth code after GC: %v", err)
}
}
if r, err := s.GarbageCollect(n.Add(time.Minute)); err != nil {
if r, err := s.GarbageCollect(expiry.Add(time.Hour)); err != nil {
t.Errorf("garbage collection failed: %v", err)
} else if r.AuthCodes != 1 {
t.Errorf("expected to garbage collect 1 objects, got %d", r.AuthCodes)
@@ -422,7 +439,7 @@ func testGC(t *testing.T, s storage.Storage) {
State: "bar",
ForceApprovalPrompt: true,
LoggedIn: true,
Expiry: n,
Expiry: expiry,
ConnectorID: "ldap",
ConnectorData: []byte(`{"some":"data"}`),
Claims: storage.Claims{
@@ -438,14 +455,21 @@ func testGC(t *testing.T, s storage.Storage) {
t.Fatalf("failed creating auth request: %v", err)
}
if _, err := s.GarbageCollect(n); err != nil {
t.Errorf("garbage collection failed: %v", err)
}
if _, err := s.GetAuthRequest(a.ID); err != nil {
t.Errorf("expected to be able to get auth code after GC: %v", err)
for _, tz := range []*time.Location{time.UTC, est, pst} {
result, err := s.GarbageCollect(expiry.Add(-time.Hour).In(tz))
if err != nil {
t.Errorf("garbage collection failed: %v", err)
} else {
if result.AuthCodes != 0 || result.AuthRequests != 0 {
t.Errorf("expected no garbage collection results, got %#v", result)
}
}
if _, err := s.GetAuthRequest(a.ID); err != nil {
t.Errorf("expected to be able to get auth code after GC: %v", err)
}
}
if r, err := s.GarbageCollect(n.Add(time.Minute)); err != nil {
if r, err := s.GarbageCollect(expiry.Add(time.Hour)); err != nil {
t.Errorf("garbage collection failed: %v", err)
} else if r.AuthRequests != 1 {
t.Errorf("expected to garbage collect 1 objects, got %d", r.AuthRequests)
@@ -457,3 +481,49 @@ func testGC(t *testing.T, s storage.Storage) {
t.Errorf("expected storage.ErrNotFound, got %v", err)
}
}
// testTimezones tests that backends either fully support timezones or
// do the correct standardization.
func testTimezones(t *testing.T, s storage.Storage) {
est, err := time.LoadLocation("America/New_York")
if err != nil {
t.Fatal(err)
}
// Create an expiry with timezone info. Only expect backends to be
// accurate to the millisecond
expiry := time.Now().In(est).Round(time.Millisecond)
c := storage.AuthCode{
ID: storage.NewID(),
ClientID: "foobar",
RedirectURI: "https://localhost:80/callback",
Nonce: "foobar",
Scopes: []string{"openid", "email"},
Expiry: expiry,
ConnectorID: "ldap",
ConnectorData: []byte(`{"some":"data"}`),
Claims: storage.Claims{
UserID: "1",
Username: "jane",
Email: "jane.doe@example.com",
EmailVerified: true,
Groups: []string{"a", "b"},
},
}
if err := s.CreateAuthCode(c); err != nil {
t.Fatalf("failed creating auth code: %v", err)
}
got, err := s.GetAuthCode(c.ID)
if err != nil {
t.Fatalf("failed to get auth code: %v", err)
}
// Ensure that if the resulting time is converted to the same
// timezone, it's the same value. We DO NOT expect timezones
// to be preserved.
gotTime := got.Expiry.In(est)
wantTime := expiry
if !gotTime.Equal(wantTime) {
t.Fatalf("expected expiry %v got %v", wantTime, gotTime)
}
}