More flexible Authorizer interface.

Authorizers can now express 'I don't care/know' and
let other Authorizers decide.

cdstar-api/src/main/java/de/gwdg/cdstar/auth/realm/Authenticator.java

@@ -17,8 +17,8 @@ public interface Authenticator extends Realm {
 	Session login(Credentials request);
 
 	/**
-	 * Explicitly logout a subject.
+	 * Explicitly logout an authenticated session, if supported by the
+	 * implementation.
 	 */
 	void logout(Session who);
-
 }

cdstar-api/src/main/java/de/gwdg/cdstar/auth/realm/Authorizer.java

@@ -20,13 +20,29 @@ public interface Authorizer extends Realm {
 	 * authentication method. For example, a token based authentication may
 	 * restrict the session to certain permission scopes (e.g. read-only access
 	 * tokens).
-	 *
+	 * 
+	 * Return {@link CheckResult#PERMIT} if the principal is allowed to perform this
+	 * action, {@link CheckResult#DENY} if the principal is blocked from performing
+	 * this action, or {@link CheckResult#SKIP} if the principal is not known or is
+	 * not explicitly allowed or denied this action.
+	 * 
+	 * The end result depends on the return values of all installed
+	 * {@link Authorizer}s: The action is granted only if at least one Authorizer
+	 * returns {@link CheckResult#PERMIT} and not a single {@link CheckResult#DENY}
+	 * is returned. In other words, {@link CheckResult#DENY} will overrule
+	 * {@link CheckResult#PERMIT} and should only be used by {@link Authorizer}s
+	 * that wish to restrict other more permissive {@link Authorizer}s.
+	 * 
 	 * @param p
-	 *            The permission to check.
+	 *                The permission to check.
 	 * @param session
-	 *            The session asking for permission, or null if anonymous access
-	 *            is requested.
-	 * @return True if the permission is granted, false otherwise.
+	 *                The session asking for permission, or null if anonymous access
+	 *                is requested.
+	 * @return {@link CheckResult}
 	 */
-	boolean isPermitted(Session session, Permission p);
+	CheckResult isPermitted(Session session, Permission p);
+
+	enum CheckResult {
+		PERMIT, DENY, SKIP;
+	}
 }

cdstar-api/src/main/java/de/gwdg/cdstar/auth/realm/PermissionResolver.java

@@ -8,11 +8,11 @@ import de.gwdg.cdstar.auth.StringPermission;
 /**
  * A {@link PermissionResolver} translates domain specific {@link Permission}
  * types into zero or more permissions of a more general type, for example
- * {{@link StringPermission}}.
+ * 	{@link StringPermission}.
  *
- * This allows allowing the use of custom {@link Permission} subclasses and
- * bridge them to other general purpose {@link Authorizer} implementations that
- * usually only support {{@link StringPermission}}.
+ * This allows the use of custom {@link Permission} subclasses and bridge them
+ * to other general purpose {@link Authorizer} implementations that usually only
+ * support {@link StringPermission}.
  *
  * The returned permissions are not resolved again to prevent loops.
  */

cdstar-api/src/main/java/de/gwdg/cdstar/auth/realm/SessionStore.java

@@ -4,8 +4,9 @@ import de.gwdg.cdstar.auth.Session;
 import de.gwdg.cdstar.auth.TokenCredentials;
 
 /**
- * A {@link SessionStore} is able to persist the principal of a session and
- * return a unique session identifier.
+ * A {@link SessionStore} is able to remember the principal of a session and
+ * return a unique session identifier, which can then be used to continue a
+ * previously remembered session.
  *
  * Implementations should also implement {@link Authenticator} and listen for
  * {@link TokenCredentials} so persisted sessions can be continued, as well as
@@ -16,8 +17,10 @@ public interface SessionStore extends Authenticator {
 
 	/**
 	 * Persist the principal of the given session. Calling this on a session
-	 * that is already persisted should update the persisted state.
+	 * that is already persisted should update the persisted state and
+	 * return the same string.
 	 */
 	String remember(Session session);
 
+
 }

cdstar-auth/src/main/java/de/gwdg/cdstar/auth/SubjectImpl.java

@@ -76,33 +76,39 @@ public class SubjectImpl implements Subject {
 		if (session == null)
 			return null;
 
-		// Check original permission
-		final List<Authorizer> authorizers = config.getRealmsByClass(Authorizer.class);
-		for (final Authorizer auth : authorizers) {
-			if (auth.isPermitted(session, p)) {
-				log.debug("Permission check passed: [{}] for [{}] (direct)", p, this);
-				return auth;
-			}
-		}
-
-		// Collect, then check derived permissions
+		// Collect derived permissions
 		final List<Permission> derieved = new ArrayList<>();
+		derieved.add(p);
 		for (final PermissionResolver realm : config.getRealmsByClass(PermissionResolver.class)) {
 			for (final Permission pr : realm.resolve(p)) {
 				derieved.add(pr);
 			}
 		}
-		// Check again for derived permissions
-		for (final Authorizer auth : authorizers) {
-			for (final Permission pr : derieved) {
-				if (auth.isPermitted(session, p)) {
-					log.debug("Permission check passed: [{}] for [{}] (derived)", pr, this);
-					return auth;
+
+		// Check permission. We need at least one PERMIT and no DENY for success.
+		Authorizer permittetBy = null;
+		final List<Authorizer> authorizers = config.getRealmsByClass(Authorizer.class);
+		for (final Permission pr : derieved) {
+			for (final Authorizer auth : authorizers) {
+				switch (auth.isPermitted(session, p)) {
+				case DENY:
+					log.info("Permission [{}] denied for [{}] by [{}]", pr, this, auth);
+					return null;
+				case PERMIT:
+					log.debug("Permission [{}] granted for [{}] by [{}]", pr, this, auth);
+					permittetBy = auth;
+					break;
+				case SKIP:
+				default:
+					break;
 				}
 			}
 		}
 
-		log.info("Permission check failed: [{}] for [{}]", p, this);
+		if (permittetBy != null)
+			return permittetBy;
+
+		log.info("Permission [{}] not granted for [{}] by any installed authenticator", p, this);
 		return null;
 	}
 

cdstar-auth/src/main/java/de/gwdg/cdstar/auth/realm/DomainAuthorizer.java

@@ -83,18 +83,18 @@ public class DomainAuthorizer implements Authorizer, GroupResolver {
 	}
 
 	@Override
-	public boolean isPermitted(Session session, Permission p) {
+	public CheckResult isPermitted(Session session, Permission p) {
 		if (!(p instanceof StringPermission))
-			return false;
+			return CheckResult.SKIP;
 
 		final String domain = session.getPrincipal().getDomain();
 		final Set<StringPermission> grants = domainGrants.get(domain);
 		if (grants == null || grants.isEmpty())
-			return false;
+			return CheckResult.SKIP;
 		for (final StringPermission grant : grants)
 			if (grant.implies(p))
-				return true;
-		return false;
+				return CheckResult.PERMIT;
+		return CheckResult.SKIP;
 	}
 
 }

cdstar-auth/src/main/java/de/gwdg/cdstar/auth/simple/SimpleAuthorizer.java

@@ -118,32 +118,32 @@ public class SimpleAuthorizer implements Authorizer, Authenticator, GroupResolve
 	}
 
 	@Override
-	public boolean isPermitted(Session session, Permission p) {
+	public CheckResult isPermitted(Session session, Permission p) {
 		final Account acc = resolveAccount(session);
 		if (acc == null)
-			return false;
+			return CheckResult.SKIP;
 
 		// TODO: Cache these.
 
 		// Check direct permissions
 		for (final StringPermission accp : acc.permissions)
 			if (accp.implies(p))
-				return true;
+				return CheckResult.PERMIT;
 
 		// Check role->permissions
 		for (final String role : acc.roles)
 			for (final StringPermission accp : roles.getOrDefault(role, Collections.emptySet()))
 				if (accp.implies(p))
-					return true;
+					return CheckResult.PERMIT;
 
 		// Check group->role->permissions
 		for (final QName group : acc.groups)
 			for (final String role : groups.getOrDefault(group, Collections.emptySet()))
 				for (final StringPermission accp : roles.getOrDefault(role, Collections.emptySet()))
 					if (accp.implies(p))
-						return true;
+						return CheckResult.PERMIT;
 
-		return false;
+		return CheckResult.SKIP;
 	}
 
 	@Override

cdstar-auth/src/test/java/de/gwdg/cdstar/auth/DomainAuthorizerTest.java

 package de.gwdg.cdstar.auth;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import org.junit.Rule;
 import org.junit.Test;
 
+import de.gwdg.cdstar.auth.realm.Authorizer.CheckResult;
 import de.gwdg.cdstar.auth.realm.DomainAuthorizer;
 import de.gwdg.cdstar.utils.test.TestLogger;
 
@@ -22,14 +24,14 @@ public class DomainAuthorizerTest {
 
 		assertTrue(a.isMemberOf(new MockSession("foo", "GWDG"), "user", "GWDG"));
 		assertTrue(a.isMemberOf(new MockSession("foo", "GWDG"), "other", "OTHER"));
-		assertTrue(a.isPermitted(new MockSession("foo", "GWDG"), StringPermission.parse("vault:GWDG:anything")));
+		assertEquals(CheckResult.PERMIT, a.isPermitted(new MockSession("foo", "GWDG"), StringPermission.parse("vault:GWDG:anything")));
 
 		assertFalse(a.isMemberOf(new MockSession("foo", "GWDG"), "other", "GWDG"));
 		assertFalse(a.isMemberOf(new MockSession("foo", "GWDG"), "user", "OTHER"));
-		assertFalse(a.isPermitted(new MockSession("foo", "GWDG"), StringPermission.parse("vault:other:anything")));
+		assertEquals(CheckResult.SKIP, a.isPermitted(new MockSession("foo", "GWDG"), StringPermission.parse("vault:other:anything")));
 
 		assertFalse(a.isMemberOf(new MockSession("foo", "GWDG2"), "user", "GWDG"));
-		assertFalse(a.isPermitted(new MockSession("foo", "GWDG2"), StringPermission.parse("vault:GWDG:anything")));
+		assertEquals(CheckResult.SKIP, a.isPermitted(new MockSession("foo", "GWDG2"), StringPermission.parse("vault:GWDG:anything")));
 
 	}
 

cdstar-auth/src/test/java/de/gwdg/cdstar/auth/StaticAuthorizerTest.java

@@ -8,6 +8,7 @@ import static org.junit.Assert.assertTrue;
 import org.junit.Before;
 import org.junit.Test;
 
+import de.gwdg.cdstar.auth.realm.Authorizer.CheckResult;
 import de.gwdg.cdstar.auth.simple.Account;
 import de.gwdg.cdstar.auth.simple.SimpleAuthorizer;
 
@@ -113,7 +114,7 @@ public class StaticAuthorizerTest {
 	}
 
 	private boolean check(Session session, String permission) {
-		return auth.isPermitted(session, StringPermission.parse(permission));
+		return CheckResult.PERMIT == auth.isPermitted(session, StringPermission.parse(permission));
 	}
 
 }

cdstar-auth/src/test/java/de/gwdg/cdstar/auth/StaticRealmTest.java

@@ -9,6 +9,7 @@ import static org.junit.Assert.assertTrue;
 import org.junit.Rule;
 import org.junit.Test;
 
+import de.gwdg.cdstar.auth.realm.Authorizer.CheckResult;
 import de.gwdg.cdstar.auth.realm.StaticRealm;
 import de.gwdg.cdstar.config.MapConfig;
 import de.gwdg.cdstar.runtime.Config;
@@ -49,9 +50,9 @@ public class StaticRealmTest {
 		assertTrue(realm.isMemberOf(session, "testGroup"));
 		assertFalse(realm.isMemberOf(session, "fakeGroup"));
 
-		assertTrue(realm.isPermitted(session, StringPermission.parse("do:stuff")));
-		assertTrue(realm.isPermitted(session, StringPermission.parse("role:stuff")));
-		assertTrue(realm.isPermitted(session, StringPermission.parse("group:stuff")));
+		assertEquals(CheckResult.PERMIT, realm.isPermitted(session, StringPermission.parse("do:stuff")));
+		assertEquals(CheckResult.PERMIT, realm.isPermitted(session, StringPermission.parse("role:stuff")));
+		assertEquals(CheckResult.PERMIT, realm.isPermitted(session, StringPermission.parse("group:stuff")));
 	}
 
 	@Test
@@ -70,8 +71,7 @@ public class StaticRealmTest {
 		final StaticRealm realm = new StaticRealm(cfg);
 		assertEquals("static", realm.getDomain());
 		assertEquals("myDomain", realm.account("test", "myDomain").getDomain());
-		assertTrue(realm.isPermitted(new MockSession("test", "myDomain"), StringPermission.parse("do:stuff")));
-
+		assertEquals(CheckResult.PERMIT, realm.isPermitted(new MockSession("test", "myDomain"), StringPermission.parse("do:stuff")));
 	}
 
 	@Test
@@ -85,7 +85,7 @@ public class StaticRealmTest {
 
 		// Still authorizes user, even if it comes from a different
 		// authenticator
-		assertTrue(realm.isPermitted(new MockSession("test", "static"), StringPermission.parse("do:stuff")));
+		assertEquals(CheckResult.PERMIT, realm.isPermitted(new MockSession("test", "static"), StringPermission.parse("do:stuff")));
 	}
 
 }

cdstar-plugins-pom/cdstar-auth-jwt/src/main/java/de/gwdg/cdstar/ext/auth/jwt/JWTRealm.java

@@ -148,26 +148,26 @@ public class JWTRealm implements Authenticator, Authorizer {
 	}
 
 	@Override
-	public boolean isPermitted(Session session, Permission p) {
+	public CheckResult isPermitted(Session session, Permission p) {
 		if (!(session instanceof JWTPrincipal))
-			return false;
+			return CheckResult.SKIP;
 		if (!(p instanceof StringPermission))
-			return false;
+			return CheckResult.SKIP;
 
 		final JWTPrincipal jwtUser = (JWTPrincipal) session;
 		try {
 			jwtUser.checkExpired();
 		} catch (final TokenExpiredException e) {
 			log.warn("Use of an expired Token: {}", session, e);
-			return false;
+			return CheckResult.DENY;
 		}
 
 		for (final Permission sp : jwtUser.getPermissions()) {
 			if (p instanceof StringPermission && ((StringPermission) sp).implies(p))
-				return true;
+				return CheckResult.PERMIT;
 		}
 
-		return false;
+		return CheckResult.SKIP;
 	}
 
 }