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.

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

Implement WEBIRC #240

Merged
merged 2 commits into from
May 2, 2016
Merged

Implement WEBIRC #240

merged 2 commits into from
May 2, 2016

Conversation

maxpoulin64
Copy link
Member

It's finally ready!

Features:

  • Works in both public/private mode
  • Supports DNS resolution (without the possible race condition of the original PR)
  • Configurable hostname prefix to mark the web client
  • In private mode, uses the IP of the client creating the new network (and is saved to JSON for server restarts)
  • IP/hostname can be overriden per user in the user's JSON
  • IP/hostname can be overriden per network in the user's JSON
  • Gracefully falls back to a standard connection of the client IP can't be obtained and issue a warning to the console. (Should only happen when editing the JSON files manually)

Overall, if enabled in the config.js, it should do what an admin would expect both in public and private mode. However, admins that wants more control can override settings as needed per user, or per user network.

EDIT: An irc-framework version to make rebasing easier or in case irc-fw gets merged first.

@maxpoulin64 maxpoulin64 added the Type: Feature Tickets that describe a desired feature or PRs that add them to the project. label Apr 3, 2016
@maxpoulin64 maxpoulin64 self-assigned this Apr 3, 2016
@maxpoulin64 maxpoulin64 force-pushed the webirc branch 4 times, most recently from 19d57dd to 93e29be Compare April 3, 2016 07:05
@xPaw
Copy link
Member

xPaw commented Apr 3, 2016

Code looks fine. I think this will have to wait for irc-fw to be merged.

Just wondering is reverse dns really that desirable in spoofed hosts? Both kiwi and qwebirc seem to do @gateway/web/cgi-irc/kiwiirc.com/ip.000.000.000.000

@maxpoulin64
Copy link
Member Author

Hmm, I'm thinking since the config file is actually Javascript, I think I'll make the webirc object accept both strings (for just the password with no extra stuff) or a function that returns the webirc object that irc-framework takes. I have noticed several ways of setting the hostnames over multiple networks. Some use just the reverse DNS. Some use ip-xx-xx-xx-xx.web.example.net, some use gateway/web/host.provider.dsl.example.net, some use just the IP. Making that option take a function would allow the admin to configure it at wish, and possibly also include more data as needed.

@maxpoulin64
Copy link
Member Author

Updated as irc-fw just got merged. Also added the function thing I said I'd do, so now the admins can do whatever they want, and if the admin only specifies the password then it follows the standard.

@@ -103,7 +111,7 @@ function init(socket, client, token) {
socket.on(
"conn",
function(data) {
client.connect(data);
client.connect(data, false);
Copy link
Member

Choose a reason for hiding this comment

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

What if instead of passing in trusted boolean, we filtered it here?

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's a great idea, I'll change that!

@xPaw
Copy link
Member

xPaw commented Apr 13, 2016

👍

};
}
} else {
console.warn("Cannot find a valid WEBIRC configuration for " + nick
Copy link
Member

Choose a reason for hiding this comment

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

Change to log.warn which is now in master.

@maxpoulin64 maxpoulin64 force-pushed the webirc branch 2 times, most recently from d1c8e1c to b9d6823 Compare April 27, 2016 07:43
@@ -79,6 +79,9 @@ function Client(manager, name, config) {
log.info("User '" + name + "' loaded");
}

Client.prototype.ip = null;
Client.prototype.host = null;

Copy link
Member

Choose a reason for hiding this comment

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

These lines can be removed.

@astorije
Copy link
Member

@maxpoulin64 and I went through the code together offline, 👏 for the feature!
As discussed, a few minor comments and then it should be fine :-)

@astorije astorije assigned astorije and unassigned maxpoulin64 Apr 30, 2016
@astorije
Copy link
Member

astorije commented May 2, 2016

👍 and merging, great job on this, @maxpoulin64!

@astorije astorije merged commit ba26724 into thelounge:master May 2, 2016
@astorije astorije added this to the ★ Next Release milestone May 15, 2016
@maxpoulin64 maxpoulin64 deleted the webirc branch May 28, 2016 22:02
@dgw dgw mentioned this pull request Jun 1, 2016
@spencerthayer
Copy link

This has been a LONG time coming. I've been waiting for Shout to implement the web IRC protocol for quite some time. Is this working out of the box? (Waiting in excitement.)

@maxpoulin64
Copy link
Member Author

@spencerthayer: what do you mean by out of the box? You still need to configure it to put your webirc password for each network, but yes it otherwise just works.

matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017
@CayneVamp-Inn
Copy link

question how do I do the format to add the webirc implement in Ive tried everything

@astorije
Copy link
Member

astorije commented Oct 1, 2018

@CayneVamp-Inn, we have not documented this on the website at the moment, so your best bet is to follow the WEBIRC section of the configuration file, which lives here.

As I am rewriting the docs, there is a planned guide for WEBIRC that is currently empty (code / preview), so once you figure it out, if you or anyone else wants to help us, you can come up with a first version of the guide.

Good luck!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature Tickets that describe a desired feature or PRs that add them to the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants