-
Notifications
You must be signed in to change notification settings - Fork 678
fix(github): made token expiration fields nullable #8707
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a MySQL datetime error that occurs when creating GitHub connections with Personal Access Tokens (PATs). The issue arose because MySQL rejects '0000-00-00' datetime values, which were being used for PAT connections that don't have OAuth-style token expiration. The fix makes the token_expires_at and refresh_token_expires_at columns nullable, allowing PAT connections to store NULL values while OAuth connections can still store actual expiration timestamps.
Changes:
- Modified
TokenExpiresAtandRefreshTokenExpiresAtfields fromtime.Timeto*time.Timein the GithubConnection model - Added nil check in token refresh logic to prevent attempting token refresh for PAT connections
- Updated all test files to use pointer values for token expiration timestamps
- Created database migration to alter columns to be nullable
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| backend/plugins/github/models/connection.go | Changed TokenExpiresAt and RefreshTokenExpiresAt fields to pointers (*time.Time) and updated UpdateToken signature |
| backend/plugins/github/models/migrationscripts/20260211_modify_connection_token_expires_at_to_nullable.go | Added new migration script to make token expiration fields nullable in database |
| backend/plugins/github/models/migrationscripts/register.go | Registered the new migration script |
| backend/plugins/github/token/token_provider.go | Added nil check for TokenExpiresAt before dereferencing, and updated to pass pointers to UpdateToken |
| backend/plugins/github/token/token_provider_test.go | Updated tests to create expiry time variables and pass pointers |
| backend/plugins/github/token/round_tripper_test.go | Updated test to create expiry time variable and pass pointer |
Comments suppressed due to low confidence (1)
backend/plugins/github/token/token_provider_test.go:75
- The test cases in this file don't include a scenario where TokenExpiresAt is nil, which represents a PAT (Personal Access Token) connection without OAuth-style token expiration. Consider adding a test case like:
// Nil TokenExpiresAt (PAT token without expiration)
tp.conn.TokenExpiresAt = nil
assert.False(t, tp.needsRefresh())This would ensure the nil check in token_provider.go (line 87-89) works correctly for PAT connections.
func TestNeedsRefresh(t *testing.T) {
tp := &TokenProvider{
conn: &models.GithubConnection{
GithubConn: models.GithubConn{
RefreshToken: "refresh_token",
},
},
}
// Not expired, outside buffer
expiry1 := time.Now().Add(10 * time.Minute)
tp.conn.TokenExpiresAt = &expiry1
assert.False(t, tp.needsRefresh())
// Inside buffer
expiry2 := time.Now().Add(1 * time.Minute)
tp.conn.TokenExpiresAt = &expiry2
assert.True(t, tp.needsRefresh())
// Expired
expiry3 := time.Now().Add(-1 * time.Minute)
tp.conn.TokenExpiresAt = &expiry3
assert.True(t, tp.needsRefresh())
// No refresh token
tp.conn.RefreshToken = ""
assert.False(t, tp.needsRefresh())
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
klesh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Good work.
Summary
What does this PR do?
This PR fixes a MySQL datetime error that occurs when creating GitHub connections with Personal Access Tokens (PATs). The fix modifies the
token_expires_atandrefresh_token_expires_atcolumns in the_tool_github_connectionstable to be nullable.The Problem:
'0000-00-00'datetime values with error:Error 1292 (22007): Incorrect datetime value: '0000-00-00' for column 'token_expires_at'The Solution:
token_expires_atandrefresh_token_expires_atcolumns to be nullable (*time.Time)Does this close any open issues?
Closes #8693
Screenshots
Other Information