Francesco Romani has posted comments on this change.

Change subject: core: VirtIO console access key management
......................................................................


Patch Set 8:

(3 comments)

https://gerrit.ovirt.org/#/c/39512/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserProfileCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddUserProfileCommand.java:

Line 34:         return getSucceeded() ? AuditLogType.USER_ADD_PROFILE : 
AuditLogType.USER_ADD_PROFILE_FAILED;
Line 35:     }
Line 36: 
Line 37:     @Override
Line 38:     protected void setActionMessageParameters() {
> this method should only contain the parameters for the generic messages, me
Got it and fixed. Thanks!
Line 39:         addCanDoActionMessage(VdcBllMessages.VAR__ACTION__ADD);
Line 40:         addCanDoActionMessage(VdcBllMessages.VAR__TYPE__USER_PROFILE);
Line 41:         
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_PROFILE_ALREADY_EXISTS);
Line 42:     }


https://gerrit.ovirt.org/#/c/39512/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveUserProfileCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveUserProfileCommand.java:

Line 23: 
Line 24:         UserProfile dbProfile = 
userProfileDao.getByUserId(getUserId());
Line 25:         UserProfile givenProfile = getParameters().getUserProfile();
Line 26: 
Line 27:         if (dbProfile == null || 
dbProfile.getId().equals(givenProfile.getId()) == false) {
> please prefer !a.equals(b) to a.equals(b) == false.
I guessed wrong :) Fixed.
Line 28:             log.warn("Profile ID mismatch: expected in profile {} 
received {}",
Line 29:                     dbProfile.getId().toString(), 
givenProfile.getId().toString());
Line 30:             setActionMessageParameters();
Line 31:             result = false;


https://gerrit.ovirt.org/#/c/39512/8/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UserProfilesOperationCommandBase.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UserProfilesOperationCommandBase.java:

Line 42:         return result;
Line 43:     }
Line 44: 
Line 45:     @Override
Line 46:     public List<PermissionSubject> getPermissionCheckSubjects() {
> i wonder about this,
I run this code before (v4: 
https://gerrit.ovirt.org/#/c/39512/4/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetUserProfileCommand.java
 )

and this worked fine, the key was updated. 
So I guess each user can edit himself (or this wasn't triggered :) ).
Line 47:         return Collections.singletonList(new 
PermissionSubject(getUserId(), VdcObjectType.User, 
ActionGroup.MANIPULATE_USERS));
Line 48:     }


-- 
To view, visit https://gerrit.ovirt.org/39512
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5ff5403823e752e695ebde76a4b7fb83e07099b6
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Martin Betak <mbe...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to