Skip to content
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.

Sign up for GitHub

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

polish for cloud social provider SSH use #350

Merged
merged 6 commits into from
Jan 27, 2016
Merged

polish for cloud social provider SSH use #350

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7c15ec9
always reject with objects
trevj Jan 22, 2016
90e9b44
add a timeout to the cloud social provider SSH connection
trevj Jan 22, 2016
df26c23
numerous fixes to ssh2 use, plus some renaming
trevj Jan 22, 2016
fbbd4f6
right args for stream end
trevj Jan 22, 2016
c944fb9
fix log line
trevj Jan 27, 2016
16e2cdc
Merge branch 'master' into trevj-cloud-social-polish
trevj Jan 27, 2016
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
122 changes: 63 additions & 59 deletions src/cloud/social/provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ const STORAGE_KEY = 'cloud-social-contacts';
const ADMIN_USERNAME = 'giver';
const REGULAR_USERNAME = 'getter';

// Timeout for establishing an SSH connection.
const CONNECT_TIMEOUT_MS = 10000;
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like the default is 20000, just curious the reason for shortening it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

20s just seemed like a long time to be sitting there waiting for an error.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 fair enough.


// Credentials for accessing a cloud instance.
// The serialised, base64 form is distributed amongst users.
// TODO: add (private) keys, for key-based auth
Expand Down Expand Up @@ -200,7 +203,7 @@ export class CloudSocialProvider {
if (invite.host in this.clients_) {
log.debug('closing old connection to %1', invite.host);
this.clients_[invite.host].then((connection: Connection) => {
connection.close();
connection.end();
});
}

Expand Down Expand Up @@ -350,7 +353,7 @@ export class CloudSocialProvider {
log.debug('logout');
for (let address in this.clients_) {
this.clients_[address].then((connection: Connection) => {
connection.close();
connection.end();
});
}
return Promise.resolve<void>();
Expand All @@ -367,11 +370,15 @@ export class CloudSocialProvider {
public inviteUser = (clientId: string): Promise<Object> => {
log.debug('inviteUser');
if (!(clientId in this.savedContacts_)) {
return Promise.reject(new Error('unknown cloud instance ' + clientId));
return Promise.reject({
message: 'unknown cloud instance ' + clientId
});
}
if (this.savedContacts_[clientId].invite.user !== ADMIN_USERNAME) {
return Promise.reject(new Error('user is logged in as non-admin user ' +
this.savedContacts_[clientId].invite.user));
return Promise.reject({
message: 'user is logged in as non-admin user ' +
this.savedContacts_[clientId].invite.user
});
}
return this.reconnect_(this.savedContacts_[clientId].invite).then(
(connection: Connection) => {
Expand Down Expand Up @@ -423,10 +430,10 @@ class Connection {

private state_ = ConnectionState.NEW;

private client_ = new Client();
private connection_ = new Client();

// The tunneled connection, i.e. secure link to Zork.
private stream_ :ssh2.Channel;
private tunnel_ :ssh2.Channel;

constructor(
private invite_: Invite,
Expand All @@ -436,14 +443,17 @@ class Connection {
// TODO: timeout
public connect = (): Promise<void> => {
if (this.state_ !== ConnectionState.NEW) {
return Promise.reject(new Error('can only connect in NEW state'));
return Promise.reject({
message: 'can only connect in NEW state'
});
}
this.state_ = ConnectionState.CONNECTING;

let connectConfig: ssh2.ConnectConfig = {
host: this.invite_.host,
port: SSH_SERVER_PORT,
username: this.invite_.user,
readyTimeout: CONNECT_TIMEOUT_MS,
// Remaining fields only for type-correctness.
tryKeyboard: false,
debug: undefined
Expand All @@ -457,20 +467,22 @@ class Connection {
}

return new Promise<void>((F, R) => {
this.client_.on('ready', () => {
this.connection_.on('ready', () => {
// TODO: set a timeout here, too
this.setState_(ConnectionState.ESTABLISHING_TUNNEL);
this.client_.forwardOut(
this.connection_.forwardOut(
// TODO: since we communicate using the stream, what does this mean?
'127.0.0.1', 0,
ZORK_HOST, ZORK_PORT, (e: Error, stream: ssh2.Channel) => {
ZORK_HOST, ZORK_PORT, (e: Error, tunnel: ssh2.Channel) => {
if (e) {
this.close();
R(new Error('error establishing tunnel: ' + e.toString()));
this.end();
R({
message: 'error establishing tunnel: ' + e.message
});
return;
}
this.setState_(ConnectionState.WAITING_FOR_PING);

this.stream_ = stream;
this.setState_(ConnectionState.WAITING_FOR_PING);

var bufferQueue = new queue.Queue<ArrayBuffer, void>();
new linefeeder.LineFeeder(bufferQueue).setSyncHandler((reply: string) => {
Expand All @@ -481,9 +493,10 @@ class Connection {
this.setState_(ConnectionState.ESTABLISHED);
F();
} else {
this.close();
R(new Error('did not receive ping from server on login: ' +
reply));
this.end();
R({
message: 'did not receive ping from server on login: ' + reply
});
}
break;
case ConnectionState.ESTABLISHED:
Expand All @@ -497,50 +510,36 @@ class Connection {
default:
log.warn('%1: did not expect message in state %2: %3',
this.name_, ConnectionState[this.state_], reply);
this.close();
this.end();
}
});

// TODO: add error handler for stream
stream.on('data', (buffer: Buffer) => {
this.tunnel_ = tunnel;
tunnel.on('data', (buffer: Buffer) => {
bufferQueue.handle(arraybuffers.bufferToArrayBuffer(buffer));
}).on('error', (e: Error) => {
// This occurs when:
// - host cannot be reached, e.g. non-existant hostname
// TODO: does this occur outside of startup, i.e. should it always reject?
log.warn('%1: tunnel error: %2', this.name_, e);
this.close();
R(new Error('could not establish tunnel: ' + e.toString()));
}).on('end', () => {
// Occurs when the stream is "over" for any reason, including
// failed connection.
log.debug('%1: tunnel end', this.name_);
this.close();
}).on('close', (hadError: boolean) => {
// TODO: when does this occur? don't see it on normal close or failure
log.debug('%1: tunnel close: %2', this.name_, hadError);
this.close();
log.debug('%1: tunnel close, with%2 error', this.name_, (hadError ? '' : 'out'));
});

stream.write('ping\n');
tunnel.write('ping\n');
});
}).on('error', (e: Error) => {
// This occurs when:
// - user supplies the wrong username or password
// - host cannot be reached, e.g. non-existant hostname
// TODO: does this occur outside of startup, i.e. should it always reject?
log.warn('%1: connection error: %2', this.name_, e);
this.close();
R(new Error('could not login: ' + e.toString()));
this.setState_(ConnectionState.TERMINATED);
R({
message: 'could not login: ' + e.message
});
}).on('end', () => {
// Occurs when the connection is "over" for any reason, including
// failed connection.
log.debug('%1: connection ended', this.name_);
this.close();
log.debug('%1: connection end', this.name_);
this.setState_(ConnectionState.TERMINATED);
}).on('close', (hadError: boolean) => {
// TODO: when does this occur? don't see it on normal close or failure
log.debug('%1: connection close: %2', this.name_, hadError);
this.close();
log.debug('%1: connection close, with%2 error', this.name_, (hadError ? '' : 'out'));
this.setState_(ConnectionState.TERMINATED);
}).connect(connectConfig);
});
}
Expand All @@ -549,17 +548,14 @@ class Connection {
if (this.state_ !== ConnectionState.ESTABLISHED) {
throw new Error('can only connect in ESTABLISHED state');
}
this.stream_.write(s + '\n');
this.tunnel_.write(s + '\n');
}

public close = (): void => {
public end = (): void => {
log.debug('%1: close', this.name_);
if (this.state_ === ConnectionState.TERMINATED) {
log.debug('%1: already closed', this.name_);
} else {
if (this.state_ !== ConnectionState.TERMINATED) {
this.setState_(ConnectionState.TERMINATED);
this.client_.end();
// TODO: what about the stream?
this.connection_.end();
}
}

Expand All @@ -578,21 +574,29 @@ class Connection {
// Executes a command, fulfilling with the command's stdout
// or rejecting if output is received on stderr.
private exec_ = (command:string): Promise<string> => {
log.debug('%1: execute command: %2', this.name_, command);
if (this.state_ !== ConnectionState.ESTABLISHED) {
return Promise.reject(new Error('can only execute commands in ESTABLISHED state'));
}
return new Promise<string>((F, R) => {
this.client_.exec(command, (e: Error, stream: ssh2.Channel) => {
this.connection_.exec(command, (e: Error, stream: ssh2.Channel) => {
if (e) {
R(e);
R({
message: 'failed to execute command: ' + e.message
});
return;
}
stream.on('data', function(data: Buffer) {

// TODO: There is a close event with a return code which
// is probably a better indication of success.
stream.on('data', (data: Buffer) => {
F(data.toString());
}).stderr.on('data', function(data: Buffer) {
R(new Error(data.toString()));
}).on('close', function(code: any, signal: any) {
log.debug('exec stream closed');
}).stderr.on('data', (data: Buffer) => {
R({
message: 'command output to STDERR: ' + data.toString()
});
}).on('end', () => {
log.debug('%1: exec stream end', this.name_);
});
});
});
Expand Down