Oved Ourfali has posted comments on this change.
Change subject: restapi : don't set jsessionid cookie when authentication
fails(#927140)
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(2 inline comments)
Some comments.
Also, please please please make sure that working with a session id works well
in all cases.
* One flow:
login (with the prefer header)
action (with the prefer header)
action (with the prefer header)
action (with the prefer header)
logout (without the prefer header)
* Other flow without prefer at all, just running actions.
and etc.
....................................................
File
backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/auth/Challenger.java
Line 97: // is successful
Line 98: httpSession = getCurrentSession(false);
Line 99:
Line 100: // If the session isn't new and doesn't carry authorization
header, we validate it
Line 101: if (validator != null && httpSession != null &&
!httpSession.isNew() && !hasAuthorizationHeader) {
I think that the httpSession.isNew can be removed, as the session will never be
new, because we always pass "false" to getCurrentSession. Can you make sure I'm
correct?
Line 102: successful = executeSessionValidation(httpSession,
preferPersistentAuth);
Line 103: } else {
Line 104: // If the session isn't new but carries authorization
header, we invalidate it first
Line 105: if (validator != null && httpSession != null &&
!httpSession.isNew()) {
Line 110: String engineSessionId =
SessionUtils.generateEngineSessionId();
Line 111: // Authenticate the session
Line 112: successful = executeBasicAuthentication(headers,
engineSessionId, preferPersistentAuth);
Line 113: if (successful && httpSession == null) {
Line 114: httpSession = getCurrentSession(true);
We shouldn't create a new session if the preferPersistentAuth isn't true, so
perhaps you can move this section to inside the if below?
Line 115: }
Line 116: if (httpSession != null) {
Line 117: SessionUtils.setEngineSessionId(httpSession,
engineSessionId);
Line 118: }
--
To view, visit http://gerrit.ovirt.org/13371
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I84907ab56e99ebb875124f42345d691edad3cdbe
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
