-
Notifications
You must be signed in to change notification settings - Fork 730
fix(amazonq): fix iam credential update logic to use custom comparator and added buffer time in cred validation #8070
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
Changes from 1 commit
cca4e38
b64cf2b
1cd2f61
c634328
7b4b8e4
02d4848
e49ec58
9614843
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,7 @@ import * as crypto from 'crypto' | |
| import { LanguageClient } from 'vscode-languageclient' | ||
| import { AuthUtil } from 'aws-core-vscode/codewhisperer' | ||
| import { Writable } from 'stream' | ||
| import { onceChanged } from 'aws-core-vscode/utils' | ||
| import { onceChanged, onceChangedWithComparator } from 'aws-core-vscode/utils' | ||
| import { getLogger, oneMinute, isSageMaker } from 'aws-core-vscode/shared' | ||
| import { isSsoConnection, isIamConnection } from 'aws-core-vscode/auth' | ||
|
|
||
|
|
@@ -108,7 +108,21 @@ export class AmazonQLspAuth { | |
| this.client.info(`UpdateBearerToken: ${JSON.stringify(request)}`) | ||
| } | ||
|
|
||
| public updateIamCredentials = onceChanged(this._updateIamCredentials.bind(this)) | ||
| private areCredentialsEqual(creds1: any, creds2: any): boolean { | ||
| if (!creds1 && !creds2) return true | ||
|
||
| if (!creds1 || !creds2) return false | ||
|
|
||
| return ( | ||
| creds1.accessKeyId === creds2.accessKeyId && | ||
| creds1.secretAccessKey === creds2.secretAccessKey && | ||
| creds1.sessionToken === creds2.sessionToken | ||
| ) | ||
| } | ||
|
|
||
| public updateIamCredentials = onceChangedWithComparator( | ||
| this._updateIamCredentials.bind(this), | ||
| ([prevCreds], [currentCreds]) => this.areCredentialsEqual(prevCreds, currentCreds) | ||
| ) | ||
| private async _updateIamCredentials(credentials: any) { | ||
| getLogger().info( | ||
| `[SageMaker Debug] Updating IAM credentials - credentials received: ${credentials ? 'YES' : 'NO'}` | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -862,6 +862,7 @@ export class Auth implements AuthService, ConnectionManager { | |
|
|
||
| private async createCachedCredentials(provider: CredentialsProvider) { | ||
| const providerId = provider.getCredentialsId() | ||
| getLogger().debug(`credentials: create cache credentials for ${provider.getProviderType()}`) | ||
| globals.loginManager.store.invalidateCredentials(providerId) | ||
| const { credentials, endpointUrl } = await globals.loginManager.store.upsertCredentials(providerId, provider) | ||
| await globals.loginManager.validateCredentials(credentials, endpointUrl, provider.getDefaultRegion()) | ||
|
|
@@ -874,6 +875,7 @@ export class Auth implements AuthService, ConnectionManager { | |
| if (creds !== undefined && creds.credentialsHashCode === provider.getHashCode()) { | ||
| return creds.credentials | ||
| } | ||
| return undefined | ||
|
||
| } | ||
|
|
||
| private readonly getToken = keyedDebounce(this._getToken.bind(this)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ import globals from '../../shared/extensionGlobals' | |
| import { getLogger } from '../../shared/logger/logger' | ||
| import { asString, CredentialsProvider, CredentialsId } from '../providers/credentials' | ||
| import { CredentialsProviderManager } from '../providers/credentialsProviderManager' | ||
| // import { get } from 'lodash' | ||
|
||
|
|
||
| export interface CachedCredentials { | ||
| credentials: AWS.Credentials | ||
|
|
@@ -31,11 +32,17 @@ export class CredentialsStore { | |
| * If the expiration property does not exist, it is assumed to never expire. | ||
| */ | ||
| public isValid(key: string): boolean { | ||
| // Apply 60-second buffer similar to SSO token expiry logic | ||
| const expirationBufferMs = 60000 | ||
|
|
||
| if (this.credentialsCache[key]) { | ||
| const expiration = this.credentialsCache[key].credentials.expiration | ||
| return expiration !== undefined ? expiration >= new globals.clock.Date() : true | ||
| const now = new globals.clock.Date() | ||
| const bufferedNow = new globals.clock.Date(now.getTime() + expirationBufferMs) | ||
| const isValid = expiration !== undefined ? expiration >= bufferedNow : true | ||
|
||
| return isValid | ||
| } | ||
|
|
||
| getLogger().debug(`credentials: no credentials found for ${key}`) | ||
| return false | ||
| } | ||
|
|
||
|
|
||
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.
nit: move to util as well.
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.
Ack, let me check if it can be moved to auth related util
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.
fixed in new commit