fix: GWT servlet review (#5190)

Signed-off-by: Nicola Timeus <nicola.timeus@eurotech.com>
eclipse-kura · Mar 26, 2024 · 3e47263 · 3e47263
1 parent 29fbc21
commit 3e47263
Show file tree

Hide file tree

Showing 24 changed files with 267 additions and 215 deletions.
diff --git a/kura/org.eclipse.kura.core.identity/about.html b/kura/org.eclipse.kura.core.identity/about.html
@@ -33,4 +33,4 @@ <h3>License</h3>
 </p>
 
 </body>
-</html>
+</html>
diff --git a/kura/org.eclipse.kura.core.identity/about_files/epl-v20.html b/kura/org.eclipse.kura.core.identity/about_files/epl-v20.html
@@ -298,4 +298,4 @@ <h2 id="exhibit-a">Exhibit A &ndash; Form of Secondary Licenses Notice</h2>
   <p>You may add additional accurate notices of copyright ownership.</p>
 </blockquote>
 </body>
-</html>
+</html>
diff --git a/...ra.rest.provider/src/main/java/org/eclipse/kura/internal/rest/auth/RestSessionHelper.java b/...ra.rest.provider/src/main/java/org/eclipse/kura/internal/rest/auth/RestSessionHelper.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2023 Eurotech and/or its affiliates and others
+ * Copyright (c) 2023, 2024 Eurotech and/or its affiliates and others
  * 
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -58,7 +58,8 @@ public HttpSession createNewAuthenticatedSession(final HttpServletRequest reques
 
         getOrCreateXsrfToken(newSession);
 
-        AuditContext.currentOrInternal().getProperties().put("session.id", newSession.getId());
+        AuditContext.currentOrInternal().getProperties().put("session.id",
+                Integer.toUnsignedString(Objects.hash(newSession.getId())));
 
         return newSession;
     }

diff --git a/....rest.provider/src/main/java/org/eclipse/kura/internal/rest/auth/SessionAuthProvider.java b/....rest.provider/src/main/java/org/eclipse/kura/internal/rest/auth/SessionAuthProvider.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2023 Eurotech and/or its affiliates and others
+ * Copyright (c) 2023, 2024 Eurotech and/or its affiliates and others
  * 
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -13,6 +13,7 @@
 package org.eclipse.kura.internal.rest.auth;
 
 import java.security.Principal;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -73,7 +74,7 @@ public Optional<Principal> authenticate(final HttpServletRequest request,
             return Optional.empty();
         }
 
-        auditContext.getProperties().put("session.id", session.get().getId());
+        auditContext.getProperties().put("session.id", Integer.toUnsignedString(Objects.hash(session.get().getId())));
 
         final Optional<Principal> result = this.sessionHelper.getPrincipalFromSession(session.get());
 

diff --git a/kura/org.eclipse.kura.web2/src/main/java/org/eclipse/kura/web/Console.java b/kura/org.eclipse.kura.web2/src/main/java/org/eclipse/kura/web/Console.java
@@ -506,7 +506,7 @@ private synchronized void initHTTPService() throws NamespaceException, ServletEx
                 this.sessionContext);
         this.httpService.registerServlet(DENALI_MODULE_PATH + "/assetsUpDownload", new ChannelServlet(), null,
                 this.sessionContext);
-        this.httpService.registerServlet(DENALI_MODULE_PATH + "/log", new LogServlet(), null, resourceContext);
+        this.httpService.registerServlet(DENALI_MODULE_PATH + "/log", new LogServlet(), null, this.sessionContext);
         this.httpService.registerServlet(DENALI_MODULE_PATH + "/skin", new SkinServlet(), null, resourceContext);
         this.httpService.registerServlet(DENALI_MODULE_PATH + "/cloudservices", new GwtCloudConnectionServiceImpl(),
                 null, this.sessionContext);

diff --git a/.../org.eclipse.kura.web2/src/main/java/org/eclipse/kura/web/server/GwtEventServiceImpl.java b/.../org.eclipse.kura.web2/src/main/java/org/eclipse/kura/web/server/GwtEventServiceImpl.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2016, 2021 Eurotech and/or its affiliates and others
+ * Copyright (c) 2016, 2024 Eurotech and/or its affiliates and others
  * 
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -20,7 +20,10 @@
 import java.util.LinkedList;
 import java.util.List;
 
+import javax.servlet.http.HttpSession;
+
 import org.eclipse.kura.core.configuration.ConfigurationChangeEvent;
+import org.eclipse.kura.web.server.util.GwtServerUtil;
 import org.eclipse.kura.web.shared.ForwardedEventTopic;
 import org.eclipse.kura.web.shared.model.GwtEventInfo;
 import org.eclipse.kura.web.shared.service.GwtEventService;
@@ -81,11 +84,13 @@ private List<GwtEventInfo> getEvents(long fromTimestamp) {
         while (i.hasNext()) {
             GwtEventInfo next = i.next();
 
+            final HttpSession session = getThreadLocalRequest().getSession(false);
+
             // ignore concurrency events raised by myself
             if (next.getTopic().equals(ConfigurationChangeEvent.CONF_CHANGE_EVENT_TOPIC)
-                    && getThreadLocalRequest().getSession(false) != null) {
+                    && session != null) {
 
-                String currentSession = getThreadLocalRequest().getSession().getId();
+                String currentSession = GwtServerUtil.getSessionIdHash(session);
                 String eventSession = (String) next.get(ConfigurationChangeEvent.CONF_CHANGE_EVENT_SESSION_PROP);
 
                 if (currentSession.equals(eventSession)) {

diff --git a/....web2/src/main/java/org/eclipse/kura/web/server/GwtPasswordAuthenticationServiceImpl.java b/....web2/src/main/java/org/eclipse/kura/web/server/GwtPasswordAuthenticationServiceImpl.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2019, 2022 Eurotech and/or its affiliates and others
+ * Copyright (c) 2019, 2024 Eurotech and/or its affiliates and others
  * 
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -20,6 +20,7 @@
 import org.eclipse.kura.audit.AuditContext;
 import org.eclipse.kura.web.Console;
 import org.eclipse.kura.web.UserManager;
+import org.eclipse.kura.web.server.util.GwtServerUtil;
 import org.eclipse.kura.web.session.Attributes;
 import org.eclipse.kura.web.shared.GwtKuraException;
 import org.eclipse.kura.web.shared.model.GwtPasswordAuthenticationResult;
@@ -57,7 +58,7 @@ public GwtPasswordAuthenticationResult authenticate(final String username, final
 
             this.userManager.authenticateWithPassword(username, password);
 
-            context.getProperties().put("session.id", session.getId());
+            context.getProperties().put("session.id", GwtServerUtil.getSessionIdHash(session));
 
             Console.instance().setAuthenticated(session, username, context.copy());
 

diff --git a/...ipse.kura.web2/src/main/java/org/eclipse/kura/web/server/GwtSecurityTokenServiceImpl.java b/...ipse.kura.web2/src/main/java/org/eclipse/kura/web/server/GwtSecurityTokenServiceImpl.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011, 2021 Eurotech and/or its affiliates and others
+ * Copyright (c) 2011, 2024 Eurotech and/or its affiliates and others
  *
  * This program and the accompanying materials are made
  * available under the terms of the Eclipse Public License 2.0
@@ -12,31 +12,21 @@
  *******************************************************************************/
 package org.eclipse.kura.web.server;
 
-import static java.util.Objects.isNull;
-
-import java.io.UnsupportedEncodingException;
-import java.security.NoSuchAlgorithmException;
-import java.util.Arrays;
 import java.util.Optional;
+import java.util.UUID;
 
-import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpSession;
 
-import org.eclipse.kura.crypto.CryptoService;
-import org.eclipse.kura.web.server.util.ServiceLocator;
-import org.eclipse.kura.web.shared.GwtKuraException;
+import org.eclipse.kura.web.session.Attributes;
 import org.eclipse.kura.web.shared.model.GwtXSRFToken;
 import org.eclipse.kura.web.shared.service.GwtSecurityTokenService;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.FrameworkUtil;
-import org.osgi.framework.ServiceReference;
 
-import com.google.gwt.user.client.rpc.RpcTokenException;
 import com.google.gwt.user.client.rpc.SerializationException;
 
 /**
- * This is the security token service, a concrete implementation to fix the XSFR security problem.
+ * This is the security token service, a concrete implementation to fix the XSFR
+ * security problem.
  */
 public class GwtSecurityTokenServiceImpl extends OsgiRemoteServiceServlet implements GwtSecurityTokenService {
 
@@ -72,23 +62,23 @@ public HttpSession getHttpSession() {
     public GwtXSRFToken generateSecurityToken() {
 
         HttpServletRequest httpServletRequest = getRequest();
-        Optional<Cookie> cookie = Arrays.stream(httpServletRequest.getCookies())
-                .filter(c -> "JSESSIONID".equals(c.getName())).findAny();
 
-        if (!cookie.isPresent() || isNull(cookie.get().getValue()) || cookie.get().getValue().isEmpty()) {
-            throw new RpcTokenException("Unable to generate XSRF cookie: the session cookie is not set or empty!");
-        }
+        final HttpSession session = httpServletRequest.getSession(false);
 
-        final BundleContext context = FrameworkUtil.getBundle(GwtSecurityTokenServiceImpl.class).getBundleContext();
-        final ServiceReference<CryptoService> ref = context.getServiceReference(CryptoService.class);
-        try {
-            CryptoService cryptoService = ServiceLocator.getInstance().getService(ref);
-            return new GwtXSRFToken(cryptoService.sha1Hash(cookie.get().getValue()));
-        } catch (GwtKuraException | NoSuchAlgorithmException | UnsupportedEncodingException e) {
-            throw new RpcTokenException("Unable to generate XSRF cookie: the crypto service is unavailable!");
-        } finally {
-            context.ungetService(ref);
+        final Optional<String> existingToken = Optional
+                .ofNullable(session.getAttribute(Attributes.XSRF_TOKEN.getValue()))
+                .filter(String.class::isInstance).map(String.class::cast);
+
+        if (existingToken.isPresent()) {
+            return new GwtXSRFToken(existingToken.get());
         }
 
+        final UUID token = UUID.randomUUID();
+
+        final String asString = token.toString();
+
+        session.setAttribute(Attributes.XSRF_TOKEN.getValue(), asString);
+
+        return new GwtXSRFToken(asString);
     }
-}
+}
diff --git a/...rg.eclipse.kura.web2/src/main/java/org/eclipse/kura/web/server/GwtSessionServiceImpl.java b/...rg.eclipse.kura.web2/src/main/java/org/eclipse/kura/web/server/GwtSessionServiceImpl.java
@@ -36,7 +36,7 @@
 
 public class GwtSessionServiceImpl extends OsgiRemoteServiceServlet implements GwtSessionService {
 
-    private static final Logger logger = LoggerFactory.getLogger(GwtSessionServiceImpl.class);
+    private static final Logger auditLogger = LoggerFactory.getLogger("AuditLogger");
 
     private final UserManager userManager;
 
@@ -60,10 +60,10 @@ public void logout(GwtXSRFToken xsrfToken) throws GwtKuraException {
 
         if (session != null) {
             final Object username = session.getAttribute(Attributes.AUTORIZED_USER.getValue());
-            final String id = session.getId();
             session.invalidate();
 
-            logger.info("UI Logout - Success - Logout succeeded for user: {}, session {}", username, id);
+            auditLogger.info("{} UI Session - Success - Logout succeeded for user: {}",
+                    AuditContext.currentOrInternal(), username);
 
             Cookie[] cookies = request.getCookies();
             for (Cookie cookie : cookies) {
@@ -102,7 +102,8 @@ public GwtUserConfig getUserConfig(final GwtXSRFToken xsrfToken) throws GwtKuraE
         try {
             return userManager.getUserConfig((String) username).orElse(null);
         } catch (KuraException e) {
-            logger.warn("failed to get configuration for identiy {}", username);
+            auditLogger.warn("{} UI Session - Failure - Failed to get configuration for user {}",
+                    AuditContext.currentOrInternal(), username);
             return null;
         }
     }
@@ -129,7 +130,8 @@ public void updatePassword(GwtXSRFToken xsrfToken, String oldPassword, String ne
         try {
             this.userManager.authenticateWithPassword(username, oldPassword);
         } catch (KuraException e) {
-            logger.warn("Wrong password");
+            auditLogger.warn("{} UI Session - Failure - Wrong password for user {}", AuditContext.currentOrInternal(),
+                    username);
             throw new GwtKuraException(GwtKuraErrorCode.INVALID_USERNAME_PASSWORD);
         }
 
@@ -139,8 +141,12 @@ public void updatePassword(GwtXSRFToken xsrfToken, String oldPassword, String ne
 
         try {
             this.userManager.setUserPassword(username, newPassword);
+
+            auditLogger.info("{} UI Session - Success - Password updated for user {}", AuditContext.currentOrInternal(),
+                    username);
         } catch (final KuraException e) {
-            logger.warn("Failed to update user password", e);
+            auditLogger.warn("{} UI Session - Failure - Failed to update password for user {}",
+                    AuditContext.currentOrInternal(), username);
             throw new GwtKuraException(GwtKuraErrorCode.INTERNAL_ERROR);
         }