Yair Zaslavsky has posted comments on this change.

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


Patch Set 1:

(3 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:
please see the commit msg.
I remember what we discussed, but -
as session invalidation runs every 30 minutes,  and we said that valid_to 
cannot exceed this, why not simply set valid to , to what is read from 
extension - if validTo is sooner than the next invalidation interval - the 
session will be removed. if validTo is after the next invalidation interval - 
the session will be removed at the next invalidation interval.
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 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 n
agreed.
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() ?
right, well anyway i will remove this following the above comment.
Line 58:                                                     Config.<Integer> 
getValue(ConfigValues.UserSessionTimeOutInterval)
Line 59:                                                     )
Line 60:                                             )
Line 61:                             )


-- 
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: 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