Ravi Nori has uploaded a new change for review.

Change subject: restapi : don't set jsessionid cookie when authentication 
fails(#927140)
......................................................................

restapi : don't set jsessionid cookie when authentication fails(#927140)

Rest APi returns session id when authentication fails and the user
makes calls with "prefer: persistent-auth"

Change-Id: I84907ab56e99ebb875124f42345d691edad3cdbe
Bug-Url: https://bugzilla.redhat.com/927140
Signed-off-by: Ravi Nori <[email protected]>
---
M 
backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/auth/Challenger.java
M 
backend/manager/modules/restapi/interface/common/jaxrs/src/test/java/org/ovirt/engine/api/common/security/auth/ChallengerTest.java
2 files changed, 16 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/42/14042/1

diff --git 
a/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/auth/Challenger.java
 
b/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/auth/Challenger.java
index d9aab40..41d7393 100644
--- 
a/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/auth/Challenger.java
+++ 
b/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/auth/Challenger.java
@@ -92,17 +92,17 @@
         boolean hasAuthorizationHeader = checkAuthorizationHeader(headers);
         Integer customHttpSessionTtl = getCustomHttpSessionTtl(headers);
 
-        // Will create a new one if it is the first session, and we want to 
persist sessions
-        // (and then the "isNew" test below will return true)
-        // Otherwise, it will return null
-        httpSession = getCurrentSession(preferPersistentAuth);
+        // Get the current session
+        // For persistent auth we will create a new session if authentication
+        // is successful
+        httpSession = getCurrentSession(false);
 
         // If the session isn't new and doesn't carry authorization header, we 
validate it
-        if (validator != null && httpSession != null && !httpSession.isNew() 
&& !hasAuthorizationHeader) {
+        if (validator != null && httpSession != null && 
!hasAuthorizationHeader) {
             successful = executeSessionValidation(httpSession, 
preferPersistentAuth);
         } else {
             // If the session isn't new but carries authorization header, we 
invalidate it first
-            if (validator != null && httpSession != null && 
!httpSession.isNew()) {
+            if (validator != null && httpSession != null) {
                 httpSession.invalidate();
                 httpSession = getCurrentSession(true);
             }
@@ -114,6 +114,9 @@
             // container will invalidate this session. An interval value of 
zero
             // or less indicates that the session should never timeout.
             if (successful && preferPersistentAuth) {
+                if (httpSession == null) {
+                    httpSession = getCurrentSession(false);
+                }
                 if (httpSession != null && customHttpSessionTtl != null) {
                     httpSession.setMaxInactiveInterval(
                             customHttpSessionTtl.intValue() * 
SECONDS_IN_MINUTE);
@@ -167,14 +170,15 @@
     private boolean executeBasicAuthentication(HttpHeaders headers, 
HttpSession httpSession, boolean preferPersistentAuth) {
         boolean successful = false;
         List<String> auth = 
headers.getRequestHeader(HttpHeaders.AUTHORIZATION);
-
-        String engineSessionId = SessionUtils.generateEngineSessionId();
-        SessionUtils.setEngineSessionId(httpSession, engineSessionId);
-
         if (auth != null && auth.size() != 0) {
             Principal principal = scheme.decode(headers);
+            String engineSessionId = SessionUtils.generateEngineSessionId();
             if (validator == null || validator.validate(principal, 
engineSessionId)) {
                 successful = true;
+                if (httpSession == null) {
+                    httpSession = getCurrentSession(true);
+                }
+                SessionUtils.setEngineSessionId(httpSession, engineSessionId);
                 updateAuthenticationProperties(preferPersistentAuth, 
principal);
             }
         }
diff --git 
a/backend/manager/modules/restapi/interface/common/jaxrs/src/test/java/org/ovirt/engine/api/common/security/auth/ChallengerTest.java
 
b/backend/manager/modules/restapi/interface/common/jaxrs/src/test/java/org/ovirt/engine/api/common/security/auth/ChallengerTest.java
index c322a7b..4f69296 100644
--- 
a/backend/manager/modules/restapi/interface/common/jaxrs/src/test/java/org/ovirt/engine/api/common/security/auth/ChallengerTest.java
+++ 
b/backend/manager/modules/restapi/interface/common/jaxrs/src/test/java/org/ovirt/engine/api/common/security/auth/ChallengerTest.java
@@ -157,7 +157,8 @@
     @Test
     public void testValidateSessionFalseOnNewSession() {
         HttpSession httpSession = new TestHttpSession(sessionId, true);
-        doReturn(httpSession).when(challenger).getCurrentSession(anyBoolean());
+        doReturn(httpSession).when(challenger).getCurrentSession(true);
+        doReturn(null).when(challenger).getCurrentSession(false);
         challenger.setValidator(new ConstValidator(true, sessionId));
         ResourceMethod resource = control.createMock(ResourceMethod.class);
         ServerResponse response = 
challenger.preProcess(setUpRequestExpectations(null, true, false), resource);


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I84907ab56e99ebb875124f42345d691edad3cdbe
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: engine_3.2
Gerrit-Owner: Ravi Nori <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to