Yair Zaslavsky has posted comments on this change.

Change subject: Introduction of filters to unify AAA flows for UI and REST-API
......................................................................


Patch Set 50:

(4 comments)

http://gerrit.ovirt.org/#/c/28022/50/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/InvalidateSessionIfAuthorizationHeaderFilter.java
File 
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/InvalidateSessionIfAuthorizationHeaderFilter.java:

> please rename to RestApiSessionValidation or something as it is now specifi
we already have RestAPiSessionMgmtFilter.
I suggest maybe RestApiSessionInvalidationFilter?
Line 1: package org.ovirt.engine.core.aaa.filters;
Line 2: 
Line 3: import java.io.IOException;
Line 4: 


Line 10: import javax.servlet.ServletResponse;
Line 11: import javax.servlet.http.HttpServletRequest;
Line 12: import javax.servlet.http.HttpSession;
Line 13: 
Line 14: public class InvalidateSessionIfAuthorizationHeaderFilter implements 
Filter {
IMHO This filter class name should be changed.
SessionInvalidationFilter?
Line 15: 
Line 16:     @Override
Line 17:     public void init(FilterConfig filterConfig) throws 
ServletException {
Line 18:     }


http://gerrit.ovirt.org/#/c/28022/50/backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/RestApiSessionMgmtFilter.java
File 
backend/manager/modules/aaa/src/main/java/org/ovirt/engine/core/aaa/filters/RestApiSessionMgmtFilter.java:

Line 35:             ServletException {
Line 36:         HttpServletRequest req = (HttpServletRequest) request;
Line 37:         boolean persistentAuth =
Line 38:                 FiltersHelper.isPersistentAuth(req);
Line 39:         HttpSession session = req.getSession();
> you don't need this here^
Done
Line 40:         if (persistentAuth) {
Line 41:             try {
Line 42:                 int ttlValue = 
Integer.parseInt(req.getHeader("Session-TTL")) * SECONDS_IN_MINUTE;
Line 43:                 if (ttlValue >= MINIMAL_SESSION_TTL) {


Line 37:         boolean persistentAuth =
Line 38:                 FiltersHelper.isPersistentAuth(req);
Line 39:         HttpSession session = req.getSession();
Line 40:         if (persistentAuth) {
Line 41:             try {
> here you should create the session.
Done
Line 42:                 int ttlValue = 
Integer.parseInt(req.getHeader("Session-TTL")) * SECONDS_IN_MINUTE;
Line 43:                 if (ttlValue >= MINIMAL_SESSION_TTL) {
Line 44:                     session.setMaxInactiveInterval(ttlValue);
Line 45:                 }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5536d123b6407acf41b6946dde796bd67d1e073
Gerrit-PatchSet: 50
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Einav Cohen <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to