Skip to content

Commit 32eafc8

Browse files
authored
fix(github): made token expiration fields nullable and updated related logic (#8707)
1 parent 80f9e2d commit 32eafc8

6 files changed

Lines changed: 80 additions & 14 deletions

File tree

backend/plugins/github/models/connection.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ type GithubConn struct {
5656
helper.MultiAuth `mapstructure:",squash"`
5757
GithubAccessToken `mapstructure:",squash" authMethod:"AccessToken"`
5858
GithubAppKey `mapstructure:",squash" authMethod:"AppKey"`
59-
RefreshToken string `mapstructure:"refreshToken" json:"refreshToken" gorm:"type:text;serializer:encdec"`
60-
TokenExpiresAt time.Time `mapstructure:"tokenExpiresAt" json:"tokenExpiresAt"`
61-
RefreshTokenExpiresAt time.Time `mapstructure:"refreshTokenExpiresAt" json:"refreshTokenExpiresAt"`
59+
RefreshToken string `mapstructure:"refreshToken" json:"refreshToken" gorm:"type:text;serializer:encdec"`
60+
TokenExpiresAt *time.Time `mapstructure:"tokenExpiresAt" json:"tokenExpiresAt"`
61+
RefreshTokenExpiresAt *time.Time `mapstructure:"refreshTokenExpiresAt" json:"refreshTokenExpiresAt"`
6262
}
6363

6464
// UpdateToken updates the token and refresh token information
65-
func (conn *GithubConn) UpdateToken(newToken, newRefreshToken string, expiry, refreshExpiry time.Time) {
65+
func (conn *GithubConn) UpdateToken(newToken, newRefreshToken string, expiry, refreshExpiry *time.Time) {
6666
conn.Token = newToken
6767
conn.RefreshToken = newRefreshToken
6868
conn.TokenExpiresAt = expiry
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/*
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
18+
package migrationscripts
19+
20+
import (
21+
"time"
22+
23+
"github.com/apache/incubator-devlake/core/context"
24+
"github.com/apache/incubator-devlake/core/errors"
25+
"github.com/apache/incubator-devlake/helpers/migrationhelper"
26+
)
27+
28+
type githubConnection20260211 struct {
29+
TokenExpiresAt *time.Time
30+
RefreshTokenExpiresAt *time.Time
31+
}
32+
33+
func (githubConnection20260211) TableName() string {
34+
return "_tool_github_connections"
35+
}
36+
37+
type modifyTokenExpiresAtToNullable struct{}
38+
39+
func (*modifyTokenExpiresAtToNullable) Up(basicRes context.BasicRes) errors.Error {
40+
return migrationhelper.AutoMigrateTables(
41+
basicRes,
42+
&githubConnection20260211{},
43+
)
44+
}
45+
46+
func (*modifyTokenExpiresAtToNullable) Version() uint64 {
47+
return 20260211000001
48+
}
49+
50+
func (*modifyTokenExpiresAtToNullable) Name() string {
51+
return "modify token_expires_at and refresh_token_expires_at to nullable"
52+
}

backend/plugins/github/models/migrationscripts/register.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,5 +56,6 @@ func All() []plugin.MigrationScript {
5656
new(changeIssueComponentType),
5757
new(addIndexToGithubJobs),
5858
new(addRefreshTokenFields),
59+
new(modifyTokenExpiresAtToNullable),
5960
}
6061
}

backend/plugins/github/token/round_tripper_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ func TestRoundTripper401Refresh(t *testing.T) {
3636
mockRT := new(MockRoundTripper)
3737
client := &http.Client{Transport: mockRT}
3838

39+
expiry := time.Now().Add(10 * time.Minute) // Not expired
3940
conn := &models.GithubConnection{
4041
GithubConn: models.GithubConn{
4142
RefreshToken: "refresh_token",
@@ -44,7 +45,7 @@ func TestRoundTripper401Refresh(t *testing.T) {
4445
Token: "old_token",
4546
},
4647
},
47-
TokenExpiresAt: time.Now().Add(10 * time.Minute), // Not expired
48+
TokenExpiresAt: &expiry,
4849
GithubAppKey: models.GithubAppKey{
4950
AppKey: api.AppKey{
5051
AppId: "123",

backend/plugins/github/token/token_provider.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ func (tp *TokenProvider) needsRefresh() bool {
8484
}
8585
}
8686

87-
return time.Now().Add(buffer).After(tp.conn.TokenExpiresAt)
87+
if tp.conn.TokenExpiresAt == nil {
88+
return false
89+
}
90+
return time.Now().Add(buffer).After(*tp.conn.TokenExpiresAt)
8891
}
8992

9093
func (tp *TokenProvider) refreshToken() errors.Error {
@@ -145,11 +148,14 @@ func (tp *TokenProvider) refreshToken() errors.Error {
145148
return errors.Default.New(fmt.Sprintf("empty access token returned; response body: %s", bodyStr))
146149
}
147150

151+
tokenExpiredAt := time.Now().Add(time.Duration(result.ExpiresIn) * time.Second)
152+
refreshTokenExpiredAt := time.Now().Add(time.Duration(result.RefreshTokenExpiresIn) * time.Second)
153+
148154
tp.conn.UpdateToken(
149155
result.AccessToken,
150156
result.RefreshToken,
151-
time.Now().Add(time.Duration(result.ExpiresIn)*time.Second),
152-
time.Now().Add(time.Duration(result.RefreshTokenExpiresIn)*time.Second),
157+
&tokenExpiredAt,
158+
&refreshTokenExpiredAt,
153159
)
154160

155161
if tp.dal != nil {

backend/plugins/github/token/token_provider_test.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,15 +55,18 @@ func TestNeedsRefresh(t *testing.T) {
5555
}
5656

5757
// Not expired, outside buffer
58-
tp.conn.TokenExpiresAt = time.Now().Add(10 * time.Minute)
58+
expiry1 := time.Now().Add(10 * time.Minute)
59+
tp.conn.TokenExpiresAt = &expiry1
5960
assert.False(t, tp.needsRefresh())
6061

6162
// Inside buffer
62-
tp.conn.TokenExpiresAt = time.Now().Add(1 * time.Minute)
63+
expiry2 := time.Now().Add(1 * time.Minute)
64+
tp.conn.TokenExpiresAt = &expiry2
6365
assert.True(t, tp.needsRefresh())
6466

6567
// Expired
66-
tp.conn.TokenExpiresAt = time.Now().Add(-1 * time.Minute)
68+
expiry3 := time.Now().Add(-1 * time.Minute)
69+
tp.conn.TokenExpiresAt = &expiry3
6770
assert.True(t, tp.needsRefresh())
6871

6972
// No refresh token
@@ -75,10 +78,11 @@ func TestTokenProviderConcurrency(t *testing.T) {
7578
mockRT := new(MockRoundTripper)
7679
client := &http.Client{Transport: mockRT}
7780

81+
expired := time.Now().Add(-1 * time.Minute) // Expired
7882
conn := &models.GithubConnection{
7983
GithubConn: models.GithubConn{
8084
RefreshToken: "refresh_token",
81-
TokenExpiresAt: time.Now().Add(-1 * time.Minute), // Expired
85+
TokenExpiresAt: &expired,
8286
GithubAppKey: models.GithubAppKey{
8387
AppKey: api.AppKey{
8488
AppId: "123",
@@ -129,11 +133,13 @@ func TestConfigurableBuffer(t *testing.T) {
129133
}
130134

131135
// 9 minutes remaining (inside 10m buffer)
132-
tp.conn.TokenExpiresAt = time.Now().Add(9 * time.Minute)
136+
expiry9 := time.Now().Add(9 * time.Minute)
137+
tp.conn.TokenExpiresAt = &expiry9
133138
assert.True(t, tp.needsRefresh())
134139

135140
// 11 minutes remaining (outside 10m buffer)
136-
tp.conn.TokenExpiresAt = time.Now().Add(11 * time.Minute)
141+
expiry11 := time.Now().Add(11 * time.Minute)
142+
tp.conn.TokenExpiresAt = &expiry11
137143
assert.False(t, tp.needsRefresh())
138144
}
139145

0 commit comments

Comments
 (0)