Aligned command with tailscale login flow #428
Conversation
[ci skip]
[ci skip]
aryanjassal
changed the title
| import * as binUtils from '../utils/index.js'; | ||
| import * as binOptions from '../utils/options.js'; | ||
| import * as errors from '../errors.js'; | ||
|
|
||
| class CommandLogin extends CommandPolykey { | ||
| constructor(...args: ConstructorParameters<typeof CommandPolykey>) { | ||
| super(...args); | ||
| this.name('login'); | ||
| this.description('Login to a platform with Polykey identity'); |
There was a problem hiding this comment.
This should be `Login to a platform with your Polykey identity', just for user clarity.
| const targetURL = new URL( | ||
| url.endsWith('/') ? url.slice(0, url.length) : url, | ||
| ); | ||
| const subPath: string = options.returnURLPath ?? '/oauth2/oidc'; |
There was a problem hiding this comment.
I don't think this check is needed anymore, because the command only expects the url now. So this can just be
const subPath = '/oauth2/oidc. Or is this for future cases, where the user should also provide the specific endpoint as well?
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ token: compactHeader }), | ||
| }); | ||
| process.stderr.write('Opening URL in browser...\n'); |
There was a problem hiding this comment.
This is all client facing code as the user will be reading this of course. So I think for clarity this should be changed to
Attempting to open this URL in your browser... just so the flow of the command is easier to understand for the user.
| @@ -13,7 +13,7 @@ | |||
| "allowSyntheticDefaultImports": true, | |||
| "resolveJsonModule": true, | |||
| "moduleResolution": "NodeNext", | |||
| "module": "ESNext", | |||
| "module": "NodeNext", | |||
There was a problem hiding this comment.
I think I mentioned this already in MatrixAI/Polykey#912 (review), but yeah I believe it should stay ESNext.
Description
Issues Fixed
polykey auth logincommand #408 (REF ENG-620)Tasks
Final checklist