Martin Peřina has posted comments on this change. Change subject: aaa: Remove userId parameter from LogoutUserCommand ......................................................................
Patch Set 1: @Alon: > > I suggest to split between various use cases to reduce the chance of breaking > security. > > 1. internal usage - restapi logout - only internal usage. Only REST API calls LogoutBySessionCommand. But internally LogoutBySessionCommand just calls LogoutUserCommand using Backend.logoff() method > > 2. user self logout - based on existing mechanism as session id is already > provided, opened for everyone. > GWT calls Backend.logoff() which executes LogoutUserCommand > > 3. force logout of admin based on session id (numeric id) - only superuser or > other role. > That is implemented in TerminateSessionCommand. Internally LogoutBySessionCommand just calls LogoutUserCommand, but I agree that is a bit different then 1. and 2. and we should put here some logging so we know that user XYZ terminated session of user ABC > > maybe in current design of engine it is 3 commands, I am fine with it. So it looks like that currently we have what you want. But 1. and 2. contains completely same logic and IMO we could merge them together to eliminate some spaghetti code. If we want to have different logic in 1. and 2. we could always split them in future. Anyway in all cases sessionId of user that should be logouted have to passed as a parameter. And in case 3. we should also pass sessionId of admin user that executed logout of other user. And also there's no way how prevent that for example some future UI patch will call LogoutBySessionCommand, that has be handled by reviewers. -- To view, visit https://gerrit.ovirt.org/38403 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia33c7dfd908c68ac06b717c0452e3de4564f35a7 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-Reviewer: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
