Skip to content
This repository has been archived by the owner on Nov 7, 2020. It is now read-only.

Commit

Permalink
Don't set connectedParticipants from bad session
Browse files Browse the repository at this point in the history 
Specifically: disallow setting connectedParticipants from users who have
the wrong hangout-url.  May be adequate to fix #280.

Also, do some more not-entirely-fruitful work on addressing test
inconsistency (#267).
  • Loading branch information
@yourcelf
yourcelf committed Apr 1, 2014
1 parent 857678a commit 40829e2
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 45 deletions.
19 changes: 13 additions & 6 deletions lib/unhangout-sockets.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,14 +407,21 @@ _.extend(UnhangoutSocketManager.prototype, events.EventEmitter.prototype, {

}.bind(this));

mgr.on("session/set-connected-participants", _.bind(function(socket, args) {
var session = this.ensureSocketInSession(mgr, socket, args.sessionId,
"session/set-connected-participants");
mgr.on("session/set-connected-participants", function(socket, args) {
var route = "session/set-connected-participants";
var session = this.ensureSocketInSession(mgr, socket, args.sessionId, route);
if (session) {
session.setConnectedParticipants(args.connectedParticipants);
return session.save();
if (session.get("hangout-url") === args["hangout-url"]) {
session.setConnectedParticipants(args.connectedParticipants);
mgr.writeAck(socket, route);
return session.save();
} else {
mgr.writeErr(socket, route, "Not in correct hangout");
}
} else {
mgr.writeErr(socket, route, "Not found");
}
}, this));
}.bind(this));

mgr.on("session/set-activities", _.bind(function(socket, args) {
var session = this.ensureSocketInSession(mgr, socket, args.sessionId,
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"postinstall": "node ./bin/compile-assets.js",
"start": "node ./bin/unhangout-server",
"setup": "source conf.sh",
"test": "NODE_ENV=testing mocha --timeout 10000 test/test.[a-r]* && sleep 1 && NODE_ENV=testing mocha --timeout 10000 test/test.[s-z]*",
"test": "NODE_ENV=testing mocha --timeout 10000 test/test.[a-r]* && sleep 1 && NODE_ENV=testing mocha test/test.s* && sleep 1 && NODE_ENV=testing mocha test/test.[t-z]*",
"forever-start": "NODE_ENV=production forever start --minUptime 1000 --spinSleepTime 1000 -a -l unhangout.log -o logs/out.log -e logs/err.log bin/unhangout-server",
"forever-stop": "forever stop bin/unhangout-server",
"forever-restart": "NODE_ENV=production forever restart bin/unhangout-server",
Expand Down
3 changes: 2 additions & 1 deletion public/js/facilitator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ var FacilitatorView = Backbone.View.extend({
session.on("change:connectedParticipants", function() {
sock.sendJSON("session/set-connected-participants", {
sessionId: session.id,
connectedParticipants: session.get("connectedParticipants")
connectedParticipants: session.get("connectedParticipants"),
"hangout-url": session.get("hangout-url")
});
});
session.on("change:hangout-broadcast-id", function() {
Expand Down
56 changes: 31 additions & 25 deletions test/common.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,8 @@ var buildBrowser = function(callback) {
return browser.executeScript("return true;").then(cb);
};
browser.waitTime = function(time) {
var waited = false;
return browser.wait(function() {
setTimeout(function() { waited = true; }, time);
return browser.then(function() { return waited; });
return new Promise(function(resolve, reject) {
setTimeout(resolve, time);
});
};
browser.waitForFunc = function(cb) {
Expand Down Expand Up @@ -104,7 +102,11 @@ exports.getSeleniumBrowser = function(callback) {
}
seleniumServer = new SeleniumServer(seleniumPath, {port: 4444});
seleniumServer.start().then(function() {
buildBrowser(callback);
// Throwing in a timeout on speculation that this makes
// intermittent timeouts in beforeAll hooks less common.
setTimeout(function() {
buildBrowser(callback);
}, 2000);
});
}
};
Expand Down Expand Up @@ -213,26 +215,30 @@ exports.sockWithPromiseClose = function() {
// 'userKey', and join it to the given room. Depends on `exports.server`
// already being inited with users.
exports.authedSock = function(userKey, room, callback) {
var newSock = exports.sockWithPromiseClose();
var user = exports.server.db.users.findWhere({"sock-key": userKey});
var onData = function(message) {
var msg = JSON.parse(message);
if (msg.type === "auth-ack") {
newSock.write(JSON.stringify({type: "join", args: {id: room}}));
} else if (msg.type === "join-ack") {
newSock.removeListener("data", onData);
callback && callback(newSock);
}
};
newSock.on("data", onData);
newSock.on("error", function(msg) {
console.log("socket error", msg);
});
newSock.once("connection", function() {
newSock.write(JSON.stringify({
type:"auth",
args:{ key: user.getSockKey(), id: user.id }
}));
return new Promise(function(resolve, reject) {
var newSock = exports.sockWithPromiseClose();
var user = exports.server.db.users.findWhere({"sock-key": userKey});
var onData = function(message) {
var msg = JSON.parse(message);
if (msg.type === "auth-ack") {
newSock.write(JSON.stringify({type: "join", args: {id: room}}));
} else if (msg.type === "join-ack") {
newSock.removeListener("data", onData);
callback && callback(newSock);
resolve(newSock);
}
};
newSock.on("data", onData);
newSock.on("error", function(msg) {
console.log("socket error", msg);
reject(new Error(msg));
});
newSock.once("connection", function() {
newSock.write(JSON.stringify({
type:"auth",
args:{ key: user.getSockKey(), id: user.id }
}));
});
});
};

Expand Down
23 changes: 17 additions & 6 deletions test/test.mock-hangout.selenium.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ describe("MOCK HANGOUT", function() {
});
});

afterEach(function(done) {
// Get a URL that won't throw modals at us.
browser.get("http://localhost:7777/public/html/test.html").then(function() {
done();
});
});

it("Communicates the hangout's URL on connction.", function(done) {
var u1 = common.server.db.users.at(0);
browser.get("http://localhost:7777/");
Expand All @@ -56,28 +63,32 @@ describe("MOCK HANGOUT", function() {
var u2 = common.server.db.users.at(1);
var u3 = common.server.db.users.at(2);
var u4 = common.server.db.users.at(3);
var url = "http://localhost:7777/test/hangout/" + session.id + "/";
var baseUrl = "http://localhost:7777/test/hangout/" + session.id + "/";
var queryUrl = baseUrl + "?mockUserIds=" + [u1.id, u2.id, u3.id].join(",");
// Set the hangout URL because connectedParticipants will be refused if
// it doesn't match the URL we get (which in the mock hangout will
// include ?mockUserIds=...).
session.set("hangout-url", queryUrl);

browser.get("http://localhost:7777/");
browser.mockAuthenticate(u1.get("sock-key"));
// First, load the hangout without extra users.
browser.get(url);
browser.get(queryUrl);
// Wait for iframe to load. Would be cleaner to introspect, but.. ugh.
browser.waitForFunc(function() {
return session.getNumConnectedParticipants() == 1;
}).then(function() {
expect(session.getNumConnectedParticipants()).to.be(1);
});
// Next, load the hangout with u2 and u3 as non-app users
browser.get("http://localhost:7777/test/hangout/" + session.id + "/?mockUserIds=" + [
u1.id, u2.id, u3.id
].join(","))
// Wait a longer time, for the cross-document message with participants to come through.
browser.get(queryUrl)
browser.waitForFunc(function() {
return session.getNumConnectedParticipants() == 3;
}).then(function() {
expect(_.pluck(session.get("connectedParticipants"), "id")).to.eql([
u1.id, u2.id, u3.id
]);
session.set("hangout-url", baseUrl);
done();
});
});
Expand Down
53 changes: 47 additions & 6 deletions test/test.session-joining.selenium.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -269,15 +269,16 @@ describe("SESSION JOINING PARTICIPANT LISTS", function() {
sock = thesock;
});
});
browser.waitTime(2000).then(function () {
expect(session.get("connectedParticipants").length).to.be(2);
browser.wait(function() {
return session.get("connectedParticipants").length == 2;
}).then(function() {
common.restartServer(function onStopped(restart) {
framedDisconnectionModalShowing(true).then(function() {
restart();
});
}, function onRestarted() {
framedDisconnectionModalShowing(false);
browser.waitTime(1000).then(function() {
browser.waitTime(5000).then(function() {
// Refresh session from new DB.
event = common.server.db.events.get(event.id);
session = event.get("sessions").get(session.id);
Expand All @@ -287,6 +288,7 @@ describe("SESSION JOINING PARTICIPANT LISTS", function() {
});
});
});

it("Warns you when you're in the wrong hangout", function(done) {
var session = event.get("sessions").at(0);
var button = "document.getElementsByTagName('iframe')[0].contentWindow" +
Expand All @@ -302,14 +304,18 @@ describe("SESSION JOINING PARTICIPANT LISTS", function() {
});
browser.executeScript("return " + button + ".href").then(function(href) {
expect(href).to.be("http://example.com/");
});
// Go to a different URL that won't throw a modal dialog up.
browser.get("http://localhost:7777/").then(function() {
done();
});
});

it("Doesn't clear hangout URL immediately, but rather after a delay.", function(done) {
var session = event.get("sessions").at(1);
session.set("hangoutConnected", false);
session.set("hangout-url", null);
common.authedSock("regular1", session.getRoomId(), function(sock) {
common.authedSock("regular1", session.getRoomId()).then(function(sock) {
sock.on("data", function(message) {
var msg = JSON.parse(message);
if (msg.type == "session/set-hangout-url-ack") {
Expand All @@ -324,9 +330,11 @@ describe("SESSION JOINING PARTICIPANT LISTS", function() {
// We don't test that it actually gets invalidated here,
// because the delay is LONG, and sinon doesn't play well
// with asynchronous socket comms.
done();
sock.promiseClose().then(done);
} else {
done(new Error(message));
sock.promiseClose().then(function() {
done(new Error("Unexpected message: " + message));
});
}
});
sock.write(JSON.stringify({
Expand All @@ -337,6 +345,39 @@ describe("SESSION JOINING PARTICIPANT LISTS", function() {
},
}));
});
});

it("Doesn't set connected participants if URL is invalid.", function(done) {
var session = event.get("sessions").at(1);
var participants = [{id: "p1", displayName: "P1", picture: ""},
{id: "p2", displayName: "P2", picture: ""},
{id: "0", displayName: "Regular1 Mock", picture: ""}];
session.set("hangout-url", "http://example.com");
session.set("connectedParticipants", participants);

common.authedSock("regular1", session.getRoomId()).then(function(sock) {
sock.on("data", function(message) {
var msg = JSON.parse(message);
if (msg.type === "session/set-connected-participants-err") {
expect(msg.args).to.eql("Not in correct hangout");
expect(session.get("connectedParticipants")).to.eql(participants);
sock.promiseClose().then(done);
} else {
sock.promiseClose().then(function() {
done(new Error("Unexpected message: " + message));
});
}
});
sock.write(JSON.stringify({
type: "session/set-connected-participants",
args: {
sessionId: session.id,
"hangout-url": "http://example2.com",
connectedParticipants: [
{id: "0", displayName: "Regular1 Mock", picture: ""}
]
}
}));
});
});
});

0 comments on commit 40829e2

Please sign in to comment.