Conversation
Closes: SDK-5713
test/commands/logout.test.js
Outdated
| assert.isTrue(mockTokenCache.get.called); | ||
| }); | ||
|
|
||
| it('should successfully logout and clear cache when revoke succeeds', async function () { |
There was a problem hiding this comment.
Shouldnt we add atleast one test case scenario covering revoke failure (retry/abort/clear)?
src/commands/logout.js
Outdated
| ); | ||
| } | ||
|
|
||
| if (environment.authMethod !== 'oauth20') { |
There was a problem hiding this comment.
I thought it was suppose to be just for box official app. I'll change it
src/commands/logout.js
Outdated
| try { | ||
| await sdk.revokeTokens(accessToken); | ||
| revoked = true; | ||
| } catch (error) { |
There was a problem hiding this comment.
How you are handling the 401 status, where an access token is already invalid, but refresh token is?
Maybe we should handle it in a different manner, like just displaying dedicated text and ask user (or even not) to proceed
There was a problem hiding this comment.
It is actually 400 error, but I'll add special handling for this case
docs/logout.md
Outdated
| `box logout` | ||
| =========== | ||
|
|
||
| Sign out and clear local authentication state. Revokes the current session's access token on Box servers and removes cached tokens from the local machine. Only supported for OAuth environments. Prompts for confirmation unless `--force` is used. |
There was a problem hiding this comment.
How about explicitly mentioning here (and in command description) that after this call, user will have to authorize again to use an cli?
There was a problem hiding this comment.
It depends on the auth type. For JWT and CCG it won't be necessary
src/commands/logout.js
Outdated
| }, | ||
| ]); | ||
|
|
||
| if (action === 'clear') { |
There was a problem hiding this comment.
How about switch case here?
There was a problem hiding this comment.
I'll rework this part of the code a bit
| @@ -0,0 +1,191 @@ | |||
| 'use strict'; | |||
There was a problem hiding this comment.
I believe that we have are using another test library from oclif for all other commands, can we use the same here or at least have one test by that lib?
4 participants
No description provided.