Follow up 944: authentication sessions are not persistent (#1003)

* #944: Fixed auth. sessions not persistent

* 944: Prevent race conditions setting authOptions

* typo correction, duplicate identifier

* prevent block of auth client service on setOptions

* consider windows cred. mgr. password len limit

Author: davegarthsimpson

Diff for: arduino-ide-extension/src/browser/auth/authentication-client-service.ts

@@ -43,15 +43,14 @@ export class AuthenticationClientService
 
   readonly onSessionDidChange = this.onSessionDidChangeEmitter.event;
 
-  onStart(): void {
+  async onStart(): Promise<void> {
     this.toDispose.push(this.onSessionDidChangeEmitter);
     this.service.setClient(this);
     this.service
       .session()
       .then((session) => this.notifySessionDidChange(session));
 
-    this.setOptions();
-    this.service.initAuthSession()
+    this.setOptions().then(() => this.service.initAuthSession());
 
     this.arduinoPreferences.onPreferenceChanged((event) => {
       if (event.preferenceName.startsWith('arduino.auth.')) {
@@ -60,8 +59,8 @@ export class AuthenticationClientService
     });
   }
 
-  setOptions(): void {
-    this.service.setOptions({
+  setOptions(): Promise<void> {
+    return this.service.setOptions({
       redirectUri: `http://localhost:${serverPort}/callback`,
       responseType: 'code',
       clientID: this.arduinoPreferences['arduino.auth.clientID'],

Diff for: arduino-ide-extension/src/common/protocol/authentication-service.ts

@@ -22,7 +22,7 @@ export interface AuthenticationService
   logout(): Promise<void>;
   session(): Promise<AuthenticationSession | undefined>;
   disposeClient(client: AuthenticationServiceClient): void;
-  setOptions(authOptions: AuthOptions): void;
+  setOptions(authOptions: AuthOptions): Promise<void>;
   initAuthSession(): Promise<void>;
 }
 

Diff for: 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<void> {
     this.authOptions = authOptions;
   }
 

Diff for: arduino-ide-extension/src/node/auth/authentication-service-impl.ts

@@ -20,7 +20,7 @@ export class AuthenticationServiceImpl
   protected readonly clients: AuthenticationServiceClient[] = [];
   protected readonly toDispose = new DisposableCollection();
 
-  private initialized = false; 
+  private initialized = false;
 
   async onStart(): Promise<void> {
     this.toDispose.pushAll([
@@ -49,12 +49,12 @@ export class AuthenticationServiceImpl
   async initAuthSession(): Promise<void> {
     if (!this.initialized) {
       await this.delegate.init();
-      this.initialized = true
+      this.initialized = true;
     }
   }
 
-  setOptions(authOptions: AuthOptions) {
-    this.delegate.setOptions(authOptions);
+  setOptions(authOptions: AuthOptions): Promise<void> {
+    return this.delegate.setOptions(authOptions);
   }
 
   async login(): Promise<AuthenticationSession> {

Diff for: 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 for: 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