SLING-10319 : Use the equals method if value comparison was intended
diff --git a/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java b/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
index df1dacc..d075e61 100644
--- a/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
+++ b/src/main/java/org/apache/sling/auth/core/impl/SlingAuthenticator.java
@@ -1306,32 +1306,32 @@
* @return <code>true</code> if the cookie has been set or cleared or
* <code>false</code> if the cookie is not modified.
*/
- private boolean setSudoCookie(HttpServletRequest req,
+ boolean setSudoCookie(HttpServletRequest req,
HttpServletResponse res, AuthenticationInfo authInfo) {
- String sudo = (String) authInfo.get(ResourceResolverFactory.USER_IMPERSONATION);
- String currentSudo = getSudoCookieValue(req);
+ final String sudo = (String) authInfo.get(ResourceResolverFactory.USER_IMPERSONATION);
+ final String currentSudo = getSudoCookieValue(req);
+
+ boolean changed = false;
// set the (new) impersonation
- final boolean setCookie = sudo != currentSudo;
- if (setCookie) {
- if (sudo == null) {
- // Parameter set to "-" to clear impersonation, which was
- // active due to cookie setting
+ if (sudo == null && currentSudo != null) {
+ // Parameter set to "-" to clear impersonation, which was
+ // active due to cookie setting
- // clear impersonation
- this.sendSudoCookie(req, res, "", 0, req.getContextPath(), authInfo.getUser());
+ // clear impersonation
+ this.sendSudoCookie(req, res, "", 0, req.getContextPath(), authInfo.getUser());
+ changed = true;
- } else if (currentSudo == null || !currentSudo.equals(sudo)) {
- // Parameter set to a name. As the cookie is not set yet
- // or is set to another name, send the cookie with current sudo
+ } else if (sudo != null && !sudo.equals(currentSudo)) {
+ // Parameter set to a name. As the cookie is not set yet
+ // or is set to another name, send the cookie with current sudo
- // (re-)set impersonation
- this.sendSudoCookie(req, res, sudo, -1, req.getContextPath(),
- sudo);
- }
+ // (re-)set impersonation
+ this.sendSudoCookie(req, res, sudo, -1, req.getContextPath(), sudo);
+ changed = true;
}
- return setCookie;
+ return changed;
}
/**
diff --git a/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticatorTest.java b/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticatorTest.java
index 4e8ca72..2c14fef 100644
--- a/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticatorTest.java
+++ b/src/test/java/org/apache/sling/auth/core/impl/SlingAuthenticatorTest.java
@@ -18,21 +18,31 @@
*/
package org.apache.sling.auth.core.impl;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.never;
+
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletRequestEvent;
+import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.SlingHttpServletResponse;
import org.apache.sling.api.resource.ResourceResolver;
+import org.apache.sling.api.resource.ResourceResolverFactory;
import org.apache.sling.auth.core.AuthenticationSupport;
import org.apache.sling.auth.core.spi.AuthenticationFeedbackHandler;
import org.apache.sling.auth.core.spi.AuthenticationInfo;
import org.apache.sling.commons.metrics.MetricsService;
import org.junit.Assert;
import org.junit.Test;
+import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;
import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
@@ -354,6 +364,61 @@
Mockito.verify(request).removeAttribute(AuthenticationSupport.REQUEST_ATTRIBUTE_RESOLVER);
}
+ @Test public void testSetSudoCookieNoSudoBeforeNoSudoAfter() {
+ final SlingAuthenticator slingAuthenticator = this.createSlingAuthenticator();
+ final AuthenticationInfo info = new AuthenticationInfo("basic");
+
+ final SlingHttpServletRequest req = Mockito.mock(SlingHttpServletRequest.class);
+ final SlingHttpServletResponse res = Mockito.mock(SlingHttpServletResponse.class);
+
+ assertFalse(slingAuthenticator.setSudoCookie(req, res, info));
+ Mockito.verify(res, never()).addCookie(Mockito.any());
+ }
+
+ @Test public void testSetSudoCookieNoSudoBeforeSudoAfter() {
+ final SlingAuthenticator slingAuthenticator = this.createSlingAuthenticator();
+ final AuthenticationInfo info = new AuthenticationInfo("basic");
+ info.put(ResourceResolverFactory.USER_IMPERSONATION, "newsudo");
+
+ final SlingHttpServletRequest req = Mockito.mock(SlingHttpServletRequest.class);
+ final SlingHttpServletResponse res = Mockito.mock(SlingHttpServletResponse.class);
+
+ assertTrue(slingAuthenticator.setSudoCookie(req, res, info));
+ ArgumentCaptor<Cookie> argument = ArgumentCaptor.forClass(Cookie.class);
+ Mockito.verify(res).addCookie(argument.capture());
+ assertEquals("\"newsudo\"", argument.getValue().getValue());
+ }
+
+ @Test public void testSetSudoCookieSudoBeforeSameSudoAfter() {
+ final SlingAuthenticator slingAuthenticator = this.createSlingAuthenticator();
+ final AuthenticationInfo info = new AuthenticationInfo("basic");
+ info.put(ResourceResolverFactory.USER_IMPERSONATION, "oldsudo");
+
+ final SlingHttpServletRequest req = Mockito.mock(SlingHttpServletRequest.class);
+ final Cookie cookie = new Cookie("sling.sudo", "\"oldsudo\"");
+ Mockito.when(req.getCookies()).thenReturn(new Cookie[] {cookie});
+ final SlingHttpServletResponse res = Mockito.mock(SlingHttpServletResponse.class);
+
+ assertFalse(slingAuthenticator.setSudoCookie(req, res, info));
+ Mockito.verify(res, never()).addCookie(Mockito.any());
+ }
+
+ @Test public void testSetSudoCookieSudoBeforeNewSudoAfter() {
+ final SlingAuthenticator slingAuthenticator = this.createSlingAuthenticator();
+ final AuthenticationInfo info = new AuthenticationInfo("basic");
+ info.put(ResourceResolverFactory.USER_IMPERSONATION, "newsudo");
+
+ final SlingHttpServletRequest req = Mockito.mock(SlingHttpServletRequest.class);
+ final Cookie cookie = new Cookie("sling.sudo", "\"oldsudo\"");
+ Mockito.when(req.getCookies()).thenReturn(new Cookie[] {cookie});
+ final SlingHttpServletResponse res = Mockito.mock(SlingHttpServletResponse.class);
+
+ assertTrue(slingAuthenticator.setSudoCookie(req, res, info));
+ ArgumentCaptor<Cookie> argument = ArgumentCaptor.forClass(Cookie.class);
+ Mockito.verify(res).addCookie(argument.capture());
+ assertEquals("\"newsudo\"", argument.getValue().getValue());
+ }
+
//---------------------------- PRIVATE METHODS -----------------------------
/**
