From d2e13ec5ad210c2a8f515d4099434b7fc1381c3b Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 7 Mar 2019 12:27:43 +0100 Subject: [PATCH 1/2] Code cleanup and improved toString() and javadocs. Signed-off-by: Simone Bordet --- .../org/eclipse/jetty/io/CyclicTimeout.java | 55 ++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/CyclicTimeout.java b/jetty-io/src/main/java/org/eclipse/jetty/io/CyclicTimeout.java index 4b26c3d184c..c9f05f95591 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/CyclicTimeout.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/CyclicTimeout.java @@ -33,6 +33,20 @@ import static java.lang.Long.MAX_VALUE; *

Subclasses should implement {@link #onTimeoutExpired()}.

*

This implementation is optimised assuming that the timeout * will mostly be cancelled and then reused with a similar value.

+ *

This implementation has a {@link Timeout} holding the time + * at which the scheduled task should fire, and a linked list of + * {@link Wakeup}, each holding the actual scheduled task.

+ *

Calling {@link #schedule(long, TimeUnit)} the first time will + * create a Timeout with an associated Wakeup and submit a task to + * the scheduler. + * Calling {@link #schedule(long, TimeUnit)} again with the same or + * a larger delay will cancel the previous Timeout, but keep the + * previous Wakeup without submitting a new task to the scheduler, + * therefore reducing the pressure on the scheduler and avoid it + * becomes a bottleneck. + * When the Wakeup task fires, it will see that the Timeout is now + * in the future and will attach a new Wakeup with the future time + * to the Timeout, and submit a scheduler task for the new Wakeup.

*/ public abstract class CyclicTimeout implements Destroyable { @@ -59,24 +73,24 @@ public abstract class CyclicTimeout implements Destroyable } /** - * Schedules a timeout, even if already set, cancelled or expired. + *

Schedules a timeout, even if already set, cancelled or expired.

+ *

If a timeout is already set, it will be cancelled and replaced + * by the new one.

* * @param delay The period of time before the timeout expires. * @param units The unit of time of the period. - * @return true if the timer was already set. + * @return true if the timeout was already set. */ public boolean schedule(long delay, TimeUnit units) { long now = System.nanoTime(); long new_timeout_at = now + units.toNanos(delay); + Wakeup new_wakeup = null; boolean result; - Wakeup new_wakeup; while (true) { Timeout timeout = _timeout.get(); - - new_wakeup = null; result = timeout._at != MAX_VALUE; // Is the current wakeup good to use? ie before our timeout time? @@ -114,14 +128,12 @@ public abstract class CyclicTimeout implements Destroyable public boolean cancel() { boolean result; - Timeout timeout; - Timeout new_timeout; while (true) { - timeout = _timeout.get(); + Timeout timeout = _timeout.get(); result = timeout._at != MAX_VALUE; Wakeup wakeup = timeout._wakeup; - new_timeout = wakeup == null ? NOT_SET : new Timeout(MAX_VALUE, wakeup); + Timeout new_timeout = wakeup == null ? NOT_SET : new Timeout(MAX_VALUE, wakeup); if (_timeout.compareAndSet(timeout, new_timeout)) break; } @@ -166,7 +178,11 @@ public abstract class CyclicTimeout implements Destroyable @Override public String toString() { - return String.format("%s@%x:%d,%s", getClass().getSimpleName(), hashCode(), _at, _wakeup); + return String.format("%s@%x:%dms,%s", + getClass().getSimpleName(), + hashCode(), + TimeUnit.NANOSECONDS.toMillis(_at - System.nanoTime()), + _wakeup); } } @@ -200,10 +216,9 @@ public abstract class CyclicTimeout implements Destroyable @Override public void run() { - long now; - Wakeup new_wakeup; - boolean has_expired; - + long now = System.nanoTime(); + Wakeup new_wakeup = null; + boolean has_expired = false; while (true) { Timeout timeout = _timeout.get(); @@ -226,16 +241,12 @@ public abstract class CyclicTimeout implements Destroyable // Not found, we become a noop. return; - now = System.nanoTime(); - new_wakeup = null; - has_expired = false; - Timeout new_timeout; - // We are in the wakeup list! So we have to act and we know our // tail has not expired (else it would have removed us from the list). // Remove ourselves (and any prior Wakeup) from the wakeup list. wakeup = wakeup._next; + Timeout new_timeout; if (timeout._at <= now) { // We have timed out! @@ -274,7 +285,11 @@ public abstract class CyclicTimeout implements Destroyable @Override public String toString() { - return String.format("%s@%x:%d->%s", getClass().getSimpleName(), hashCode(), _at, _next); + return String.format("%s@%x:%dms->%s", + getClass().getSimpleName(), + hashCode(), + _at == MAX_VALUE ? _at : TimeUnit.NANOSECONDS.toMillis(_at - System.nanoTime()), + _next); } } } From beb4881af42b53b0e7c598e592ed75263904d29c Mon Sep 17 00:00:00 2001 From: Jan Bartel Date: Fri, 8 Mar 2019 10:44:39 +1100 Subject: [PATCH 2/2] Issue #3394 Remove unused JAASGroup,RoleCheckPolicy,StrictRoleCheckPolicy (#3433) Signed-off-by: Jan Bartel --- .../configuring/security/jaas-support.adoc | 14 --- .../org/eclipse/jetty/jaas/JAASGroup.java | 117 ------------------ .../eclipse/jetty/jaas/RoleCheckPolicy.java | 35 ------ .../jetty/jaas/StrictRoleCheckPolicy.java | 63 ---------- .../config/demo-base/webapps/test-jaas.xml | 2 +- 5 files changed, 1 insertion(+), 230 deletions(-) delete mode 100644 jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASGroup.java delete mode 100644 jetty-jaas/src/main/java/org/eclipse/jetty/jaas/RoleCheckPolicy.java delete mode 100644 jetty-jaas/src/main/java/org/eclipse/jetty/jaas/StrictRoleCheckPolicy.java diff --git a/jetty-documentation/src/main/asciidoc/configuring/security/jaas-support.adoc b/jetty-documentation/src/main/asciidoc/configuring/security/jaas-support.adoc index ce69083655b..d6e486c86e4 100644 --- a/jetty-documentation/src/main/asciidoc/configuring/security/jaas-support.adoc +++ b/jetty-documentation/src/main/asciidoc/configuring/security/jaas-support.adoc @@ -179,7 +179,6 @@ To allow the greatest degree of flexibility in using JAAS with web applications, Note that you don't ordinarily need to set these explicitly, as Jetty has defaults which will work in 99% of cases. However, should you need to, you can configure: -* a policy for role-based authorization (Default: `org.eclipse.jetty.jaas.StrictRoleCheckPolicy`) * a CallbackHandler (Default: `org.eclipse.jetty.jaas.callback.DefaultCallbackHandler`) * a list of classnames for the Principal implementation that equate to a user role (Default: `org.eclipse.jetty.jaas.JAASRole`) @@ -190,9 +189,6 @@ Here's an example of setting each of these (to their default values): Test JAAS Realm xyz - - - org.eclipse.jetty.jaas.callback.DefaultCallbackHandler @@ -204,16 +200,6 @@ Here's an example of setting each of these (to their default values): ---- -===== RoleCheckPolicy - -The `RoleCheckPolicy` must be an implementation of the `org.eclipse.jetty.jaas.RoleCheckPolicy` interface and its purpose is to help answer the question "is User X in Role Y" for role-based authorization requests. -The default implementation distributed with Jetty is the `org.eclipse.jetty.jaas.StrictRoleCheckPolicy`, which will assess a user as having a particular role if that role is at the top of the stack of roles that have been temporarily pushed onto the user. -If the user has no temporarily assigned roles, the role is amongst those configured for the user. - -Roles can be temporarily assigned to a user programmatically by using the `pushRole(String rolename)` method of the `org.eclipse.jetty.jaas.JAASUserPrincipal` class. - -For the majority of webapps, the default `StrictRoleCheckPolicy` will be quite adequate, however you may provide your own implementation and set it on your `JAASLoginService` instance. - ===== CallbackHandler A CallbackHandler is responsible for interfacing with the user to obtain usernames and credentials to be authenticated. diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASGroup.java b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASGroup.java deleted file mode 100644 index fe87b01d4bf..00000000000 --- a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/JAASGroup.java +++ /dev/null @@ -1,117 +0,0 @@ -// -// ======================================================================== -// 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; - -import java.security.Principal; -import java.security.acl.Group; -import java.util.Enumeration; -import java.util.HashSet; -import java.util.Iterator; - -public class JAASGroup implements Group -{ - public static final String ROLES = "__roles__"; - - private String _name = null; - private HashSet _members = null; - - public JAASGroup(String n) - { - this._name = n; - this._members = new HashSet(); - } - - /* ------------------------------------------------------------ */ - @Override - public synchronized boolean addMember(Principal principal) - { - return _members.add(principal); - } - - @Override - public synchronized boolean removeMember(Principal principal) - { - return _members.remove(principal); - } - - @Override - public boolean isMember(Principal principal) - { - return _members.contains(principal); - } - - @Override - public Enumeration members() - { - - class MembersEnumeration implements Enumeration - { - private Iterator itor; - - public MembersEnumeration (Iterator itor) - { - this.itor = itor; - } - - @Override - public boolean hasMoreElements () - { - return this.itor.hasNext(); - } - - - @Override - public Principal nextElement () - { - return this.itor.next(); - } - - } - - return new MembersEnumeration (_members.iterator()); - } - - @Override - public int hashCode() - { - return getName().hashCode(); - } - - @Override - public boolean equals(Object object) - { - if (! (object instanceof JAASGroup)) - return false; - - return ((JAASGroup)object).getName().equals(getName()); - } - - @Override - public String toString() - { - return getName(); - } - - @Override - public String getName() - { - - return _name; - } -} diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/RoleCheckPolicy.java b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/RoleCheckPolicy.java deleted file mode 100644 index d83f647f9d6..00000000000 --- a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/RoleCheckPolicy.java +++ /dev/null @@ -1,35 +0,0 @@ -// -// ======================================================================== -// 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; - -import java.security.Principal; -import java.security.acl.Group; - -public interface RoleCheckPolicy -{ - /* ------------------------------------------------ */ - /** Check if a role is either a runAsRole or in a set of roles - * @param roleName the role to check - * @param runAsRole a pushed role (can be null) - * @param roles a Group whose Principals are role names - * @return true if role equals runAsRole or is a member of roles. - */ - public boolean checkRole (String roleName, Principal runAsRole, Group roles); - -} diff --git a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/StrictRoleCheckPolicy.java b/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/StrictRoleCheckPolicy.java deleted file mode 100644 index 712a4bc4911..00000000000 --- a/jetty-jaas/src/main/java/org/eclipse/jetty/jaas/StrictRoleCheckPolicy.java +++ /dev/null @@ -1,63 +0,0 @@ -// -// ======================================================================== -// 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; - -import java.security.Principal; -import java.security.acl.Group; -import java.util.Enumeration; - - -/* ---------------------------------------------------- */ -/** StrictRoleCheckPolicy - *

Enforces that if a runAsRole is present, then the - * role to check must be the same as that runAsRole and - * the set of static roles is ignored. - * - * - * - */ -public class StrictRoleCheckPolicy implements RoleCheckPolicy -{ - - @Override - public boolean checkRole (String roleName, Principal runAsRole, Group roles) - { - //check if this user has had any temporary role pushed onto - //them. If so, then only check if the user has that role. - if (runAsRole != null) - { - return (roleName.equals(runAsRole.getName())); - } - else - { - if (roles == null) - return false; - Enumeration rolesEnum = roles.members(); - boolean found = false; - while (rolesEnum.hasMoreElements() && !found) - { - Principal p = (Principal)rolesEnum.nextElement(); - found = roleName.equals(p.getName()); - } - return found; - } - - } - -} diff --git a/tests/test-webapps/test-jaas-webapp/src/main/config/demo-base/webapps/test-jaas.xml b/tests/test-webapps/test-jaas-webapp/src/main/config/demo-base/webapps/test-jaas.xml index c56276b0a7e..eb031c87a3d 100644 --- a/tests/test-webapps/test-jaas-webapp/src/main/config/demo-base/webapps/test-jaas.xml +++ b/tests/test-webapps/test-jaas-webapp/src/main/config/demo-base/webapps/test-jaas.xml @@ -2,7 +2,7 @@ - +