Skip to content

Commit 611556c

Browse files
lrf-nitrospiffazSpiff Azetadncrewsyamoyamoto
authored
feat: GitHub App token refresh (#8746)
* feat(github): auto-refresh GitHub App installation tokens Add transport-level token refresh for GitHub App (AppKey) connections. GitHub App installation tokens expire after ~1 hour; this adds proactive refresh (before expiry) and reactive refresh (on 401) using the existing TokenProvider/RefreshRoundTripper infrastructure. New files: - app_installation_refresh.go: refresh logic + DB persistence - refresh_api_client.go: minimal ApiClient for token refresh POST - cmd/test_refresh/main.go: manual test script for real GitHub Apps Modified: - connection.go: export GetInstallationAccessToken, parse ExpiresAt - token_provider.go: add refreshFn for pluggable refresh strategies - round_tripper.go: document dual Authorization header interaction - api_client.go: wire AppKey connections into refresh infrastructure - Tests updated for new constructors and AppKey refresh flow * feat(github): add diagnostic logging to GitHub App token refresh Add structured logging at key decision points for token refresh: - Token provider creation (connection ID, installation ID, expiry) - Round tripper installation (connection ID, auth method) - Proactive refresh trigger (near-expiry detection) - Refresh start/success/failure (old/new token prefixes, expiry times) - DB persistence success/failure - Reactive 401 refresh and skip-due-to-concurrent-refresh All logs route through the DevLake logger to pipeline log files. * fix(github): prevent deadlock and fix token persistence in App token refresh Deadlock fix: NewAppInstallationTokenProvider now captures client.Transport (the base transport) before wrapping with RefreshRoundTripper. The refresh function uses newRefreshApiClientWithTransport(baseTransport) to POST for new installation tokens, bypassing the RefreshRoundTripper entirely. Token persistence fix: PersistEncryptedTokenColumns() manually encrypts tokens via plugin.Encrypt() then writes ciphertext via dal.UpdateColumns with conn.TableName() (a string) as the first argument. Passing the table name string makes GORM use Table() instead of Model(), preventing the encdec serializer from corrupting the in-memory token value. The encryption secret is threaded from taskCtx.GetConfig(ENCRYPTION_SECRET) through CreateApiClient to TokenProvider to persist functions. Also persists the initial App token at startup for DB consistency, and adds TestProactiveRefreshNoDeadlock with a real RSA key to verify the deadlock scenario is resolved. * fix(grafana): update dashboard descriptions to list all supported data sources (#8741) Several dashboard introduction panels hardcoded "GitHub and Jira" as required data sources, even though the underlying queries use generic domain layer tables that work with any supported Git tool or issue tracker. Updated to list all supported sources following the pattern already used by DORA and WorkLogs dashboards. Closes #8740 Co-authored-by: Spiff Azeta <spiffazeta@gmail.com> * fix: modify cicd_deployments name from varchar to text (#8724) * fix: modify cicd_deployments name from varchar to text * fix: update the year * fix(q_dev): replace MariaDB-specific IF NOT EXISTS syntax with DAL methods for MySQL 8.x compatibility (#8745) * fix(azuredevops): default empty entities and add CROSS to repo scope in makeScopeV200 (#8751) When scopeConfig.Entities is empty (common when no entities are explicitly selected in the UI), makeScopeV200 produced zero scopes, leaving project_mapping with no rows. Additionally, the repo scope condition did not check for DOMAIN_TYPE_CROSS, so selecting only CROSS would not create a repo scope, breaking DORA metrics. This adds the same fixes applied to GitLab in #8743. Closes #8749 * fix(bitbucket): default empty entities to all domain types in makeScopesV200 (#8750) When scopeConfig.Entities is empty (common when no entities are explicitly selected in the UI), makeScopesV200 produced zero scopes, leaving project_mapping with no repo rows. This adds the same empty-entities default applied to GitLab in #8743. Closes #8748 * fix(github): remove unused refresh client constructor and update tests --------- Co-authored-by: Spiff Azeta <35563797+spiffaz@users.noreply.github.com> Co-authored-by: Spiff Azeta <spiffazeta@gmail.com> Co-authored-by: Dan Crews <crewsd@gmail.com> Co-authored-by: Tomoya Kawaguchi <68677002+yamoyamoto@users.noreply.github.com>
1 parent e92596f commit 611556c

10 files changed

Lines changed: 1019 additions & 64 deletions

File tree

‎backend/plugins/github/models/connection.go‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,13 +81,15 @@ func (conn *GithubConn) PrepareApiClient(apiClient plugin.ApiClient) errors.Erro
8181
}
8282

8383
if conn.AuthMethod == AppKey && conn.InstallationID != 0 {
84-
token, err := conn.getInstallationAccessToken(apiClient)
84+
token, err := conn.GetInstallationAccessToken(apiClient)
8585
if err != nil {
8686
return err
8787
}
88-
89-
conn.Token = token.Token
90-
conn.tokens = []string{token.Token}
88+
var expiresAt *time.Time
89+
if !token.ExpiresAt.IsZero() {
90+
expiresAt = &token.ExpiresAt
91+
}
92+
conn.UpdateToken(token.Token, "", expiresAt, nil)
9193
}
9294

9395
return nil
@@ -354,7 +356,8 @@ type GithubUserOfToken struct {
354356
}
355357

356358
type InstallationToken struct {
357-
Token string `json:"token"`
359+
Token string `json:"token"`
360+
ExpiresAt time.Time `json:"expires_at"`
358361
}
359362

360363
type GithubApp struct {
@@ -397,7 +400,7 @@ func (gak *GithubAppKey) CreateJwt() (string, errors.Error) {
397400
return tokenString, nil
398401
}
399402

400-
func (gak *GithubAppKey) getInstallationAccessToken(
403+
func (gak *GithubAppKey) GetInstallationAccessToken(
401404
apiClient plugin.ApiClient,
402405
) (*InstallationToken, errors.Error) {
403406

@@ -417,12 +420,18 @@ func (gak *GithubAppKey) getInstallationAccessToken(
417420
if err != nil {
418421
return nil, err
419422
}
423+
if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK {
424+
return nil, errors.HttpStatus(resp.StatusCode).New(fmt.Sprintf("unexpected status code while getting installation access token: %s", string(body)))
425+
}
420426

421427
var installationToken InstallationToken
422428
err = errors.Convert(json.Unmarshal(body, &installationToken))
423429
if err != nil {
424430
return nil, err
425431
}
432+
if installationToken.Token == "" {
433+
return nil, errors.Default.New("empty installation access token returned")
434+
}
426435

427436
return &installationToken, nil
428437
}

‎backend/plugins/github/tasks/api_client.go‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,14 +35,18 @@ func CreateApiClient(taskCtx plugin.TaskContext, connection *models.GithubConnec
3535
return nil, err
3636
}
3737

38-
// Inject TokenProvider if refresh token is present
39-
if connection.RefreshToken != "" {
40-
logger := taskCtx.GetLogger()
41-
db := taskCtx.GetDal()
42-
43-
// Create TokenProvider
44-
tp := token.NewTokenProvider(connection, db, apiClient.GetClient(), logger)
38+
logger := taskCtx.GetLogger()
39+
db := taskCtx.GetDal()
40+
encryptionSecret := taskCtx.GetConfig(plugin.EncodeKeyEnvStr)
4541

42+
// Inject TokenProvider for OAuth refresh or GitHub App installation tokens.
43+
var tp *token.TokenProvider
44+
if connection.RefreshToken != "" {
45+
tp = token.NewTokenProvider(connection, db, apiClient.GetClient(), logger, encryptionSecret)
46+
} else if connection.AuthMethod == models.AppKey && connection.InstallationID != 0 {
47+
tp = token.NewAppInstallationTokenProvider(connection, db, apiClient.GetClient(), logger, encryptionSecret)
48+
}
49+
if tp != nil {
4650
// Wrap the transport
4751
baseTransport := apiClient.GetClient().Transport
4852
if baseTransport == nil {
@@ -51,6 +55,20 @@ func CreateApiClient(taskCtx plugin.TaskContext, connection *models.GithubConnec
5155

5256
rt := token.NewRefreshRoundTripper(baseTransport, tp)
5357
apiClient.GetClient().Transport = rt
58+
logger.Info("Installed token refresh round tripper for connection %d (authMethod=%s)",
59+
connection.ID, connection.AuthMethod)
60+
}
61+
62+
// Persist the freshly minted token so the DB has a correctly encrypted value.
63+
// PrepareApiClient (called by NewApiClientFromConnection) mints the token
64+
// in-memory but does not persist it; without this, the DB may contain a stale
65+
// or corrupted token that breaks GET /connections.
66+
if connection.AuthMethod == models.AppKey && connection.Token != "" {
67+
if err := token.PersistEncryptedTokenColumns(db, connection, encryptionSecret, logger, false); err != nil {
68+
logger.Warn(err, "Failed to persist initial token for connection %d", connection.ID)
69+
} else {
70+
logger.Info("Persisted initial token for connection %d", connection.ID)
71+
}
5472
}
5573

5674
// create rate limit calculator

‎backend/plugins/github/token/app_installation_refresh.go‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
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 token
19+
20+
import (
21+
"time"
22+
23+
"github.com/apache/incubator-devlake/core/dal"
24+
"github.com/apache/incubator-devlake/core/errors"
25+
"github.com/apache/incubator-devlake/core/log"
26+
"github.com/apache/incubator-devlake/core/plugin"
27+
"github.com/apache/incubator-devlake/plugins/github/models"
28+
)
29+
30+
// tokenPrefix returns the first n characters of a token for safe logging.
31+
func tokenPrefix(token string, n int) string {
32+
if len(token) <= n {
33+
return token
34+
}
35+
return token[:n] + "..."
36+
}
37+
38+
func refreshGitHubAppInstallationToken(tp *TokenProvider) errors.Error {
39+
if tp == nil || tp.conn == nil {
40+
return errors.Default.New("missing github connection for app token refresh")
41+
}
42+
if tp.conn.AuthMethod != models.AppKey || tp.conn.InstallationID == 0 {
43+
return errors.Default.New("invalid github app connection for token refresh")
44+
}
45+
if tp.conn.Endpoint == "" {
46+
return errors.Default.New("missing github endpoint for token refresh")
47+
}
48+
49+
oldToken := tp.conn.Token
50+
if tp.logger != nil {
51+
expiresStr := "unknown"
52+
if tp.conn.TokenExpiresAt != nil {
53+
expiresStr = tp.conn.TokenExpiresAt.Format(time.RFC3339)
54+
}
55+
tp.logger.Info(
56+
"Refreshing GitHub App installation token for connection %d (installation %d), old token=%s, expires_at=%s",
57+
tp.conn.ID, tp.conn.InstallationID,
58+
tokenPrefix(oldToken, 8),
59+
expiresStr,
60+
)
61+
}
62+
63+
// Use baseTransport (the unwrapped transport) to avoid deadlock.
64+
// The httpClient's transport may be the RefreshRoundTripper which would
65+
// re-enter GetToken() and deadlock on the mutex.
66+
apiClient := newRefreshApiClientWithTransport(tp.conn.Endpoint, tp.baseTransport)
67+
installationToken, err := tp.conn.GithubAppKey.GetInstallationAccessToken(apiClient)
68+
if err != nil {
69+
if tp.logger != nil {
70+
tp.logger.Error(err, "Failed to refresh GitHub App installation token for connection %d", tp.conn.ID)
71+
}
72+
return err
73+
}
74+
75+
var expiresAt *time.Time
76+
if !installationToken.ExpiresAt.IsZero() {
77+
expiresAt = &installationToken.ExpiresAt
78+
}
79+
tp.conn.UpdateToken(installationToken.Token, "", expiresAt, nil)
80+
81+
if tp.logger != nil {
82+
tp.logger.Info(
83+
"Successfully refreshed GitHub App installation token for connection %d, new token=%s, new expires_at=%s",
84+
tp.conn.ID,
85+
tokenPrefix(installationToken.Token, 8),
86+
installationToken.ExpiresAt.Format(time.RFC3339),
87+
)
88+
}
89+
90+
persistAppToken(tp.dal, tp.conn, tp.encryptionSecret, tp.logger)
91+
return nil
92+
}
93+
94+
func persistAppToken(d dal.Dal, conn *models.GithubConnection, encryptionSecret string, logger log.Logger) {
95+
if d == nil || conn == nil {
96+
return
97+
}
98+
if err := PersistEncryptedTokenColumns(d, conn, encryptionSecret, logger, false); err != nil {
99+
if logger != nil {
100+
logger.Warn(err, "Failed to persist refreshed app installation token for connection %d", conn.ID)
101+
}
102+
} else if logger != nil {
103+
logger.Info("Persisted refreshed app installation token for connection %d", conn.ID)
104+
}
105+
}
106+
107+
// PersistEncryptedTokenColumns manually encrypts token fields and writes them
108+
// to the DB using UpdateColumns (map-based), which only touches the specified
109+
// columns. This avoids two problems:
110+
// - dal.Update (GORM Save) writes ALL fields, including refresh_token_expires_at
111+
// which may have Go zero time that MySQL rejects as '0000-00-00'.
112+
// - dal.UpdateColumns with plaintext bypasses the GORM encdec serializer,
113+
// writing unencrypted tokens that corrupt subsequent reads.
114+
//
115+
// IMPORTANT: We pass the table name string (not the conn struct) to UpdateColumns
116+
// so that GORM uses Table() instead of Model(). When Model(conn) is used, GORM
117+
// processes the encdec serializer on the struct's Token field during statement
118+
// preparation, which overwrites conn.Token in memory with the encrypted ciphertext.
119+
// This corrupts the in-memory token causing immediate 401s on the next API call.
120+
//
121+
// If includeRefreshToken is true, refresh_token and refresh_token_expires_at
122+
// are also written (used by the OAuth refresh path where these values are valid).
123+
func PersistEncryptedTokenColumns(d dal.Dal, conn *models.GithubConnection, encryptionSecret string, logger log.Logger, includeRefreshToken bool) errors.Error {
124+
encToken, err := plugin.Encrypt(encryptionSecret, conn.Token)
125+
if err != nil {
126+
return errors.Default.Wrap(err, "failed to encrypt token for persistence")
127+
}
128+
129+
sets := []dal.DalSet{
130+
{ColumnName: "token", Value: encToken},
131+
{ColumnName: "token_expires_at", Value: conn.TokenExpiresAt},
132+
}
133+
134+
if includeRefreshToken {
135+
encRefreshToken, err := plugin.Encrypt(encryptionSecret, conn.RefreshToken)
136+
if err != nil {
137+
return errors.Default.Wrap(err, "failed to encrypt refresh_token for persistence")
138+
}
139+
sets = append(sets,
140+
dal.DalSet{ColumnName: "refresh_token", Value: encRefreshToken},
141+
dal.DalSet{ColumnName: "refresh_token_expires_at", Value: conn.RefreshTokenExpiresAt},
142+
)
143+
}
144+
145+
// Use the table name string instead of the conn struct to prevent GORM from
146+
// running the encdec serializer on conn.Token during Model() processing.
147+
return d.UpdateColumns(
148+
conn.TableName(),
149+
sets,
150+
dal.Where("id = ?", conn.ID),
151+
)
152+
}

0 commit comments

Comments
 (0)