Merge pull request #1091 from sharelatex/ta-entity-authorization-refactor

UserMembershipAuthorization Refactor

GitOrigin-RevId: 055cebcd9298ed6dace081198f68491a000cf4a3

services/web/app/coffee/Features/UserMembership/UserMembershipAuthorization.coffee

@@ -6,21 +6,36 @@ Errors = require('../Errors/Errors')
 logger = require("logger-sharelatex")
 
 module.exports =
-	requireEntityAccess: (entityName, entityIdOverride = null) ->
-		(req, res, next) ->
-			loggedInUser = AuthenticationController.getSessionUser(req)
-			unless loggedInUser
-				return AuthorizationMiddlewear.redirectToRestricted req, res, next
+	requireTeamAccess: (req, res, next) ->
+		requireAccessToEntity('team', req.params.id, req, res, next)
 
-			entityId = entityIdOverride or req.params.id
-			getEntity entityName, entityId, loggedInUser, (error, entity, entityConfig) ->
-				return next(error) if error?
-				unless entity?
-					return AuthorizationMiddlewear.redirectToRestricted(req, res, next)
+	requireGroupAccess: (req, res, next) ->
+		requireAccessToEntity('group', req.params.id, req, res, next)
 
-				req.entity = entity
-				req.entityConfig = entityConfig
-				next()
+	requireGroupManagersAccess: (req, res, next) ->
+		requireAccessToEntity('groupManagers', req.params.id, req, res, next)
+
+	requireInstitutionAccess: (req, res, next) ->
+		requireAccessToEntity('institution', req.params.id, req, res, next)
+
+	requireGraphAccess: (req, res, next) ->
+		requireAccessToEntity(
+			req.query.resource_type, req.query.resource_id, req, res, next
+		)
+
+requireAccessToEntity = (entityName, entityId, req, res, next) ->
+	loggedInUser = AuthenticationController.getSessionUser(req)
+	unless loggedInUser
+		return AuthorizationMiddlewear.redirectToRestricted req, res, next
+
+	getEntity entityName, entityId, loggedInUser, (error, entity, entityConfig) ->
+		return next(error) if error?
+		unless entity?
+			return AuthorizationMiddlewear.redirectToRestricted(req, res, next)
+
+		req.entity = entity
+		req.entityConfig = entityConfig
+		next()
 
 getEntity = (entityName, entityId, userId, callback = (error, entity, entityConfig)->) ->
 	entityConfig = EntityConfigs[entityName]

services/web/app/coffee/Features/UserMembership/UserMembershipRouter.coffee

@@ -5,36 +5,41 @@ TeamInvitesController = require '../Subscription/TeamInvitesController'
 
 module.exports =
 	apply: (webRouter) ->
+		# group members routes
 		webRouter.get '/manage/groups/:id/members',
-			UserMembershipAuthorization.requireEntityAccess('group'),
+			UserMembershipAuthorization.requireGroupAccess,
 			UserMembershipController.index
 		webRouter.post '/manage/groups/:id/invites',
-			UserMembershipAuthorization.requireEntityAccess('group'),
+			UserMembershipAuthorization.requireGroupAccess,
 			TeamInvitesController.createInvite
 		webRouter.delete '/manage/groups/:id/user/:user_id',
-			UserMembershipAuthorization.requireEntityAccess('group'),
+			UserMembershipAuthorization.requireGroupAccess,
 			SubscriptionGroupController.removeUserFromGroup
 		webRouter.delete '/manage/groups/:id/invites/:email',
-			UserMembershipAuthorization.requireEntityAccess('group'),
+			UserMembershipAuthorization.requireGroupAccess,
 			TeamInvitesController.revokeInvite
 		webRouter.get '/manage/groups/:id/members/export',
-			UserMembershipAuthorization.requireEntityAccess('group'),
+			UserMembershipAuthorization.requireGroupAccess,
 			UserMembershipController.exportCsv
 
+		# group managers routes
+		webRouter.get "/manage/groups/:id/managers",
+			UserMembershipAuthorization.requireGroupManagersAccess,
+			UserMembershipController.index
+		webRouter.post "/manage/groups/:id/managers",
+			UserMembershipAuthorization.requireGroupManagersAccess,
+			UserMembershipController.add
+		webRouter.delete "/manage/groups/:id/managers/:userId",
+			UserMembershipAuthorization.requireGroupManagersAccess,
+			UserMembershipController.remove
 
-		regularEntitites =
-			groups: 'groupManagers'
-			institutions: 'institution'
-		for pathName, entityName of regularEntitites
-			do (pathName, entityName) ->
-				webRouter.get "/manage/#{pathName}/:id/managers",
-					UserMembershipAuthorization.requireEntityAccess(entityName),
-					UserMembershipController.index
-
-				webRouter.post "/manage/#{pathName}/:id/managers",
-					UserMembershipAuthorization.requireEntityAccess(entityName),
-					UserMembershipController.add
-
-				webRouter.delete "/manage/#{pathName}/:id/managers/:userId",
-					UserMembershipAuthorization.requireEntityAccess(entityName),
-					UserMembershipController.remove
+		# institution members routes
+		webRouter.get "/manage/institutions/:id/managers",
+			UserMembershipAuthorization.requireInstitutionAccess,
+			UserMembershipController.index
+		webRouter.post "/manage/institutions/:id/managers",
+			UserMembershipAuthorization.requireInstitutionAccess,
+			UserMembershipController.add
+		webRouter.delete "/manage/institutions/:id/managers/:userId",
+			UserMembershipAuthorization.requireInstitutionAccess,
+			UserMembershipController.remove

services/web/test/unit/coffee/UserMembership/UserMembershipAuthorizationTests.coffee

@@ -30,10 +30,9 @@ describe "UserMembershipAuthorization", ->
 				log: ->
 				err: ->
 
-	describe 'requireEntityAccess', ->
+	describe 'requireAccessToEntity', ->
 		it 'get entity', (done) ->
-			middlewear = @UserMembershipAuthorization.requireEntityAccess 'group'
-			middlewear @req, null, (error) =>
+			@UserMembershipAuthorization.requireGroupAccess @req, null, (error) =>
 				expect(error).to.not.extist
 				sinon.assert.calledWithMatch(
 					@UserMembershipHandler.getEntity,
@@ -45,19 +44,9 @@ describe "UserMembershipAuthorization", ->
 				expect(@req.entityConfig).to.exist
 				done()
 
-		it 'handle unknown entity', (done) ->
-			middlewear = @UserMembershipAuthorization.requireEntityAccess 'foo'
-			middlewear @req, null, (error) =>
-				expect(error).to.extist
-				expect(error).to.be.an.instanceof(Errors.NotFoundError)
-				sinon.assert.notCalled(@UserMembershipHandler.getEntity)
-				expect(@req.entity).to.not.exist
-				done()
-
 		it 'handle entity not found', (done) ->
 			@UserMembershipHandler.getEntity.yields(null, null)
-			middlewear = @UserMembershipAuthorization.requireEntityAccess 'institution'
-			middlewear @req, null, (error) =>
+			@UserMembershipAuthorization.requireGroupAccess @req, null, (error) =>
 				expect(error).to.extist
 				sinon.assert.called(@AuthorizationMiddlewear.redirectToRestricted)
 				sinon.assert.called(@UserMembershipHandler.getEntity)
@@ -66,34 +55,63 @@ describe "UserMembershipAuthorization", ->
 
 		it 'handle anonymous user', (done) ->
 			@AuthenticationController.getSessionUser.returns(null)
-			middlewear = @UserMembershipAuthorization.requireEntityAccess 'institution'
-			middlewear @req, null, (error) =>
+			@UserMembershipAuthorization.requireGroupAccess @req, null, (error) =>
 				expect(error).to.extist
 				sinon.assert.called(@AuthorizationMiddlewear.redirectToRestricted)
 				sinon.assert.notCalled(@UserMembershipHandler.getEntity)
 				expect(@req.entity).to.not.exist
 				done()
 
-		it 'can override entity id', (done) ->
-			middlewear = @UserMembershipAuthorization.requireEntityAccess 'group', 'entity-id-override'
+	describe 'requireEntityAccess', ->
+		it 'handle team access', (done) ->
+			@UserMembershipAuthorization.requireTeamAccess @req, null, (error) =>
+				expect(error).to.not.extist
+				sinon.assert.calledWithMatch(
+					@UserMembershipHandler.getEntity,
+					@req.params.id,
+					fields: primaryKey: 'overleaf.id'
+				)
+				done()
+
+		it 'handle group access', (done) ->
+			@UserMembershipAuthorization.requireGroupAccess @req, null, (error) =>
+				expect(error).to.not.extist
+				sinon.assert.calledWithMatch(
+					@UserMembershipHandler.getEntity,
+					@req.params.id,
+					translations: title: 'group_account'
+				)
+				done()
+
+		it 'handle group managers access', (done) ->
+			@UserMembershipAuthorization.requireGroupManagersAccess @req, null, (error) =>
+				expect(error).to.not.extist
+				sinon.assert.calledWithMatch(
+					@UserMembershipHandler.getEntity,
+					@req.params.id,
+					translations: subtitle: 'managers_management'
+				)
+				done()
+
+		it 'handle institution access', (done) ->
+			@UserMembershipAuthorization.requireInstitutionAccess @req, null, (error) =>
+				expect(error).to.not.extist
+				sinon.assert.calledWithMatch(
+					@UserMembershipHandler.getEntity,
+					@req.params.id,
+					modelName: 'Institution',
+				)
+				done()
+
+		it 'handle graph access', (done) ->
+			@req.query.resource_id = 'mock-resource-id'
+			@req.query.resource_type = 'institution'
+			middlewear = @UserMembershipAuthorization.requireGraphAccess
 			middlewear @req, null, (error) =>
 				expect(error).to.not.extist
 				sinon.assert.calledWithMatch(
 					@UserMembershipHandler.getEntity,
-					'entity-id-override',
+					@req.query.resource_id,
+					modelName: 'Institution',
 				)
 				done()
-
-		it "doesn't cache entity id between requests", (done) ->
-			middlewear = @UserMembershipAuthorization.requireEntityAccess 'group'
-			middlewear @req, null, (error) =>
-				expect(error).to.not.extist
-				lastCallArs = @UserMembershipHandler.getEntity.lastCall.args
-				expect(lastCallArs[0]).to.equal @req.params.id
-				newEntityId = 'another-mock-id'
-				@req.params.id = newEntityId
-				middlewear @req, null, (error) =>
-					expect(error).to.not.extist
-					lastCallArs = @UserMembershipHandler.getEntity.lastCall.args
-					expect(lastCallArs[0]).to.equal newEntityId
-					done()