Skip to content

Commit bb97530

Browse files
🩹 [Patch]: Prevent Concurrent Access Token Refresh with Mutex Lock (#393)
This pull request introduces significant updates to the GitHub authentication and API handling mechanisms, improving token management, adding support for GitHub App private keys, and enhancing concurrency handling. The changes also include adjustments to data types and configurations for better precision and compatibility. - Fixes #392 ### Authentication Updates * Added support for GitHub App private key handling by introducing a new `PrivateKey` property in `AppGitHubContext` and ensuring it is securely processed during authentication. * Enhanced token refresh logic to handle both user access tokens and GitHub App PEM tokens, with mutex-based concurrency control for token updates. ### API Handling Improvements * Removed redundant token refresh logic from `Invoke-GitHubAPI` and centralized it in the context resolution process for better maintainability. ### Configuration and Type Adjustments * Changed `AccessTokenGracePeriodInHours` from `int` to `double` for increased precision in token expiration calculations. * Updated time-related calculations to use `[datetime]::Now` instead of `Get-Date` for consistency across the module. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: MariusStorhaug <[email protected]> Co-authored-by: Marius Storhaug <[email protected]>
1 parent 78ff374 commit bb97530

File tree

10 files changed

+93
-87
lines changed

10 files changed

+93
-87
lines changed

‎src/classes/public/Config/GitHubConfig.ps1‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
[string] $ID
44

55
# The access token grace period in hours.
6-
[System.Nullable[int]] $AccessTokenGracePeriodInHours
6+
[System.Nullable[double]] $AccessTokenGracePeriodInHours
77

88
# The default context.
99
[string] $DefaultContext

‎src/classes/public/Context/GitHubContext/AppGitHubContext.ps1‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22
# Client ID for GitHub Apps
33
[string] $ClientID
44

5+
# The private key for the app.
6+
[securestring] $PrivateKey
7+
58
# Owner of the GitHub App
69
[string] $OwnerName
710

@@ -36,6 +39,7 @@
3639
$this.HttpVersion = $Object.HttpVersion
3740
$this.PerPage = $Object.PerPage
3841
$this.ClientID = $Object.ClientID
42+
$this.PrivateKey = $Object.PrivateKey
3943
$this.OwnerName = $Object.OwnerName
4044
$this.OwnerType = $Object.OwnerType
4145
$this.Permissions = $Object.Permissions

‎src/functions/private/Auth/Context/Resolve-GitHubContext.ps1‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,24 @@
5656
if ($Context -is [string]) {
5757
$contextName = $Context
5858
Write-Verbose "Getting context: [$contextName]"
59-
return Get-GitHubContext -Context $contextName
59+
$Context = Get-GitHubContext -Context $contextName
6060
}
6161

6262
if ($null -eq $Context) {
6363
Write-Verbose 'Context is null, returning default context.'
64-
return Get-GitHubContext
64+
$Context = Get-GitHubContext
65+
}
66+
67+
switch ($Context.TokenType) {
68+
'ghu' {
69+
Write-Verbose 'Using GitHub User Access Token.'
70+
$Context = Update-GitHubUserAccessToken -Context $Context -PassThru
71+
}
72+
'PEM' {
73+
Write-Verbose 'Using GitHub App PEM Token.'
74+
$jwt = Get-GitHubAppJSONWebToken -ClientId $Context.ClientID -PrivateKey $Context.PrivateKey
75+
$Context.Token = $jwt.Token
76+
}
6577
}
6678

6779
# TODO: Implement App installation context resolution

‎src/functions/private/Auth/DeviceFlow/Test-GitHubAccessTokenRefreshRequired.ps1‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@
2727

2828
process {
2929
$tokenExpirationDate = $Context.TokenExpirationDate
30-
$currentDateTime = Get-Date
31-
$remainingDuration = [datetime]$tokenExpirationDate - $currentDateTime
32-
$remainingDuration.TotalHours -lt $script:GitHub.Config.AccessTokenGracePeriodInHours
30+
$remainingDuration = [datetime]$tokenExpirationDate - [datetime]::Now
31+
$updateToken = $remainingDuration.TotalHours -lt $script:GitHub.Config.AccessTokenGracePeriodInHours
32+
$updateToken
3333
}
3434

3535
end {

‎src/functions/private/Auth/DeviceFlow/Update-GitHubUserAccessToken.ps1‎

Lines changed: 61 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
[Refreshing user access tokens](https://docs.github.com/apps/creating-github-apps/authenticating-with-a-github-app/refreshing-user-access-tokens)
2121
#>
2222
[CmdletBinding(SupportsShouldProcess)]
23-
[OutputType([securestring])]
23+
[OutputType([GitHubContext])]
2424
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingWriteHost', '', Justification = 'Is the CLI part of the module.')]
2525
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidUsingConvertToSecureStringWithPlainText', '', Justification = 'The tokens are recieved as clear text. Mitigating exposure by removing variables and performing garbage collection.')]
2626
[Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSAvoidLongLines', '', Justification = 'Reason for suppressing')]
@@ -32,7 +32,15 @@
3232

3333
# Return the new access token.
3434
[Parameter()]
35-
[switch] $PassThru
35+
[switch] $PassThru,
36+
37+
# Suppress output messages.
38+
[Parameter()]
39+
[switch] $Silent,
40+
41+
# Timeout in milliseconds for waiting on mutex. Default is 30 seconds.
42+
[Parameter()]
43+
[int] $TimeoutMs = 30000
3644
)
3745

3846
begin {
@@ -42,56 +50,63 @@
4250
}
4351

4452
process {
45-
Write-Verbose "Reusing previously stored ClientID: [$($Context.AuthClientID)]"
46-
$authClientID = $Context.AuthClientID
47-
$accessTokenValidity = [datetime]($Context.TokenExpirationDate) - (Get-Date)
48-
$accessTokenIsValid = $accessTokenValidity.Seconds -gt 0
49-
$hours = $accessTokenValidity.Hours.ToString().PadLeft(2, '0')
50-
$minutes = $accessTokenValidity.Minutes.ToString().PadLeft(2, '0')
51-
$seconds = $accessTokenValidity.Seconds.ToString().PadLeft(2, '0')
52-
$accessTokenValidityText = "$hours`:$minutes`:$seconds"
53-
if ($accessTokenIsValid) {
54-
if ($accessTokenValidity.TotalHours -gt $script:GitHub.Config.AccessTokenGracePeriodInHours) {
55-
if (-not $Silent) {
56-
Write-Host '✓ ' -ForegroundColor Green -NoNewline
57-
Write-Host "Access token is still valid for $accessTokenValidityText ..."
58-
}
59-
return
60-
} else {
61-
if (-not $Silent) {
62-
Write-Host 'âš  ' -ForegroundColor Yellow -NoNewline
63-
Write-Host "Access token remaining validity $accessTokenValidityText. Refreshing access token..."
53+
if (Test-GitHubAccessTokenRefreshRequired -Context $Context) {
54+
$lockName = "PSModule.GitHub/$($Context.ID)"
55+
$lock = $null
56+
try {
57+
$lock = [System.Threading.Mutex]::new($false, $lockName)
58+
$updateToken = $lock.WaitOne(0)
59+
60+
if ($updateToken) {
61+
try {
62+
$refreshTokenValidity = [datetime]($Context.RefreshTokenExpirationDate) - [datetime]::Now
63+
$refreshTokenIsValid = $refreshTokenValidity.TotalSeconds -gt 0
64+
if ($refreshTokenIsValid) {
65+
if (-not $Silent) {
66+
Write-Host 'âš  ' -ForegroundColor Yellow -NoNewline
67+
Write-Host 'Access token expired. Refreshing access token...'
68+
}
69+
$tokenResponse = Invoke-GitHubDeviceFlowLogin -ClientID $Context.AuthClientID -RefreshToken $Context.RefreshToken -HostName $Context.HostName
70+
} else {
71+
Write-Verbose "Using $($Context.DeviceFlowType) authentication..."
72+
$tokenResponse = Invoke-GitHubDeviceFlowLogin -ClientID $Context.AuthClientID -HostName $Context.HostName
73+
}
74+
$Context.Token = ConvertTo-SecureString -AsPlainText $tokenResponse.access_token
75+
$Context.TokenExpirationDate = (Get-Date).AddSeconds($tokenResponse.expires_in)
76+
$Context.TokenType = $tokenResponse.access_token -replace $script:GitHub.TokenPrefixPattern
77+
$Context.RefreshToken = ConvertTo-SecureString -AsPlainText $tokenResponse.refresh_token
78+
$Context.RefreshTokenExpirationDate = (Get-Date).AddSeconds($tokenResponse.refresh_token_expires_in)
79+
80+
if ($PSCmdlet.ShouldProcess('Access token', 'Update/refresh')) {
81+
Set-Context -Context $Context -Vault $script:GitHub.ContextVault
82+
}
83+
} finally {
84+
$lock.ReleaseMutex()
85+
}
86+
} else {
87+
Write-Verbose "Access token is not valid. Waiting for mutex to be released (timeout: $($TimeoutMs)ms)..."
88+
try {
89+
if ($lock.WaitOne($TimeoutMs)) {
90+
# Re-read context to get updated token from other process
91+
$Context = Resolve-GitHubContext -Context $Context.ID
92+
$lock.ReleaseMutex()
93+
} else {
94+
Write-Warning 'Timeout waiting for token update. Proceeding with current token state.'
95+
}
96+
} catch [System.Threading.AbandonedMutexException] {
97+
Write-Debug 'Mutex was abandoned by another process. Re-checking token state...'
98+
$Context = Get-Context -Context $Context.ID -Vault $script:GitHub.ContextVault
99+
}
64100
}
65-
$tokenResponse = Invoke-GitHubDeviceFlowLogin -ClientID $authClientID -RefreshToken ($Context.RefreshToken) -HostName $Context.HostName
66-
}
67-
} else {
68-
$refreshTokenValidity = [datetime]($Context.RefreshTokenExpirationDate) - (Get-Date)
69-
$refreshTokenIsValid = $refreshTokenValidity.Seconds -gt 0
70-
if ($refreshTokenIsValid) {
71-
if (-not $Silent) {
72-
Write-Host 'âš  ' -ForegroundColor Yellow -NoNewline
73-
Write-Host 'Access token expired. Refreshing access token...'
101+
} finally {
102+
if ($lock) {
103+
$lock.Dispose()
74104
}
75-
$tokenResponse = Invoke-GitHubDeviceFlowLogin -ClientID $authClientID -RefreshToken ($Context.RefreshToken) -HostName $Context.HostName
76-
} else {
77-
Write-Verbose "Using $Mode authentication..."
78-
$tokenResponse = Invoke-GitHubDeviceFlowLogin -ClientID $authClientID -Scope $Scope -HostName $Context.HostName
79105
}
80106
}
81-
$Context.Token = ConvertTo-SecureString -AsPlainText $tokenResponse.access_token
82-
$Context.TokenExpirationDate = (Get-Date).AddSeconds($tokenResponse.expires_in)
83-
$Context.TokenType = $tokenResponse.access_token -replace $script:GitHub.TokenPrefixPattern
84-
$Context.RefreshToken = ConvertTo-SecureString -AsPlainText $tokenResponse.refresh_token
85-
$Context.RefreshTokenExpirationDate = (Get-Date).AddSeconds($tokenResponse.refresh_token_expires_in)
86-
87-
if ($PSCmdlet.ShouldProcess('Access token', 'Update/refresh')) {
88-
Set-Context -Context $Context -Vault $script:GitHub.ContextVault
89-
}
90-
91107
if ($PassThru) {
92-
$Context.Token
108+
return $Context
93109
}
94-
95110
}
96111

97112
end {

‎src/functions/public/API/Invoke-GitHubAPI.ps1‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ function Invoke-GitHubAPI {
120120

121121
process {
122122
$Token = $Context.Token
123-
124123
$HttpVersion = Resolve-GitHubContextSetting -Name 'HttpVersion' -Value $HttpVersion -Context $Context
125124
$ApiBaseUri = Resolve-GitHubContextSetting -Name 'ApiBaseUri' -Value $ApiBaseUri -Context $Context
126125
$ApiVersion = Resolve-GitHubContextSetting -Name 'ApiVersion' -Value $ApiVersion -Context $Context
@@ -136,18 +135,6 @@ function Invoke-GitHubAPI {
136135
TokenType = $TokenType
137136
} | Format-List | Out-String -Stream | ForEach-Object { Write-Debug $_ }
138137
}
139-
$jwt = $null
140-
switch ($TokenType) {
141-
'ghu' {
142-
if (Test-GitHubAccessTokenRefreshRequired -Context $Context) {
143-
$Token = Update-GitHubUserAccessToken -Context $Context -PassThru
144-
}
145-
}
146-
'PEM' {
147-
$jwt = Get-GitHubAppJSONWebToken -ClientId $Context.ClientID -PrivateKey $Context.Token
148-
$Token = $jwt.Token
149-
}
150-
}
151138

152139
$headers = @{
153140
Accept = $Accept
@@ -209,20 +196,6 @@ function Invoke-GitHubAPI {
209196
Write-Debug '----------------------------------'
210197
}
211198
do {
212-
switch ($TokenType) {
213-
'ghu' {
214-
if (Test-GitHubAccessTokenRefreshRequired -Context $Context) {
215-
$Token = Update-GitHubUserAccessToken -Context $Context -PassThru
216-
}
217-
}
218-
'PEM' {
219-
if ($jwt.ExpiresAt -lt (Get-Date)) {
220-
$jwt = Get-GitHubAppJSONWebToken -ClientId $Context.ClientID -PrivateKey $Context.Token
221-
$Token = $jwt.Token
222-
$APICall['Token'] = $Token
223-
}
224-
}
225-
}
226199
$response = Invoke-WebRequest @APICall -ProgressAction 'SilentlyContinue' -Debug:$false -Verbose:$false
227200

228201
$headers = @{}

‎src/functions/public/Auth/Connect-GitHubAccount.ps1‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@
9898
Mandatory,
9999
ParameterSetName = 'App'
100100
)]
101-
[string] $PrivateKey,
101+
[object] $PrivateKey,
102102

103103
# Automatically load installations for the GitHub App.
104104
[Parameter(
@@ -246,10 +246,13 @@
246246
}
247247
'App' {
248248
Write-Verbose 'Logging in as a GitHub App...'
249+
if (-not($PrivateKey -is [System.Security.SecureString])) {
250+
$PrivateKey = $PrivateKey | ConvertTo-SecureString -AsPlainText
251+
}
249252
$context += @{
250-
Token = ConvertTo-SecureString -AsPlainText $PrivateKey
251-
TokenType = 'PEM'
252-
ClientID = $ClientID
253+
PrivateKey = $PrivateKey
254+
TokenType = 'PEM'
255+
ClientID = $ClientID
253256
}
254257
}
255258
'PAT' {

‎src/functions/public/Auth/Connect-GitHubApp.ps1‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@
8585
}
8686

8787
process {
88-
8988
$installations = Get-GitHubAppInstallation -Context $Context
9089
$selectedInstallations = @()
9190
Write-Verbose "Found [$($installations.Count)] installations."

‎src/types/GitHubContext.Types.ps1xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
<Name>Remaining</Name>
88
<GetScriptBlock>
99
if ($null -eq $this.TokenExpirationDate) { return }
10-
New-TimeSpan -Start (Get-Date) -End $this.TokenExpirationDate
10+
New-TimeSpan -Start ([datetime]::Now) -End $this.TokenExpirationDate
1111
</GetScriptBlock>
1212
</ScriptProperty>
1313
</Members>
@@ -19,7 +19,7 @@
1919
<Name>Remaining</Name>
2020
<GetScriptBlock>
2121
if ($null -eq $this.TokenExpirationDate) { return }
22-
New-TimeSpan -Start (Get-Date) -End $this.TokenExpirationDate
22+
New-TimeSpan -Start ([datetime]::Now) -End $this.TokenExpirationDate
2323
</GetScriptBlock>
2424
</ScriptProperty>
2525
</Members>

‎src/variables/private/GitHub.ps1‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ $script:GitHub = [pscustomobject]@{
99
ID = 'Module'
1010
HostName = ($env:GITHUB_SERVER_URL ?? 'github.com') -replace '^https?://'
1111
ApiBaseUri = "https://api.$(($env:GITHUB_SERVER_URL ?? 'github.com') -replace '^https?://')"
12-
AccessTokenGracePeriodInHours = 4
12+
AccessTokenGracePeriodInHours = 4.0
1313
GitHubAppClientID = 'Iv1.f26b61bc99e69405'
1414
OAuthAppClientID = '7204ae9b0580f2cb8288'
1515
DefaultContext = ''

0 commit comments

Comments
 (0)