From bb886ad9320c170b9b12a111e00f6671978c8cee Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Wed, 11 Nov 2020 17:48:08 +0100 Subject: [PATCH 1/3] Fix #5575 SEARCH method (#5576) + Added all IANA methods + Used Trie for most lookups + Fixed ArrayTernayTrie lookup + optimised GET, POST and HEAD Co-authored-by: Joakim Erdfelt --- .../org/eclipse/jetty/http/HttpMethod.java | 286 ++++++++++-------- .../eclipse/jetty/http/HttpParserTest.java | 30 +- .../jetty/http/jmh/HttpMethodBenchmark.java | 127 ++++++++ .../eclipse/jetty/util/ArrayTernaryTrie.java | 6 + 4 files changed, 321 insertions(+), 128 deletions(-) create mode 100644 jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java index 83f6bfc7365..e0e035072dc 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpMethod.java @@ -26,137 +26,72 @@ import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.Trie; /** - * + * Known HTTP Methods */ public enum HttpMethod { - GET, - POST, - HEAD, - PUT, - OPTIONS, - DELETE, - TRACE, - CONNECT, - MOVE, - PROXY, - PRI; + // From https://www.iana.org/assignments/http-methods/http-methods.xhtml + ACL(Type.IDEMPOTENT), + BASELINE_CONTROL(Type.IDEMPOTENT), + BIND(Type.IDEMPOTENT), + CHECKIN(Type.IDEMPOTENT), + CHECKOUT(Type.IDEMPOTENT), + CONNECT(Type.NORMAL), + COPY(Type.IDEMPOTENT), + DELETE(Type.IDEMPOTENT), + GET(Type.SAFE), + HEAD(Type.SAFE), + LABEL(Type.IDEMPOTENT), + LINK(Type.IDEMPOTENT), + LOCK(Type.NORMAL), + MERGE(Type.IDEMPOTENT), + MKACTIVITY(Type.IDEMPOTENT), + MKCALENDAR(Type.IDEMPOTENT), + MKCOL(Type.IDEMPOTENT), + MKREDIRECTREF(Type.IDEMPOTENT), + MKWORKSPACE(Type.IDEMPOTENT), + MOVE(Type.IDEMPOTENT), + OPTIONS(Type.SAFE), + ORDERPATCH(Type.IDEMPOTENT), + PATCH(Type.NORMAL), + POST(Type.NORMAL), + PRI(Type.SAFE), + PROPFIND(Type.SAFE), + PROPPATCH(Type.IDEMPOTENT), + PUT(Type.IDEMPOTENT), + REBIND(Type.IDEMPOTENT), + REPORT(Type.SAFE), + SEARCH(Type.SAFE), + TRACE(Type.SAFE), + UNBIND(Type.IDEMPOTENT), + UNCHECKOUT(Type.IDEMPOTENT), + UNLINK(Type.IDEMPOTENT), + UNLOCK(Type.IDEMPOTENT), + UPDATE(Type.IDEMPOTENT), + UPDATEREDIRECTREF(Type.IDEMPOTENT), + VERSION_CONTROL(Type.IDEMPOTENT), - /** - * Optimized lookup to find a method name and trailing space in a byte array. - * - * @param bytes Array containing ISO-8859-1 characters - * @param position The first valid index - * @param limit The first non valid index - * @return An HttpMethod if a match or null if no easy match. - */ - public static HttpMethod lookAheadGet(byte[] bytes, final int position, int limit) + // Other methods + PROXY(Type.NORMAL); + + // The type of the method + private enum Type { - int length = limit - position; - if (length < 4) - return null; - switch (bytes[position]) - { - case 'G': - if (bytes[position + 1] == 'E' && bytes[position + 2] == 'T' && bytes[position + 3] == ' ') - return GET; - break; - case 'P': - if (bytes[position + 1] == 'O' && bytes[position + 2] == 'S' && bytes[position + 3] == 'T' && length >= 5 && bytes[position + 4] == ' ') - return POST; - if (bytes[position + 1] == 'R' && bytes[position + 2] == 'O' && bytes[position + 3] == 'X' && length >= 6 && bytes[position + 4] == 'Y' && bytes[position + 5] == ' ') - return PROXY; - if (bytes[position + 1] == 'U' && bytes[position + 2] == 'T' && bytes[position + 3] == ' ') - return PUT; - if (bytes[position + 1] == 'R' && bytes[position + 2] == 'I' && bytes[position + 3] == ' ') - return PRI; - break; - case 'H': - if (bytes[position + 1] == 'E' && bytes[position + 2] == 'A' && bytes[position + 3] == 'D' && length >= 5 && bytes[position + 4] == ' ') - return HEAD; - break; - case 'O': - if (bytes[position + 1] == 'P' && bytes[position + 2] == 'T' && bytes[position + 3] == 'I' && length >= 8 && - bytes[position + 4] == 'O' && bytes[position + 5] == 'N' && bytes[position + 6] == 'S' && bytes[position + 7] == ' ') - return OPTIONS; - break; - case 'D': - if (bytes[position + 1] == 'E' && bytes[position + 2] == 'L' && bytes[position + 3] == 'E' && length >= 7 && - bytes[position + 4] == 'T' && bytes[position + 5] == 'E' && bytes[position + 6] == ' ') - return DELETE; - break; - case 'T': - if (bytes[position + 1] == 'R' && bytes[position + 2] == 'A' && bytes[position + 3] == 'C' && length >= 6 && - bytes[position + 4] == 'E' && bytes[position + 5] == ' ') - return TRACE; - break; - case 'C': - if (bytes[position + 1] == 'O' && bytes[position + 2] == 'N' && bytes[position + 3] == 'N' && length >= 8 && - bytes[position + 4] == 'E' && bytes[position + 5] == 'C' && bytes[position + 6] == 'T' && bytes[position + 7] == ' ') - return CONNECT; - break; - case 'M': - if (bytes[position + 1] == 'O' && bytes[position + 2] == 'V' && bytes[position + 3] == 'E' && length >= 5 && bytes[position + 4] == ' ') - return MOVE; - break; - - default: - break; - } - return null; + NORMAL, + IDEMPOTENT, + SAFE } - /** - * Optimized lookup to find a method name and trailing space in a byte array. - * - * @param buffer buffer containing ISO-8859-1 characters, it is not modified. - * @return An HttpMethod if a match or null if no easy match. - */ - public static HttpMethod lookAheadGet(ByteBuffer buffer) - { - if (buffer.hasArray()) - return lookAheadGet(buffer.array(), buffer.arrayOffset() + buffer.position(), buffer.arrayOffset() + buffer.limit()); - - int l = buffer.remaining(); - if (l >= 4) - { - HttpMethod m = CACHE.getBest(buffer, 0, l); - if (m != null) - { - int ml = m.asString().length(); - if (l > ml && buffer.get(buffer.position() + ml) == ' ') - return m; - } - } - return null; - } - - public static final Trie INSENSITIVE_CACHE = new ArrayTrie<>(); - - static - { - for (HttpMethod method : HttpMethod.values()) - { - INSENSITIVE_CACHE.put(method.toString(), method); - } - } - - public static final Trie CACHE = new ArrayTernaryTrie<>(false); - - static - { - for (HttpMethod method : HttpMethod.values()) - { - CACHE.put(method.toString(), method); - } - } - - private final ByteBuffer _buffer; + private final String _method; private final byte[] _bytes; + private final ByteBuffer _buffer; + private final Type _type; - HttpMethod() + HttpMethod(Type type) { - _bytes = StringUtil.getBytes(toString()); + _method = name().replace('_', '-'); + _type = type; + _bytes = StringUtil.getBytes(_method); _buffer = ByteBuffer.wrap(_bytes); } @@ -170,6 +105,28 @@ public enum HttpMethod return toString().equalsIgnoreCase(s); } + /** + * An HTTP method is safe if it doesn't alter the state of the server. + * In other words, a method is safe if it leads to a read-only operation. + * Several common HTTP methods are safe: GET , HEAD , or OPTIONS . + * All safe methods are also idempotent, but not all idempotent methods are safe + * @return if the method is safe. + */ + public boolean isSafe() + { + return _type == Type.SAFE; + } + + /** + * An idempotent HTTP method is an HTTP method that can be called many times without different outcomes. + * It would not matter if the method is called only once, or ten times over. The result should be the same. + * @return true if the method is idempotent. + */ + public boolean isIdempotent() + { + return _type.ordinal() >= Type.IDEMPOTENT.ordinal(); + } + public ByteBuffer asBuffer() { return _buffer.asReadOnlyBuffer(); @@ -177,11 +134,94 @@ public enum HttpMethod public String asString() { - return toString(); + return _method; + } + + public String toString() + { + return _method; + } + + public static final Trie INSENSITIVE_CACHE = new ArrayTrie<>(252); + public static final Trie CACHE = new ArrayTernaryTrie<>(false, 300); + public static final Trie LOOK_AHEAD = new ArrayTernaryTrie<>(false, 330); + public static final int ACL_AS_INT = ('A' & 0xff) << 24 | ('C' & 0xFF) << 16 | ('L' & 0xFF) << 8 | (' ' & 0xFF); + public static final int GET_AS_INT = ('G' & 0xff) << 24 | ('E' & 0xFF) << 16 | ('T' & 0xFF) << 8 | (' ' & 0xFF); + public static final int PRI_AS_INT = ('P' & 0xff) << 24 | ('R' & 0xFF) << 16 | ('I' & 0xFF) << 8 | (' ' & 0xFF); + public static final int PUT_AS_INT = ('P' & 0xff) << 24 | ('U' & 0xFF) << 16 | ('T' & 0xFF) << 8 | (' ' & 0xFF); + public static final int POST_AS_INT = ('P' & 0xff) << 24 | ('O' & 0xFF) << 16 | ('S' & 0xFF) << 8 | ('T' & 0xFF); + public static final int HEAD_AS_INT = ('H' & 0xff) << 24 | ('E' & 0xFF) << 16 | ('A' & 0xFF) << 8 | ('D' & 0xFF); + static + { + for (HttpMethod method : HttpMethod.values()) + { + if (!INSENSITIVE_CACHE.put(method.asString(), method)) + throw new IllegalStateException("INSENSITIVE_CACHE too small: " + method); + + if (!CACHE.put(method.asString(), method)) + throw new IllegalStateException("CACHE too small: " + method); + + if (!LOOK_AHEAD.put(method.asString() + ' ', method)) + throw new IllegalStateException("LOOK_AHEAD too small: " + method); + } } /** - * Converts the given String parameter to an HttpMethod + * Optimized lookup to find a method name and trailing space in a byte array. + * + * @param bytes Array containing ISO-8859-1 characters + * @param position The first valid index + * @param limit The first non valid index + * @return An HttpMethod if a match or null if no easy match. + * @deprecated Not used + */ + @Deprecated + public static HttpMethod lookAheadGet(byte[] bytes, final int position, int limit) + { + return LOOK_AHEAD.getBest(bytes, position, limit - position); + } + + /** + * Optimized lookup to find a method name and trailing space in a byte array. + * + * @param buffer buffer containing ISO-8859-1 characters, it is not modified. + * @return An HttpMethod if a match or null if no easy match. + */ + public static HttpMethod lookAheadGet(ByteBuffer buffer) + { + int len = buffer.remaining(); + // Short cut for 3 char methods, mostly for GET optimisation + if (len > 3) + { + switch (buffer.getInt(buffer.position())) + { + case ACL_AS_INT: + return ACL; + case GET_AS_INT: + return GET; + case PRI_AS_INT: + return PRI; + case PUT_AS_INT: + return PUT; + case POST_AS_INT: + if (len > 4 && buffer.get(buffer.position() + 4) == ' ') + return POST; + break; + case HEAD_AS_INT: + if (len > 4 && buffer.get(buffer.position() + 4) == ' ') + return HEAD; + break; + default: + break; + } + } + return LOOK_AHEAD.getBest(buffer, 0, len); + } + + /** + * Converts the given String parameter to an HttpMethod. + * The string may differ from the Enum name as a '-' in the method + * name is represented as a '_' in the Enum name. * * @param method the String to get the equivalent HttpMethod from * @return the HttpMethod or null if the parameter method is unknown diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java index d454d5e49eb..39d72a13be6 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java @@ -23,6 +23,7 @@ import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Locale; import org.eclipse.jetty.http.HttpParser.State; import org.eclipse.jetty.toolchain.test.Net; @@ -32,6 +33,8 @@ import org.hamcrest.Matchers; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import static org.eclipse.jetty.http.HttpComplianceSection.NO_FIELD_FOLDING; import static org.hamcrest.MatcherAssert.assertThat; @@ -85,12 +88,20 @@ public class HttpParserTest @Test public void httpMethodTest() { - assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("Wibble "))); - assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("GET"))); - assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer("MO"))); + for (HttpMethod m : HttpMethod.values()) + { + assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString().substring(0,2)))); + assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString()))); + assertNull(HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString() + "FOO"))); + assertEquals(m, HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString() + " "))); + assertEquals(m, HttpMethod.lookAheadGet(BufferUtil.toBuffer(m.asString() + " /foo/bar"))); - assertEquals(HttpMethod.GET, HttpMethod.lookAheadGet(BufferUtil.toBuffer("GET "))); - assertEquals(HttpMethod.MOVE, HttpMethod.lookAheadGet(BufferUtil.toBuffer("MOVE "))); + assertNull(HttpMethod.lookAheadGet(m.asString().substring(0,2).getBytes(), 0,2)); + assertNull(HttpMethod.lookAheadGet(m.asString().getBytes(), 0, m.asString().length())); + assertNull(HttpMethod.lookAheadGet((m.asString() + "FOO").getBytes(), 0, m.asString().length() + 3)); + assertEquals(m, HttpMethod.lookAheadGet(("\n" + m.asString() + " ").getBytes(), 1, m.asString().length() + 2)); + assertEquals(m, HttpMethod.lookAheadGet(("\n" + m.asString() + " /foo").getBytes(), 1, m.asString().length() + 6)); + } ByteBuffer b = BufferUtil.allocateDirect(128); BufferUtil.append(b, BufferUtil.toBuffer("GET")); @@ -100,6 +111,15 @@ public class HttpParserTest assertEquals(HttpMethod.GET, HttpMethod.lookAheadGet(b)); } + @ParameterizedTest + @ValueSource(strings = {"GET", "POST", "VERSION-CONTROL"}) + public void httpMethodNameTest(String methodName) + { + HttpMethod method = HttpMethod.fromString(methodName); + assertNotNull(method, "Method should have been found: " + methodName); + assertEquals(methodName.toUpperCase(Locale.US), method.toString()); + } + @Test public void testLineParseMockIP() { diff --git a/jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java b/jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java new file mode 100644 index 00000000000..283a866dab1 --- /dev/null +++ b/jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java @@ -0,0 +1,127 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// ------------------------------------------------------------------------ +// 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.http.jmh; + +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import org.eclipse.jetty.http.HttpMethod; +import org.eclipse.jetty.util.BufferUtil; +import org.openjdk.jmh.annotations.Benchmark; +import org.openjdk.jmh.annotations.BenchmarkMode; +import org.openjdk.jmh.annotations.Measurement; +import org.openjdk.jmh.annotations.Mode; +import org.openjdk.jmh.annotations.Scope; +import org.openjdk.jmh.annotations.State; +import org.openjdk.jmh.annotations.Threads; +import org.openjdk.jmh.annotations.Warmup; +import org.openjdk.jmh.profile.GCProfiler; +import org.openjdk.jmh.runner.Runner; +import org.openjdk.jmh.runner.RunnerException; +import org.openjdk.jmh.runner.options.Options; +import org.openjdk.jmh.runner.options.OptionsBuilder; + +@State(Scope.Benchmark) +@Threads(4) +@Warmup(iterations = 5, time = 2000, timeUnit = TimeUnit.MILLISECONDS) +@Measurement(iterations = 5, time = 2000, timeUnit = TimeUnit.MILLISECONDS) +public class HttpMethodBenchmark +{ + private static final ByteBuffer GET = BufferUtil.toBuffer("GET / HTTP/1.1\r\n\r\n"); + private static final ByteBuffer POST = BufferUtil.toBuffer("POST / HTTP/1.1\r\n\r\n"); + private static final ByteBuffer MOVE = BufferUtil.toBuffer("MOVE / HTTP/1.1\r\n\r\n"); + private static final Map MAP = new HashMap<>(); + + static + { + for (HttpMethod m : HttpMethod.values()) + MAP.put(m.asString(), m); + } + + @Benchmark + @BenchmarkMode({Mode.Throughput}) + public HttpMethod testTrieGetBest() throws Exception + { + return HttpMethod.LOOK_AHEAD.getBest(GET, 0, GET.remaining()); + } + + @Benchmark + @BenchmarkMode({Mode.Throughput}) + public HttpMethod testIntSwitch() throws Exception + { + switch (GET.getInt(0)) + { + case HttpMethod.ACL_AS_INT: + return HttpMethod.ACL; + case HttpMethod.GET_AS_INT: + return HttpMethod.GET; + case HttpMethod.PRI_AS_INT: + return HttpMethod.PRI; + case HttpMethod.PUT_AS_INT: + return HttpMethod.PUT; + default: + return null; + } + } + + @Benchmark + @BenchmarkMode({Mode.Throughput}) + public HttpMethod testMapGet() throws Exception + { + for (int i = 0; i < GET.remaining(); i++) + { + if (GET.get(i) == (byte)' ') + return MAP.get(BufferUtil.toString(GET, 0, i, StandardCharsets.US_ASCII)); + } + return null; + } + + @Benchmark + @BenchmarkMode({Mode.Throughput}) + public HttpMethod testHttpMethodPost() throws Exception + { + return HttpMethod.lookAheadGet(POST); + } + + @Benchmark + @BenchmarkMode({Mode.Throughput}) + public HttpMethod testHttpMethodMove() throws Exception + { + return HttpMethod.lookAheadGet(MOVE); + } + + public static void main(String[] args) throws RunnerException + { + Options opt = new OptionsBuilder() + .include(HttpMethodBenchmark.class.getSimpleName()) + .warmupIterations(10) + .measurementIterations(10) + .addProfiler(GCProfiler.class) + .forks(1) + .threads(1) + .build(); + + new Runner(opt).run(); + } +} + + diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTernaryTrie.java b/jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTernaryTrie.java index a1eecc1846c..1c8b976d605 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTernaryTrie.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/ArrayTernaryTrie.java @@ -368,6 +368,12 @@ public class ArrayTernaryTrie extends AbstractTrie return getBest(0, b, offset, len); } + @Override + public V getBest(byte[] b, int offset, int len) + { + return getBest(0, b, offset, len); + } + private V getBest(int t, byte[] b, int offset, int len) { int node = t; From dd86adad66a89319a0092aa3da1bc132c53b1037 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 11 Nov 2020 18:18:50 +0100 Subject: [PATCH 2/3] Fixed file header --- .../jetty/http/jmh/HttpMethodBenchmark.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java b/jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java index 283a866dab1..d5ea6726db8 100644 --- a/jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java +++ b/jetty-jmh/src/main/java/org/eclipse/jetty/http/jmh/HttpMethodBenchmark.java @@ -1,19 +1,19 @@ // -// ======================================================================== -// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. -// ------------------------------------------------------------------------ -// 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. +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. // -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html +// This program and the accompanying materials are made available under +// the terms of the Eclipse Public License 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0 // -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php +// This Source Code may also be made available under the following +// Secondary Licenses when the conditions for such availability set +// forth in the Eclipse Public License, v. 2.0 are satisfied: +// the Apache License v2.0 which is available at +// https://www.apache.org/licenses/LICENSE-2.0 // -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== // package org.eclipse.jetty.http.jmh; From 3a99c89350947fc99b97741f1a53c09f9c8a7e01 Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Wed, 11 Nov 2020 18:21:06 +0100 Subject: [PATCH 3/3] Issue #5486 PropertyFileLoginModule retains PropertyUserStores (#5518) * Issue #5486 PropertyFileLoginModule retains PropertyUserStores Signed-off-by: Jan Bartel --- .../eclipse/jetty/jaas/JAASLoginService.java | 138 ++++++------------ .../jetty/jaas/PropertyUserStoreManager.java | 101 +++++++++++++ .../jaas/callback/DefaultCallbackHandler.java | 37 ++--- .../jaas/spi/PropertyFileLoginModule.java | 89 ++++++----- .../jetty/jaas/JAASLoginServiceTest.java | 55 +++---- .../jaas/spi/PropertyFileLoginModuleTest.java | 76 +++++++--- 6 files changed, 301 insertions(+), 195 deletions(-) create mode 100644 jetty-jaas/src/main/java/org/eclipse/jetty/jaas/PropertyUserStoreManager.java diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASLoginService.java b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASLoginService.java index 35b1cfd1337..45470de1e2c 100644 --- a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASLoginService.java +++ b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASLoginService.java @@ -20,15 +20,16 @@ package org.eclipse.jetty.jaas; import java.io.IOException; import java.security.Principal; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.LinkedHashSet; +import java.util.List; import java.util.Set; import javax.security.auth.Subject; import javax.security.auth.callback.Callback; import javax.security.auth.callback.CallbackHandler; import javax.security.auth.callback.NameCallback; -import javax.security.auth.callback.PasswordCallback; import javax.security.auth.callback.UnsupportedCallbackException; import javax.security.auth.login.Configuration; import javax.security.auth.login.FailedLoginException; @@ -37,9 +38,6 @@ import javax.security.auth.login.LoginException; import javax.servlet.ServletRequest; import org.eclipse.jetty.jaas.callback.DefaultCallbackHandler; -import org.eclipse.jetty.jaas.callback.ObjectCallback; -import org.eclipse.jetty.jaas.callback.RequestParameterCallback; -import org.eclipse.jetty.jaas.callback.ServletRequestCallback; import org.eclipse.jetty.security.DefaultIdentityService; import org.eclipse.jetty.security.IdentityService; import org.eclipse.jetty.security.LoginService; @@ -47,7 +45,7 @@ import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.UserIdentity; import org.eclipse.jetty.util.ArrayUtil; import org.eclipse.jetty.util.Loader; -import org.eclipse.jetty.util.component.AbstractLifeCycle; +import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; @@ -58,13 +56,14 @@ import org.eclipse.jetty.util.log.Logger; * Implementation of jetty's LoginService that works with JAAS for * authorization and authentication. */ -public class JAASLoginService extends AbstractLifeCycle implements LoginService +public class JAASLoginService extends ContainerLifeCycle implements LoginService { private static final Logger LOG = Log.getLogger(JAASLoginService.class); public static final String DEFAULT_ROLE_CLASS_NAME = "org.eclipse.jetty.jaas.JAASRole"; public static final String[] DEFAULT_ROLE_CLASS_NAMES = {DEFAULT_ROLE_CLASS_NAME}; - + public static final ThreadLocal INSTANCE = new ThreadLocal<>(); + protected String[] _roleClassNames = DEFAULT_ROLE_CLASS_NAMES; protected String _callbackHandlerClass; protected String _realmName; @@ -183,6 +182,7 @@ public class JAASLoginService extends AbstractLifeCycle implements LoginService { if (_identityService == null) _identityService = new DefaultIdentityService(); + addBean(new PropertyUserStoreManager()); super.doStart(); } @@ -193,59 +193,27 @@ public class JAASLoginService extends AbstractLifeCycle implements LoginService { CallbackHandler callbackHandler = null; if (_callbackHandlerClass == null) - { - callbackHandler = new CallbackHandler() - { - @Override - public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException - { - for (Callback callback : callbacks) - { - if (callback instanceof NameCallback) - { - ((NameCallback)callback).setName(username); - } - else if (callback instanceof PasswordCallback) - { - ((PasswordCallback)callback).setPassword(credentials.toString().toCharArray()); - } - else if (callback instanceof ObjectCallback) - { - ((ObjectCallback)callback).setObject(credentials); - } - else if (callback instanceof RequestParameterCallback) - { - RequestParameterCallback rpc = (RequestParameterCallback)callback; - if (request != null) - rpc.setParameterValues(Arrays.asList(request.getParameterValues(rpc.getParameterName()))); - } - else if (callback instanceof ServletRequestCallback) - { - ((ServletRequestCallback)callback).setRequest(request); - } - else - throw new UnsupportedCallbackException(callback); - } - } - }; - } + callbackHandler = new DefaultCallbackHandler(); else { Class clazz = Loader.loadClass(_callbackHandlerClass); callbackHandler = (CallbackHandler)clazz.getDeclaredConstructor().newInstance(); - if (DefaultCallbackHandler.class.isAssignableFrom(clazz)) - { - DefaultCallbackHandler dch = (DefaultCallbackHandler)callbackHandler; - if (request instanceof Request) - dch.setRequest((Request)request); - dch.setCredential(credentials); - dch.setUserName(username); - } + } + + if (callbackHandler instanceof DefaultCallbackHandler) + { + DefaultCallbackHandler dch = (DefaultCallbackHandler)callbackHandler; + if (request instanceof Request) + dch.setRequest((Request)request); + dch.setCredential(credentials); + dch.setUserName(username); } //set up the login context Subject subject = new Subject(); - LoginContext loginContext = (_configuration == null ? new LoginContext(_loginModuleName, subject, callbackHandler) + INSTANCE.set(this); + LoginContext loginContext = + (_configuration == null ? new LoginContext(_loginModuleName, subject, callbackHandler) : new LoginContext(_loginModuleName, subject, callbackHandler, _configuration)); loginContext.login(); @@ -265,6 +233,10 @@ public class JAASLoginService extends AbstractLifeCycle implements LoginService { LOG.ignore(e); } + finally + { + INSTANCE.remove(); + } return null; } @@ -306,52 +278,36 @@ public class JAASLoginService extends AbstractLifeCycle implements LoginService protected String[] getGroups(Subject subject) { Collection groups = new LinkedHashSet<>(); - Set principals = subject.getPrincipals(); - for (Principal principal : principals) + for (Principal principal : subject.getPrincipals()) { - Class c = principal.getClass(); - while (c != null) - { - if (roleClassNameMatches(c.getName())) - { - groups.add(principal.getName()); - break; - } - - boolean added = false; - for (Class ci : c.getInterfaces()) - { - if (roleClassNameMatches(ci.getName())) - { - groups.add(principal.getName()); - added = true; - break; - } - } - - if (!added) - { - c = c.getSuperclass(); - } - else - break; - } + if (isRoleClass(principal.getClass(), Arrays.asList(getRoleClassNames()))) + groups.add(principal.getName()); } return groups.toArray(new String[groups.size()]); } - - private boolean roleClassNameMatches(String classname) + + /** + * Check whether the class, its superclasses or any interfaces they implement + * is one of the classes that represents a role. + * + * @param clazz the class to check + * @param roleClassNames the list of classnames that represent roles + * @return true if the class is a role class + */ + private static boolean isRoleClass(Class clazz, List roleClassNames) { - boolean result = false; - for (String roleClassName : getRoleClassNames()) + Class c = clazz; + + //add the class, its interfaces and superclasses to the list to test + List classnames = new ArrayList<>(); + while (c != null) { - if (roleClassName.equals(classname)) - { - result = true; - break; - } + classnames.add(c.getName()); + Arrays.stream(c.getInterfaces()).map(Class::getName).forEach(classnames::add); + c = c.getSuperclass(); } - return result; + + return roleClassNames.stream().anyMatch(classnames::contains); } } diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/PropertyUserStoreManager.java b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/PropertyUserStoreManager.java new file mode 100644 index 00000000000..bf9f00e677b --- /dev/null +++ b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/PropertyUserStoreManager.java @@ -0,0 +1,101 @@ +// +// ======================================================================== +// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others. +// ------------------------------------------------------------------------ +// 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; + +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; + +import org.eclipse.jetty.jaas.spi.PropertyFileLoginModule; +import org.eclipse.jetty.security.PropertyUserStore; +import org.eclipse.jetty.util.component.AbstractLifeCycle; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; + +/** + * PropertyUserStoreManager + * + * Maintains a map of PropertyUserStores, keyed off the location of the property file containing + * the authentication and authorization information. + * + * This class is used to enable the PropertyUserStores to be cached and shared. This is essential + * for the PropertyFileLoginModules, whose lifecycle is controlled by the JAAS api and instantiated + * afresh whenever a user needs to be authenticated. Without this class, every PropertyFileLoginModule + * instantiation would re-read and reload in all the user information just to authenticate a single user. + */ +public class PropertyUserStoreManager extends AbstractLifeCycle +{ + private static final Logger LOG = Log.getLogger(PropertyFileLoginModule.class); + + /** + * Map of user authentication and authorization information loaded in from a property file. + * The map is keyed off the location of the file. + */ + private Map _propertyUserStores; + + public PropertyUserStore getPropertyUserStore(String file) + { + synchronized (this) + { + if (_propertyUserStores == null) + return null; + + return _propertyUserStores.get(file); + } + } + + public PropertyUserStore addPropertyUserStore(String file, PropertyUserStore store) + { + synchronized (this) + { + Objects.requireNonNull(_propertyUserStores); + PropertyUserStore existing = _propertyUserStores.get(file); + if (existing != null) + return existing; + + _propertyUserStores.put(file, store); + return store; + } + } + + @Override + protected void doStart() throws Exception + { + _propertyUserStores = new HashMap(); + super.doStart(); + } + + @Override + protected void doStop() throws Exception + { + for (Map.Entry entry: _propertyUserStores.entrySet()) + { + try + { + entry.getValue().stop(); + } + catch (Exception e) + { + LOG.warn("Error stopping PropertyUserStore at {}", entry.getKey(), e); + } + } + _propertyUserStores = null; + super.doStop(); + } +} diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/callback/DefaultCallbackHandler.java b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/callback/DefaultCallbackHandler.java index 1b3523fcb4d..c6316c077b1 100644 --- a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/callback/DefaultCallbackHandler.java +++ b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/callback/DefaultCallbackHandler.java @@ -26,7 +26,6 @@ import javax.security.auth.callback.PasswordCallback; import javax.security.auth.callback.UnsupportedCallbackException; import org.eclipse.jetty.server.Request; -import org.eclipse.jetty.util.security.Password; /** * DefaultCallbackHandler @@ -47,38 +46,34 @@ public class DefaultCallbackHandler extends AbstractCallbackHandler public void handle(Callback[] callbacks) throws IOException, UnsupportedCallbackException { - for (int i = 0; i < callbacks.length; i++) + for (Callback callback : callbacks) { - if (callbacks[i] instanceof NameCallback) + if (callback instanceof NameCallback) { - ((NameCallback)callbacks[i]).setName(getUserName()); + ((NameCallback)callback).setName(getUserName()); } - else if (callbacks[i] instanceof ObjectCallback) + else if (callback instanceof ObjectCallback) { - ((ObjectCallback)callbacks[i]).setObject(getCredential()); + ((ObjectCallback)callback).setObject(getCredential()); } - else if (callbacks[i] instanceof PasswordCallback) + else if (callback instanceof PasswordCallback) { - if (getCredential() instanceof Password) - ((PasswordCallback)callbacks[i]).setPassword(getCredential().toString().toCharArray()); - else if (getCredential() instanceof String) + ((PasswordCallback)callback).setPassword(getCredential().toString().toCharArray()); + } + else if (callback instanceof RequestParameterCallback) + { + if (_request != null) { - ((PasswordCallback)callbacks[i]).setPassword(((String)getCredential()).toCharArray()); + RequestParameterCallback rpc = (RequestParameterCallback)callback; + rpc.setParameterValues(Arrays.asList(_request.getParameterValues(rpc.getParameterName()))); } - else - throw new UnsupportedCallbackException(callbacks[i], "User supplied credentials cannot be converted to char[] for PasswordCallback: try using an ObjectCallback instead"); } - else if (callbacks[i] instanceof RequestParameterCallback) + else if (callback instanceof ServletRequestCallback) { - RequestParameterCallback callback = (RequestParameterCallback)callbacks[i]; - callback.setParameterValues(Arrays.asList(_request.getParameterValues(callback.getParameterName()))); - } - else if (callbacks[i] instanceof ServletRequestCallback) - { - ((ServletRequestCallback)callbacks[i]).setRequest(_request); + ((ServletRequestCallback)callback).setRequest(_request); } else - throw new UnsupportedCallbackException(callbacks[i]); + throw new UnsupportedCallbackException(callback); } } } 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 79d59fd3626..47ab456c7d0 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 @@ -21,11 +21,12 @@ package org.eclipse.jetty.jaas.spi; 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.jaas.JAASLoginService; +import org.eclipse.jetty.jaas.PropertyUserStoreManager; import org.eclipse.jetty.security.AbstractLoginService; import org.eclipse.jetty.security.PropertyUserStore; import org.eclipse.jetty.server.UserIdentity; @@ -41,14 +42,12 @@ public class PropertyFileLoginModule extends AbstractLoginModule public static final String DEFAULT_FILENAME = "realm.properties"; private static final Logger LOG = Log.getLogger(PropertyFileLoginModule.class); - - private static ConcurrentHashMap _propertyUserStores = new ConcurrentHashMap(); - - private int _refreshInterval = 0; - private String _filename = DEFAULT_FILENAME; + + private PropertyUserStore _store; /** - * Read contents of the configured property file. + * Use a PropertyUserStore to read the authentication and authorizaton information contained in + * the file named by the option "file". * * @param subject the subject * @param callbackHandler the callback handler @@ -64,40 +63,61 @@ public class PropertyFileLoginModule extends AbstractLoginModule setupPropertyUserStore(options); } + /** + * Get an existing, or create a new PropertyUserStore to read the + * authentication and authorization information from the file named by + * the option "file". + * + * @param options configuration options + */ private void setupPropertyUserStore(Map options) { - parseConfig(options); + String filename = (String)options.get("file"); + filename = (filename == null ? DEFAULT_FILENAME : filename); - if (_propertyUserStores.get(_filename) == null) + PropertyUserStoreManager mgr = JAASLoginService.INSTANCE.get().getBean(PropertyUserStoreManager.class); + if (mgr == null) + throw new IllegalStateException("No PropertyUserStoreManager"); + + _store = mgr.getPropertyUserStore(filename); + if (_store == null) { - PropertyUserStore propertyUserStore = new PropertyUserStore(); - propertyUserStore.setConfig(_filename); - - PropertyUserStore prev = _propertyUserStores.putIfAbsent(_filename, propertyUserStore); - if (prev == null) + boolean hotReload = false; + String tmp = (String)options.get("hotReload"); + if (tmp != null) + hotReload = Boolean.parseBoolean(tmp); + else { - LOG.debug("setupPropertyUserStore: Starting new PropertyUserStore. PropertiesFile: " + _filename + " refreshInterval: " + _refreshInterval); - - try + //refreshInterval is deprecated, use hotReload instead + tmp = (String)options.get("refreshInterval"); + if (tmp != null) { - propertyUserStore.start(); - } - catch (Exception e) - { - LOG.warn("Exception while starting propertyUserStore: ", e); + LOG.warn("Use 'hotReload' boolean property instead of 'refreshInterval'"); + try + { + hotReload = (Integer.parseInt(tmp) > 0); + } + catch (NumberFormatException e) + { + LOG.warn("'refreshInterval' is not an integer"); + } } } + PropertyUserStore newStore = new PropertyUserStore(); + newStore.setConfig(filename); + newStore.setHotReload(hotReload); + _store = mgr.addPropertyUserStore(filename, newStore); + try + { + _store.start(); + } + catch (Exception e) + { + LOG.warn("Exception starting propertyUserStore {} ", filename, e); + } } } - private void parseConfig(Map options) - { - String tmp = (String)options.get("file"); - _filename = (tmp == null ? DEFAULT_FILENAME : tmp); - tmp = (String)options.get("refreshInterval"); - _refreshInterval = (tmp == null ? _refreshInterval : Integer.parseInt(tmp)); - } - /** * @param userName the user name * @throws Exception if unable to get the user information @@ -105,12 +125,8 @@ public class PropertyFileLoginModule extends AbstractLoginModule @Override public UserInfo getUserInfo(String userName) throws Exception { - PropertyUserStore propertyUserStore = _propertyUserStores.get(_filename); - if (propertyUserStore == null) - throw new IllegalStateException("PropertyUserStore should never be null here!"); - - LOG.debug("Checking PropertyUserStore " + _filename + " for " + userName); - UserIdentity userIdentity = propertyUserStore.getUserIdentity(userName); + LOG.debug("Checking PropertyUserStore {} for {}", _store.getConfig(), userName); + UserIdentity userIdentity = _store.getUserIdentity(userName); if (userIdentity == null) return null; @@ -123,7 +139,6 @@ public class PropertyFileLoginModule extends AbstractLoginModule .collect(Collectors.toList()); Credential credential = (Credential)userIdentity.getSubject().getPrivateCredentials().iterator().next(); - LOG.debug("Found: " + userName + " in PropertyUserStore " + _filename); return new UserInfo(userName, credential, roles); } } diff --git a/jetty-jaas/src/test/java/org/eclipse/jetty/jaas/JAASLoginServiceTest.java b/jetty-jaas/src/test/java/org/eclipse/jetty/jaas/JAASLoginServiceTest.java index 52a45952bfd..f62fa0070ed 100644 --- a/jetty-jaas/src/test/java/org/eclipse/jetty/jaas/JAASLoginServiceTest.java +++ b/jetty-jaas/src/test/java/org/eclipse/jetty/jaas/JAASLoginServiceTest.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.jaas; import java.security.Principal; +import java.util.Arrays; import java.util.Collections; import javax.security.auth.Subject; import javax.security.auth.login.AppConfigurationEntry; @@ -29,25 +30,17 @@ import org.eclipse.jetty.security.DefaultIdentityService; import org.eclipse.jetty.server.Request; import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; /** * JAASLoginServiceTest */ public class JAASLoginServiceTest { - public static class TestConfiguration extends Configuration - { - AppConfigurationEntry _entry = new AppConfigurationEntry(TestLoginModule.class.getCanonicalName(), LoginModuleControlFlag.REQUIRED, Collections.emptyMap()); - - @Override - public AppConfigurationEntry[] getAppConfigurationEntry(String name) - { - return new AppConfigurationEntry[]{_entry}; - } - } - interface SomeRole { @@ -94,18 +87,31 @@ public class JAASLoginServiceTest @Test public void testServletRequestCallback() throws Exception { + Configuration config = new Configuration() + { + @Override + public AppConfigurationEntry[] getAppConfigurationEntry(String name) + { + return new AppConfigurationEntry[] { + new AppConfigurationEntry(TestLoginModule.class.getCanonicalName(), + LoginModuleControlFlag.REQUIRED, + Collections.emptyMap()) + }; + } + }; + //Test with the DefaultCallbackHandler JAASLoginService ls = new JAASLoginService("foo"); ls.setCallbackHandlerClass("org.eclipse.jetty.jaas.callback.DefaultCallbackHandler"); ls.setIdentityService(new DefaultIdentityService()); - ls.setConfiguration(new TestConfiguration()); + ls.setConfiguration(config); Request request = new Request(null, null); ls.login("aaardvaark", "aaa", request); //Test with the fallback CallbackHandler ls = new JAASLoginService("foo"); ls.setIdentityService(new DefaultIdentityService()); - ls.setConfiguration(new TestConfiguration()); + ls.setConfiguration(config); ls.login("aaardvaark", "aaa", request); } @@ -137,12 +143,8 @@ public class JAASLoginServiceTest subject.getPrincipals().add(new AnotherTestRole("z")); String[] groups = ls.getGroups(subject); - assertEquals(3, groups.length); - for (String g : groups) - { - assertTrue(g.equals("x") || g.equals("y") || g.equals("z")); - } - + assertThat(Arrays.asList(groups), containsInAnyOrder("x", "y", "z")); + //test a custom role class ls.setRoleClassNames(new String[]{AnotherTestRole.class.getName()}); Subject subject2 = new Subject(); @@ -150,8 +152,9 @@ public class JAASLoginServiceTest subject2.getPrincipals().add(new TestRole("x")); subject2.getPrincipals().add(new TestRole("y")); subject2.getPrincipals().add(new AnotherTestRole("z")); - assertEquals(1, ls.getGroups(subject2).length); - assertEquals("z", ls.getGroups(subject2)[0]); + String[] s2groups = ls.getGroups(subject2); + assertThat(s2groups, is(notNullValue())); + assertThat(Arrays.asList(s2groups), containsInAnyOrder("z")); //test a custom role class that implements an interface ls.setRoleClassNames(new String[]{SomeRole.class.getName()}); @@ -160,11 +163,9 @@ public class JAASLoginServiceTest subject3.getPrincipals().add(new TestRole("x")); subject3.getPrincipals().add(new TestRole("y")); subject3.getPrincipals().add(new AnotherTestRole("z")); - assertEquals(3, ls.getGroups(subject3).length); - for (String g : groups) - { - assertTrue(g.equals("x") || g.equals("y") || g.equals("z")); - } + String[] s3groups = ls.getGroups(subject3); + assertThat(s3groups, is(notNullValue())); + assertThat(Arrays.asList(s3groups), containsInAnyOrder("x", "y", "z")); //test a class that doesn't match ls.setRoleClassNames(new String[]{NotTestRole.class.getName()}); 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 index 49fad3f36df..529bc95b54d 100644 --- 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 @@ -19,34 +19,72 @@ package org.eclipse.jetty.jaas.spi; import java.io.File; -import java.util.HashMap; -import javax.security.auth.Subject; +import java.util.Collections; +import javax.security.auth.login.AppConfigurationEntry; +import javax.security.auth.login.AppConfigurationEntry.LoginModuleControlFlag; +import javax.security.auth.login.Configuration; -import org.eclipse.jetty.jaas.callback.DefaultCallbackHandler; +import org.eclipse.jetty.jaas.JAASLoginService; +import org.eclipse.jetty.jaas.PropertyUserStoreManager; +import org.eclipse.jetty.security.DefaultIdentityService; +import org.eclipse.jetty.security.PropertyUserStore; +import org.eclipse.jetty.server.Request; +import org.eclipse.jetty.server.UserIdentity; import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.junit.jupiter.api.Test; 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 static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; public class PropertyFileLoginModuleTest { @Test - public void testRoles() - throws Exception + public void testPropertyFileLoginModule() throws Exception { - File file = MavenTestingUtils.getTestResourceFile("login.properties"); - PropertyFileLoginModule module = new PropertyFileLoginModule(); - Subject subject = new Subject(); - HashMap options = new HashMap<>(); - options.put("file", file.getCanonicalPath()); - module.initialize(subject, new DefaultCallbackHandler(), new HashMap(), options); - UserInfo fred = module.getUserInfo("fred"); - assertEquals("fred", fred.getUserName()); - assertThat(fred.getRoleNames(), containsInAnyOrder("role1", "role2", "role3")); - assertThat(fred.getRoleNames(), not(contains("fred"))); + //configure for PropertyFileLoginModule + File loginProperties = MavenTestingUtils.getTestResourceFile("login.properties"); + + Configuration testConfig = new Configuration() + { + @Override + public AppConfigurationEntry[] getAppConfigurationEntry(String name) + { + return new AppConfigurationEntry[]{new AppConfigurationEntry(PropertyFileLoginModule.class.getName(), + LoginModuleControlFlag.REQUIRED, + Collections.singletonMap("file", loginProperties.getAbsolutePath()))}; + } + }; + + JAASLoginService ls = new JAASLoginService("foo"); + ls.setCallbackHandlerClass("org.eclipse.jetty.jaas.callback.DefaultCallbackHandler"); + ls.setIdentityService(new DefaultIdentityService()); + ls.setConfiguration(testConfig); + ls.start(); + + //test that the manager is created when the JAASLoginService starts + PropertyUserStoreManager mgr = ls.getBean(PropertyUserStoreManager.class); + assertThat(mgr, notNullValue()); + + //test the PropertyFileLoginModule authentication and authorization + Request request = new Request(null, null); + UserIdentity uid = ls.login("fred", "pwd", request); + assertThat(uid.isUserInRole("role1", null), is(true)); + assertThat(uid.isUserInRole("role2", null), is(true)); + assertThat(uid.isUserInRole("role3", null), is(true)); + assertThat(uid.isUserInRole("role4", null), is(false)); + + //Test that the PropertyUserStore is created by the PropertyFileLoginModule + PropertyUserStore store = mgr.getPropertyUserStore(loginProperties.getAbsolutePath()); + assertThat(store, is(notNullValue())); + assertThat(store.isRunning(), is(true)); + assertThat(store.isHotReload(), is(false)); + + //test that the PropertyUserStoreManager is stopped and all PropertyUserStores stopped + ls.stop(); + assertThat(mgr.isStopped(), is(true)); + assertThat(mgr.getPropertyUserStore(loginProperties.getAbsolutePath()), is(nullValue())); + assertThat(store.isStopped(), is(true)); } }