From cf4e7412d440c581bc351de69fee1d00eac1a33e Mon Sep 17 00:00:00 2001
From: Jan Bartel <janb@webtide.com>
Date: Mon, 1 Jul 2019 15:46:14 +0200
Subject: [PATCH] Issue #3815 Ensure user not in roles for
 PropertyFileLoginModule (#3826)

Signed-off-by: Jan Bartel <janb@webtide.com>
---
 .../jaas/spi/PropertyFileLoginModule.java     | 16 +++---
 .../jaas/spi/PropertyFileLoginModuleTest.java | 53 +++++++++++++++++++
 .../src/test/resources/login.properties       |  1 +
 3 files changed, 61 insertions(+), 9 deletions(-)
 create mode 100644 jetty-jaas/src/test/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModuleTest.java
 create mode 100644 jetty-jaas/src/test/resources/login.properties

diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModule.java b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModule.java
index bb99c620441..4626e4a7709 100644
--- a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModule.java
+++ b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModule.java
@@ -18,15 +18,16 @@
 
 package org.eclipse.jetty.jaas.spi;
 
-import java.security.Principal;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.stream.Collectors;
+
 import javax.security.auth.Subject;
 import javax.security.auth.callback.CallbackHandler;
 
+import org.eclipse.jetty.security.AbstractLoginService;
 import org.eclipse.jetty.security.PropertyUserStore;
 import org.eclipse.jetty.server.UserIdentity;
 import org.eclipse.jetty.util.log.Log;
@@ -116,14 +117,11 @@ public class PropertyFileLoginModule extends AbstractLoginModule
 
         //TODO in future versions change the impl of PropertyUserStore so its not
         //storing Subjects etc, just UserInfo
-        Set<Principal> principals = userIdentity.getSubject().getPrincipals();
+        Set<AbstractLoginService.RolePrincipal> principals = userIdentity.getSubject().getPrincipals(AbstractLoginService.RolePrincipal.class);
 
-        List<String> roles = new ArrayList<String>();
-
-        for (Principal principal : principals)
-        {
-            roles.add(principal.getName());
-        }
+        List<String> roles = principals.stream()
+            .map(AbstractLoginService.RolePrincipal::getName)
+            .collect(Collectors.toList());
 
         Credential credential = (Credential)userIdentity.getSubject().getPrivateCredentials().iterator().next();
         LOG.debug("Found: " + userName + " in PropertyUserStore " + _filename);
diff --git a/jetty-jaas/src/test/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModuleTest.java b/jetty-jaas/src/test/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModuleTest.java
new file mode 100644
index 00000000000..7f9cd1cafc5
--- /dev/null
+++ b/jetty-jaas/src/test/java/org/eclipse/jetty/jaas/spi/PropertyFileLoginModuleTest.java
@@ -0,0 +1,53 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.jaas.spi;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.not;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.io.File;
+import java.util.HashMap;
+
+import javax.security.auth.Subject;
+
+import org.eclipse.jetty.jaas.callback.DefaultCallbackHandler;
+import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
+import org.junit.jupiter.api.Test;
+
+public class PropertyFileLoginModuleTest
+{
+    @Test
+    public void testRoles()
+        throws Exception
+    {
+        File file = MavenTestingUtils.getTestResourceFile("login.properties");
+        PropertyFileLoginModule module = new PropertyFileLoginModule();
+        Subject subject = new Subject();
+        HashMap<String, String> options = new HashMap<>();
+        options.put("file", file.getCanonicalPath());
+        module.initialize(subject, new DefaultCallbackHandler(), new HashMap<String, String>(), options);
+        UserInfo fred = module.getUserInfo("fred");
+        assertEquals("fred", fred.getUserName());
+        assertThat(fred.getRoleNames(), containsInAnyOrder("role1", "role2", "role3"));
+        assertThat(fred.getRoleNames(), not(contains("fred")));
+    }
+}
diff --git a/jetty-jaas/src/test/resources/login.properties b/jetty-jaas/src/test/resources/login.properties
new file mode 100644
index 00000000000..22a4bedc7b7
--- /dev/null
+++ b/jetty-jaas/src/test/resources/login.properties
@@ -0,0 +1 @@
+fred=pwd,role1,role2,role3
\ No newline at end of file