From d8b8f6617a5886ba0d2c490ccddfa483dbb6b2df Mon Sep 17 00:00:00 2001 From: Athou Date: Sat, 30 Mar 2013 19:06:32 +0100 Subject: [PATCH] change role to enum --- .../com/commafeed/backend/StartupBean.java | 2 +- .../backend/dao/UserRoleService.java | 5 ++- .../commafeed/backend/dao/UserService.java | 6 +-- .../com/commafeed/backend/model/UserRole.java | 15 +++++-- .../com/commafeed/backend/security/Role.java | 6 --- .../frontend/CommaFeedApplication.java | 41 +++++++++++++------ .../commafeed/frontend/CommaFeedSession.java | 12 +++++- .../frontend/{rest => }/SecurityCheck.java | 6 ++- .../commafeed/frontend/pages/HomePage.java | 6 +-- .../commafeed/frontend/pages/LoginPage.html | 3 -- .../frontend/pages/SettingsPage.html | 25 ----------- .../frontend/pages/SettingsPage.java | 8 ---- .../frontend/rest/resources/AbstractREST.java | 12 +++--- .../rest/resources/AdminUsersREST.java | 14 +++---- src/main/webapp/js/controllers.js | 2 +- 15 files changed, 77 insertions(+), 86 deletions(-) delete mode 100644 src/main/java/com/commafeed/backend/security/Role.java rename src/main/java/com/commafeed/frontend/{rest => }/SecurityCheck.java (77%) delete mode 100644 src/main/java/com/commafeed/frontend/pages/SettingsPage.html delete mode 100644 src/main/java/com/commafeed/frontend/pages/SettingsPage.java diff --git a/src/main/java/com/commafeed/backend/StartupBean.java b/src/main/java/com/commafeed/backend/StartupBean.java index b35e527a..f120de2d 100644 --- a/src/main/java/com/commafeed/backend/StartupBean.java +++ b/src/main/java/com/commafeed/backend/StartupBean.java @@ -18,8 +18,8 @@ import com.commafeed.backend.model.Feed; import com.commafeed.backend.model.FeedCategory; import com.commafeed.backend.model.FeedSubscription; import com.commafeed.backend.model.User; +import com.commafeed.backend.model.UserRole.Role; import com.commafeed.backend.security.PasswordEncryptionService; -import com.commafeed.backend.security.Role; @Startup @Singleton diff --git a/src/main/java/com/commafeed/backend/dao/UserRoleService.java b/src/main/java/com/commafeed/backend/dao/UserRoleService.java index f7280233..a2d0c896 100644 --- a/src/main/java/com/commafeed/backend/dao/UserRoleService.java +++ b/src/main/java/com/commafeed/backend/dao/UserRoleService.java @@ -7,6 +7,7 @@ import javax.ejb.Stateless; import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserRole; +import com.commafeed.backend.model.UserRole.Role; import com.commafeed.frontend.utils.ModelFactory.MF; import com.google.common.collect.Sets; @@ -18,8 +19,8 @@ public class UserRoleService extends GenericDAO { return findByField(MF.i(MF.p(UserRole.class).getUser()), user); } - public Set getRoles(User user) { - Set list = Sets.newHashSet(); + public Set getRoles(User user) { + Set list = Sets.newHashSet(); for (UserRole role : findByField(MF.i(proxy().getUser()), user)) { list.add(role.getRole()); } diff --git a/src/main/java/com/commafeed/backend/dao/UserService.java b/src/main/java/com/commafeed/backend/dao/UserService.java index 86417aa4..864dfefd 100644 --- a/src/main/java/com/commafeed/backend/dao/UserService.java +++ b/src/main/java/com/commafeed/backend/dao/UserService.java @@ -8,8 +8,8 @@ import javax.inject.Inject; import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserRole; +import com.commafeed.backend.model.UserRole.Role; import com.commafeed.backend.security.PasswordEncryptionService; -import com.commafeed.backend.security.Role; import com.commafeed.frontend.utils.ModelFactory.MF; import com.google.common.collect.Iterables; @@ -34,7 +34,7 @@ public class UserService extends GenericDAO { return null; } - public User register(String name, String password, Collection roles) { + public User register(String name, String password, Collection roles) { List users = findByField(MF.i(proxy().getName()), name); if (!users.isEmpty()) { return null; @@ -45,7 +45,7 @@ public class UserService extends GenericDAO { user.setSalt(salt); user.setPassword(encryptionService.getEncryptedPassword(password, salt)); user.getRoles().add(new UserRole(user, Role.USER)); - for (String role : roles) { + for (Role role : roles) { user.getRoles().add(new UserRole(user, role)); user.getRoles().add(new UserRole(user, role)); } diff --git a/src/main/java/com/commafeed/backend/model/UserRole.java b/src/main/java/com/commafeed/backend/model/UserRole.java index a9d51a54..152fcc10 100644 --- a/src/main/java/com/commafeed/backend/model/UserRole.java +++ b/src/main/java/com/commafeed/backend/model/UserRole.java @@ -2,6 +2,8 @@ package com.commafeed.backend.model; import javax.persistence.Column; import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.Enumerated; import javax.persistence.JoinColumn; import javax.persistence.OneToOne; import javax.persistence.Table; @@ -11,18 +13,23 @@ import javax.persistence.Table; @SuppressWarnings("serial") public class UserRole extends AbstractModel { + public static enum Role { + USER, ADMIN + } + @OneToOne @JoinColumn(name = "user_id", nullable = false) private User user; @Column(name = "roleName", nullable = false) - private String role; + @Enumerated(EnumType.STRING) + private Role role; public UserRole() { } - public UserRole(User user, String role) { + public UserRole(User user, Role role) { this.user = user; this.role = role; } @@ -35,11 +42,11 @@ public class UserRole extends AbstractModel { this.user = user; } - public String getRole() { + public Role getRole() { return role; } - public void setRole(String role) { + public void setRole(Role role) { this.role = role; } diff --git a/src/main/java/com/commafeed/backend/security/Role.java b/src/main/java/com/commafeed/backend/security/Role.java deleted file mode 100644 index f3639cec..00000000 --- a/src/main/java/com/commafeed/backend/security/Role.java +++ /dev/null @@ -1,6 +0,0 @@ -package com.commafeed.backend.security; - -public class Role { - public static final String USER = "user"; - public static final String ADMIN = "admin"; -} diff --git a/src/main/java/com/commafeed/frontend/CommaFeedApplication.java b/src/main/java/com/commafeed/frontend/CommaFeedApplication.java index 11de2336..5f9496e0 100644 --- a/src/main/java/com/commafeed/frontend/CommaFeedApplication.java +++ b/src/main/java/com/commafeed/frontend/CommaFeedApplication.java @@ -9,14 +9,15 @@ import javax.naming.InitialContext; import javax.naming.NamingException; import org.apache.wicket.Application; +import org.apache.wicket.Component; import org.apache.wicket.Page; import org.apache.wicket.Session; import org.apache.wicket.ajax.AjaxRequestTarget; +import org.apache.wicket.authorization.Action; +import org.apache.wicket.authorization.IAuthorizationStrategy; import org.apache.wicket.authroles.authentication.AbstractAuthenticatedWebSession; import org.apache.wicket.authroles.authentication.AuthenticatedWebApplication; -import org.apache.wicket.authroles.authorization.strategies.role.IRoleCheckingStrategy; import org.apache.wicket.authroles.authorization.strategies.role.Roles; -import org.apache.wicket.authroles.authorization.strategies.role.annotations.AnnotationsRoleAuthorizationStrategy; import org.apache.wicket.cdi.CdiConfiguration; import org.apache.wicket.cdi.ConversationPropagation; import org.apache.wicket.core.request.handler.PageProvider; @@ -26,6 +27,7 @@ import org.apache.wicket.markup.html.WebPage; import org.apache.wicket.request.IRequestHandler; import org.apache.wicket.request.Request; import org.apache.wicket.request.Response; +import org.apache.wicket.request.component.IRequestableComponent; import org.apache.wicket.request.cycle.AbstractRequestCycleListener; import org.apache.wicket.request.cycle.RequestCycle; import org.jboss.vfs.VirtualFile; @@ -39,7 +41,6 @@ import org.slf4j.LoggerFactory; import com.commafeed.frontend.pages.HomePage; import com.commafeed.frontend.pages.LoginPage; import com.commafeed.frontend.pages.LogoutPage; -import com.commafeed.frontend.pages.SettingsPage; import com.commafeed.frontend.utils.exception.DisplayExceptionPage; import de.agilecoders.wicket.Bootstrap; @@ -56,7 +57,6 @@ public class CommaFeedApplication extends AuthenticatedWebApplication { mountPage("login", LoginPage.class); mountPage("logout", LogoutPage.class); mountPage("error", DisplayExceptionPage.class); - mountPage("settings", SettingsPage.class); setupInjection(); @@ -65,14 +65,31 @@ public class CommaFeedApplication extends AuthenticatedWebApplication { getMarkupSettings().setDefaultMarkupEncoding("UTF-8"); getSecuritySettings().setAuthorizationStrategy( - new AnnotationsRoleAuthorizationStrategy( - new IRoleCheckingStrategy() { - @Override - public boolean hasAnyRole(Roles roles) { - return CommaFeedSession.get().getRoles() - .hasAnyRole(roles); - } - })); + new IAuthorizationStrategy() { + + @Override + public boolean isInstantiationAuthorized( + Class componentClass) { + boolean authorized = true; + + boolean restricted = componentClass + .isAnnotationPresent(SecurityCheck.class); + if (restricted) { + SecurityCheck annotation = componentClass + .getAnnotation(SecurityCheck.class); + Roles roles = CommaFeedSession.get().getRoles(); + authorized = roles.hasAnyRole(new Roles(annotation + .value().name())); + } + return authorized; + } + + @Override + public boolean isActionAuthorized(Component component, + Action action) { + return true; + } + }); getRequestCycleListeners().add(new AbstractRequestCycleListener() { @Override diff --git a/src/main/java/com/commafeed/frontend/CommaFeedSession.java b/src/main/java/com/commafeed/frontend/CommaFeedSession.java index 04526e24..c4edc548 100644 --- a/src/main/java/com/commafeed/frontend/CommaFeedSession.java +++ b/src/main/java/com/commafeed/frontend/CommaFeedSession.java @@ -1,5 +1,7 @@ package com.commafeed.frontend; +import java.util.Set; + import javax.inject.Inject; import org.apache.wicket.Session; @@ -10,6 +12,8 @@ import org.apache.wicket.request.Request; import com.commafeed.backend.dao.UserRoleService; import com.commafeed.backend.dao.UserService; import com.commafeed.backend.model.User; +import com.commafeed.backend.model.UserRole.Role; +import com.google.common.collect.Sets; @SuppressWarnings("serial") public class CommaFeedSession extends AuthenticatedWebSession { @@ -51,9 +55,13 @@ public class CommaFeedSession extends AuthenticatedWebSession { this.user = null; this.roles = new Roles(); } else { + + Set roleSet = Sets.newHashSet(); + for (Role role : userRoleService.getRoles(user)) { + roleSet.add(role.name()); + } this.user = user; - this.roles = new Roles(userRoleService.getRoles(user).toArray( - new String[0])); + this.roles = new Roles(roleSet.toArray(new String[0])); } return user != null; } diff --git a/src/main/java/com/commafeed/frontend/rest/SecurityCheck.java b/src/main/java/com/commafeed/frontend/SecurityCheck.java similarity index 77% rename from src/main/java/com/commafeed/frontend/rest/SecurityCheck.java rename to src/main/java/com/commafeed/frontend/SecurityCheck.java index a862944f..49965c19 100644 --- a/src/main/java/com/commafeed/frontend/rest/SecurityCheck.java +++ b/src/main/java/com/commafeed/frontend/SecurityCheck.java @@ -1,4 +1,4 @@ -package com.commafeed.frontend.rest; +package com.commafeed.frontend; import java.lang.annotation.ElementType; import java.lang.annotation.Inherited; @@ -9,6 +9,8 @@ import java.lang.annotation.Target; import javax.enterprise.util.Nonbinding; import javax.interceptor.InterceptorBinding; +import com.commafeed.backend.model.UserRole.Role; + @Inherited @InterceptorBinding @Target({ ElementType.TYPE, ElementType.METHOD }) @@ -19,5 +21,5 @@ public @interface SecurityCheck { * Roles needed. */ @Nonbinding - String[] value() default {}; + Role value() default Role.USER; } \ No newline at end of file diff --git a/src/main/java/com/commafeed/frontend/pages/HomePage.java b/src/main/java/com/commafeed/frontend/pages/HomePage.java index 20b7c8cd..8771ea95 100644 --- a/src/main/java/com/commafeed/frontend/pages/HomePage.java +++ b/src/main/java/com/commafeed/frontend/pages/HomePage.java @@ -1,11 +1,11 @@ package com.commafeed.frontend.pages; -import org.apache.wicket.authroles.authorization.strategies.role.annotations.AuthorizeInstantiation; import org.apache.wicket.markup.head.CssHeaderItem; import org.apache.wicket.markup.head.IHeaderResponse; import org.apache.wicket.markup.head.JavaScriptHeaderItem; -import com.commafeed.backend.security.Role; +import com.commafeed.backend.model.UserRole.Role; +import com.commafeed.frontend.SecurityCheck; import com.commafeed.frontend.references.angular.AngularReference; import com.commafeed.frontend.references.angular.AngularResourceReference; import com.commafeed.frontend.references.angular.AngularSanitizeReference; @@ -21,7 +21,7 @@ import com.commafeed.frontend.references.select2.Select2Reference; import com.commafeed.frontend.references.spinjs.SpinJSReference; @SuppressWarnings("serial") -@AuthorizeInstantiation(Role.USER) +@SecurityCheck(Role.USER) public class HomePage extends BasePage { @Override diff --git a/src/main/java/com/commafeed/frontend/pages/LoginPage.html b/src/main/java/com/commafeed/frontend/pages/LoginPage.html index b07d875a..10e89550 100644 --- a/src/main/java/com/commafeed/frontend/pages/LoginPage.html +++ b/src/main/java/com/commafeed/frontend/pages/LoginPage.html @@ -1,8 +1,6 @@ - - @@ -14,6 +12,5 @@ - diff --git a/src/main/java/com/commafeed/frontend/pages/SettingsPage.html b/src/main/java/com/commafeed/frontend/pages/SettingsPage.html deleted file mode 100644 index dbdc0819..00000000 --- a/src/main/java/com/commafeed/frontend/pages/SettingsPage.html +++ /dev/null @@ -1,25 +0,0 @@ - - - - - - - - - - - - - Placeholder - - - - - - - - - - - - diff --git a/src/main/java/com/commafeed/frontend/pages/SettingsPage.java b/src/main/java/com/commafeed/frontend/pages/SettingsPage.java deleted file mode 100644 index 3960b761..00000000 --- a/src/main/java/com/commafeed/frontend/pages/SettingsPage.java +++ /dev/null @@ -1,8 +0,0 @@ -package com.commafeed.frontend.pages; - -import org.apache.wicket.markup.html.WebPage; - -@SuppressWarnings("serial") -public class SettingsPage extends WebPage { - -} diff --git a/src/main/java/com/commafeed/frontend/rest/resources/AbstractREST.java b/src/main/java/com/commafeed/frontend/rest/resources/AbstractREST.java index 68d52378..e859eee4 100644 --- a/src/main/java/com/commafeed/frontend/rest/resources/AbstractREST.java +++ b/src/main/java/com/commafeed/frontend/rest/resources/AbstractREST.java @@ -33,11 +33,11 @@ import com.commafeed.backend.dao.UserService; import com.commafeed.backend.dao.UserSettingsService; import com.commafeed.backend.feeds.OPMLImporter; import com.commafeed.backend.model.User; +import com.commafeed.backend.model.UserRole.Role; import com.commafeed.backend.security.PasswordEncryptionService; -import com.commafeed.backend.security.Role; import com.commafeed.frontend.CommaFeedApplication; import com.commafeed.frontend.CommaFeedSession; -import com.commafeed.frontend.rest.SecurityCheck; +import com.commafeed.frontend.SecurityCheck; @Produces(MediaType.APPLICATION_JSON) @Consumes(MediaType.APPLICATION_JSON) @@ -133,11 +133,9 @@ public abstract class AbstractREST { } private boolean checkRole(User user, SecurityCheck annotation) { - Set roles = userRoleService.getRoles(user); - for (String role : annotation.value()) { - if (!roles.contains(role)) { - return false; - } + Set roles = userRoleService.getRoles(user); + if (!roles.contains(annotation.value())) { + return false; } return true; } diff --git a/src/main/java/com/commafeed/frontend/rest/resources/AdminUsersREST.java b/src/main/java/com/commafeed/frontend/rest/resources/AdminUsersREST.java index 991cd90a..c47f8253 100644 --- a/src/main/java/com/commafeed/frontend/rest/resources/AdminUsersREST.java +++ b/src/main/java/com/commafeed/frontend/rest/resources/AdminUsersREST.java @@ -16,9 +16,9 @@ import org.apache.commons.lang.StringUtils; import com.commafeed.backend.StartupBean; import com.commafeed.backend.model.User; import com.commafeed.backend.model.UserRole; -import com.commafeed.backend.security.Role; +import com.commafeed.backend.model.UserRole.Role; +import com.commafeed.frontend.SecurityCheck; import com.commafeed.frontend.model.UserModel; -import com.commafeed.frontend.rest.SecurityCheck; import com.google.common.base.Preconditions; import com.google.common.collect.Maps; import com.google.common.collect.Sets; @@ -37,7 +37,7 @@ public class AdminUsersREST extends AbstractREST { if (id == null) { Preconditions.checkNotNull(userModel.getPassword()); - Set roles = Sets.newHashSet(Role.USER); + Set roles = Sets.newHashSet(Role.USER); if (userModel.isAdmin()) { roles.add(Role.ADMIN); } @@ -63,12 +63,12 @@ public class AdminUsersREST extends AbstractREST { user.setDisabled(!userModel.isEnabled()); userService.update(user); - Set roles = userRoleService.getRoles(user); + Set roles = userRoleService.getRoles(user); if (userModel.isAdmin() && !roles.contains(Role.ADMIN)) { userRoleService.save(new UserRole(user, Role.ADMIN)); } else if (!userModel.isAdmin() && roles.contains(Role.ADMIN)) { for (UserRole userRole : userRoleService.findAll(user)) { - if (Role.ADMIN.equals(userRole.getRole())) { + if (userRole.getRole() == Role.ADMIN) { userRoleService.delete(userRole); } } @@ -88,7 +88,7 @@ public class AdminUsersREST extends AbstractREST { userModel.setName(user.getName()); userModel.setEnabled(!user.isDisabled()); for (UserRole role : userRoleService.findAll(user)) { - if (Role.ADMIN.equals(role.getRole())) { + if (role.getRole() == Role.ADMIN) { userModel.setAdmin(true); } } @@ -110,7 +110,7 @@ public class AdminUsersREST extends AbstractREST { userModel.setEnabled(!user.isDisabled()); users.put(key, userModel); } - if (Role.ADMIN.equals(role.getRole())) { + if (role.getRole() == Role.ADMIN) { userModel.setAdmin(true); } } diff --git a/src/main/webapp/js/controllers.js b/src/main/webapp/js/controllers.js index dd4aa088..29fe8b30 100644 --- a/src/main/webapp/js/controllers.js +++ b/src/main/webapp/js/controllers.js @@ -277,7 +277,7 @@ module.controller('ManageUserCtrl', function($scope, $state, $stateParams, AdminUsersService) { $scope.user = $stateParams._id ? AdminUsersService.get({ id : $stateParams._id - }) : {}; + }) : {enabled: true}; $scope.alerts = []; $scope.closeAlert = function(index) { $scope.alerts.splice(index, 1);