From 3e03164ed4e6f29f669d74fe0cab7f19e2359dca Mon Sep 17 00:00:00 2001 From: James Allen Date: Thu, 10 Mar 2016 17:15:14 +0000 Subject: [PATCH] Remove dead auth_token code --- .../AuthenticationController.coffee | 35 +----- .../AuthenticationManager.coffee | 16 --- .../CollaboratorsController.coffee | 32 ----- .../Collaborators/CollaboratorsRouter.coffee | 1 - .../Features/User/UserInfoController.coffee | 4 - services/web/app/coffee/router.coffee | 3 +- .../AuthenticationControllerTests.coffee | 114 ++++-------------- .../AuthenticationManagerTests.coffee | 46 ------- .../CollaboratorsControllerTests.coffee | 46 ------- 9 files changed, 30 insertions(+), 267 deletions(-) diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee index 312b441158..e7db5d9f65 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationController.coffee @@ -44,50 +44,27 @@ module.exports = AuthenticationController = text: req.i18n.translate("email_or_password_wrong_try_again"), type: 'error' - getAuthToken: (req, res, next = (error) ->) -> - AuthenticationController.getLoggedInUserId req, (error, user_id) -> - return next(error) if error? - AuthenticationManager.getAuthToken user_id, (error, auth_token) -> - return next(error) if error? - res.send(auth_token) - getLoggedInUserId: (req, callback = (error, user_id) ->) -> if req?.session?.user?._id? callback null, req.session.user._id.toString() else callback null, null - getLoggedInUser: (req, options = {allow_auth_token: false}, callback = (error, user) ->) -> - if typeof(options) == "function" - callback = options - options = {allow_auth_token: false} - + getLoggedInUser: (req, callback = (error, user) ->) -> if req.session?.user?._id? query = req.session.user._id - else if req.query?.auth_token? and options.allow_auth_token - query = { auth_token: req.query.auth_token } else return callback null, null UserGetter.getUser query, callback - requireLogin: (options = {allow_auth_token: false, load_from_db: false}) -> + requireLogin: () -> doRequest = (req, res, next = (error) ->) -> - load_from_db = options.load_from_db - if req.query?.auth_token? and options.allow_auth_token - load_from_db = true - if load_from_db - AuthenticationController.getLoggedInUser req, { allow_auth_token: options.allow_auth_token }, (error, user) -> - return next(error) if error? - return AuthenticationController._redirectToLoginOrRegisterPage(req, res) if !user? - req.user = user - return next() + if !req.session.user? + AuthenticationController._redirectToLoginOrRegisterPage(req, res) else - if !req.session.user? - AuthenticationController._redirectToLoginOrRegisterPage(req, res) - else - req.user = req.session.user - return next() + req.user = req.session.user + return next() return doRequest diff --git a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee index 254c5c6c41..d2422be90b 100644 --- a/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee +++ b/services/web/app/coffee/Features/Authentication/AuthenticationManager.coffee @@ -36,19 +36,3 @@ module.exports = AuthenticationManager = $unset: password: true }, callback) - getAuthToken: (user_id, callback = (error, auth_token) ->) -> - db.users.findOne { _id: ObjectId(user_id.toString()) }, { auth_token : true }, (error, user) => - return callback(error) if error? - return callback(new Error("user could not be found: #{user_id}")) if !user? - if user.auth_token? - callback null, user.auth_token - else - @_createSecureToken (error, auth_token) -> - db.users.update { _id: ObjectId(user_id.toString()) }, { $set : auth_token: auth_token }, (error) -> - return callback(error) if error? - callback null, auth_token - - _createSecureToken: (callback = (error, token) ->) -> - crypto.randomBytes 48, (error, buffer) -> - return callback(error) if error? - callback null, buffer.toString("hex") diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsController.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsController.coffee index 82912e1b6b..1905d0d0a6 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsController.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsController.coffee @@ -7,14 +7,6 @@ UserGetter = require "../User/UserGetter" mimelib = require("mimelib") module.exports = CollaboratorsController = - getCollaborators: (req, res, next = (error) ->) -> - project_id = req.params.Project_id - CollaboratorsHandler.getMembersWithPrivilegeLevels project_id, (error, members) -> - return next(error) if error? - CollaboratorsController._formatCollaborators members, (error, collaborators) -> - return next(error) if error? - res.json(collaborators) - addUserToProject: (req, res, next) -> project_id = req.params.Project_id LimitationsManager.canAddXCollaborators project_id, 1, (error, allowed) => @@ -58,27 +50,3 @@ module.exports = CollaboratorsController = EditorRealTimeController.emitToRoom(project_id, 'userRemovedFromProject', user_id) callback() - _formatCollaborators: (members, callback = (error, collaborators) ->) -> - collaborators = [] - - pushCollaborator = (user, permissions, owner) -> - collaborators.push { - id: user._id.toString() - first_name: user.first_name - last_name: user.last_name - email: user.email - permissions: permissions - owner: owner - } - - for member in members - {user, privilegeLevel} = member - if privilegeLevel == "admin" - pushCollaborator(user, ["read", "write", "admin"], true) - else if privilegeLevel == "readAndWrite" - pushCollaborator(user, ["read", "write"], false) - else - pushCollaborator(user, ["read"], false) - - callback null, collaborators - diff --git a/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee b/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee index d5abf23350..f4f1b0343c 100644 --- a/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee +++ b/services/web/app/coffee/Features/Collaborators/CollaboratorsRouter.coffee @@ -5,7 +5,6 @@ AuthenticationController = require('../Authentication/AuthenticationController') module.exports = apply: (webRouter, apiRouter) -> webRouter.post '/project/:Project_id/leave', AuthenticationController.requireLogin(), CollaboratorsController.removeSelfFromProject - apiRouter.get '/project/:Project_id/collaborators', SecurityManager.requestCanAccessProject(allow_auth_token: true), CollaboratorsController.getCollaborators webRouter.post '/project/:Project_id/users', SecurityManager.requestIsOwner, CollaboratorsController.addUserToProject webRouter.delete '/project/:Project_id/users/:user_id', SecurityManager.requestIsOwner, CollaboratorsController.removeUserFromProject diff --git a/services/web/app/coffee/Features/User/UserInfoController.coffee b/services/web/app/coffee/Features/User/UserInfoController.coffee index ac7556bc90..edaf836c80 100644 --- a/services/web/app/coffee/Features/User/UserInfoController.coffee +++ b/services/web/app/coffee/Features/User/UserInfoController.coffee @@ -6,10 +6,6 @@ sanitize = require('sanitizer') module.exports = UserController = getLoggedInUsersPersonalInfo: (req, res, next = (error) ->) -> - # this is funcky as hell, we don't use the current session to get the user - # we use the auth token, actually destroying session from the chat api request - if req.query?.auth_token? - req.session?.destroy() logger.log user: req.user, "reciving request for getting logged in users personal info" return next(new Error("User is not logged in")) if !req.user? UserGetter.getUser req.user._id, { diff --git a/services/web/app/coffee/router.coffee b/services/web/app/coffee/router.coffee index eafe41470b..36b1bd7ea2 100644 --- a/services/web/app/coffee/router.coffee +++ b/services/web/app/coffee/router.coffee @@ -88,8 +88,7 @@ module.exports = class Router webRouter.delete '/user/newsletter/unsubscribe', AuthenticationController.requireLogin(), UserController.unsubscribe webRouter.delete '/user', AuthenticationController.requireLogin(), UserController.deleteUser - webRouter.get '/user/auth_token', AuthenticationController.requireLogin(), AuthenticationController.getAuthToken - webRouter.get '/user/personal_info', AuthenticationController.requireLogin(allow_auth_token: true), UserInfoController.getLoggedInUsersPersonalInfo + webRouter.get '/user/personal_info', AuthenticationController.requireLogin(), UserInfoController.getLoggedInUsersPersonalInfo apiRouter.get '/user/:user_id/personal_info', AuthenticationController.httpAuth, UserInfoController.getPersonalInfo webRouter.get '/project', AuthenticationController.requireLogin(), ProjectController.projectListPage diff --git a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee index 63ff12a374..f83b38617b 100644 --- a/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Authentication/AuthenticationControllerTests.coffee @@ -173,7 +173,7 @@ describe "AuthenticationController", -> beforeEach -> @req.session = user: @user - @AuthenticationController.getLoggedInUser(@req, {}, @callback) + @AuthenticationController.getLoggedInUser(@req, @callback) it "should look up the user in the database", -> @UserGetter.getUser @@ -183,105 +183,37 @@ describe "AuthenticationController", -> it "should return the user", -> @callback.calledWith(null, @user).should.equal true - describe "with an auth token, but without auth_token_allowed set to true", -> - beforeEach -> - @req.query = - auth_token: "auth-token" - @AuthenticationController.getLoggedInUser(@req, {}, @callback) - - it "should not look up the user in the database", -> - @UserGetter.getUser.called.should.equal false - - it "should return null in the callback", -> - @callback.calledWith(null, null).should.equal true - - describe "with an auth token and auth_token_allowed set to true", -> - beforeEach -> - @req.query = - auth_token: "auth-token" - @AuthenticationController.getLoggedInUser(@req, {allow_auth_token: true}, @callback) - - it "should look up the user in the database", -> - @UserGetter.getUser - .calledWith(auth_token: @req.query.auth_token) - .should.equal true - - it "should return the user", -> - @callback.calledWith(null, @user).should.equal true - describe "requireLogin", -> beforeEach -> @user = _id: "user-id-123" email: "user@sharelatex.com" + @middleware = @AuthenticationController.requireLogin() - describe "when loading from the database", -> + describe "when the user is logged in", -> beforeEach -> - @middleware = @AuthenticationController.requireLogin(@options = { allow_auth_token: true, load_from_db: true }) - - describe "when the user is logged in", -> - beforeEach -> - @AuthenticationController.getLoggedInUser = sinon.stub().callsArgWith(2, null, @user) - @middleware(@req, @res, @next) - - it "should call getLoggedInUser with the passed options", -> - @AuthenticationController.getLoggedInUser.calledWith(@req, { allow_auth_token: true }).should.equal true - - it "should set the user property on the request", -> - @req.user.should.deep.equal @user - - it "should call the next method in the chain", -> - @next.called.should.equal true - - describe "when the user is not logged in", -> - beforeEach -> - @AuthenticationController._redirectToLoginOrRegisterPage = sinon.stub() - @AuthenticationController.getLoggedInUser = sinon.stub().callsArgWith(2, null, null) - @middleware(@req, @res, @next) - - it "should redirect to the register page", -> - @AuthenticationController._redirectToLoginOrRegisterPage.calledWith(@req, @res).should.equal true - - describe "when not loading from the database", -> - beforeEach -> - @middleware = @AuthenticationController.requireLogin(@options = { load_from_db: false }) - - describe "when the user is logged in", -> - beforeEach -> - @req.session = - user: @user = { - _id: "user-id-123" - email: "user@sharelatex.com" - } - @middleware(@req, @res, @next) - - it "should set the user property on the request", -> - @req.user.should.deep.equal @user - - it "should call the next method in the chain", -> - @next.called.should.equal true - - describe "when the user is not logged in", -> - beforeEach -> - @req.session = {} - @AuthenticationController._redirectToLoginOrRegisterPage = sinon.stub() - @req.query = {} - @middleware(@req, @res, @next) - - it "should redirect to the register or login page", -> - @AuthenticationController._redirectToLoginOrRegisterPage.calledWith(@req, @res).should.equal true - - describe "when not loading from the database but an auth_token is provided", -> - beforeEach -> - @AuthenticationController.getLoggedInUser = sinon.stub().callsArgWith(2, null, @user) - @middleware = @AuthenticationController.requireLogin(@options = { load_from_db: false, allow_auth_token: true }) - @req.query = auth_token: @auth_token = "auth-token-provided" + @req.session = + user: @user = { + _id: "user-id-123" + email: "user@sharelatex.com" + } @middleware(@req, @res, @next) - it "should try to load the user from the database anyway", -> - @AuthenticationController.getLoggedInUser - .calledWith(@req, {allow_auth_token: true}) - .should.equal true + it "should set the user property on the request", -> + @req.user.should.deep.equal @user + + it "should call the next method in the chain", -> + @next.called.should.equal true + + describe "when the user is not logged in", -> + beforeEach -> + @req.session = {} + @AuthenticationController._redirectToLoginOrRegisterPage = sinon.stub() + @req.query = {} + @middleware(@req, @res, @next) + + it "should redirect to the register or login page", -> + @AuthenticationController._redirectToLoginOrRegisterPage.calledWith(@req, @res).should.equal true describe "requireGlobalLogin", -> beforeEach -> diff --git a/services/web/test/UnitTests/coffee/Authentication/AuthenticationManagerTests.coffee b/services/web/test/UnitTests/coffee/Authentication/AuthenticationManagerTests.coffee index 3a5c60cf66..f4d5e72c26 100644 --- a/services/web/test/UnitTests/coffee/Authentication/AuthenticationManagerTests.coffee +++ b/services/web/test/UnitTests/coffee/Authentication/AuthenticationManagerTests.coffee @@ -95,49 +95,3 @@ describe "AuthenticationManager", -> it "should call the callback", -> @callback.called.should.equal true - - describe "getAuthToken", -> - beforeEach -> - @auth_token = "auth-token" - - describe "when the user has an auth token set", -> - beforeEach -> - @db.users.findOne = sinon.stub().callsArgWith(2, null, auth_token: @auth_token) - @AuthenticationManager.getAuthToken(@user_id, @callback) - - it "should look up the auth token in the db", -> - @db.users.findOne - .calledWith({ - _id: ObjectId(@user_id.toString()) - }, { - auth_token: true - }) - .should.equal true - - it "should return the auth token", -> - @callback.calledWith(null, @auth_token).should.equal true - - describe "when the user does not have an auth token set", -> - beforeEach -> - @db.users.findOne = sinon.stub().callsArgWith(2, null, auth_token: null) - @db.users.update = sinon.stub().callsArgWith(2, null) - @AuthenticationManager._createSecureToken = sinon.stub().callsArgWith(0, null, @auth_token) - @AuthenticationManager.getAuthToken(@user_id, @callback) - - it "should generate a new auth token", -> - @AuthenticationManager._createSecureToken.called.should.equal true - - it "should set the auth token on the user document in the db", -> - @db.users.update - .calledWith({ - _id: ObjectId(@user_id.toString()) - }, { - $set: auth_token: @auth_token - }) - .should.equal true - - it "should return the auth token", -> - @callback.calledWith(null, @auth_token).should.equal true - - - diff --git a/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsControllerTests.coffee b/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsControllerTests.coffee index 84b0a250d3..32de9ebe0a 100644 --- a/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsControllerTests.coffee +++ b/services/web/test/UnitTests/coffee/Collaborators/CollaboratorsControllerTests.coffee @@ -24,52 +24,6 @@ describe "CollaboratorsController", -> @project_id = "project-id-123" @callback = sinon.stub() - describe "getCollaborators", -> - beforeEach -> - @req.params = - Project_id: @project_id - @members = [ - { - user: { _id: "admin-id", email: "admin@example.com", first_name: "Joe", last_name: "Admin", foo: "bar" } - privilegeLevel: "admin" - }, - { - user: { _id: "rw-id", email: "rw@example.com", first_name: "Jane", last_name: "Write", foo: "bar" } - privilegeLevel: "readAndWrite" - }, - { - user: { _id: "ro-id", email: "ro@example.com", first_name: "Joe", last_name: "Read", foo: "bar" } - privilegeLevel: "readOnly" - } - ] - @CollaboratorsHandler.getMembersWithPrivilegeLevels = sinon.stub() - @CollaboratorsHandler.getMembersWithPrivilegeLevels - .withArgs(@project_id) - .yields(null, @members) - @res.json = sinon.stub() - @CollaboratorsController.getCollaborators(@req, @res) - - it "should return the formatted collaborators", -> - @res.json - .calledWith([ - { - id: "admin-id", email: "admin@example.com", first_name: "Joe", last_name: "Admin" - permissions: ["read", "write", "admin"] - owner: true - } - { - id: "rw-id", email: "rw@example.com", first_name: "Jane", last_name: "Write" - permissions: ["read", "write"] - owner: false - } - { - id: "ro-id", email: "ro@example.com", first_name: "Joe", last_name: "Read" - permissions: ["read"] - owner: false - } - ]) - .should.equal true - describe "addUserToProject", -> beforeEach -> @req.params =