Alon Bar-Lev has posted comments on this change.

Change subject: aaa: Introducing usage of Acct
......................................................................


Patch Set 16:

(11 comments)

http://gerrit.ovirt.org/#/c/27070/16/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AcctUtils.java
File 
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/AcctUtils.java:

Line 36:     public static void reportRecords(
Line 37:             int reportReason,
Line 38:             ExtMap authRecord,
Line 39:             ExtMap principalRecord,
Line 40:             String user,
please move user to 2nd parameter to be ordered.... user, auth record, 
principal record per what we have in reality
Line 41:             String message,
Line 42:             Object... msgArgs
Line 43:             ) {
Line 44:         ExtMap input = new ExtMap();


http://gerrit.ovirt.org/#/c/27070/16/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/BasicAuthenticationFilter.java
File 
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/BasicAuthenticationFilter.java:

Line 143:                             "Basic authentication failed for User 
%1$s.",
Line 144:                             userProfile.userName
Line 145:                             );
Line 146:                 }
Line 147:                 if (outputMap.<Integer> get(Authn.InvokeKeys.RESULT) 
!= Authn.AuthResult.SUCCESS) {
use or with two conditions as logic is the same?
Line 148:                     AcctUtils.reportRecords(
Line 149:                             Acct.ReportReason.PRINCIPAL_LOGIN_FAILED,
Line 150:                             null,
Line 151:                             null,


http://gerrit.ovirt.org/#/c/27070/16/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/NegotiationFilter.java
File 
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/NegotiationFilter.java:

Line 153:                         case Authn.AuthResult.SUCCESS:
Line 154:                             ExtMap authRecord = output.<ExtMap> 
get(Authn.InvokeKeys.AUTH_RECORD);
Line 155:                             
req.setAttribute(FiltersHelper.Constants.REQUEST_AUTH_RECORD_KEY, authRecord);
Line 156:                         
req.setAttribute(FiltersHelper.Constants.REQUEST_AUTH_TYPE_KEY,
Line 157:                                 AuthType.NEGOTIATION);
indent?
Line 158:                             
req.setAttribute(FiltersHelper.Constants.REQUEST_PROFILE_KEY, profile);
Line 159:                             session.removeAttribute(STACK_ATTR);
Line 160:                         stack.clear();
Line 161:                             break;


http://gerrit.ovirt.org/#/c/27070/16/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java:

Line 188:                         null,
Line 189:                         null,
Line 190:                         null,
Line 191:                         "Invalid user was passed. Credentials based 
authentication has failed"
Line 192:                         );
send the user as field and within message?
Line 193:                 return false;
Line 194:             }
Line 195:             String password = getParameters().getPassword();
Line 196:             if (password == null) {


Line 202:                         
Acct.ReportReason.PRINCIPAL_CREDENTIALS_INVALID,
Line 203:                         null,
Line 204:                         null,
Line 205:                         null,
Line 206:                         "Invalid credentials were passed. Credentails 
based authentication has failed"
same... but why here it is credentials? you can always send auth record and 
principal record, so if null they will just be ignored... but at least all 
calls will be consistent.
Line 207:                         );
Line 208:                 return false;
Line 209:             }
Line 210: 


http://gerrit.ovirt.org/#/c/27070/16/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/session/SessionDataContainer.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/session/SessionDataContainer.java:

Line 95:      * @param sessionId
Line 96:      *            - id of current session
Line 97:      */
Line 98:     public final void removeSessionOnLogout(String sessionId) {
Line 99:         removeSessionImpl(sessionId, 
Acct.ReportReason.PRINCIPAL_LOGOUT, "Prinicial has performed logout");
add principal name to message?
Line 100:     }
Line 101: 
Line 102:     /**
Line 103:      * Will run the process of cleaning expired sessions.


Line 111:             ConcurrentMap<String, Object> sessionMap = 
entry.getValue().contentOfSession;
Line 112:             Date hardLimit = (Date) 
sessionMap.get(HARD_LIMIT_PARAMETER_NAME);
Line 113:             Date softLimit = (Date) 
sessionMap.get(SOFT_LIMIT_PARAMETER_NAME);
Line 114:             if ((hardLimit != null && hardLimit.before(now)) || 
(softLimit != null && softLimit.before(now))) {
Line 115:                 removeSessionImpl(entry.getKey(), 
Acct.ReportReason.PRINCIPAL_SESSION_EXPIRED, "Session has expired");
add principal name to message?
Line 116:             }
Line 117:         }
Line 118:     }
Line 119: 


http://gerrit.ovirt.org/#/c/27070/16/backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/aaa/Acct.java
File 
backend/manager/modules/extensions-api-root/extensions-api/src/main/java/org/ovirt/engine/api/extensions/aaa/Acct.java:

Line 64:         public static final int STARTUP = 0;
Line 65:         /** Application shutdown. */
Line 66:         public static final int SHUTDOWN = 1;
Line 67:         /**
Line 68:          * User is invalid.
Will have {@link PrincipalRecord}.
Line 69:          */
Line 70:         public static final int USER_INVALID = 2;
Line 71:         /**
Line 72:          * Principal record was not found.


Line 68:          * User is invalid.
Line 69:          */
Line 70:         public static final int USER_INVALID = 2;
Line 71:         /**
Line 72:          * Principal record was not found.
Will have {@link PrincipalRecord}.
Line 73:          */
Line 74:         public static final int PRINCIPAL_INVALID = 3;
Line 75:         /**
Line 76:          * Login failed by any reason but locked.


Line 117:          * Will have {@link PrincipalRecord}.
Line 118:          */
Line 119:         public static final int PRINCIPAL_CREDENTIALS_CHANGED = 12;
Line 120:         /**
Line 121:          * Invalid credentials.
Will have {@link PrincipalRecord}.
Line 122:          */
Line 123:         public static final int PRINCIPAL_CREDENTIALS_INVALID = 13;
Line 124:         /**
Line 125:          * Access denied.


Line 119:         public static final int PRINCIPAL_CREDENTIALS_CHANGED = 12;
Line 120:         /**
Line 121:          * Invalid credentials.
Line 122:          */
Line 123:         public static final int PRINCIPAL_CREDENTIALS_INVALID = 13;
this should be PRINCIPAL_LOGIN_FAILED, no?
Line 124:         /**
Line 125:          * Access denied.
Line 126:          */
Line 127:         public static final int ACCESS_DENIED = 14;


-- 
To view, visit http://gerrit.ovirt.org/27070
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ief13d233d11b7ab32b328735b4f58ec7cffff567
Gerrit-PatchSet: 16
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@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