Skip to content

Aligned command with tailscale login flow #428

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: staging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion npmDepsHash
Original file line number Diff line number Diff line change
@@ -1 +1 @@
sha256-23Yx1RJcaNEL1YmNz/LRFYL+5g++B+aoT0CSSbreAx8=
sha256-XCPXt+ESgufgJBBTvy+TZBjcovFef4pW/E0f4/JaAc0=
134 changes: 134 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 6 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -126,21 +126,21 @@
},
"optionalDependencies": {
"@matrixai/db": "*",
"@matrixai/exec-darwin-universal": "*",
"@matrixai/exec-linux-x64": "*",
"@matrixai/mdns-linux-x64": "*",
"@matrixai/quic-darwin-universal": "*",
"@matrixai/quic-linux-x64": "*",
"@matrixai/quic-win32-x64": "*",
"@matrixai/exec-darwin-universal": "*",
"@matrixai/exec-linux-x64": "*",
"fd-lock": "*",
"sodium-native": "*"
},
"devDependencies": {
"@matrixai/lint": "^0.2.11",
"@fast-check/jest": "^2.1.1",
"@matrixai/errors": "^2.1.3",
"@matrixai/logger": "^4.0.3",
"@matrixai/exec": "^1.0.3",
"@fast-check/jest": "^2.1.1",
"@matrixai/lint": "^0.2.11",
"@matrixai/logger": "^4.0.3",
"@swc/core": "1.3.82",
"@swc/jest": "^0.2.29",
"@types/jest": "^29.5.14",
Expand All @@ -161,6 +161,7 @@
"mocked-env": "^1.3.5",
"nexpect": "^0.6.0",
"node-gyp-build": "^4.8.4",
"open": "^10.1.2",
"polykey": "^2.4.0",
"shelljs": "^0.8.5",
"shx": "^0.3.4",
Expand Down
73 changes: 22 additions & 51 deletions src/auth/CommandLogin.ts
Original file line number Diff line number Diff line change
@@ -1,35 +1,24 @@
import type PolykeyClient from 'polykey/PolykeyClient.js';
import type {
TokenPayloadEncoded,
TokenProtectedHeaderEncoded,
TokenSignatureEncoded,
} from 'polykey/tokens/types.js';
import type { IdentityRequestData } from 'polykey/client/types.js';
import CommandPolykey from '../CommandPolykey.js';
import * as binProcessors from '../utils/processors.js';
import * as binParsers from '../utils/parsers.js';
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');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be `Login to a platform with your Polykey identity', just for user clarity.

this.argument(
'<token>',
'Token provided by platform for logging in',
binParsers.parseCompactJWT,
);
this.argument('<url>', 'The URL to login using Polykey');
this.addOption(binOptions.nodeId);
this.addOption(binOptions.clientHost);
this.addOption(binOptions.clientPort);
this.action(async (encodedToken, options) => {
this.addOption(binOptions.returnURLPath);
this.action(async (url: string, options) => {
const { default: PolykeyClient } = await import(
'polykey/PolykeyClient.js'
);
const tokensUtils = await import('polykey/tokens/utils.js');
const { default: open } = await import('open');
const clientOptions = await binProcessors.processClientOptions(
options.nodePath,
options.nodeId,
Expand Down Expand Up @@ -58,51 +47,33 @@ class CommandLogin extends CommandPolykey {
logger: this.logger.getChild(PolykeyClient.name),
});

// Create a JSON representation of the encoded header
const [protectedHeader, payload, signature] = encodedToken;
const incomingTokenEncoded = {
payload: payload as TokenPayloadEncoded,
signatures: [
{
protected: protectedHeader as TokenProtectedHeaderEncoded,
signature: signature as TokenSignatureEncoded,
},
],
};

// Get it verified and signed by the agent
// Get a signed token by the agent
const response = await binUtils.retryAuthentication(
(auth) =>
pkClient.rpcClient.methods.authSignToken({
metadata: auth,
...incomingTokenEncoded,
}),
pkClient.rpcClient.methods.authIdentityToken({ metadata: auth }),
meta,
);

// Send the returned JWT to the returnURL provided by the initial token
const compactHeader = binUtils.jsonToCompactJWT(response);
const incomingPayload =
tokensUtils.parseTokenPayload<IdentityRequestData>(payload);
let result: Response;
const targetURL = new URL(
url.endsWith('/') ? url.slice(0, url.length) : url,
);
const subPath: string = options.returnURLPath ?? '/oauth2/oidc';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

targetURL.pathname = subPath.startsWith('/') ? subPath : `/${subPath}`;
targetURL.searchParams.append('token', compactHeader);

// Print out the URL to stderr
process.stderr.write(
`Open the following URL in your browser:\n\t${targetURL}\n`,
);

// Try to open the URL in the browser
try {
result = await fetch(incomingPayload.returnURL, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ token: compactHeader }),
});
process.stderr.write('Opening URL in browser...\n');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

await open(targetURL.toString());
} catch (e) {
throw new errors.ErrorPolykeyCLILoginFailed(
'Failed to send token to return url',
{ cause: e },
);
}

// Handle non-200 response
if (!result.ok) {
throw new errors.ErrorPolykeyCLILoginFailed(
`Return url returned failure with code ${result.status}`,
);
process.stderr.write(`Failed to open browser: ${e.message}\n`);
}
} finally {
if (pkClient! != null) await pkClient.stop();
Expand Down
6 changes: 0 additions & 6 deletions src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,11 +201,6 @@ class ErrorPolykeyCLIInvalidJWT<T> extends ErrorPolykeyCLI<T> {
exitCode = sysexits.USAGE;
}

class ErrorPolykeyCLILoginFailed<T> extends ErrorPolykeyCLI<T> {
static description = 'Failed to login using Polykey';
exitCode = sysexits.SOFTWARE;
}

export {
ErrorPolykeyCLI,
ErrorPolykeyCLIUncaughtException,
Expand Down Expand Up @@ -235,5 +230,4 @@ export {
ErrorPolykeyCLIEditSecret,
ErrorPolykeyCLITouchSecret,
ErrorPolykeyCLIInvalidJWT,
ErrorPolykeyCLILoginFailed,
};
6 changes: 6 additions & 0 deletions src/utils/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,11 @@ const preserveNewline = new Option(
})
.default([]);

const returnURLPath = new Option(
'--url-path <path>',
'Which path on the website to send the token to',
);

export {
nodePath,
format,
Expand Down Expand Up @@ -365,4 +370,5 @@ export {
recursive,
parents,
preserveNewline,
returnURLPath,
};
Loading