Author: bleny Date: 2010-10-05 15:34:48 +0200 (Tue, 05 Oct 2010) New Revision: 386 Url: http://nuiton.org/repositories/revision/wikitty/386 Log: better security support : more checks Modified: trunk/wikitty-api/src/main/java/org/nuiton/wikitty/WikittyServiceSecurity.java trunk/wikitty-api/src/main/java/org/nuiton/wikitty/WikittyUtil.java trunk/wikitty-api/src/test/java/org/nuiton/wikitty/layers/WikittyServiceSecurityTest.java trunk/wikitty-api/src/test/resources/log4j.properties Modified: trunk/wikitty-api/src/main/java/org/nuiton/wikitty/WikittyServiceSecurity.java =================================================================== --- trunk/wikitty-api/src/main/java/org/nuiton/wikitty/WikittyServiceSecurity.java 2010-10-05 07:43:24 UTC (rev 385) +++ trunk/wikitty-api/src/main/java/org/nuiton/wikitty/WikittyServiceSecurity.java 2010-10-05 13:34:48 UTC (rev 386) @@ -121,8 +121,8 @@ } } - protected String extensionToWikittySecurityId(WikittyExtension extension) { - return String.format("WikittySecurity:%s", extension.getName()); + protected String extensionToWikittySecurityId(String extensionName) { + return String.format("WikittySecurity:%s", extensionName); } public void createAccount(String securityToken, String login, String password) { @@ -167,8 +167,8 @@ boolean creationAllowed = userIsAnonymousOrAppAdmin(securityToken, userId); if (creationAllowed) { - if (restoreExtensionAuthorisation(securityToken, extension) == null) { - String wikittyAuthorisationId = extensionToWikittySecurityId(extension); + if (restoreExtensionAuthorisation(securityToken, extension.getName()) == null) { + String wikittyAuthorisationId = extensionToWikittySecurityId(extension.getName()); Wikitty wikittyAuthorisation = new WikittyImpl(wikittyAuthorisationId); WikittyAuthorisationHelper.addExtension(wikittyAuthorisation); WikittyAuthorisationHelper.setOwner(wikittyAuthorisation, userId); @@ -202,20 +202,29 @@ } return userIsAnonymousOrAppAdmin; } - - /** restore the wikitty authorisation attached to given extension + + /** restore the wikitty authorisation attached to given extension. * * @return a wikitty with WikittyAuthorisation extension, or null if given * extension has no security policy attached - * @throws SecurityException if user don't have rights required */ public Wikitty restoreExtensionAuthorisation(String securityToken, WikittyExtension extension) { + return restoreExtensionAuthorisation(securityToken, extension.getName()); + } + + /** restore the wikitty authorisation attached to given extension. + * + * @return a wikitty with WikittyAuthorisation extension, or null if given + * extension has no security policy attached + */ + public Wikitty restoreExtensionAuthorisation(String securityToken, + String extensionName) { String userId = getUserId(securityToken); - String wikittyAuthorisationId = extensionToWikittySecurityId(extension); + String wikittyAuthorisationId = extensionToWikittySecurityId(extensionName); Wikitty wikittyAuthorisation = ws.restore(securityToken, wikittyAuthorisationId); if (wikittyAuthorisation == null) { - log.debug(extension + " has no authorization attached"); + log.debug(extensionName + " has no authorization attached"); } else { /* if ( ! canAdmin(securityToken, userId, wikittyAuthorisation)) { @@ -311,44 +320,58 @@ List<Wikitty> wikittiesToStore = new ArrayList<Wikitty>(); for (Wikitty wikitty : wikitties) { - // FIXME 20100930 bleny what if user store wikitty authorisation - // usual case, a user want to store a wikitty Wikitty oldVersion = ws.restore(securityToken, wikitty.getId()); - if (oldVersion == null) { // it's a creation - - // check that **reader** right on Security for all extension - for (WikittyExtension extension: wikitty.getExtensions()) { - Wikitty extensionRights = restoreExtensionAuthorisation(securityToken, extension); - - boolean canCreate = extensionRights == null || - canRead(securityToken, userId, null, extensionRights); - if ( ! canCreate ) { - throw new SecurityException(_( - "user %s can't create instance of extension %s", - userId, extensionRights)); - } + + Collection<String> newExtensions = wikitty.getExtensionNames(); + if (oldVersion != null) { + // we already checked the rights for those extension + // re-do the check has too much cost, avoid it + newExtensions.removeAll(oldVersion.getExtensionNames()); + } + + // check that **reader** right on Security for all extension + for (String extensionName: newExtensions) { + + Wikitty extensionRights = restoreExtensionAuthorisation(securityToken, extensionName); + boolean canCreate = extensionRights == null || + canRead(securityToken, userId, null, extensionRights); + if ( ! canCreate ) { + throw new SecurityException(_( + "user %s can't create instance of extension %s", + userId, extensionRights)); } + } - } else { // it's an update + if (oldVersion != null) { // it's an update for (String fqFieldDirtyName : wikitty.getDirty()) { String concernedExtensionName = WikittyUtil.getExtensionNameFromFQFieldName(fqFieldDirtyName); if (log.isTraceEnabled()) { - log.trace("will update field " + fqFieldDirtyName); - log.trace("it's extension " + concernedExtensionName); + log.trace(String.format("will update field %s from extension %s", + fqFieldDirtyName, concernedExtensionName)); } - if (canWrite(securityToken, userId, concernedExtensionName, wikitty)) { + boolean canChange; + if (WikittyAuthorisation.EXT_WIKITTYAUTHORISATION.equals(concernedExtensionName)) { + canChange = canAdmin(securityToken, userId, concernedExtensionName, wikitty); + } else { + canChange = canWrite(securityToken, userId, concernedExtensionName, wikitty); + } + + if (canChange) { Object newValue = wikitty.getFqField(fqFieldDirtyName); oldVersion.setFqField(fqFieldDirtyName, newValue); + } else { + throw new SecurityException(_("user %s can't write field %s on wikitty %s", + userId, fqFieldDirtyName, wikitty)); } } + } - wikittiesToStore.add(wikitty); - } + wikittiesToStore.add(wikitty); } return wikittiesToStore; } @@ -389,7 +412,7 @@ Wikitty wikitty) { for (String extensionName : wikitty.getExtensionNames()) { if ( ! canRead(securityToken, userId, extensionName, wikitty)) { - throw new SecurityException(_("user %s can't read extension %s on wikitty %s", + throw new SecurityException(_("user %s can't read extension %s on wikitty %s, it may be due to a global policy on the wikitty", userId, extensionName, wikitty)); } } @@ -411,7 +434,7 @@ // there is no policy for this extension // but there is a policy for all extension of wikitty canRead = isReader(securityToken, userId, wikitty, null) - || canWrite(securityToken, userId, extensionName, wikitty); + || canWrite(securityToken, userId, extensionName, wikitty); } else { // no security policy, everything is allowed canRead = true; @@ -544,7 +567,7 @@ String userId = getUserId(securityToken); if ( ! userIsAnonymousOrAppAdmin(securityToken, userId)) { for (WikittyExtension extension : exts) { - Wikitty extensionAuthorisation = restoreExtensionAuthorisation(securityToken, extension); + Wikitty extensionAuthorisation = restoreExtensionAuthorisation(securityToken, extension.getName()); if ( ! canWrite(securityToken, userId, null, extensionAuthorisation)) { throw new SecurityException(_("user %s don't have write right for extension %s", userId, extension)); } @@ -757,7 +780,7 @@ String metaFieldName = WikittyUtil.getMetaFieldName( WikittyAuthorisation.EXT_WIKITTYAUTHORISATION, extensionName, WikittyAuthorisation.FIELD_WIKITTYAUTHORISATION_READER); - result = isMember(securityToken, userId, wikitty, metaFieldName); + result = isMember(securityToken, userId, wikitty, metaFieldName, true); return result; } @@ -825,24 +848,36 @@ } protected boolean isMember(String securityToken, String userId, Wikitty extensionRights, String fqFieldName) { + // by default, user is considered not member if she is not in the group, so passing "false" + return isMember(securityToken, userId, extensionRights, fqFieldName, false); + } + protected boolean isMember(String securityToken, String userId, + Wikitty extensionRights, String fqFieldName, boolean considerEmptyGroupAsMembership) { + String extensionName = WikittyUtil.getExtensionNameFromFQFieldName(fqFieldName); String fieldName = WikittyUtil.getFieldNameFromFQFieldName(fqFieldName); - + Set<String> groupOrUser = extensionRights.getFieldAsSet(extensionName, fieldName, String.class); - boolean result = isMember(securityToken, userId, groupOrUser); - if ( ! result) { + boolean isMember; + if (groupOrUser == null || groupOrUser.isEmpty()) { + isMember = considerEmptyGroupAsMembership; + } else { + isMember = isMember(securityToken, userId, groupOrUser); + } + + if ( ! isMember) { // user don't have right on current object, check parent right String parentId = WikittyAuthorisationHelper.getParent(extensionRights); if (parentId != null) { Wikitty parent = ws.restore(securityToken, parentId); - result = isMember(securityToken, userId, parent, fqFieldName); + isMember = isMember(securityToken, userId, parent, fqFieldName); } } - return result; + return isMember; } // /** Modified: trunk/wikitty-api/src/main/java/org/nuiton/wikitty/WikittyUtil.java =================================================================== --- trunk/wikitty-api/src/main/java/org/nuiton/wikitty/WikittyUtil.java 2010-10-05 07:43:24 UTC (rev 385) +++ trunk/wikitty-api/src/main/java/org/nuiton/wikitty/WikittyUtil.java 2010-10-05 13:34:48 UTC (rev 386) @@ -820,21 +820,27 @@ return fqFieldName; } - /** given a fully qualified meta-extension name, return the name of the meta-extension */ + /** given a fully qualified meta-extension name, return the name of the meta-extension. + * @return null if fqFieldName doesn't contains a meta extension + */ public static String getMetaExtensionNameFromFQMetaExtensionName(String fqFieldName) { - String[] fqElements = fqFieldName.split(FQ_META_EXTENSION_SEPARATOR_REGEX); - return fqElements[0]; + String metaExtensionName = null; + if (fqFieldName.indexOf(FQ_META_EXTENSION_SEPARATOR) != -1) { + String[] fqElements = fqFieldName.split(FQ_META_EXTENSION_SEPARATOR_REGEX); + metaExtensionName = fqElements[1]; + } + return metaExtensionName; } /** given a fully qualified meta-extension name, return the name of the extension */ public static String getExtensionNameFromFQMetaExtensionName(String fqFieldName) { String[] fqElements = fqFieldName.split(FQ_META_EXTENSION_SEPARATOR_REGEX); - return fqElements[1]; + return fqElements[0]; } /** given names of meta-extension and extension, return a fully qualified meta-extension name */ public static String getFQMetaExtensionName(String metaExtensionName, String extensionName) { - String fqFieldName = metaExtensionName + FQ_META_EXTENSION_SEPARATOR + extensionName; + String fqFieldName = extensionName + FQ_META_EXTENSION_SEPARATOR + metaExtensionName; return fqFieldName; } Modified: trunk/wikitty-api/src/test/java/org/nuiton/wikitty/layers/WikittyServiceSecurityTest.java =================================================================== --- trunk/wikitty-api/src/test/java/org/nuiton/wikitty/layers/WikittyServiceSecurityTest.java 2010-10-05 07:43:24 UTC (rev 385) +++ trunk/wikitty-api/src/test/java/org/nuiton/wikitty/layers/WikittyServiceSecurityTest.java 2010-10-05 13:34:48 UTC (rev 386) @@ -2,6 +2,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -23,9 +24,9 @@ import org.nuiton.wikitty.WikittyService; import org.nuiton.wikitty.WikittyServiceInMemory; import org.nuiton.wikitty.WikittyServiceSecurity; +import org.nuiton.wikitty.WikittyUtil; -/** test {@link org.nuiton.wikitty.WikittyServiceSecurity} */ -@Ignore("not ready") +/** test {@link org.nuiton.wikitty.WikittyServiceSecurity}. */ public class WikittyServiceSecurityTest extends AbstractWikittyServiceTest { private static final Log log = LogFactory.getLog(WikittyServiceSecurityTest.class); @@ -87,7 +88,7 @@ @Test public void testInvalidToken() { // try to store with invalid token - String invalidToken = "INVALID TOKEN"; + String invalidToken = WikittyUtil.genSecurityTokenId(); try { service.store(invalidToken, aWikitty); fail(); @@ -110,7 +111,7 @@ // now try to make a valid token invalid service.logout(readerToken); try { - service.store(token, aWikitty); + service.store(readerToken, aWikitty); fail(); } catch (SecurityException e) {} } @@ -121,7 +122,7 @@ public void testReaderRightOnWikitty() { aWikitty.addExtension(WikittyAuthorisationAbstract.extensionWikittyAuthorisation); WikittyAuthorisation auth = new WikittyAuthorisationImpl(aWikitty); - + String readerId = securityService.getUserWikittyId(null, "reader"); auth.clearReader(); @@ -129,14 +130,42 @@ log.debug("will store wikitty" + aWikitty); service.store(ownerToken, aWikitty); - + try { - service.restore(null, aWikitty.getId()); + Wikitty restoredWikitty = service.restore(null, aWikitty.getId()); + log.debug("restored wikitty is " + restoredWikitty); fail("an exception should have been raised"); } catch (SecurityException e) { - log.info(e); + log.info("raised exception : " + e); } } + + @Test + public void testWriterRightOnWikitty() { + aWikitty.addExtension(WikittyAuthorisationAbstract.extensionWikittyAuthorisation); + WikittyAuthorisation auth = new WikittyAuthorisationImpl(aWikitty); + service.store(ownerToken, aWikitty); + + String adminId = securityService.getUserWikittyId(null, "admin"); + + auth.clearReader(); + auth.clearWriter(); + auth.clearAdmin(); + auth.addAdmin(adminId); + + log.debug("will store wikitty" + aWikitty); + + try { + service.store(writerToken, aWikitty); + fail("an exception should have been raised"); + } catch (SecurityException e) { + log.info("raised exception : " + e); + } + + Wikitty restoredWikitty = service.restore(null, aWikitty.getId()); + log.debug("restored wikitty is " + restoredWikitty); + assertNotNull(restoredWikitty); + } /* *** level 2 security tests ***/ @@ -194,7 +223,7 @@ } @Test - public void checkAdminRightOnExtention() { + public void checkAdminRightOnExtension() { // TODO 20100923 bleny check that store with no sufficient rights fail Wikitty extensionAuthorisation = securityService.restoreExtensionAuthorisation(adminToken, extension); Modified: trunk/wikitty-api/src/test/resources/log4j.properties =================================================================== --- trunk/wikitty-api/src/test/resources/log4j.properties 2010-10-05 07:43:24 UTC (rev 385) +++ trunk/wikitty-api/src/test/resources/log4j.properties 2010-10-05 13:34:48 UTC (rev 386) @@ -4,7 +4,8 @@ # Appender and Layout log4j.appender.logConsole=org.apache.log4j.ConsoleAppender log4j.appender.logConsole.layout=org.apache.log4j.PatternLayout -log4j.appender.logConsole.layout.ConversionPattern=%d{yy/MM/dd HH:mm:ss} %p %c{2}: %m%n +log4j.appender.logConsole.layout.ConversionPattern=%d %5p [%t] (%F:%L) %M - %m%n +# security layer : # log4j.category.org.nuiton.wikitty.WikittyServiceSecurity=TRACE # log4j.category.org.nuiton.wikitty.layers.WikittyServiceSecurityTest=TRACE