Oved Ourfali has posted comments on this change. Change subject: aaa: Intorduce filters ......................................................................
Patch Set 21: (2 comments) http://gerrit.ovirt.org/#/c/28022/21/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 107: ) Line 108: ); Line 109: if (outputMap.<Integer> get(Base.InvokeKeys.RESULT) == Base.InvokeResult.SUCCESS && Line 110: outputMap.<Integer> get(Authn.InvokeKeys.RESULT) == Authn.AuthResult.SUCCESS) { Line 111: request.getSession(true); > IMHO this should be the place the session should be created if doesn't exis iiuc you ask whether it is right to do request.getSession(true) in this scope, right? If so, makes sense to me. I'd extract it to another method, though, just to emphasize it. Line 112: request.setAttribute(FiltersHelper.Constants.REQUEST_AUTH_RECORD_KEY, Line 113: outputMap.<ExtMap> get(Authn.InvokeKeys.AUTH_RECORD)); Line 114: request.setAttribute(FiltersHelper.Constants.REQUEST_PROFILE_KEY, userProfile.profile); Line 115: } else { Line 108: ); Line 109: if (outputMap.<Integer> get(Base.InvokeKeys.RESULT) == Base.InvokeResult.SUCCESS && Line 110: outputMap.<Integer> get(Authn.InvokeKeys.RESULT) == Authn.AuthResult.SUCCESS) { Line 111: request.getSession(true); Line 112: request.setAttribute(FiltersHelper.Constants.REQUEST_AUTH_RECORD_KEY, What about having the same session creation also on the negotiation filter? Perhaps you can even have a generic function of acquiring the session and setting the authentication record, as you'll probably need it in other filters as well. Thoughts? Line 113: outputMap.<ExtMap> get(Authn.InvokeKeys.AUTH_RECORD)); Line 114: request.setAttribute(FiltersHelper.Constants.REQUEST_PROFILE_KEY, userProfile.profile); Line 115: } else { Line 116: log.error( -- 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: 21 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@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
