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

Jump to bottom

Allow locking network configuration #82

Merged
merged 1 commit into from Mar 2, 2016
Merged

Allow locking network configuration #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions client/index.html
View file
Open in desktop
Expand Up @@ -103,11 +103,11 @@ <h2>Network settings</h2>
<label>Server</label>
</div>
<div class="col-sm-6 col-xs-8">
<input class="input" name="host" value="<%= defaults.host %>">
<input class="input" name="host" value="<%= defaults.host %>" <%= typeof(lockNetwork) !== "undefined" && lockNetwork ? 'disabled' : ''%>>
</div>
<div class="col-sm-3 col-xs-4">
<div class="port">
<input class="input" name="port" value="<%= defaults.port %>">
<input class="input" name="port" value="<%= defaults.port %>" <%= typeof(lockNetwork) !== "undefined" && lockNetwork ? 'disabled' : ''%>>
</div>
</div>
<div class="clearfix"></div>
Expand All @@ -120,7 +120,7 @@ <h2>Network settings</h2>
<div class="col-sm-3"></div>
<div class="col-sm-9">
<label class="tls">
<input type="checkbox" name="tls" <%= defaults.tls ? 'checked="checked"' : '' %>>
<input type="checkbox" name="tls" <%= defaults.tls ? 'checked="checked"' : '' %> <%= typeof(lockNetwork) !== "undefined" && lockNetwork ? 'disabled' : ''%>>
Copy link
Member

Choose a reason for hiding this comment

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

[Very minor comments] For these 3 changes, could you choose either single or double quotes, rather than a mix of both? Also, you might want to do '' %> > or '' %>> instead of ''%>>.

Copy link
Member

Choose a reason for hiding this comment

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

This seems reasonable to me, to avoid the annoying escaping of the double-quotes for the HTML. This is consistent with what's already there. Double-quotes for Javascript expressions, single quotes for the HTML output.

Copy link
Member

Choose a reason for hiding this comment

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

I thought about that at first, but actually there is no need to escape here, unless I am being mistaken at that time of the day: <%= typeof(lockNetwork) !== "undefined" && lockNetwork ? "disabled" : "" %> seems to be fine to me.

Copy link
Member

Choose a reason for hiding this comment

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

(And actually, for checkboxes, no need to do 'checked="checked"', "checked" would be enough here)

Enable TLS/SSL
</label>
</div>
Expand Down
14 changes: 12 additions & 2 deletions defaults/config.js
View file
Open in desktop
Expand Up @@ -79,14 +79,24 @@ module.exports = {
//
// Display network
//
// If set to false The Lounge will not expose network settings in login
// form, limiting client to connect to the configured network.
// If set to false network settings will not be shown in the login form.
//
// @type boolean
// @default true
//
displayNetwork: true,

//
// Lock network
//
// If set to true, users will not be able to modify host, port and tls
// settings and will be limited to the configured network.
//
// @type boolean
// @default false
//
lockNetwork: false,
Copy link
Member

Choose a reason for hiding this comment

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

Instead of locking to the default, could we have an array of hostnames that we can lock to. Would allow people that want to allow multiple irc servers, but not just any random one.

Copy link
Member Author

Choose a reason for hiding this comment

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

That would require some restructuring, plus have a dropdown on the client show allowed servers. I'm not keen on doing that myself.


//
// Log settings
//
Expand Down
34 changes: 32 additions & 2 deletions src/client.js
View file
Open in desktop
Expand Up @@ -123,13 +123,43 @@ Client.prototype.find = function(id) {
Client.prototype.connect = function(args) {
var config = Helper.getConfig();
var client = this;

if (config.lockNetwork) {
// This check is needed to prevent invalid user configurations
if (args.host && args.host.length > 0 && args.host !== config.defaults.host) {
var invalidHostnameMsg = new Msg({
type: Msg.Type.ERROR,
text: "Hostname you specified is not allowed."
});
client.emit("msg", {
msg: invalidHostnameMsg
});
return;
}

args.host = config.defaults.host;
args.port = config.defaults.port;
args.tls = config.defaults.tls;
}

var server = {
name: args.name || "",
host: args.host || "chat.freenode.net",
port: args.port || (args.tls ? 6697 : 6667),
host: args.host || "",
port: parseInt(args.port, 10) || (args.tls ? 6697 : 6667),
rejectUnauthorized: false
};

if (server.host.length === 0) {
var emptyHostnameMsg = new Msg({
type: Msg.Type.ERROR,
text: "You must specify a hostname to connect."
});
client.emit("msg", {
msg: emptyHostnameMsg
});
return;
}

if (config.bind) {
server.localAddress = config.bind;
if (args.tls) {
Expand Down