Vojtech Szocs has posted comments on this change.

Change subject: restapi: CSRF protection filter
......................................................................


Patch Set 1: Code-Review+1

(2 comments)

Looks good to me, please see my minor comment in CSRFProtectionFilter.

http://gerrit.ovirt.org/#/c/29681/1/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/CSRFProtectionFilter.java
File 
backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/CSRFProtectionFilter.java:

Line 104:     private void doFilter(HttpServletRequest request, 
HttpServletResponse response, FilterChain chain)
Line 105:             throws IOException, ServletException {
Line 106:         // If protection is globally disabled then we don't need to 
do anything else, jump directly to the next filter
Line 107:         // in the chain:
Line 108:         boolean enabled = 
Config.getValue(ConfigValues.CSRFProtection);
If ConfigValues.CSRFProtection parameter is not "reloadable" (its change must 
be followed by Engine restart), above line could be placed in filter's init 
method.
Line 109:         if (!enabled) {
Line 110:             chain.doFilter(request, response);
Line 111:             return;
Line 112:         }


Line 121: 
Line 122:         // At this point we know that protection is globally enabled, 
and that there isn't a session already created. So
Line 123:         // we should first let the other filters and the application 
do their work. As a result a new session may be
Line 124:         // created. In that case we need to check if protection has 
been requested for that session and store the result
Line 125:         // for use in future requests.
Excellent comment.
Line 126:         try {
Line 127:             chain.doFilter(request, response);
Line 128:         }
Line 129:         finally {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5700192b62e514091c9f29910596f312c068c5b2
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[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