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

Refactor authentication flow #1411

Merged
merged 1 commit into from Aug 13, 2017
Merged

Refactor authentication flow #1411

merged 1 commit into from Aug 13, 2017

Conversation

xPaw
Copy link
Member

@xPaw xPaw commented Aug 13, 2017

This PR is whitespace heavy so review with ?w=1.

@xPaw xPaw added Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. Type: Feature Tickets that describe a desired feature or PRs that add them to the project. labels Aug 13, 2017
@xPaw xPaw added this to the 2.5.0 milestone Aug 13, 2017
@@ -470,3 +466,13 @@ function auth(data) {
localAuth(client, data.user, data.password, authCallback);
}
}

function reverseDnsLookup(ip, callback) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Rewrote this function to be an independent helper. This improves authentication flow (e.g. remember me in webirc most likely did not work before).

const socket = this;
let client;

const finalInit = () => initializeClient(socket, client, !!data.remember, data.token || null);
Copy link
Member Author

Choose a reason for hiding this comment

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

I am now passing token so that we have a session-like reference to it. This will be required for push notifs and other stuff later.

} else {
init(socket);
socket.emit("auth", {success: true});
Copy link
Member Author

Choose a reason for hiding this comment

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

Moved these 2 lines here so that we don't have a big if/else in init.

@@ -300,13 +300,13 @@ Client.prototype.updateSession = function(token, ip, request) {
const agent = UAParser(request.headers["user-agent"] || "");
let friendlyAgent = "";

if (agent.browser.name.length) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixes crash if UA is missing.

@astorije astorije merged commit bd53055 into master Aug 13, 2017
@astorije astorije deleted the xpaw/refactor-auth branch August 13, 2017 20:35
matburnham pushed a commit to matburnham/lounge that referenced this pull request Sep 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. 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

2 participants