From ff13922e78f344aeaece9263e63d01ae10f11ef2 Mon Sep 17 00:00:00 2001 From: David Simpson <45690499+davegarthsimpson@users.noreply.github.com> Date: Wed, 18 May 2022 16:33:44 +0200 Subject: [PATCH 1/5] #944: Fixed auth. sessions not persistent --- .../src/browser/auth/authentication-client-service.ts | 3 +++ .../src/common/protocol/authentication-service.ts | 1 + .../src/node/auth/authentication-service-impl.ts | 10 +++++++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/arduino-ide-extension/src/browser/auth/authentication-client-service.ts b/arduino-ide-extension/src/browser/auth/authentication-client-service.ts index 2b0fb9cdf..3990ecd56 100644 --- a/arduino-ide-extension/src/browser/auth/authentication-client-service.ts +++ b/arduino-ide-extension/src/browser/auth/authentication-client-service.ts @@ -49,7 +49,10 @@ export class AuthenticationClientService this.service .session() .then((session) => this.notifySessionDidChange(session)); + this.setOptions(); + this.service.initAuthSession() + this.arduinoPreferences.onPreferenceChanged((event) => { if (event.preferenceName.startsWith('arduino.auth.')) { this.setOptions(); diff --git a/arduino-ide-extension/src/common/protocol/authentication-service.ts b/arduino-ide-extension/src/common/protocol/authentication-service.ts index 227427793..df9662ffe 100644 --- a/arduino-ide-extension/src/common/protocol/authentication-service.ts +++ b/arduino-ide-extension/src/common/protocol/authentication-service.ts @@ -23,6 +23,7 @@ export interface AuthenticationService session(): Promise; disposeClient(client: AuthenticationServiceClient): void; setOptions(authOptions: AuthOptions): void; + initAuthSession(): Promise; } export interface AuthenticationServiceClient { diff --git a/arduino-ide-extension/src/node/auth/authentication-service-impl.ts b/arduino-ide-extension/src/node/auth/authentication-service-impl.ts index c0ec42c8c..73906cee4 100644 --- a/arduino-ide-extension/src/node/auth/authentication-service-impl.ts +++ b/arduino-ide-extension/src/node/auth/authentication-service-impl.ts @@ -19,6 +19,8 @@ export class AuthenticationServiceImpl protected readonly delegate = new ArduinoAuthenticationProvider(); protected readonly clients: AuthenticationServiceClient[] = []; protected readonly toDispose = new DisposableCollection(); + + private initialized = false; async onStart(): Promise { this.toDispose.pushAll([ @@ -42,7 +44,13 @@ export class AuthenticationServiceImpl this.clients.forEach((client) => this.disposeClient(client)) ), ]); - await this.delegate.init(); + } + + async initAuthSession(): Promise { + if (!this.initialized) { + await this.delegate.init(); + this.initialized = true + } } setOptions(authOptions: AuthOptions) { From 0a7dbccc202a5727e86a03b5f6cf144268982c37 Mon Sep 17 00:00:00 2001 From: David Simpson <45690499+davegarthsimpson@users.noreply.github.com> Date: Tue, 24 May 2022 09:59:44 +0200 Subject: [PATCH 2/5] 944: Prevent race conditions setting authOptions --- .../src/browser/auth/authentication-client-service.ts | 10 +++++----- .../src/common/protocol/authentication-service.ts | 2 +- .../src/node/auth/arduino-auth-provider.ts | 2 +- .../src/node/auth/authentication-service-impl.ts | 10 +++++----- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/arduino-ide-extension/src/browser/auth/authentication-client-service.ts b/arduino-ide-extension/src/browser/auth/authentication-client-service.ts index 3990ecd56..4dfa8252a 100644 --- a/arduino-ide-extension/src/browser/auth/authentication-client-service.ts +++ b/arduino-ide-extension/src/browser/auth/authentication-client-service.ts @@ -43,15 +43,15 @@ export class AuthenticationClientService readonly onSessionDidChange = this.onSessionDidChangeEmitter.event; - onStart(): void { + async onStart(): Promise { this.toDispose.push(this.onSessionDidChangeEmitter); this.service.setClient(this); this.service .session() .then((session) => this.notifySessionDidChange(session)); - this.setOptions(); - this.service.initAuthSession() + await this.setOptions(); + this.service.initAuthSession(); this.arduinoPreferences.onPreferenceChanged((event) => { if (event.preferenceName.startsWith('arduino.auth.')) { @@ -60,8 +60,8 @@ export class AuthenticationClientService }); } - setOptions(): void { - this.service.setOptions({ + setOptions(): Promise { + return this.service.setOptions({ redirectUri: `http://localhost:${serverPort}/callback`, responseType: 'code', clientID: this.arduinoPreferences['arduino.auth.clientID'], diff --git a/arduino-ide-extension/src/common/protocol/authentication-service.ts b/arduino-ide-extension/src/common/protocol/authentication-service.ts index df9662ffe..cb2c87e74 100644 --- a/arduino-ide-extension/src/common/protocol/authentication-service.ts +++ b/arduino-ide-extension/src/common/protocol/authentication-service.ts @@ -22,7 +22,7 @@ export interface AuthenticationService logout(): Promise; session(): Promise; disposeClient(client: AuthenticationServiceClient): void; - setOptions(authOptions: AuthOptions): void; + setOptions(authOptions: AuthOptions): Promise; initAuthSession(): Promise; } diff --git a/arduino-ide-extension/src/node/auth/arduino-auth-provider.ts b/arduino-ide-extension/src/node/auth/arduino-auth-provider.ts index 652da1cd0..85fb37e0a 100644 --- a/arduino-ide-extension/src/node/auth/arduino-auth-provider.ts +++ b/arduino-ide-extension/src/node/auth/arduino-auth-provider.ts @@ -89,7 +89,7 @@ export class ArduinoAuthenticationProvider implements AuthenticationProvider { setInterval(checkToken, REFRESH_INTERVAL); } - public setOptions(authOptions: AuthOptions) { + public async setOptions(authOptions: AuthOptions): Promise { this.authOptions = authOptions; } diff --git a/arduino-ide-extension/src/node/auth/authentication-service-impl.ts b/arduino-ide-extension/src/node/auth/authentication-service-impl.ts index 73906cee4..d79be4ac6 100644 --- a/arduino-ide-extension/src/node/auth/authentication-service-impl.ts +++ b/arduino-ide-extension/src/node/auth/authentication-service-impl.ts @@ -19,8 +19,8 @@ export class AuthenticationServiceImpl protected readonly delegate = new ArduinoAuthenticationProvider(); protected readonly clients: AuthenticationServiceClient[] = []; protected readonly toDispose = new DisposableCollection(); - - private initialized = false; + + private initialized = false; async onStart(): Promise { this.toDispose.pushAll([ @@ -49,12 +49,12 @@ export class AuthenticationServiceImpl async initAuthSession(): Promise { if (!this.initialized) { await this.delegate.init(); - this.initialized = true + this.initialized = true; } } - setOptions(authOptions: AuthOptions) { - this.delegate.setOptions(authOptions); + setOptions(authOptions: AuthOptions): Promise { + return this.delegate.setOptions(authOptions); } async login(): Promise { From 9746e9469f3546ad5fbdec2c17c6c64fa1f67bb3 Mon Sep 17 00:00:00 2001 From: David Simpson <45690499+davegarthsimpson@users.noreply.github.com> Date: Tue, 24 May 2022 10:42:39 +0200 Subject: [PATCH 3/5] typo correction, duplicate identifier --- .../src/node/auth/authentication-service-impl.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/arduino-ide-extension/src/node/auth/authentication-service-impl.ts b/arduino-ide-extension/src/node/auth/authentication-service-impl.ts index d65e15552..91944f903 100644 --- a/arduino-ide-extension/src/node/auth/authentication-service-impl.ts +++ b/arduino-ide-extension/src/node/auth/authentication-service-impl.ts @@ -20,8 +20,6 @@ export class AuthenticationServiceImpl protected readonly clients: AuthenticationServiceClient[] = []; protected readonly toDispose = new DisposableCollection(); - private initialized = false; - private initialized = false; async onStart(): Promise { From 15bb3b519319c1943a8c0d00ad74b8223387c064 Mon Sep 17 00:00:00 2001 From: David Simpson <45690499+davegarthsimpson@users.noreply.github.com> Date: Tue, 24 May 2022 10:51:18 +0200 Subject: [PATCH 4/5] prevent block of auth client service on setOptions --- .../src/browser/auth/authentication-client-service.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/arduino-ide-extension/src/browser/auth/authentication-client-service.ts b/arduino-ide-extension/src/browser/auth/authentication-client-service.ts index 4dfa8252a..696c41ef8 100644 --- a/arduino-ide-extension/src/browser/auth/authentication-client-service.ts +++ b/arduino-ide-extension/src/browser/auth/authentication-client-service.ts @@ -50,8 +50,7 @@ export class AuthenticationClientService .session() .then((session) => this.notifySessionDidChange(session)); - await this.setOptions(); - this.service.initAuthSession(); + this.setOptions().then(() => this.service.initAuthSession()); this.arduinoPreferences.onPreferenceChanged((event) => { if (event.preferenceName.startsWith('arduino.auth.')) { From 8e9e5ad5ac1956dee22173e96e5610473c10a2ed Mon Sep 17 00:00:00 2001 From: David Simpson <45690499+davegarthsimpson@users.noreply.github.com> Date: Mon, 6 Jun 2022 16:46:18 +0200 Subject: [PATCH 5/5] consider windows cred. mgr. password len limit --- arduino-ide-extension/src/node/auth/keychain.ts | 9 +++++++++ arduino-ide-extension/src/node/auth/utils.ts | 10 +++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/arduino-ide-extension/src/node/auth/keychain.ts b/arduino-ide-extension/src/node/auth/keychain.ts index c5835e2e8..7b7d8c743 100644 --- a/arduino-ide-extension/src/node/auth/keychain.ts +++ b/arduino-ide-extension/src/node/auth/keychain.ts @@ -47,6 +47,15 @@ export class Keychain { return false; } try { + const stringifiedTokenLength = stringifiedToken.length; + const tokenLengthNotSupported = + stringifiedTokenLength > 2500 && process.platform === 'win32'; + + if (tokenLengthNotSupported) { + // TODO manage this specific error appropriately + return false; + } + await keytar.setPassword( this.credentialsSection, this.account, diff --git a/arduino-ide-extension/src/node/auth/utils.ts b/arduino-ide-extension/src/node/auth/utils.ts index 496dc6b87..a7bc36110 100644 --- a/arduino-ide-extension/src/node/auth/utils.ts +++ b/arduino-ide-extension/src/node/auth/utils.ts @@ -44,7 +44,15 @@ export function token2IToken(token: Token): IToken { (token.id_token && jwt_decode(token.id_token)) || {}; return { - idToken: token.id_token, + /* + * ".id_token" is already decoded for account details above + * so we probably don't need to keep it around as "idToken". + * If we do, and subsequently try to store it with + * Windows Credential Manager (WCM) it's probable we'll + * exceed WCMs' 2500 password character limit breaking + * our auth functionality + */ + // ! idToken: token.id_token, expiresIn: token.expires_in, expiresAt: token.expires_in ? Date.now() + token.expires_in * 1000