From b85f8b87d42855fe19db9c151d1b8faf955d0327 Mon Sep 17 00:00:00 2001 From: Yuvraj Singh Chauhan Date: Wed, 11 Feb 2026 08:38:50 +0530 Subject: [PATCH] fix(github): made token expiration fields nullable and updated related logic --- backend/plugins/github/models/connection.go | 8 +-- ...connection_token_expires_at_to_nullable.go | 52 +++++++++++++++++++ .../models/migrationscripts/register.go | 1 + .../github/token/round_tripper_test.go | 3 +- .../plugins/github/token/token_provider.go | 12 +++-- .../github/token/token_provider_test.go | 18 ++++--- 6 files changed, 80 insertions(+), 14 deletions(-) create mode 100644 backend/plugins/github/models/migrationscripts/20260211_modify_connection_token_expires_at_to_nullable.go diff --git a/backend/plugins/github/models/connection.go b/backend/plugins/github/models/connection.go index 4dba2f75689..d03353c1881 100644 --- a/backend/plugins/github/models/connection.go +++ b/backend/plugins/github/models/connection.go @@ -56,13 +56,13 @@ type GithubConn struct { helper.MultiAuth `mapstructure:",squash"` GithubAccessToken `mapstructure:",squash" authMethod:"AccessToken"` GithubAppKey `mapstructure:",squash" authMethod:"AppKey"` - RefreshToken string `mapstructure:"refreshToken" json:"refreshToken" gorm:"type:text;serializer:encdec"` - TokenExpiresAt time.Time `mapstructure:"tokenExpiresAt" json:"tokenExpiresAt"` - RefreshTokenExpiresAt time.Time `mapstructure:"refreshTokenExpiresAt" json:"refreshTokenExpiresAt"` + RefreshToken string `mapstructure:"refreshToken" json:"refreshToken" gorm:"type:text;serializer:encdec"` + TokenExpiresAt *time.Time `mapstructure:"tokenExpiresAt" json:"tokenExpiresAt"` + RefreshTokenExpiresAt *time.Time `mapstructure:"refreshTokenExpiresAt" json:"refreshTokenExpiresAt"` } // UpdateToken updates the token and refresh token information -func (conn *GithubConn) UpdateToken(newToken, newRefreshToken string, expiry, refreshExpiry time.Time) { +func (conn *GithubConn) UpdateToken(newToken, newRefreshToken string, expiry, refreshExpiry *time.Time) { conn.Token = newToken conn.RefreshToken = newRefreshToken conn.TokenExpiresAt = expiry diff --git a/backend/plugins/github/models/migrationscripts/20260211_modify_connection_token_expires_at_to_nullable.go b/backend/plugins/github/models/migrationscripts/20260211_modify_connection_token_expires_at_to_nullable.go new file mode 100644 index 00000000000..ff7b127fb09 --- /dev/null +++ b/backend/plugins/github/models/migrationscripts/20260211_modify_connection_token_expires_at_to_nullable.go @@ -0,0 +1,52 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with +this work for additional information regarding copyright ownership. +The ASF licenses this file to You under the Apache License, Version 2.0 +(the "License"); you may not use this file except in compliance with +the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package migrationscripts + +import ( + "time" + + "github.com/apache/incubator-devlake/core/context" + "github.com/apache/incubator-devlake/core/errors" + "github.com/apache/incubator-devlake/helpers/migrationhelper" +) + +type githubConnection20260211 struct { + TokenExpiresAt *time.Time + RefreshTokenExpiresAt *time.Time +} + +func (githubConnection20260211) TableName() string { + return "_tool_github_connections" +} + +type modifyTokenExpiresAtToNullable struct{} + +func (*modifyTokenExpiresAtToNullable) Up(basicRes context.BasicRes) errors.Error { + return migrationhelper.AutoMigrateTables( + basicRes, + &githubConnection20260211{}, + ) +} + +func (*modifyTokenExpiresAtToNullable) Version() uint64 { + return 20260211000001 +} + +func (*modifyTokenExpiresAtToNullable) Name() string { + return "modify token_expires_at and refresh_token_expires_at to nullable" +} diff --git a/backend/plugins/github/models/migrationscripts/register.go b/backend/plugins/github/models/migrationscripts/register.go index 74f9d712b4c..0094972890e 100644 --- a/backend/plugins/github/models/migrationscripts/register.go +++ b/backend/plugins/github/models/migrationscripts/register.go @@ -56,5 +56,6 @@ func All() []plugin.MigrationScript { new(changeIssueComponentType), new(addIndexToGithubJobs), new(addRefreshTokenFields), + new(modifyTokenExpiresAtToNullable), } } diff --git a/backend/plugins/github/token/round_tripper_test.go b/backend/plugins/github/token/round_tripper_test.go index 6767d8ccfd1..e766adb8824 100644 --- a/backend/plugins/github/token/round_tripper_test.go +++ b/backend/plugins/github/token/round_tripper_test.go @@ -36,6 +36,7 @@ func TestRoundTripper401Refresh(t *testing.T) { mockRT := new(MockRoundTripper) client := &http.Client{Transport: mockRT} + expiry := time.Now().Add(10 * time.Minute) // Not expired conn := &models.GithubConnection{ GithubConn: models.GithubConn{ RefreshToken: "refresh_token", @@ -44,7 +45,7 @@ func TestRoundTripper401Refresh(t *testing.T) { Token: "old_token", }, }, - TokenExpiresAt: time.Now().Add(10 * time.Minute), // Not expired + TokenExpiresAt: &expiry, GithubAppKey: models.GithubAppKey{ AppKey: api.AppKey{ AppId: "123", diff --git a/backend/plugins/github/token/token_provider.go b/backend/plugins/github/token/token_provider.go index ba9941cd47d..ed3fa2256e4 100644 --- a/backend/plugins/github/token/token_provider.go +++ b/backend/plugins/github/token/token_provider.go @@ -84,7 +84,10 @@ func (tp *TokenProvider) needsRefresh() bool { } } - return time.Now().Add(buffer).After(tp.conn.TokenExpiresAt) + if tp.conn.TokenExpiresAt == nil { + return false + } + return time.Now().Add(buffer).After(*tp.conn.TokenExpiresAt) } func (tp *TokenProvider) refreshToken() errors.Error { @@ -145,11 +148,14 @@ func (tp *TokenProvider) refreshToken() errors.Error { return errors.Default.New(fmt.Sprintf("empty access token returned; response body: %s", bodyStr)) } + tokenExpiredAt := time.Now().Add(time.Duration(result.ExpiresIn) * time.Second) + refreshTokenExpiredAt := time.Now().Add(time.Duration(result.RefreshTokenExpiresIn) * time.Second) + tp.conn.UpdateToken( result.AccessToken, result.RefreshToken, - time.Now().Add(time.Duration(result.ExpiresIn)*time.Second), - time.Now().Add(time.Duration(result.RefreshTokenExpiresIn)*time.Second), + &tokenExpiredAt, + &refreshTokenExpiredAt, ) if tp.dal != nil { diff --git a/backend/plugins/github/token/token_provider_test.go b/backend/plugins/github/token/token_provider_test.go index 1c296376a6e..3319f55910f 100644 --- a/backend/plugins/github/token/token_provider_test.go +++ b/backend/plugins/github/token/token_provider_test.go @@ -55,15 +55,18 @@ func TestNeedsRefresh(t *testing.T) { } // Not expired, outside buffer - tp.conn.TokenExpiresAt = time.Now().Add(10 * time.Minute) + expiry1 := time.Now().Add(10 * time.Minute) + tp.conn.TokenExpiresAt = &expiry1 assert.False(t, tp.needsRefresh()) // Inside buffer - tp.conn.TokenExpiresAt = time.Now().Add(1 * time.Minute) + expiry2 := time.Now().Add(1 * time.Minute) + tp.conn.TokenExpiresAt = &expiry2 assert.True(t, tp.needsRefresh()) // Expired - tp.conn.TokenExpiresAt = time.Now().Add(-1 * time.Minute) + expiry3 := time.Now().Add(-1 * time.Minute) + tp.conn.TokenExpiresAt = &expiry3 assert.True(t, tp.needsRefresh()) // No refresh token @@ -75,10 +78,11 @@ func TestTokenProviderConcurrency(t *testing.T) { mockRT := new(MockRoundTripper) client := &http.Client{Transport: mockRT} + expired := time.Now().Add(-1 * time.Minute) // Expired conn := &models.GithubConnection{ GithubConn: models.GithubConn{ RefreshToken: "refresh_token", - TokenExpiresAt: time.Now().Add(-1 * time.Minute), // Expired + TokenExpiresAt: &expired, GithubAppKey: models.GithubAppKey{ AppKey: api.AppKey{ AppId: "123", @@ -129,11 +133,13 @@ func TestConfigurableBuffer(t *testing.T) { } // 9 minutes remaining (inside 10m buffer) - tp.conn.TokenExpiresAt = time.Now().Add(9 * time.Minute) + expiry9 := time.Now().Add(9 * time.Minute) + tp.conn.TokenExpiresAt = &expiry9 assert.True(t, tp.needsRefresh()) // 11 minutes remaining (outside 10m buffer) - tp.conn.TokenExpiresAt = time.Now().Add(11 * time.Minute) + expiry11 := time.Now().Add(11 * time.Minute) + tp.conn.TokenExpiresAt = &expiry11 assert.False(t, tp.needsRefresh()) }