Alon Bar-Lev has posted comments on this change.

Change subject: aaa: Added usage of AuthRecord.VALID_TO
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.ovirt.org/#/c/26975/1/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 128:             } catch (ParseException e) {
Line 129:                 log.warn("Error parsing AuthRecord.VALID_TO . Default 
VALID_TO value will be set on session");
Line 130:                 validTo = DateUtils.addMinutes(new 
Date(System.currentTimeMillis()), Config.<Integer> 
getValue(ConfigValues.UserSessionTimeOutInterval));
Line 131:             }
Line 132:             SessionDataContainer.getInstance().setValidTo(validTo);
should be:

 valid_to = default
 if we have valid_to from extesion
    valid_to = min(valid_to, valid_to_from_extension
Line 133:         }
Line 134:         return true;
Line 135:     }
Line 136: 


http://gerrit.ovirt.org/#/c/26975/1/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/internal/InternalAuthn.java
File 
backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/internal/InternalAuthn.java:

Line 10: import org.ovirt.engine.api.extensions.ExtMap;
Line 11: import org.ovirt.engine.api.extensions.Extension;
Line 12: import org.ovirt.engine.api.extensions.aaa.Authn;
Line 13: import org.ovirt.engine.core.common.config.Config;
Line 14: import org.ovirt.engine.core.common.config.ConfigValues;
this implies that you are doing something wrong, please do not add core classes.
Line 15: 
Line 16: /**
Line 17:  * This authenticator authenticates the internal user as specified in 
the {@code AdminUser} and {@code AdminPassword}
Line 18:  * configuration parameters stored in the database. Currently it is in 
an interim status of development as


Line 50:                     new ExtMap().mput(
Line 51:                             Authn.AuthRecord.PRINCIPAL,
Line 52:                             adminUser
Line 53:                             ).mput(
Line 54:                                     Authn.AuthRecord.VALID_TO,
but this is not required here... you just do not set VALID_TO if you have no 
restriction for this user, then the core will set the config value.
Line 55:                                     new SimpleDateFormat("yyyy-MM-dd 
HH:mm:ssZ").format(
Line 56:                                             DateUtils.addMinutes(
Line 57:                                                     new 
Date(System.currentTimeMillis()),
Line 58:                                                     Config.<Integer> 
getValue(ConfigValues.UserSessionTimeOutInterval)


Line 53:                             ).mput(
Line 54:                                     Authn.AuthRecord.VALID_TO,
Line 55:                                     new SimpleDateFormat("yyyy-MM-dd 
HH:mm:ssZ").format(
Line 56:                                             DateUtils.addMinutes(
Line 57:                                                     new 
Date(System.currentTimeMillis()),
why not just new Date() ?
Line 58:                                                     Config.<Integer> 
getValue(ConfigValues.UserSessionTimeOutInterval)
Line 59:                                                     )
Line 60:                                             )
Line 61:                             )


http://gerrit.ovirt.org/#/c/26975/1/backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/LdapAuthenticateUserCommand.java
File 
backend/manager/modules/builtin-extensions/src/main/java/org/ovirt/engine/extensions/aaa/builtin/kerberosldap/LdapAuthenticateUserCommand.java:

Line 74:                                                                 
Config.<Integer> getValue(ConfigValues.UserSessionTimeOutInterval)
Line 75:                                                                 )
Line 76:                                                         )
Line 77:                                         )
Line 78:                         );
same...
Line 79: 
Line 80: 
Line 81:                 setSucceeded(true);
Line 82:             } else {


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

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