Revert "Track the isRestrictedUser flag on clients"

This commit is contained in:
Simon Detheridge 2019-10-22 10:17:38 +01:00 committed by GitHub
parent c31c2d292d
commit 925a8651c1
6 changed files with 91 additions and 210 deletions

View file

@ -4,7 +4,7 @@ logger = require "logger-sharelatex"
{ CodedError } = require "./Errors"
module.exports = WebApiManager =
joinProject: (project_id, user, callback = (error, project, privilegeLevel, isRestrictedUser) ->) ->
joinProject: (project_id, user, callback = (error, project, privilegeLevel) ->) ->
user_id = user._id
logger.log {project_id, user_id}, "sending join project request to web"
url = "#{settings.apis.web.url}/project/#{project_id}/join"
@ -24,11 +24,7 @@ module.exports = WebApiManager =
}, (error, response, data) ->
return callback(error) if error?
if 200 <= response.statusCode < 300
if !data? || !data?.project?
err = new Error('no data returned from joinProject request')
logger.error {err, project_id, user_id}, "error accessing web api"
return callback(err)
callback null, data.project, data.privilegeLevel, data.isRestrictedUser
callback null, data?.project, data?.privilegeLevel
else if response.statusCode == 429
logger.log(project_id, user_id, "rate-limit hit when joining project")
callback(new CodedError("rate-limit hit when joining project", "TooManyRequests"))

View file

@ -18,7 +18,7 @@ module.exports = WebsocketController =
user_id = user?._id
logger.log {user_id, project_id, client_id: client.id}, "user joining project"
metrics.inc "editor.join-project"
WebApiManager.joinProject project_id, user, (error, project, privilegeLevel, isRestrictedUser) ->
WebApiManager.joinProject project_id, user, (error, project, privilegeLevel) ->
return callback(error) if error?
if !privilegeLevel or privilegeLevel == ""
@ -26,6 +26,7 @@ module.exports = WebsocketController =
logger.warn {err, project_id, user_id, client_id: client.id}, "user is not authorized to join project"
return callback(err)
client.set("privilege_level", privilegeLevel)
client.set("user_id", user_id)
client.set("project_id", project_id)
@ -36,7 +37,6 @@ module.exports = WebsocketController =
client.set("connected_time", new Date())
client.set("signup_date", user?.signUpDate)
client.set("login_count", user?.loginCount)
client.set("is_restricted_user", !!(isRestrictedUser))
RoomManager.joinProject client, project_id, (err) ->
logger.log {user_id, project_id, client_id: client.id}, "user joined project"
@ -178,11 +178,8 @@ module.exports = WebsocketController =
CLIENT_REFRESH_DELAY: 1000
getConnectedUsers: (client, callback = (error, users) ->) ->
metrics.inc "editor.get-connected-users"
Utils.getClientAttributes client, ["project_id", "user_id", "is_restricted_user"], (error, clientAttributes) ->
Utils.getClientAttributes client, ["project_id", "user_id"], (error, {project_id, user_id}) ->
return callback(error) if error?
{project_id, user_id, is_restricted_user} = clientAttributes
if is_restricted_user
return callback(null, [])
return callback(new Error("no project_id found on client")) if !project_id?
logger.log {user_id, project_id, client_id: client.id}, "getting connected users"
AuthorizationManager.assertClientCanViewProject client, (error) ->

View file

@ -7,19 +7,6 @@ HealthCheckManager = require "./HealthCheckManager"
RoomManager = require "./RoomManager"
ChannelManager = require "./ChannelManager"
ConnectedUsersManager = require "./ConnectedUsersManager"
Utils = require './Utils'
Async = require 'async'
RESTRICTED_USER_MESSAGE_TYPE_PASS_LIST = [
'connectionAccepted',
'otUpdateApplied',
'otUpdateError',
'joinDoc',
'reciveNewDoc',
'reciveNewFile',
'reciveNewFolder',
'removeEntity'
]
module.exports = WebsocketLoadBalancer =
rclientPubList: RedisClientManager.createClientList(Settings.redis.pubsub)
@ -82,28 +69,12 @@ module.exports = WebsocketLoadBalancer =
clientList = io.sockets.clients(message.room_id)
# avoid unnecessary work if no clients are connected
return if clientList.length is 0
logger.log {
channel: channel,
message: message.message,
room_id: message.room_id,
message_id: message._id,
socketIoClients: (client.id for client in clientList)
}, "distributing event to clients"
logger.log {channel:channel, message: message.message, room_id: message.room_id, message_id: message._id, socketIoClients: (client.id for client in clientList)}, "distributing event to clients"
seen = {}
# Send the messages to clients async, don't wait for them all to finish
Async.eachLimit clientList
, 2
, (client, cb) ->
Utils.getClientAttributes client, ['is_restricted_user'], (err, {is_restricted_user}) ->
return cb(err) if err?
if !seen[client.id]
for client in clientList when not seen[client.id]
seen[client.id] = true
if !(is_restricted_user && message.message not in RESTRICTED_USER_MESSAGE_TYPE_PASS_LIST)
client.emit(message.message, message.payload...)
cb()
, (err) ->
if err?
logger.err {err, message}, "Error sending message to clients"
else if message.health_check?
logger.debug {message}, "got health check message in editor events channel"
HealthCheckManager.check channel, message.key

View file

@ -26,8 +26,7 @@ describe 'WebApiManager', ->
beforeEach ->
@response = {
project: { name: "Test project" }
privilegeLevel: "owner",
isRestrictedUser: true
privilegeLevel: "owner"
}
@request.post = sinon.stub().callsArgWith(1, null, {statusCode: 200}, @response)
@WebApiManager.joinProject @project_id, @user, @callback
@ -48,9 +47,9 @@ describe 'WebApiManager', ->
})
.should.equal true
it "should return the project, privilegeLevel, and restricted flag", ->
it "should return the project and privilegeLevel", ->
@callback
.calledWith(null, @response.project, @response.privilegeLevel, @response.isRestrictedUser)
.calledWith(null, @response.project, @response.privilegeLevel)
.should.equal true
describe "with an error from web", ->
@ -63,16 +62,6 @@ describe 'WebApiManager', ->
.calledWith(new Error("non-success code from web: 500"))
.should.equal true
describe "with no data from web", ->
beforeEach ->
@request.post = sinon.stub().callsArgWith(1, null, {statusCode: 200}, null)
@WebApiManager.joinProject @project_id, @user_id, @callback
it "should call the callback with an error", ->
@callback
.calledWith(new Error("no data returned from joinProject request"))
.should.equal true
describe "when the project is over its rate limit", ->
beforeEach ->
@request.post = sinon.stub().callsArgWith(1, null, {statusCode: 429}, null)

View file

@ -53,8 +53,7 @@ describe 'WebsocketController', ->
}
@privilegeLevel = "owner"
@ConnectedUsersManager.updateUserPosition = sinon.stub().callsArg(4)
@isRestrictedUser = true
@WebApiManager.joinProject = sinon.stub().callsArgWith(2, null, @project, @privilegeLevel, @isRestrictedUser)
@WebApiManager.joinProject = sinon.stub().callsArgWith(2, null, @project, @privilegeLevel)
@RoomManager.joinProject = sinon.stub().callsArg(2)
@WebsocketController.joinProject @client, @user, @project_id, @callback
@ -96,9 +95,6 @@ describe 'WebsocketController', ->
it "should set the project owner id on the client", ->
@client.set.calledWith("owner_id", @owner_id).should.equal true
it "should set the is_restricted_user flag on the client", ->
@client.set.calledWith("is_restricted_user", @isRestrictedUser).should.equal true
it "should call the callback with the project, privilegeLevel and protocolVersion", ->
@callback
.calledWith(null, @project, @privilegeLevel, @WebsocketController.PROTOCOL_VERSION)
@ -403,20 +399,6 @@ describe 'WebsocketController', ->
it "should return an error", ->
@callback.calledWith(@err).should.equal true
describe "when restricted user", ->
beforeEach ->
@client.params.is_restricted_user = true
@AuthorizationManager.assertClientCanViewProject = sinon.stub().callsArgWith(1, null)
@WebsocketController.getConnectedUsers @client, @callback
it "should return an empty array of users", ->
@callback.calledWith(null, []).should.equal true
it "should not get the connected users for the project", ->
@ConnectedUsersManager.getConnectedUsers
.called
.should.equal false
describe "updateClientPosition", ->
beforeEach ->
@WebsocketLoadBalancer.emitToRoom = sinon.stub()

View file

@ -18,12 +18,6 @@ describe "WebsocketLoadBalancer", ->
"./RoomManager" : @RoomManager = {eventSource: sinon.stub().returns @RoomEvents}
"./ChannelManager": @ChannelManager = {publish: sinon.stub()}
"./ConnectedUsersManager": @ConnectedUsersManager = {refreshClient: sinon.stub()}
"./Utils": @Utils = {
getClientAttributes: sinon.spy(
(client, _attrs, callback) ->
callback(null, {is_restricted_user: !!client.__isRestricted})
)
}
@io = {}
@WebsocketLoadBalancer.rclientPubList = [{publish: sinon.stub()}]
@WebsocketLoadBalancer.rclientSubList = [{
@ -32,7 +26,7 @@ describe "WebsocketLoadBalancer", ->
}]
@room_id = "room-id"
@message = "otUpdateApplied"
@message = "message-to-editor"
@payload = ["argument one", 42]
describe "emitToRoom", ->
@ -76,7 +70,6 @@ describe "WebsocketLoadBalancer", ->
describe "_processEditorEvent", ->
describe "with bad JSON", ->
beforeEach ->
@isRestrictedUser = false
@SafeJsonParse.parse = sinon.stub().callsArgWith 1, new Error("oops")
@WebsocketLoadBalancer._processEditorEvent(@io, "editor-events", "blah")
@ -105,54 +98,6 @@ describe "WebsocketLoadBalancer", ->
@emit2.calledWith(@message, @payload...).should.equal true
@emit3.called.should.equal false # duplicate client should be ignored
describe "with a designated room, and restricted clients, not restricted message", ->
beforeEach ->
@io.sockets =
clients: sinon.stub().returns([
{id: 'client-id-1', emit: @emit1 = sinon.stub()}
{id: 'client-id-2', emit: @emit2 = sinon.stub()}
{id: 'client-id-1', emit: @emit3 = sinon.stub()} # duplicate client
{id: 'client-id-4', emit: @emit4 = sinon.stub(), __isRestricted: true}
])
data = JSON.stringify
room_id: @room_id
message: @message
payload: @payload
@WebsocketLoadBalancer._processEditorEvent(@io, "editor-events", data)
it "should send the message to all (unique) clients in the room", ->
@io.sockets.clients
.calledWith(@room_id)
.should.equal true
@emit1.calledWith(@message, @payload...).should.equal true
@emit2.calledWith(@message, @payload...).should.equal true
@emit3.called.should.equal false # duplicate client should be ignored
@emit4.called.should.equal true # restricted client, but should be called
describe "with a designated room, and restricted clients, restricted message", ->
beforeEach ->
@io.sockets =
clients: sinon.stub().returns([
{id: 'client-id-1', emit: @emit1 = sinon.stub()}
{id: 'client-id-2', emit: @emit2 = sinon.stub()}
{id: 'client-id-1', emit: @emit3 = sinon.stub()} # duplicate client
{id: 'client-id-4', emit: @emit4 = sinon.stub(), __isRestricted: true}
])
data = JSON.stringify
room_id: @room_id
message: @restrictedMessage = 'new-comment'
payload: @payload
@WebsocketLoadBalancer._processEditorEvent(@io, "editor-events", data)
it "should send the message to all (unique) clients in the room, who are not restricted", ->
@io.sockets.clients
.calledWith(@room_id)
.should.equal true
@emit1.calledWith(@restrictedMessage, @payload...).should.equal true
@emit2.calledWith(@restrictedMessage, @payload...).should.equal true
@emit3.called.should.equal false # duplicate client should be ignored
@emit4.called.should.equal false # restricted client, should not be called
describe "when emitting to all", ->
beforeEach ->
@io.sockets =
@ -165,3 +110,4 @@ describe "WebsocketLoadBalancer", ->
it "should send the message to all clients", ->
@emit.calledWith(@message, @payload...).should.equal true