Fixes according to review. Handle refresh token expiration time separ…

…atly.

Signed-off-by: Mykola Morhun <mmorhun@redhat.com>

package.json

@@ -36,9 +36,8 @@
     "execa": "^4.0.3",
     "fancy-test": "^1.4.9",
     "fs-extra": "^9.0.1",
-    "js-yaml": "^3.14.0",
     "inquirer": "^7.3.3",
-    "jsonwebtoken": "^8.5.1",
+    "js-yaml": "^3.14.0",
     "listr": "^0.14.3",
     "listr-verbose-renderer": "^0.6.0",
     "lodash": "^4.17.20",
@@ -57,7 +56,6 @@
     "@types/chai": "^4",
     "@types/jest": "26.0.14",
     "@types/js-yaml": "^3.12.5",
-    "@types/jsonwebtoken": "^8.5.0",
     "@types/listr": "^0.14.2",
     "@types/node": "^12",
     "@types/node-forge": "^0.9.5",
@@ -100,11 +98,14 @@
       "@oclif/plugin-update"
     ],
     "topics": {
+      "auth": {
+        "description": "Manage Eclipse Che server login sessions"
+      },
       "server": {
-        "description": "control Eclipse Che server"
+        "description": "Control Eclipse Che server"
       },
       "workspace": {
-        "description": "control Che workspaces"
+        "description": "Control Che workspaces"
       }
     },
     "update": {

src/api/che-login-manager.ts

@@ -11,7 +11,6 @@
 import axios, { AxiosInstance } from 'axios'
 import * as fs from 'fs'
 import * as https from 'https'
-import * as jwt from 'jsonwebtoken'
 import * as path from 'path'
 import * as querystring from 'querystring'
 
@@ -28,8 +27,12 @@ export interface LoginData {
 
 // Credentials file format
 export interface CheServerLoginConfig {
+  // Defines file format version
+  version?: string
+  // Define current login session. Empty if none.
   lastLoginUrl?: string
   lastUserName?: string
+  // Registered logins
   logins?: Logins
 }
 
@@ -41,6 +44,8 @@ export type LoginRecord = RefreshTokenLoginRecord | PasswordLoginRecord | OcUser
 
 export interface RefreshTokenLoginRecord {
   refreshToken: string
+  // Expiration datetime (in seconds) for local timezone
+  expires: number
 }
 
 export interface OcUserTokenLoginRecord {
@@ -88,11 +93,11 @@ interface CheKeycloakSettings {
 // Response structure from Keycloak get access token endpoint
 interface KeycloakAuthTokenResponse {
   access_token: string
-  expires_in: number
+  expires_in: number | string
   refresh_token: string
-  refresh_expires_in: number
+  refresh_expires_in?: number | string
   token_type: string
-  scope: string
+  scope?: string
 }
 
 const REQUEST_TIMEOUT_MS = 10000
@@ -194,8 +199,14 @@ export class CheServerLoginManager {
 
     // Check whether provided login credentials valid and get refresh token.
     const keycloakAuthData = await this.keycloakAuth(apiUrl, loginRecord, cheKeycloakSettings)
+    const now = (Date.now() / 1000)
+    let refreshTokenExpiresIn: string | number = keycloakAuthData.refresh_expires_in ? keycloakAuthData.refresh_expires_in : keycloakAuthData.expires_in
+    if (typeof refreshTokenExpiresIn === 'string') {
+      refreshTokenExpiresIn = parseFloat(refreshTokenExpiresIn)
+    }
     const refreshTokenLoginRecord: RefreshTokenLoginRecord = {
       refreshToken: keycloakAuthData.refresh_token,
+      expires: now + refreshTokenExpiresIn
     }
 
     const username = isPasswordLoginData(loginRecord) ? loginRecord.username :
@@ -291,6 +302,11 @@ export class CheServerLoginManager {
     if (!this.loginData.logins) {
       this.loginData.logins = {}
     }
+
+    if (!this.loginData.version) {
+      // So far there is only one existing file format
+      this.loginData.version = 'v1'
+    }
   }
 
   private saveLoginData(): void {
@@ -347,8 +363,7 @@ export class CheServerLoginManager {
     const now = Date.now() / 1000
     for (const [apiUrl, serverLogins] of Object.entries(this.loginData.logins)) {
       for (const [username, loginRecord] of Object.entries(serverLogins)) {
-        const decodedToken = jwt.decode(loginRecord.refreshToken)
-        if (decodedToken && typeof decodedToken !== 'string' && decodedToken.exp && decodedToken.exp <= now) {
+        if (loginRecord.expires <= now) {
           // Token is expired, delete it
           delete serverLogins[username]
         }

src/commands/auth/delete.ts

@@ -22,7 +22,6 @@ export default class Delete extends Command {
     {
       name: CHE_API_ENDPOINT_KEY,
       description: 'Eclipse Che server API endpoint',
-      env: 'CHE_API_ENDPOINT',
       required: true
     }
   ]
@@ -32,33 +31,33 @@ export default class Delete extends Command {
   }
 
   static examples = [
-    '# Delete given user login information for specified cluster:\n' +
-    'auth:delete che-che.apps-crc.testing/api -u username',
-    '\n# Delete all existing logins for specified cluster:\n' +
-    'auth:delete che-che.apps-crc.testing',
+    '# Delete login session of the specified user on the cluster:\n' +
+    'chectl auth:delete che-che.apps-crc.testing/api -u username',
+    '\n\n# Delete all login sessions on the cluster:\n' +
+    'chectl auth:delete che-che.apps-crc.testing',
   ]
 
   async run() {
     const { args, flags } = this.parse(Delete)
 
     let cheApiEndpoint = CheApiClient.normalizeCheApiEndpointUrl(args[CHE_API_ENDPOINT_KEY])
-    let username: string | undefined = flags[USERNAME_KEY]
+    const username: string | undefined = flags[USERNAME_KEY]
 
     const loginManager = await CheServerLoginManager.getInstance(this.config.configDir)
 
     if (!loginManager.hasLoginFor(cheApiEndpoint)) {
       // Maybe /api suffix isn't provided
       const cheApiEndpointGuess = cheApiEndpoint + '/api'
       if (!loginManager.hasLoginFor(cheApiEndpointGuess)) {
-        cli.info(`No registered logins on server ${cheApiEndpoint}`)
+        cli.info(`No registered login sessions on server ${cheApiEndpoint}`)
         return
       }
       cheApiEndpoint = cheApiEndpointGuess
     }
 
     if (username) {
       if (!loginManager.hasLoginFor(cheApiEndpoint, username)) {
-        cli.info(`No existing logins for ${username} on server ${cheApiEndpoint}`)
+        cli.info(`${username} is not logged in on ${cheApiEndpoint}. Nothing to delete.`)
         return
       }
     }
@@ -67,7 +66,7 @@ export default class Delete extends Command {
     if (username) {
       cli.info(`Succesfully logged out ${username} on ${cheApiEndpoint}`)
     } else {
-      cli.info(`Succesfully logged out of ${cheApiEndpoint}`)
+      cli.info(`Succesfully logged out all users on ${cheApiEndpoint}`)
     }
   }
 

src/commands/auth/list.ts

@@ -38,7 +38,7 @@ export default class List extends Command {
         }
       })
     } else {
-      output = 'No registered logins'
+      output = 'There are no login sessions'
     }
 
     cli.info(output)

src/commands/auth/login.ts

@@ -56,13 +56,13 @@ export default class Login extends Command {
 
   static examples = [
     '# Log in with username and password (when OpenShift OAuth is not enabled):\n' +
-    'auth:login https://che-che.apps-crc.testing/api -u username -p password',
-    '\n# Log in with username and password (password will be asked interactively):\n' +
-    'auth:login che-che.apps-crc.testing -u username',
-    '\n# Log in with token (when OpenShift OAuth is enabled):\n' +
-    'auth:login che.openshift.io -t token',
-    '\n# Log in with oc token (when logged into an OpenShift cluster with oc and OpenShift OAuth is enabled):\n' +
-    'auth:login che.my.server.net',
+    'chectl auth:login https://che-che.apps-crc.testing/api -u username -p password',
+    '\n\n# Log in with username and password (password will be asked interactively):\n' +
+    'chectl auth:login che-che.apps-crc.testing -u username',
+    '\n\n# Log in with token (when OpenShift OAuth is enabled):\n' +
+    'chectl auth:login che.openshift.io -t token',
+    '\n\n# Log in with oc token (when logged into an OpenShift cluster with oc and OpenShift OAuth is enabled):\n' +
+    'chectl auth:login che.my.server.net',
   ]
 
   async run() {

src/commands/auth/use.ts

@@ -23,7 +23,6 @@ export default class Use extends Command {
     {
       name: CHE_API_ENDPOINT_KEY,
       description: 'Eclipse Che server API endpoint',
-      env: 'CHE_API_ENDPOINT',
       required: false
     }
   ]
@@ -39,14 +38,14 @@ export default class Use extends Command {
   }
 
   static examples = [
-    '# Make given user on specified cluster current:\n' +
-    'auth:use che-che.apps-crc.testing/api -u username',
-    '# Switch to another user on the same cluster:\n' +
-    'auth:use -u another-user-on-this-server',
-    '# Switch to the user on the given cluster (requires to have only one user logged in the given cluster):\n' +
-    'auth:use my.cluster.net',
-    '# Interactively select current login:\n' +
-    'auth:use -i',
+    '# Set an active login session for the specified user on the given cluster:\n' +
+    'chectl auth:use che-che.apps-crc.testing/api -u username',
+    '\n\n# Switch to another user on the same cluster:\n' +
+    'chectl auth:use -u another-user-on-this-server',
+    '\n\n# Switch to the only user on the given cluster:\n' +
+    'chectl auth:use my.cluster.net',
+    '\n\n# Select active login session in interactive mode:\n' +
+    'chectl auth:use -i',
   ]
 
   async run() {
@@ -87,7 +86,7 @@ export default class Use extends Command {
         // Maybe /api suffix isn't provided
         const cheApiEndpointGuess = cheApiEndpoint + '/api'
         if (!loginManager.hasLoginFor(cheApiEndpointGuess)) {
-          cli.info(`No registered logins on server ${cheApiEndpoint}`)
+          cli.info(`No registered login sessions on server ${cheApiEndpoint}`)
           return
         }
         cheApiEndpoint = cheApiEndpointGuess
@@ -98,7 +97,7 @@ export default class Use extends Command {
       // Check if given server has only one login session to use
       const serverLogins = loginManager.getAllLogins().get(cheApiEndpoint)
       if (!serverLogins || (serverLogins && serverLogins.length < 1)) {
-        cli.info(`No registered logins for ${cheApiEndpoint} server`)
+        cli.info(`No registered login sessions for ${cheApiEndpoint} server`)
         return
       }
       if (serverLogins.length !== 1) {

src/common-flags.ts

@@ -63,7 +63,7 @@ export const cheOperatorCRPatchYaml = string({
 export const USERNAME_KEY = 'username'
 export const username = string({
   char: 'u',
-  description: 'Eclipse Che user name',
+  description: 'Eclipse Che username',
   env: 'CHE_USER_NAME',
   required: false,
 })