From 06c8ab5aa71e9df22a6e093ad060fb1a6cb25d99 Mon Sep 17 00:00:00 2001 From: "m.nabokikh" Date: Sun, 15 Nov 2020 22:26:34 +0400 Subject: [PATCH] Fixes of naming and code style Signed-off-by: m.nabokikh --- cmd/dex/config.go | 4 ++-- cmd/dex/serve.go | 10 +++++----- server/handlers.go | 7 ++++++- server/rotation.go | 12 ++++++------ server/rotation_test.go | 7 +++---- server/server_test.go | 22 ++++++++-------------- storage/sql/migrate.go | 10 +++++++--- 7 files changed, 37 insertions(+), 35 deletions(-) diff --git a/cmd/dex/config.go b/cmd/dex/config.go index 6683c39e..a75ddaee 100644 --- a/cmd/dex/config.go +++ b/cmd/dex/config.go @@ -305,8 +305,8 @@ type Expiry struct { // DeviceRequests defines the duration of time for which the DeviceRequests will be valid. DeviceRequests string `json:"deviceRequests"` - // RefreshToken defines refresh tokens expiry policy - RefreshToken RefreshTokenExpiry `json:"refreshTokens"` + // RefreshTokens defines refresh tokens expiry policy + RefreshTokens RefreshTokenExpiry `json:"refreshTokens"` } // Logger holds configuration required to customize logging for dex. diff --git a/cmd/dex/serve.go b/cmd/dex/serve.go index aa8f5ad6..9e6f8b9a 100644 --- a/cmd/dex/serve.go +++ b/cmd/dex/serve.go @@ -317,12 +317,12 @@ func runServe(options serveOptions) error { logger.Infof("config device requests valid for: %v", deviceRequests) serverConfig.DeviceRequestsValidFor = deviceRequests } - refreshTokenPolicy, err := server.NewRefreshTokenPolicyFromConfig( + refreshTokenPolicy, err := server.NewRefreshTokenPolicy( logger, - c.Expiry.RefreshToken.DisableRotation, - c.Expiry.RefreshToken.ValidIfNotUsedFor, - c.Expiry.RefreshToken.AbsoluteLifetime, - c.Expiry.RefreshToken.ReuseInterval, + c.Expiry.RefreshTokens.DisableRotation, + c.Expiry.RefreshTokens.ValidIfNotUsedFor, + c.Expiry.RefreshTokens.AbsoluteLifetime, + c.Expiry.RefreshTokens.ReuseInterval, ) if err != nil { return fmt.Errorf("invalid refresh token expiration policy config: %v", err) diff --git a/server/handlers.go b/server/handlers.go index 0f76c410..aa08b7a5 100644 --- a/server/handlers.go +++ b/server/handlers.go @@ -1042,7 +1042,12 @@ func (s *Server) handleRefreshToken(w http.ResponseWriter, r *http.Request, clie return } if refresh.Token != token.Token { - if !s.refreshTokenPolicy.AllowedToReuse(refresh.LastUsed) || refresh.ObsoleteToken != token.Token { + switch { + case !s.refreshTokenPolicy.AllowedToReuse(refresh.LastUsed): + fallthrough + case refresh.ObsoleteToken != token.Token: + fallthrough + case refresh.ObsoleteToken == "": s.logger.Errorf("refresh token with id %s claimed twice", refresh.ID) s.tokenErrHelper(w, errInvalidRequest, "Refresh token is invalid or has already been claimed by another client.", http.StatusBadRequest) return diff --git a/server/rotation.go b/server/rotation.go index 48593ede..98489767 100644 --- a/server/rotation.go +++ b/server/rotation.go @@ -185,13 +185,13 @@ type RefreshTokenPolicy struct { validIfNotUsedFor time.Duration // interval from last token update to the end of its life reuseInterval time.Duration // interval within which old refresh token is allowed to be reused - Clock func() time.Time + now func() time.Time logger log.Logger } -func NewRefreshTokenPolicyFromConfig(logger log.Logger, rotation bool, validIfNotUsedFor, absoluteLifetime, reuseInterval string) (*RefreshTokenPolicy, error) { - r := RefreshTokenPolicy{Clock: time.Now, logger: logger} +func NewRefreshTokenPolicy(logger log.Logger, rotation bool, validIfNotUsedFor, absoluteLifetime, reuseInterval string) (*RefreshTokenPolicy, error) { + r := RefreshTokenPolicy{now: time.Now, logger: logger} var err error if validIfNotUsedFor != "" { @@ -231,19 +231,19 @@ func (r *RefreshTokenPolicy) CompletelyExpired(lastUsed time.Time) bool { if r.absoluteLifetime == 0 { return false // expiration disabled } - return r.Clock().After(lastUsed.Add(r.absoluteLifetime)) + return r.now().After(lastUsed.Add(r.absoluteLifetime)) } func (r *RefreshTokenPolicy) ExpiredBecauseUnused(lastUsed time.Time) bool { if r.validIfNotUsedFor == 0 { return false // expiration disabled } - return r.Clock().After(lastUsed.Add(r.validIfNotUsedFor)) + return r.now().After(lastUsed.Add(r.validIfNotUsedFor)) } func (r *RefreshTokenPolicy) AllowedToReuse(lastUsed time.Time) bool { if r.reuseInterval == 0 { return false // expiration disabled } - return !r.Clock().After(lastUsed.Add(r.reuseInterval)) + return !r.now().After(lastUsed.Add(r.reuseInterval)) } diff --git a/server/rotation_test.go b/server/rotation_test.go index a75a2435..e279bf54 100644 --- a/server/rotation_test.go +++ b/server/rotation_test.go @@ -110,19 +110,18 @@ func TestRefreshTokenPolicy(t *testing.T) { Level: logrus.DebugLevel, } - r, err := NewRefreshTokenPolicyFromConfig(l, true, "1m", "1m", "1m") + r, err := NewRefreshTokenPolicy(l, true, "1m", "1m", "1m") require.NoError(t, err) t.Run("Allowed", func(t *testing.T) { - r.Clock = func() time.Time { return lastTime } + r.now = func() time.Time { return lastTime } require.Equal(t, true, r.AllowedToReuse(lastTime)) require.Equal(t, false, r.ExpiredBecauseUnused(lastTime)) require.Equal(t, false, r.CompletelyExpired(lastTime)) }) t.Run("Expired", func(t *testing.T) { - r.Clock = func() time.Time { return lastTime.Add(2 * time.Minute) } - time.Sleep(1 * time.Second) + r.now = func() time.Time { return lastTime.Add(2 * time.Minute) } require.Equal(t, false, r.AllowedToReuse(lastTime)) require.Equal(t, true, r.ExpiredBecauseUnused(lastTime)) require.Equal(t, true, r.CompletelyExpired(lastTime)) diff --git a/server/server_test.go b/server/server_test.go index 2ef4f613..d8b40991 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -117,6 +117,14 @@ func newTestServer(ctx context.Context, t *testing.T, updateConfig func(c *Confi t.Fatal(err) } server.skipApproval = true // Don't prompt for approval, just immediately redirect with code. + + // Default rotation policy + server.refreshTokenPolicy, err = NewRefreshTokenPolicy(logger, false, "", "", "") + if err != nil { + t.Fatalf("failed to prepare rotation policy: %v", err) + } + server.refreshTokenPolicy.now = config.Now + return s, server } @@ -677,13 +685,6 @@ func TestOAuth2CodeFlow(t *testing.T) { }) defer httpServer.Close() - policy, err := NewRefreshTokenPolicyFromConfig(s.logger, false, "", "", "") - if err != nil { - t.Fatalf("failed to prepare rotation policy: %v", err) - } - policy.Clock = now - s.refreshTokenPolicy = policy - mockConn := s.connectors["mock"] conn = mockConn.Connector.(*mock.Callback) @@ -1515,13 +1516,6 @@ func TestOAuth2DeviceFlow(t *testing.T) { }) defer httpServer.Close() - policy, err := NewRefreshTokenPolicyFromConfig(s.logger, false, "", "", "") - if err != nil { - t.Fatalf("failed to prepare rotation policy: %v", err) - } - policy.Clock = now - s.refreshTokenPolicy = policy - mockConn := s.connectors["mock"] conn = mockConn.Connector.(*mock.Callback) diff --git a/storage/sql/migrate.go b/storage/sql/migrate.go index 0f2666bf..498db252 100644 --- a/storage/sql/migrate.go +++ b/storage/sql/migrate.go @@ -176,9 +176,6 @@ var migrations = []migration{ alter table refresh_token add column token text not null default '';`, ` - alter table refresh_token - add column obsolete_token text default '';`, - ` alter table refresh_token add column created_at timestamptz not null default '0001-01-01 00:00:00 UTC';`, ` @@ -277,4 +274,11 @@ var migrations = []migration{ add column code_challenge_method text not null default '';`, }, }, + { + stmts: []string{ + ` + alter table refresh_token + add column obsolete_token text default '';`, + }, + }, }