diff --git a/profiles/killbill/src/main/java/org/killbill/billing/server/modules/KillBillShiroWebModule.java b/profiles/killbill/src/main/java/org/killbill/billing/server/modules/KillBillShiroWebModule.java index b8a7992da7..ad6713de04 100644 --- a/profiles/killbill/src/main/java/org/killbill/billing/server/modules/KillBillShiroWebModule.java +++ b/profiles/killbill/src/main/java/org/killbill/billing/server/modules/KillBillShiroWebModule.java @@ -1,7 +1,7 @@ /* * Copyright 2010-2013 Ning, Inc. - * Copyright 2014-2018 Groupon, Inc - * Copyright 2014-2018 The Billing Project, LLC + * Copyright 2014-2019 Groupon, Inc + * Copyright 2014-2019 The Billing Project, LLC * * The Billing Project licenses this file to you under the Apache License, version 2.0 * (the "License"); you may not use this file except in compliance with the @@ -19,7 +19,7 @@ package org.killbill.billing.server.modules; import java.util.Collection; -import java.util.LinkedList; +import java.util.Set; import javax.servlet.ServletContext; import javax.servlet.ServletRequest; @@ -28,11 +28,12 @@ import org.apache.shiro.authc.pam.ModularRealmAuthenticator; import org.apache.shiro.authc.pam.ModularRealmAuthenticatorWith540; -import org.apache.shiro.authz.ModularRealmAuthorizer; import org.apache.shiro.cache.CacheManager; import org.apache.shiro.guice.web.ShiroWebModuleWith435; +import org.apache.shiro.mgt.DefaultSecurityManager; import org.apache.shiro.mgt.SubjectDAO; import org.apache.shiro.realm.Realm; +import org.apache.shiro.realm.text.IniRealm; import org.apache.shiro.session.mgt.SessionManager; import org.apache.shiro.session.mgt.eis.SessionDAO; import org.apache.shiro.web.filter.authc.BasicHttpAuthenticationFilter; @@ -45,6 +46,7 @@ import org.killbill.billing.server.security.KillbillJdbcTenantRealm; import org.killbill.billing.util.config.definition.RbacConfig; import org.killbill.billing.util.config.definition.RedisCacheConfig; +import org.killbill.billing.util.config.definition.SecurityConfig; import org.killbill.billing.util.glue.EhcacheShiroManagerProvider; import org.killbill.billing.util.glue.KillBillShiroModule; import org.killbill.billing.util.glue.RealmsFromShiroIniProvider; @@ -56,7 +58,10 @@ import org.skife.config.ConfigSource; import org.skife.config.ConfigurationObjectFactory; +import com.google.common.collect.ImmutableSet; +import com.google.inject.Inject; import com.google.inject.Key; +import com.google.inject.Provider; import com.google.inject.TypeLiteral; import com.google.inject.binder.AnnotatedBindingBuilder; import com.google.inject.matcher.AbstractMatcher; @@ -70,10 +75,12 @@ public class KillBillShiroWebModule extends ShiroWebModuleWith435 { private final ConfigSource configSource; + private final DefaultSecurityManager defaultSecurityManager; public KillBillShiroWebModule(final ServletContext servletContext, final ConfigSource configSource) { super(servletContext); this.configSource = configSource; + this.defaultSecurityManager = RealmsFromShiroIniProvider.get(configSource); } @Override @@ -92,9 +99,18 @@ public String getString(final String propertyName) { bind(CacheManager.class).toProvider(EhcacheShiroManagerProvider.class).asEagerSingleton(); } + final SecurityConfig securityConfig = new ConfigurationObjectFactory(configSource).build(SecurityConfig.class); + final Collection realms = defaultSecurityManager.getRealms() != null ? defaultSecurityManager.getRealms() : + ImmutableSet.of(new IniRealm(securityConfig.getShiroResourcePath())); // Mainly for testing + for (final Realm realm : realms) { + bindRealm().toInstance(realm); + } + configureShiroForRBAC(); configureShiroForTenants(); + + expose(new TypeLiteral>() {}); } private void configureShiroForRBAC() { @@ -129,6 +145,32 @@ private void configureShiroForTenants() { expose(KillbillJdbcTenantRealm.class); } + @Override + protected void bindWebSecurityManager(final AnnotatedBindingBuilder bind) { + //super.bindWebSecurityManager(bind); + // This following is to work around obscure Guice issues + bind.toProvider(KillBillWebSecurityManagerProvider.class).asEagerSingleton(); + } + + public static final class KillBillWebSecurityManagerProvider implements Provider { + + private final Collection realms; + private final SessionManager sessionManager; + + @Inject + public KillBillWebSecurityManagerProvider(final Collection realms, final SessionManager sessionManager) { + this.realms = realms; + this.sessionManager = sessionManager; + } + + @Override + public DefaultWebSecurityManager get() { + final DefaultWebSecurityManager defaultWebSecurityManager = new DefaultWebSecurityManager(realms); + defaultWebSecurityManager.setSessionManager(sessionManager); + return defaultWebSecurityManager; + } + } + @Override protected void bindSessionManager(final AnnotatedBindingBuilder bind) { // Bypass the servlet container completely for session management and delegate it to Shiro. @@ -175,20 +217,10 @@ public void hear(final TypeLiteral typeLiteral, final TypeEncounter ty public void afterInjection(final Object o) { final DefaultWebSecurityManager webSecurityManager = (DefaultWebSecurityManager) o; - // Other realms have been injected by Guice (bindRealm().toInstance(...) makes Guice throw a ClassCastException?!) - final Collection realmsFromShiroIni = RealmsFromShiroIniProvider.get(configSource); - - if (webSecurityManager.getAuthorizer() instanceof ModularRealmAuthorizer) { - final ModularRealmAuthorizer modularRealmAuthorizer = (ModularRealmAuthorizer) webSecurityManager.getAuthorizer(); - final Collection realms = new LinkedList(realmsFromShiroIni); - realms.addAll(modularRealmAuthorizer.getRealms()); - modularRealmAuthorizer.setRealms(realms); - } - if (webSecurityManager.getAuthenticator() instanceof ModularRealmAuthenticator) { final ModularRealmAuthenticator authenticator = (ModularRealmAuthenticator) webSecurityManager.getAuthenticator(); authenticator.setAuthenticationStrategy(new FirstSuccessfulStrategyWith540()); - webSecurityManager.setAuthenticator(new ModularRealmAuthenticatorWith540(realmsFromShiroIni, authenticator)); + webSecurityManager.setAuthenticator(new ModularRealmAuthenticatorWith540(webSecurityManager.getRealms(), authenticator)); } } }); diff --git a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestSecurity.java b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestSecurity.java index 0405446faf..9f62521644 100644 --- a/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestSecurity.java +++ b/profiles/killbill/src/test/java/org/killbill/billing/jaxrs/TestSecurity.java @@ -152,15 +152,15 @@ public void testUserWithUpdates() throws KillBillClientException { logout(); login(username, password); List permissions = securityApi.getCurrentUserPermissions(requestOptions); - Assert.assertEquals(permissions.size(), Permission.values().length); + Assert.assertEquals(permissions, ImmutableList.of("*")); - String newPassword = "IamTheBestWarrior"; + final String newPassword = "IamTheBestWarrior"; securityApi.updateUserPassword(username, new UserRoles(username, newPassword, null), requestOptions); logout(); login(username, newPassword); permissions = securityApi.getCurrentUserPermissions(requestOptions); - Assert.assertEquals(permissions.size(), Permission.values().length); + Assert.assertEquals(permissions, ImmutableList.of("*")); final String newRoleDefinition = "somethingLessNice"; // Only enough permissions to invalidate itself in the last step... diff --git a/util/src/main/java/org/killbill/billing/util/glue/KillBillShiroModule.java b/util/src/main/java/org/killbill/billing/util/glue/KillBillShiroModule.java index 2f9a3e09b4..050127bd0c 100644 --- a/util/src/main/java/org/killbill/billing/util/glue/KillBillShiroModule.java +++ b/util/src/main/java/org/killbill/billing/util/glue/KillBillShiroModule.java @@ -1,7 +1,7 @@ /* * Copyright 2010-2013 Ning, Inc. - * Copyright 2014-2018 Groupon, Inc - * Copyright 2014-2018 The Billing Project, LLC + * Copyright 2014-2019 Groupon, Inc + * Copyright 2014-2019 The Billing Project, LLC * * The Billing Project licenses this file to you under the Apache License, version 2.0 * (the "License"); you may not use this file except in compliance with the @@ -18,10 +18,15 @@ package org.killbill.billing.util.glue; +import java.util.Collection; +import java.util.Set; + import org.apache.shiro.cache.CacheManager; import org.apache.shiro.guice.ShiroModule; +import org.apache.shiro.mgt.DefaultSecurityManager; import org.apache.shiro.mgt.SecurityManager; import org.apache.shiro.mgt.SubjectDAO; +import org.apache.shiro.realm.Realm; import org.apache.shiro.realm.text.IniRealm; import org.apache.shiro.session.mgt.DefaultSessionManager; import org.apache.shiro.session.mgt.SessionManager; @@ -29,13 +34,15 @@ import org.killbill.billing.platform.api.KillbillConfigSource; import org.killbill.billing.util.config.definition.RbacConfig; import org.killbill.billing.util.config.definition.RedisCacheConfig; +import org.killbill.billing.util.config.definition.SecurityConfig; import org.killbill.billing.util.security.shiro.realm.KillBillJdbcRealm; import org.killbill.billing.util.security.shiro.realm.KillBillJndiLdapRealm; import org.killbill.billing.util.security.shiro.realm.KillBillOktaRealm; import org.skife.config.ConfigSource; import org.skife.config.ConfigurationObjectFactory; -import com.google.inject.Provider; +import com.google.common.collect.ImmutableSet; +import com.google.inject.TypeLiteral; import com.google.inject.binder.AnnotatedBindingBuilder; // For Kill Bill library only. @@ -59,9 +66,18 @@ public static boolean isRBACEnabled() { } private final KillbillConfigSource configSource; + private final ConfigSource skifeConfigSource; + private final DefaultSecurityManager defaultSecurityManager; public KillBillShiroModule(final KillbillConfigSource configSource) { this.configSource = configSource; + this.skifeConfigSource = new ConfigSource() { + @Override + public String getString(final String propertyName) { + return configSource.getString(propertyName); + } + }; + this.defaultSecurityManager = RealmsFromShiroIniProvider.get(skifeConfigSource); } protected void configureShiro() { @@ -73,25 +89,22 @@ public String getString(final String propertyName) { }).build(RbacConfig.class); bind(RbacConfig.class).toInstance(config); - final ConfigSource skifeConfigSource = new ConfigSource() { - @Override - public String getString(final String propertyName) { - return configSource.getString(propertyName); - } - }; - bind(RbacConfig.class).toInstance(config); - final Provider iniRealmProvider = RealmsFromShiroIniProvider.getIniRealmProvider(skifeConfigSource); - // Hack for Kill Bill library to work around weird Guice ClassCastException when using - // bindRealm().toInstance(...) -- this means we don't support custom realms when embedding Kill Bill - bindRealm().toProvider(iniRealmProvider).asEagerSingleton(); + final SecurityConfig securityConfig = new ConfigurationObjectFactory(skifeConfigSource).build(SecurityConfig.class); + final Collection realms = defaultSecurityManager.getRealms() != null ? defaultSecurityManager.getRealms() : + ImmutableSet.of(new IniRealm(securityConfig.getShiroResourcePath())); // Mainly for testing + for (final Realm realm : realms) { + bindRealm().toInstance(realm); + } configureJDBCRealm(); configureLDAPRealm(); configureOktaRealm(); + + expose(new TypeLiteral>() {}); } protected void configureJDBCRealm() { @@ -112,7 +125,8 @@ protected void configureOktaRealm() { @Override protected void bindSecurityManager(final AnnotatedBindingBuilder bind) { - super.bindSecurityManager(bind); + //super.bindSecurityManager(bind); + bind.toInstance(defaultSecurityManager); final RedisCacheConfig redisCacheConfig = new ConfigurationObjectFactory(new ConfigSource() { @Override diff --git a/util/src/main/java/org/killbill/billing/util/glue/RealmsFromShiroIniProvider.java b/util/src/main/java/org/killbill/billing/util/glue/RealmsFromShiroIniProvider.java index 4c29d16cd1..4eeb94667b 100644 --- a/util/src/main/java/org/killbill/billing/util/glue/RealmsFromShiroIniProvider.java +++ b/util/src/main/java/org/killbill/billing/util/glue/RealmsFromShiroIniProvider.java @@ -1,7 +1,7 @@ /* * Copyright 2010-2013 Ning, Inc. - * Copyright 2014-2018 Groupon, Inc - * Copyright 2014-2018 The Billing Project, LLC + * Copyright 2014-2019 Groupon, Inc + * Copyright 2014-2019 The Billing Project, LLC * * The Billing Project licenses this file to you under the Apache License, version 2.0 * (the "License"); you may not use this file except in compliance with the @@ -18,14 +18,10 @@ package org.killbill.billing.util.glue; -import java.util.Collection; - import org.apache.shiro.config.ConfigurationException; import org.apache.shiro.config.IniSecurityManagerFactory; import org.apache.shiro.mgt.DefaultSecurityManager; import org.apache.shiro.mgt.SecurityManager; -import org.apache.shiro.realm.Realm; -import org.apache.shiro.realm.text.IniRealm; import org.apache.shiro.util.Factory; import org.killbill.billing.util.config.definition.SecurityConfig; import org.skife.config.ConfigSource; @@ -33,44 +29,23 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.collect.ImmutableSet; -import com.google.inject.Provider; - public class RealmsFromShiroIniProvider { private static final Logger log = LoggerFactory.getLogger(RealmsFromShiroIniProvider.class); - public static Collection get(final ConfigSource configSource) { + public static DefaultSecurityManager get(final ConfigSource configSource) { final SecurityConfig securityConfig = new ConfigurationObjectFactory(configSource).build(SecurityConfig.class); - Collection realms = null; try { final Factory factory = new IniSecurityManagerFactory(securityConfig.getShiroResourcePath()); // TODO Pierre hack - lame cast here, but we need to have Shiro go through its reflection magic // to parse the [main] section of the ini file. Without duplicating code, this seems to be possible only // by going through IniSecurityManagerFactory. - final DefaultSecurityManager securityManager = (DefaultSecurityManager) factory.getInstance(); - realms = securityManager.getRealms(); + return (DefaultSecurityManager) factory.getInstance(); } catch (final ConfigurationException e) { log.warn("Unable to configure RBAC", e); } - return realms != null ? realms : - ImmutableSet.of(new IniRealm(securityConfig.getShiroResourcePath())); // Mainly for testing - } - - public static Provider getIniRealmProvider(final ConfigSource configSource) { - for (final Realm cur : get(configSource)) { - if (cur instanceof IniRealm) { - return new Provider() { - @Override - public IniRealm get() { - return (IniRealm) cur; - } - }; - } - } - return null; } } diff --git a/util/src/main/java/org/killbill/billing/util/security/api/DefaultSecurityApi.java b/util/src/main/java/org/killbill/billing/util/security/api/DefaultSecurityApi.java index 876f56b2b5..4f75c70ccc 100644 --- a/util/src/main/java/org/killbill/billing/util/security/api/DefaultSecurityApi.java +++ b/util/src/main/java/org/killbill/billing/util/security/api/DefaultSecurityApi.java @@ -18,12 +18,15 @@ package org.killbill.billing.util.security.api; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import javax.annotation.Nullable; @@ -33,7 +36,9 @@ import org.apache.shiro.authc.AuthenticationToken; import org.apache.shiro.authc.UsernamePasswordToken; import org.apache.shiro.authz.AuthorizationException; +import org.apache.shiro.authz.AuthorizationInfo; import org.apache.shiro.mgt.DefaultSecurityManager; +import org.apache.shiro.realm.AuthorizingRealm; import org.apache.shiro.realm.Realm; import org.apache.shiro.subject.SimplePrincipalCollection; import org.apache.shiro.subject.Subject; @@ -48,6 +53,8 @@ import org.killbill.billing.util.security.shiro.dao.UserDao; import org.killbill.billing.util.security.shiro.dao.UserRolesModelDao; import org.killbill.billing.util.security.shiro.realm.KillBillJdbcRealm; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.base.Function; import com.google.common.base.Functions; @@ -58,17 +65,21 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; -import com.google.common.collect.Sets; public class DefaultSecurityApi implements SecurityApi { - private static final String[] allPermissions = new String[Permission.values().length]; + // Custom Realm implementors are encouraged to enable DEBUG level logging for development + private static final Logger logger = LoggerFactory.getLogger(DefaultSecurityApi.class); private final UserDao userDao; + private final Set realms; + private final Map doGetAuthorizationInfoMethods = new HashMap(); @Inject - public DefaultSecurityApi(final UserDao userDao) { + public DefaultSecurityApi(final UserDao userDao, final Set realms) { this.userDao = userDao; + this.realms = realms; + buildDoGetAuthorizationInfoMethods(); } @Override @@ -121,28 +132,51 @@ public boolean isSubjectAuthenticated() { @Override public Set getCurrentUserPermissions(final TenantContext context) { - // Built-in permissions - final String[] killbillPermissionsString = getAllPermissionsAsStrings(); - // Add user-defined permissions, if any - final Set allPermissions = Sets.newHashSet(killbillPermissionsString); - allPermissions.addAll(userDao.getAllPermissions()); - final String[] allPermissionsArray = allPermissions.toArray(new String[]{}); - final Subject subject = SecurityUtils.getSubject(); - // Bulk (optimized) call - final boolean[] permissions = subject.isPermitted(allPermissionsArray); - - final Set userPermissions = new HashSet(); - for (int i = 0; i < permissions.length; i++) { - final String permissionName = allPermissionsArray[i]; - if (permissions[i] && - // Don't return the "*" version - !("*".equals(permissionName) || permissionName.endsWith(":*"))) { - userPermissions.add(permissionName); + + final Set allPermissions = new HashSet(); + for (final Entry realmAndMethod : doGetAuthorizationInfoMethods.entrySet()) { + try { + final AuthorizationInfo authorizationInfo = (AuthorizationInfo) realmAndMethod.getValue().invoke(realmAndMethod.getKey(), subject.getPrincipals()); + if (authorizationInfo == null) { + logger.debug("No AuthorizationInfo returned from Realm {}", realmAndMethod.getKey()); + } else { + final Collection realmObjectPermissions = authorizationInfo.getObjectPermissions(); + if (realmObjectPermissions == null) { + logger.debug("No ObjectPermissions returned from Realm {}", realmAndMethod.getKey()); + } else { + for (final org.apache.shiro.authz.Permission realmPermission : realmObjectPermissions) { + // Note: this assumes custom realms return something sensible here + final String realmPermissionAsString = realmPermission.toString(); + if (realmPermissionAsString == null) { + logger.debug("Null ObjectPermission#toString returned from Realm {}", realmAndMethod.getKey()); + } else { + allPermissions.add(realmPermissionAsString); + } + } + } + // The Javadoc says that getObjectPermissions should contain the results from getStringPermissions, + // but this is incorrect in practice (JdbcRealm for instance) + final Collection realmStringPermissions = authorizationInfo.getStringPermissions(); + if (realmStringPermissions == null) { + logger.debug("No StringPermissions returned from Realm {}", realmAndMethod.getKey()); + } else { + allPermissions.addAll(authorizationInfo.getStringPermissions()); + } + } + } catch (final IllegalAccessException e) { + // Ignore + logger.debug("Unable to retrieve permissions for Realm {}", realmAndMethod.getKey(), e); + } catch (final InvocationTargetException e) { + // Ignore + logger.debug("Unable to retrieve permissions for Realm {}", realmAndMethod.getKey(), e); + } catch (final RuntimeException e) { + // Ignore + logger.debug("Unable to retrieve permissions for Realm {}", realmAndMethod.getKey(), e); } } - return userPermissions; + return allPermissions; } @Override @@ -279,20 +313,6 @@ public String apply(final String input) { return expandedPermissions; } - private String[] getAllPermissionsAsStrings() { - if (allPermissions[0] == null) { - synchronized (allPermissions) { - if (allPermissions[0] == null) { - final Permission[] killbillPermissions = Permission.values(); - for (int i = 0; i < killbillPermissions.length; i++) { - allPermissions[i] = killbillPermissions[i].toString(); - } - } - } - } - return allPermissions; - } - private void invalidateJDBCAuthorizationCache(final String username) { final Collection realms = ((DefaultSecurityManager) SecurityUtils.getSecurityManager()).getRealms(); final KillBillJdbcRealm killBillJdbcRealm = (KillBillJdbcRealm) Iterables.tryFind(realms, new Predicate() { @@ -308,4 +328,33 @@ public boolean apply(@Nullable final Realm input) { killBillJdbcRealm.clearCachedAuthorizationInfo(principals); } } + + private void buildDoGetAuthorizationInfoMethods() { + for (final Realm realm : realms) { + if (!(realm instanceof AuthorizingRealm)) { + logger.debug("Unable to retrieve doGetAuthorizationInfo method from Realm {}: not an AuthorizingRealm", realm); + continue; + } + + Method doGetAuthorizationInfoMethod = null; + Class clazz = realm.getClass(); + while (clazz != null) { + final Method[] methods = clazz.getDeclaredMethods(); + for (final Method method : methods) { + if ("doGetAuthorizationInfo".equals(method.getName())) { + doGetAuthorizationInfoMethod = method; + doGetAuthorizationInfoMethod.setAccessible(true); + break; + } + } + clazz = clazz.getSuperclass(); + } + if (doGetAuthorizationInfoMethod == null) { + logger.debug("Unable to retrieve doGetAuthorizationInfo method from Realm {}", realm); + continue; + } + + doGetAuthorizationInfoMethods.put(realm, doGetAuthorizationInfoMethod); + } + } } diff --git a/util/src/test/java/org/killbill/billing/util/UtilTestSuiteWithEmbeddedDB.java b/util/src/test/java/org/killbill/billing/util/UtilTestSuiteWithEmbeddedDB.java index 77500b0f3a..1fa8947753 100644 --- a/util/src/test/java/org/killbill/billing/util/UtilTestSuiteWithEmbeddedDB.java +++ b/util/src/test/java/org/killbill/billing/util/UtilTestSuiteWithEmbeddedDB.java @@ -1,7 +1,7 @@ /* * Copyright 2010-2014 Ning, Inc. - * Copyright 2014-2018 Groupon, Inc - * Copyright 2014-2018 The Billing Project, LLC + * Copyright 2014-2019 Groupon, Inc + * Copyright 2014-2019 The Billing Project, LLC * * The Billing Project licenses this file to you under the Apache License, version 2.0 * (the "License"); you may not use this file except in compliance with the @@ -18,8 +18,11 @@ package org.killbill.billing.util; +import java.util.Set; + import javax.inject.Inject; +import org.apache.shiro.realm.Realm; import org.killbill.billing.GuicyKillbillTestSuiteWithEmbeddedDB; import org.killbill.billing.account.api.ImmutableAccountInternalApi; import org.killbill.billing.api.TestApiListener; @@ -91,6 +94,8 @@ public abstract class UtilTestSuiteWithEmbeddedDB extends GuicyKillbillTestSuite protected NodeInfoDao nodeInfoDao; @Inject protected BroadcastDao broadcastDao; + @Inject + protected Set realms; @BeforeClass(groups = "slow") public void beforeClass() throws Exception { diff --git a/util/src/test/java/org/killbill/billing/util/glue/TestUtilModuleWithEmbeddedDB.java b/util/src/test/java/org/killbill/billing/util/glue/TestUtilModuleWithEmbeddedDB.java index 772d545b80..e6e29360d5 100644 --- a/util/src/test/java/org/killbill/billing/util/glue/TestUtilModuleWithEmbeddedDB.java +++ b/util/src/test/java/org/killbill/billing/util/glue/TestUtilModuleWithEmbeddedDB.java @@ -1,7 +1,7 @@ /* * Copyright 2010-2013 Ning, Inc. - * Copyright 2014-2018 Groupon, Inc - * Copyright 2014-2018 The Billing Project, LLC + * Copyright 2014-2019 Groupon, Inc + * Copyright 2014-2019 The Billing Project, LLC * * The Billing Project licenses this file to you under the Apache License, version 2.0 * (the "License"); you may not use this file except in compliance with the @@ -18,13 +18,26 @@ package org.killbill.billing.util.glue; +import java.io.IOException; +import java.util.Set; + +import org.apache.shiro.config.Ini; +import org.apache.shiro.realm.Realm; +import org.apache.shiro.realm.text.IniRealm; import org.killbill.billing.GuicyKillbillTestWithEmbeddedDBModule; import org.killbill.billing.api.TestApiListener; import org.killbill.billing.osgi.api.PluginsInfoApi; import org.killbill.billing.platform.api.KillbillConfigSource; +import org.killbill.billing.util.config.definition.SecurityConfig; +import org.killbill.billing.util.security.shiro.realm.KillBillJdbcRealm; import org.killbill.clock.ClockMock; +import org.killbill.commons.embeddeddb.EmbeddedDB; import org.mockito.Mockito; +import com.google.common.collect.ImmutableSet; +import com.google.inject.Provides; +import com.google.inject.Singleton; + public class TestUtilModuleWithEmbeddedDB extends TestUtilModule { private final ClockMock clock; @@ -49,6 +62,20 @@ protected void configure() { bind(TestApiListener.class).asEagerSingleton(); } + @Provides + @Singleton + protected Set provideRealms(final EmbeddedDB embeddedDB, final SecurityConfig securityConfig) throws IOException { + final Ini ini = new Ini(); + ini.load("[users]\n" + + "tester = tester, creditor\n" + + "[roles]\n" + + "creditor = invoice:credit, customx:customy\n"); + final Realm iniRealm = new IniRealm(ini); + final Realm killBillJdbcRealm = new KillBillJdbcRealm(embeddedDB.getDataSource(), securityConfig); + + return ImmutableSet.of(iniRealm, killBillJdbcRealm); + } + private final class SecurityModuleWithNoSecurityManager extends SecurityModule { public SecurityModuleWithNoSecurityManager(final KillbillConfigSource configSource) { @@ -69,6 +96,5 @@ protected void installUserApi() { bind(PluginsInfoApi.class).toInstance(Mockito.mock(PluginsInfoApi.class)); super.installUserApi(); } - } } diff --git a/util/src/test/java/org/killbill/billing/util/security/api/TestDefaultSecurityApi.java b/util/src/test/java/org/killbill/billing/util/security/api/TestDefaultSecurityApi.java index 915c400f12..dd4fb879c7 100644 --- a/util/src/test/java/org/killbill/billing/util/security/api/TestDefaultSecurityApi.java +++ b/util/src/test/java/org/killbill/billing/util/security/api/TestDefaultSecurityApi.java @@ -20,16 +20,17 @@ import java.util.Set; +import org.apache.shiro.realm.Realm; +import org.killbill.billing.security.Permission; +import org.killbill.billing.security.api.SecurityApi; +import org.killbill.billing.util.UtilTestSuiteNoDB; import org.killbill.billing.util.security.shiro.dao.UserDao; import org.mockito.Mockito; import org.testng.Assert; import org.testng.annotations.Test; -import org.killbill.billing.security.Permission; -import org.killbill.billing.security.api.SecurityApi; -import org.killbill.billing.util.UtilTestSuiteNoDB; - import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; public class TestDefaultSecurityApi extends UtilTestSuiteNoDB { @@ -38,7 +39,7 @@ public void testRetrievePermissions() throws Exception { configureShiro(); // We don't want the Guice injected one (it has Shiro disabled) - final SecurityApi securityApi = new DefaultSecurityApi(Mockito.mock(UserDao.class)); + final SecurityApi securityApi = new DefaultSecurityApi(Mockito.mock(UserDao.class), ImmutableSet.of()); logout(); final Set anonsPermissions = securityApi.getCurrentUserPermissions(callContext); diff --git a/util/src/test/java/org/killbill/billing/util/security/shiro/realm/TestKillBillJdbcRealm.java b/util/src/test/java/org/killbill/billing/util/security/shiro/realm/TestKillBillJdbcRealm.java index e84155460f..e373d742b6 100644 --- a/util/src/test/java/org/killbill/billing/util/security/shiro/realm/TestKillBillJdbcRealm.java +++ b/util/src/test/java/org/killbill/billing/util/security/shiro/realm/TestKillBillJdbcRealm.java @@ -17,6 +17,7 @@ package org.killbill.billing.util.security.shiro.realm; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -55,8 +56,8 @@ public void beforeMethod() throws Exception { } super.beforeMethod(); - final KillBillJdbcRealm realm = new KillBillJdbcRealm(helper.getDataSource(), securityConfig); - securityManager = new DefaultSecurityManager(realm); + + securityManager = new DefaultSecurityManager(realms); SecurityUtils.setSecurityManager(securityManager); } @@ -147,7 +148,7 @@ public void testSanityOfPermissions() throws SecurityApiException { } @Test(groups = "slow") - public void testCustomPermissions() throws Exception { + public void testCustomPermissionsAcrossRealms() throws Exception { final String role = "writer_off"; final ImmutableList rolePermissions = ImmutableList.of(Permission.INVOICE_CAN_DELETE_CBA.toString(), /* Built-in permission */ "invoice:write_off" /* Built-in group but custom value */, @@ -159,8 +160,8 @@ public void testCustomPermissions() throws Exception { final List roleDefinitions = securityApi.getRoleDefinition(role, callContext); Assert.assertEqualsNoOrder(roleDefinitions.toArray(), rolePermissions.toArray()); - final String username = "pierre"; - final String password = "password"; + final String username = "tester"; + final String password = "tester"; securityApi.addUserRoles(username, password, ImmutableList.of(role), callContext); final AuthenticationToken goodToken = new UsernamePasswordToken(username, password); @@ -168,9 +169,13 @@ public void testCustomPermissions() throws Exception { try { ThreadContext.bind(subject); + // JDBC Realm subject.checkPermission(Permission.INVOICE_CAN_DELETE_CBA.toString()); subject.checkPermission("invoice:write_off"); subject.checkPermission("acme:kb_dev"); + // Shiro Realm + subject.checkPermission("invoice:credit"); + subject.checkPermission("customx:customy"); try { subject.checkPermission("acme:kb_deployer"); Assert.fail("Subject should not have rights to deploy Kill Bill"); @@ -178,7 +183,10 @@ public void testCustomPermissions() throws Exception { } final Set permissions = securityApi.getCurrentUserPermissions(callContext); - Assert.assertEquals(permissions, rolePermissions); + final Set expectedPermissions = new HashSet(rolePermissions); + expectedPermissions.add("invoice:credit"); + expectedPermissions.add("customx:customy"); + Assert.assertEquals(permissions, expectedPermissions); } finally { ThreadContext.unbindSubject(); subject.logout(); @@ -242,23 +250,23 @@ public void testAuthorizationV2() throws SecurityApiException { subject.checkPermission(Permission.TAG_CAN_CREATE_TAG_DEFINITION.toString()); subject.checkPermission("acme:kb_dev"); - // "*" is not expanded here + // "*" is not expanded final List roleDefinitions = securityApi.getRoleDefinition(role, callContext); Assert.assertEqualsNoOrder(roleDefinitions.toArray(), rolePermissions.toArray()); - // "*" is expanded here + // "*" is not expanded final Set permissions = securityApi.getCurrentUserPermissions(callContext); - Assert.assertEquals(permissions.size(), Permission.values().length + 1 /* acme */); + Assert.assertEquals(permissions, ImmutableList.of("*")); securityApi.addRoleDefinition("for yet another user", ImmutableList.of("acme:kb_deployer"), callContext); - // "*" is not expanded here + // "*" is not expanded final List roleDefinitions2 = securityApi.getRoleDefinition(role, callContext); Assert.assertEqualsNoOrder(roleDefinitions2.toArray(), rolePermissions.toArray()); - // "*" is expanded here + // "*" is not expanded final Set permissions2 = securityApi.getCurrentUserPermissions(callContext); - Assert.assertEquals(permissions2.size(), Permission.values().length + 2 /* acme */); + Assert.assertEquals(permissions2, ImmutableList.of("*")); } finally { ThreadContext.unbindSubject(); subject.logout(); @@ -287,23 +295,23 @@ public void testAuthorizationV3() throws SecurityApiException { final Object[] rolePermissions = ImmutableList.of("account:*", "invoice:*", "tag:create_tag_definition", "acme:*").toArray(); - // "*" is not expanded here + // "*" is not expanded final List roleDefinitions = securityApi.getRoleDefinition(role, callContext); Assert.assertEqualsNoOrder(roleDefinitions.toArray(), rolePermissions); - // "*" is expanded here + // "*" is not expanded final Set permissions = securityApi.getCurrentUserPermissions(callContext); - Assert.assertEquals(permissions.size(), 6 /* All account */ + 5 /* All invoice */ + 1 /* Tag */ + 1 /* acme */); + Assert.assertEqualsNoOrder(permissions.toArray(), rolePermissions); securityApi.addRoleDefinition("for yet another user", ImmutableList.of("acme:kb_deployer"), callContext); - // "*" is not expanded here + // "*" is not expanded final List roleDefinitions2 = securityApi.getRoleDefinition(role, callContext); Assert.assertEqualsNoOrder(roleDefinitions2.toArray(), rolePermissions); - // "*" is expanded here + // "*" is not expanded final Set permissions2 = securityApi.getCurrentUserPermissions(callContext); - Assert.assertEquals(permissions2.size(), 6 /* All account */ + 5 /* All invoice */ + 1 /* Tag */ + 2 /* acme */); + Assert.assertEqualsNoOrder(permissions2.toArray(), rolePermissions); } finally { ThreadContext.unbindSubject(); subject.logout(); diff --git a/util/src/test/resources/shiro.ini b/util/src/test/resources/shiro.ini index af1509194a..c6681da136 100644 --- a/util/src/test/resources/shiro.ini +++ b/util/src/test/resources/shiro.ini @@ -16,4 +16,8 @@ # # ################################################################################### -# Dummy file to avoid exceptions in the logs +[users] +admin = password, root + +[roles] +root = *:*