- Avatar for the user at login is now downloaded and saved/removed in a separate thread

- Redirect after the login to the original requested page is performed when response has not already commited due to redirect to another URI due to tokens expiration or errors

.classpath

@@ -9,6 +9,7 @@
 	<classpathentry excluding="**" kind="src" output="target/classes" path="src/main/resources">
 		<attributes>
 			<attribute name="maven.pomderived" value="true"/>
+			<attribute name="optional" value="true"/>
 		</attributes>
 	</classpathentry>
 	<classpathentry kind="src" output="target/test-classes" path="src/test/java">
@@ -28,6 +29,7 @@
 		<attributes>
 			<attribute name="maven.pomderived" value="true"/>
 			<attribute name="test" value="true"/>
+			<attribute name="optional" value="true"/>
 		</attributes>
 	</classpathentry>
 	<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER/org.eclipse.jdt.internal.debug.ui.launcher.StandardVMType/JavaSE-1.8">

.settings/org.eclipse.jdt.core.prefs

@@ -6,6 +6,6 @@ org.eclipse.jdt.core.compiler.problem.assertIdentifier=error
 org.eclipse.jdt.core.compiler.problem.enablePreviewFeatures=disabled
 org.eclipse.jdt.core.compiler.problem.enumIdentifier=error
 org.eclipse.jdt.core.compiler.problem.forbiddenReference=warning
-org.eclipse.jdt.core.compiler.problem.reportPreviewFeatures=ignore
+org.eclipse.jdt.core.compiler.problem.reportPreviewFeatures=warning
 org.eclipse.jdt.core.compiler.release=disabled
 org.eclipse.jdt.core.compiler.source=1.8

.settings/org.eclipse.wst.common.component

@@ -1,11 +1,31 @@
 <?xml version="1.0" encoding="UTF-8"?><project-modules id="moduleCoreId" project-version="1.5.0">
-    <wb-module deploy-name="oidc-enrollment-hook">
+            
+    
+    <wb-module deploy-name="oidc-enrollment-hook-1.2.1-SNAPSHOT">
+                        
+        
         <wb-resource deploy-path="/" source-path="/target/m2e-wtp/web-resources"/>
+                        
+        
         <wb-resource deploy-path="/" source-path="/src/main/webapp" tag="defaultRootSource"/>
+                        
+        
         <wb-resource deploy-path="/WEB-INF/classes" source-path="/src/main/java"/>
+                        
+        
         <wb-resource deploy-path="/WEB-INF/classes" source-path="/src/main/resources"/>
+                        
+        
         <property name="java-output-path" value="/oidc-portal-enrollment/target/classes"/>
+                        
+        
         <property name="context-root" value="oidc-enrollment-hook"/>
+                        
+        
         <property name="component.exclusion.patterns"/>
+                    
+    
     </wb-module>
+        
+
 </project-modules>

.settings/org.eclipse.wst.common.project.facet.core.xml

@@ -3,7 +3,7 @@
   <fixed facet="wst.jsdt.web"/>
   <installed facet="wst.jsdt.web" version="1.0"/>
   <installed facet="liferay.hook" version="6.0"/>
-  <installed facet="java" version="1.8"/>
   <installed facet="jst.web" version="2.5"/>
   <installed facet="jst.jaxrs" version="2.1"/>
+  <installed facet="java" version="1.8"/>
 </faceted-project>

CHANGELOG.md

@@ -2,6 +2,17 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 # Changelog for "oidc-enrollment-hook"
 
+## [v1.2.1-SNAPSHOT]
+- Better handling of tokens remove from proxy in the `finally` statement in the `SessionDestroyAction` class
+- Avatar for the user at login is now downloaded and saved/removed in a separate thread
+- Redirect after the login to the original requested page is performed when response has not already commited due to redirect to another URI due to tokens expiration or errors
+
+## [v1.2.0]
+- Now an UMA token is issued also after the login in the `PostLoginAction` to assure token's presence for the context without have to wait the next HTTP call and the Valve's intervention, in the case when it is not necessary the redirect to an origin URI after the login. (#20591)
+
+## [v1.1.3]
+- Now user reconciliation/identification from OIDC token after the login is performed no more checking by using the email address but by using the User's username, the Liferay `screenname`. (#20827) (#20840)
+
 ## [v1.1.2]
 - Added some info of the user is about to create in the logs expecially for screen name (auto-generated or externally provided) (#20413). Restored per-session token removal. Logs revised. (#20445)
 

README.md

@@ -1,6 +1,6 @@
-# Event Publisher Library
+# OIDC Enrollment Hook
 
-**Event Publisher library** is a [Liferay](https://liferay.com) 6.2 hook that and basically implements OIDC portal login in the gCube framework
+**OIDC Enrollment Hook** is a [Liferay](https://liferay.com) 6.2 hook that basically implements OIDC portal login in the gCube framework
 
 ## Structure of the project
 

pom.xml

@@ -1,18 +1,25 @@
 <project xmlns="http://maven.apache.org/POM/4.0.0"
 	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
 	xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+
 	<modelVersion>4.0.0</modelVersion>
+
 	<parent>
 		<artifactId>maven-parent</artifactId>
 		<groupId>org.gcube.tools</groupId>
-		<version>1.1.0</version>
+		<version>1.2.0</version>
 		<relativePath />
 	</parent>
+
 	<groupId>org.gcube.portal</groupId>
 	<artifactId>oidc-enrollment-hook</artifactId>
 	<packaging>war</packaging>
-	<version>1.1.2</version>
+	<version>1.2.1-SNAPSHOT</version>
+
 	<properties>
+		<java.version>8</java.version>
+		<maven.compiler.source>${java.version}</maven.compiler.source>
+		<maven.compiler.target>${java.version}</maven.compiler.target>
 		<liferay.version>6.2.5</liferay.version>
 		<liferay.maven.plugin.version>6.2.10.12</liferay.maven.plugin.version>
 		<liferay.auto.deploy.dir>/Users/themaxx/Development/Server/liferay-portal-6.2-ce-ga6/deploy</liferay.auto.deploy.dir>
@@ -20,33 +27,44 @@
 		<liferay.app.server.lib.global.dir>/Users/themaxx/Development/Server/liferay-portal-6.2-ce-ga6/tomcat-7.0.62/lib/ext</liferay.app.server.lib.global.dir>
 		<liferay.app.server.portal.dir>/Users/themaxx/Development/Server/liferay-portal-6.2-ce-ga6/tomcat-7.0.62/webapps/ROOT</liferay.app.server.portal.dir>
 	</properties>
+
 	<dependencyManagement>
 		<dependencies>
 			<dependency>
 				<groupId>org.gcube.distribution</groupId>
 				<artifactId>maven-portal-bom</artifactId>
-				<version>3.6.0</version>
+				<version>3.7.0</version>
 				<type>pom</type>
 				<scope>import</scope>
 			</dependency>
 		</dependencies>
 	</dependencyManagement>
+
 	<scm>
 		<connection>scm:git:https://code-repo.d4science.org/gCubeSystem/${project.artifactId}.git</connection>
 		<developerConnection>scm:git:https://code-repo.d4science.org/gCubeSystem/${project.artifactId}.git</developerConnection>
 		<url>https://code-repo.d4science.org/gCubeSystem/${project.artifactId}</url>
 	</scm>
+
 	<dependencies>
 		<dependency>
 			<groupId>org.gcube.common</groupId>
 			<artifactId>oidc-library</artifactId>
-			<version>[1.0.0-SNAPSHOT, 2.0.0-SNAPSHOT)</version>
 			<scope>provided</scope>
 		</dependency>
 		<dependency>
 			<groupId>org.gcube.portal</groupId>
 			<artifactId>oidc-library-portal</artifactId>
-			<version>[1.0.0-SNAPSHOT, 2.0.0-SNAPSHOT)</version>
+			<scope>provided</scope>
+		</dependency>
+		<dependency>
+			<groupId>org.gcube.common.portal</groupId>
+			<artifactId>portal-manager</artifactId>
+			<scope>provided</scope>
+		</dependency>
+		<dependency>
+			<groupId>org.gcube.core</groupId>
+			<artifactId>common-scope</artifactId>
 			<scope>provided</scope>
 		</dependency>
 		<dependency>
@@ -70,6 +88,7 @@
 			<scope>provided</scope>
 		</dependency>
 	</dependencies>
+
 	<build>
 		<plugins>
 			<plugin>
@@ -85,21 +104,7 @@
 					<pluginType>hook</pluginType>
 				</configuration>
 			</plugin>
-			<plugin>
-				<artifactId>maven-compiler-plugin</artifactId>
-				<configuration>
-					<encoding>UTF-8</encoding>
-					<source>${maven.compiler.source}</source>
-					<target>${maven.compiler.target}</target>
-				</configuration>
-			</plugin>
-<!-- 			<plugin> -->
-<!-- 				<artifactId>maven-resources-plugin</artifactId> -->
-<!-- 				<version>2.5</version> -->
-<!-- 				<configuration> -->
-<!-- 					<encoding>UTF-8</encoding> -->
-<!-- 				</configuration> -->
-<!-- 			</plugin> -->
 		</plugins>
 	</build>
+
 </project>

src/main/java/org/gcube/portal/oidc/lr62/OpenIdConnectAutoLogin.java

@@ -33,8 +33,9 @@ public class OpenIdConnectAutoLogin extends BaseAutoLogin {
 
     private static final Log log = LogFactoryUtil.getLog(OpenIdConnectAutoLogin.class);
 
-    private static boolean ASSURE_AVATAR_FORMAT = true;
+    private static final boolean ENSURE_AVATAR_FORMAT = true;
-    private static String DEFAULT_AVATAR_FORMAT = "png";
+    private static final String DEFAULT_AVATAR_FORMAT = "png";
+    private static final boolean DELETE_AVATAR_IF_NOT_FOUND_ON_SERVER = false;
 
     @Override
     public String[] doLogin(HttpServletRequest request, HttpServletResponse response) throws Exception {
@@ -66,7 +67,9 @@ public class OpenIdConnectAutoLogin extends BaseAutoLogin {
                 // TODO: to be removed when tested in depth
                 log.error("Applying strategy", t);
             }
+            if (log.isDebugEnabled()) {
                 log.debug("Returning logged in user's info");
+            }
             return new String[] { String.valueOf(user.getUserId()), UUID.randomUUID().toString(), "false" };
         } else {
             log.warn("User is null");
@@ -77,83 +80,80 @@ public class OpenIdConnectAutoLogin extends BaseAutoLogin {
     public static User createOrUpdateUser(JWTToken token, long companyId, long groupId, String portalURL,
             LiferayOpenIdConnectConfiguration configuration) throws Exception {
 
+        String username = token.getUserName();
         String email = token.getEmail();
         String given = token.getGiven();
         String family = token.getFamily();
         String subject = token.getSub();
-        String username = token.getUserName();
         User user = null;
         try {
-            boolean updateUser = false;
+            user = UserLocalServiceUtil.fetchUserByScreenName(companyId, username);
-            // Search by email first
-            user = UserLocalServiceUtil.fetchUserByEmailAddress(companyId, email);
             if (user == null) {
-                log.debug("No Liferay user found with email address=" + email + ", trying with openId");
+                // Then search by openId, in case an admin changed the username on OIDC server
-                // Then search by openId, in case user has changed the email address
+                if (log.isDebugEnabled()) {
+                    log.debug("No Liferay user found with username=" + username + ", trying with openId");
+                }
                 user = UserLocalServiceUtil.fetchUserByOpenId(companyId, subject);
                 if (user == null) {
-                    log.debug("No Liferay user found with openid=" + subject + " and email address=" + email);
+                    if (log.isDebugEnabled()) {
+                        log.debug("No Liferay user found with openid=" + subject + " and username=" + username);
+                    }
                     if (configuration.createUnexistingUser()) {
                         log.info("A new user will be created [email=" + email + ",given=" + given + ",family=" + family
                                 + ",subject=" + subject + ",username=" + username);
 
                         user = addUser(companyId, groupId, portalURL, email, given, family, subject, username);
                     } else {
-                        log.info("User will not be created according to configuration");
+                        log.warn("Unexisting user will not be created according to configuration");
                         return null;
                     }
-                } else {
+                } else if (log.isDebugEnabled()) {
-                    log.info("User found by its openId, the email will be updated");
+                    log.debug("User found by its openId, other info will be updated");
-                    updateUser = true;
                 }
             }
+            boolean updateUser = false;
             if (user != null) {
-                log.debug("User found, updating name details with info from userinfo if changed");
+                if (log.isDebugEnabled()) {
-                if (given != user.getFirstName()) {
+                    log.debug("User found, checking its details against userinfo for changes");
+                }
+                if (given != null && !given.equals(user.getFirstName())) {
+                    if (log.isTraceEnabled()) {
+                        log.trace("Given name is changed");
+                    }
                     user.setFirstName(given);
                     updateUser = true;
                 }
-                if (family != user.getLastName()) {
+                if (family != null && !family.equals(user.getLastName())) {
+                    if (log.isTraceEnabled()) {
+                        log.trace("Last name is changed");
+                    }
                     user.setLastName(family);
                     updateUser = true;
                 }
-                if (email != user.getEmailAddress()) {
+                if (email != null && !email.equals(user.getEmailAddress())) {
+                    if (log.isTraceEnabled()) {
+                        log.trace("Email address is changed");
+                    }
                     user.setEmailAddress(email);
                     updateUser = true;
                 }
+                if (subject != null && !subject.equals(user.getOpenId())) {
+                    if (log.isTraceEnabled()) {
+                        log.trace("Setting OOID subject as openid");
+                    }
+                    user.setOpenId(subject);
+                    updateUser = true;
+                }
             }
             if (updateUser) {
+                if (log.isDebugEnabled()) {
+                    log.debug("Updating user's details with info from userinfo");
+                }
                 UserLocalServiceUtil.updateUser(user);
             }
 
-            try {
+            // Performing avatar download and save/delete in new thread to avoid distributed network persistence delays
-                byte[] userAvatar = OpenIdConnectRESTHelper.getUserAvatar(configuration.getAvatarURL(), token);
+            new AvatarThread(configuration, user, token).start();
-                if (userAvatar != null && userAvatar.length > 0) {
-                    if (ASSURE_AVATAR_FORMAT) {
-                        log.debug("Assuring avatar image format as: " + DEFAULT_AVATAR_FORMAT);
-                        log.debug("Reading image stream with length: " + userAvatar.length);
-                        BufferedImage bi = ImageIO.read(new ByteArrayInputStream(userAvatar));
-                        if (bi != null) {
-                            ByteArrayOutputStream baos = new ByteArrayOutputStream();
-                            log.debug("Converting avatar stream image format to: " + DEFAULT_AVATAR_FORMAT);
-                            ImageIO.write(bi, DEFAULT_AVATAR_FORMAT, baos);
-                            baos.flush();
-                            baos.close();
-                            log.debug("Reading converted image from the BAOS");
-                            userAvatar = baos.toByteArray();
-                        } else {
-                            log.warn("Buffered image read is null!");
-                        }
-                    }
-                    log.debug("Saving the retrieved avatar as user's portrait");
-                    UserLocalServiceUtil.updatePortrait(user.getUserId(), userAvatar);
-                } else {
-                    log.debug("Deleting the user's portrait since no avatar has been found for the user");
-                    UserLocalServiceUtil.deletePortrait(user.getUserId());
-                }
-            } catch (Throwable t) {
-                log.error("Cannot save/update/delete user's portrait", t);
-            }
         } catch (SystemException | PortalException e) {
             throw new RuntimeException(e);
         }
@@ -171,9 +171,13 @@ public class OpenIdConnectAutoLogin extends BaseAutoLogin {
         boolean autoScreenName = username == null;
         String screenName = StringPool.BLANK;
         if (autoScreenName) {
+            if (log.isDebugEnabled()) {
                 log.debug("Screen name will be auto-generated");
+            }
         } else {
+            if (log.isDebugEnabled()) {
                 log.debug("Screen name will be set to: " + username);
+            }
             screenName = username;
         }
         long facebookId = 0;
@@ -211,4 +215,60 @@ public class OpenIdConnectAutoLogin extends BaseAutoLogin {
         return user;
     }
 
+    public static class AvatarThread extends Thread {
+
+        private LiferayOpenIdConnectConfiguration configuration;
+        private User user;
+        private JWTToken token;
+
+        public AvatarThread(LiferayOpenIdConnectConfiguration configuration, User user, JWTToken token) {
+            this.configuration = configuration;
+            this.user = user;
+            this.token = token;
+        }
+
+        @Override
+        public void run() {
+            log.debug("Starting avatar download and save thread for: " + user.getScreenName());
+            try {
+                byte[] userAvatar = OpenIdConnectRESTHelper.getUserAvatar(configuration.getAvatarURL(), token);
+                if (userAvatar != null && userAvatar.length > 0) {
+                    if (ENSURE_AVATAR_FORMAT) {
+                        if (log.isDebugEnabled()) {
+                            log.debug("Assuring avatar image format as: " + DEFAULT_AVATAR_FORMAT);
+                            log.debug("Reading image stream with length: " + userAvatar.length);
+                        }
+                        BufferedImage bi = ImageIO.read(new ByteArrayInputStream(userAvatar));
+                        if (bi != null) {
+                            ByteArrayOutputStream baos = new ByteArrayOutputStream();
+                            if (log.isDebugEnabled()) {
+                                log.debug("Converting avatar stream image format to: " + DEFAULT_AVATAR_FORMAT);
+                            }
+                            ImageIO.write(bi, DEFAULT_AVATAR_FORMAT, baos);
+                            baos.flush();
+                            baos.close();
+                            if (log.isDebugEnabled()) {
+                                log.debug("Reading converted image from the BAOS");
+                            }
+                            userAvatar = baos.toByteArray();
+                        } else {
+                            log.warn("Buffered image read is null!");
+                        }
+                    }
+                    if (log.isDebugEnabled()) {
+                        log.debug("Saving the retrieved avatar as user's portrait");
+                    }
+                    UserLocalServiceUtil.updatePortrait(user.getUserId(), userAvatar);
+                } else if (DELETE_AVATAR_IF_NOT_FOUND_ON_SERVER) {
+                    if (log.isDebugEnabled()) {
+                        log.debug("Deleting the user's portrait since no avatar has been found for the user");
+                    }
+                    UserLocalServiceUtil.deletePortrait(user.getUserId());
+                }
+            } catch (Throwable t) {
+                log.error("Cannot save/update/delete user's portrait", t);
+            }
+        }
+    }
+
 }

src/main/java/org/gcube/portal/oidc/lr62/PostLoginAction.java

@@ -7,6 +7,7 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
+import org.gcube.common.portal.PortalContext;
 import org.gcube.oidc.rest.JWTToken;
 
 import com.liferay.portal.kernel.events.Action;
@@ -20,6 +21,8 @@ public class PostLoginAction extends Action {
 
     protected static final Log log = LogFactoryUtil.getLog(PostLoginAction.class);
 
+    public static boolean REQUEST_UMA_ALSO_WITH_REDIRECT = true;
+
     @Override
     public void run(HttpServletRequest request, HttpServletResponse response) throws ActionException {
         if (log.isInfoEnabled()) {
@@ -27,6 +30,7 @@ public class PostLoginAction extends Action {
         }
         JWTToken token = JWTTokenUtil.getOIDCFromRequest(request);
         HttpSession session = request.getSession(false);
+        String redirect = (String) request.getAttribute(OpenIdConnectLoginFilter.REDIRECT_ATTRIBUTE);;
         if (token != null && session != null) {
             User user = (User) session.getAttribute(WebKeys.USER);
             if (user != null) {
@@ -38,6 +42,15 @@ public class PostLoginAction extends Action {
                 log.error("User object not found in session " + session.getId() + " ["
                         + Integer.toHexString(session.hashCode()) + "]");
             }
+            if (redirect == null || REQUEST_UMA_ALSO_WITH_REDIRECT) {
+                if (log.isDebugEnabled()) {
+                    log.debug("Getting current infrastructure context via portal context class");
+                }
+                String currentContext = "/" + PortalContext.getConfiguration().getInfrastructureName();
+                OIDCUmaUtil.checkUMATicketAndProvideInThreadLocal(request, response, user, session, currentContext);
+            } else if (log.isDebugEnabled()) {
+                log.debug("UMA token will be set by the valve after the redirection to: " + redirect);
+            }
         } else {
             if (token == null) {
                 log.error("OIDC token object is null in request");
@@ -46,18 +59,26 @@ public class PostLoginAction extends Action {
                 log.error("Session is null");
             }
         }
-        String redirect = (String) request.getAttribute(OpenIdConnectLoginFilter.REDIRECT_ATTRIBUTE);
+
         if (redirect != null) {
+            // Checking if a redirect has been already set (e.g. after an exception and/or a redirect to the logout URI)
+            if (!response.isCommitted()) {
                 if (log.isDebugEnabled()) {
                     log.debug("Redirecting to the original requested URI: " + redirect);
                 }
                 try {
+                    // I'm not sure I can use this LR facility since it's used also by landing-page-hook.
+                    // Indeed perhaps it should also be discussed if it takes precedence over this redirect in the case.
+                    // session.setAttribute(WebKeys.LAST_PATH, new LastPath(null, URLDecoder.decode(redirect, "UTF-8"))
                     response.sendRedirect(URLDecoder.decode(redirect, "UTF-8"));
                 } catch (IOException e) {
                     new ActionException("Redirecting to original requested URI: " + redirect, e);
                 }
-        } else if (log.isTraceEnabled()) {
+            } else {
-            log.trace("No original requested URI has been found in session");
+                log.warn("Cannot redirect to original redirect URI (" + redirect + ") since the response is already commited");
+            }
+        } else if (log.isDebugEnabled()) {
+            log.debug("No original requested URI has been found in session");
         }
     }
 

src/main/java/org/gcube/portal/oidc/lr62/SessionDestroyAction.java

@@ -40,6 +40,13 @@ public class SessionDestroyAction extends SessionAction {
                     OpenIdConnectRESTHelper.logout(configuration.getLogoutURL(), token);
                 } catch (IOException e) {
                     throw new ActionException("Performing logut on OIDC server", e);
+                } finally {
+                    if (log.isInfoEnabled()) {
+                        log.info("Removing OIDC tokens from cache proxy for user " + user.getScreenName() + " and session "
+                                + session.getId());
+                    }
+                    JWTCacheProxy.getInstance().removeOIDCToken(user, session.getId());
+                    JWTCacheProxy.getInstance().removeUMAToken(user, session.getId());
                 }
             } else {
                 log.warn("Cannot find the OIDC token in session " + session.getId() + " ["
@@ -50,12 +57,6 @@ public class SessionDestroyAction extends SessionAction {
                 log.debug("Don't perform OIDC logout according to configuration for user: " + user.getScreenName());
             }
         }
-        if (log.isDebugEnabled()) {
-            log.debug("Removing OIDC tokens from cache proxy for user " + user.getScreenName() + " and session "
-                    + session.getId());
-        }
-        JWTCacheProxy.getInstance().removeOIDCToken(user, session.getId());
-        JWTCacheProxy.getInstance().removeUMAToken(user, session.getId());
     }
 
 }