Fixes of naming and code style

Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>

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
+	// RefreshTokens defines refresh tokens expiry policy
-	RefreshToken RefreshTokenExpiry `json:"refreshTokens"`
+	RefreshTokens RefreshTokenExpiry `json:"refreshTokens"`
 }
 
 // Logger holds configuration required to customize logging for dex.

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.RefreshTokens.DisableRotation,
-		c.Expiry.RefreshToken.ValidIfNotUsedFor,
+		c.Expiry.RefreshTokens.ValidIfNotUsedFor,
-		c.Expiry.RefreshToken.AbsoluteLifetime,
+		c.Expiry.RefreshTokens.AbsoluteLifetime,
-		c.Expiry.RefreshToken.ReuseInterval,
+		c.Expiry.RefreshTokens.ReuseInterval,
 	)
 	if err != nil {
 		return fmt.Errorf("invalid refresh token expiration policy config: %v", err)

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

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) {
+func NewRefreshTokenPolicy(logger log.Logger, rotation bool, validIfNotUsedFor, absoluteLifetime, reuseInterval string) (*RefreshTokenPolicy, error) {
-	r := RefreshTokenPolicy{Clock: time.Now, logger: logger}
+	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))
 }

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) }
+		r.now = func() time.Time { return lastTime.Add(2 * time.Minute) }
-		time.Sleep(1 * time.Second)
 		require.Equal(t, false, r.AllowedToReuse(lastTime))
 		require.Equal(t, true, r.ExpiredBecauseUnused(lastTime))
 		require.Equal(t, true, r.CompletelyExpired(lastTime))

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)
 

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 '';`,
+		},
+	},
 }