views:

277

answers:

1

Hi,

I know it's a good idea to use well implemented standard software for authorization and authentication. But this time I wrote it on my own. It is used in a intranet only application, so security requirements are low. But I, as the developer, would like to know, how secure it really is. I would please you to review the code and give me some feedback on

  • security relevant issues
  • any other improvemnts I could or should make

One concrete question: If a user does a logout, the session will be destroyed and the user will be forwarded to the default page. Now it is possible to go back and see the last page with priviliges of the loged out user. A refresh solves this problem. Is this because of the browser's cache? Is there any way to prevent not logged on users from seeing restricted resources through the browser history?

Thanks in advane.

Short description: There is a User class and a Role class. Each role holds a set of accessable resources. This is something like a whitelist approach. Only granted ressources can be accessed. Each user can have multiple roles. The current User object is stored within the session. If there is no user a default user will be used. Each session / user is bound to a host. If anybody logs in the old session will be destroyed and a new session will be created. The following file is a JEE filter which does authentication and authorization. It should be mapped with /* URL pattern to be consulted before any resource.

public class SecurityFilter implements Filter {

    private static final long serialVersionUID = -3992323601572130181L;
    private static final String DEFAULT_PAGE = "/";
    private static final String LOGIN_PAGE = "/login.xhtml";
    private static final String LOGOUT_PAGE = "/logout.xhtml";
    private static final String ATTRIBUTE_USER = "USER";
    private static final String ATTRIBUTE_REQUESTED_RESOURCE = "REQUESTED_RESOURCE";
    private static final String PARAMETER_USERNAME = "USERNAME";
    private static final String PARAMETER_PASSWORD = "PASSWORD";

    public void doFilter(ServletRequest request, ServletResponse response, FilterChain filterChain) throws IOException, ServletException {
        HttpServletRequest httpRequest = (HttpServletRequest) request;
        String contextPath = httpRequest.getContextPath();
        String resource = httpRequest.getRequestURI().replace(contextPath, "");
        System.out.println("Requested Resource:\t" + resource);
        if(LOGOUT_PAGE.equals(resource)){
            HttpSession session = httpRequest.getSession();
            session.invalidate();
        }
        User user = getSessionUser(httpRequest);
        String host = httpRequest.getRemoteHost();
        if(user.isGranted(resource) && user.isSameHost(host)) {
            System.out.println(user + ": GRANTED");
            httpRequest.getSession().removeAttribute(ATTRIBUTE_REQUESTED_RESOURCE);
            filterChain.doFilter(request, response);
        } else {
            System.out.println(user + ": DENIED");
            httpRequest.getSession().setAttribute(ATTRIBUTE_REQUESTED_RESOURCE, contextPath + resource);
            RequestDispatcher requestDispatcher = httpRequest.getRequestDispatcher(LOGIN_PAGE);
            requestDispatcher.forward(request, response);
        }
    }

    private User getSessionUser(HttpServletRequest request){
        User user = null;
        HttpSession session = request.getSession();
        Object attribute = session.getAttribute(ATTRIBUTE_USER);
        if (attribute instanceof User) {
            user = (User) attribute;
        }

        String username = request.getParameter(PARAMETER_USERNAME);
        String password = request.getParameter(PARAMETER_PASSWORD);
        if(username != null && password != null){
            user = getUserFromDatabase(username, password);
            String host = request.getRemoteHost();
            user.setCurrentHost(host);
            //-------------------------------------- Achtung --------------------------------------
            //Beendet die aktuelle Session, die vor dem Login bestanden hat und erzeugt eine neue.
            //Diese Vorgehensweise dient zum Schutz gegen XSRF (Cross-Site Request Forgery).
            session.invalidate();
            session = request.getSession(true);
            //-------------------------------------------------------------------------------------
        }
        if(user == null){
            user = getDefaultUser();
            String host = request.getRemoteHost();
            user.setCurrentHost(host);
        }
        session.setAttribute(ATTRIBUTE_USER, user);
        return user;
    }

    private User getDefaultUser(){
        return getUserFromDatabase(null, null);
    }

    private User getUserFromDatabase(String username, String password){
        //This is a mock method
        User user = null;
        Role anonym = new Role("Anonym");
        anonym.grantResource("/index.xhtml");
        anonym.grantResource("/login.xhtml");
        anonym.grantResource("/logout.xhtml");
        anonym.grantResource("/Startseite.xhtml");
        anonym.grantResource("/");
        anonym.grantResource("/resources/styles.css");
        anonym.grantResource("/resources/background_header.png");
        anonym.grantResource("/resources/background_middle.png");
        anonym.grantResource("/resources/bg.gif");
        anonym.grantResource("/resources/favicon.ico");
        anonym.grantResource("/resources/foot.gif");
        if(username != null && password != null){
            if("dboegner".equals(username) && "a".equals(password)){
                Role entwickler = new Role("Entwickler", anonym);
                entwickler.grantResource("/IT.xhtml");
                user = new User(username, password);
                user.addRole(entwickler);
            }
        } else {
            Set<Role> roles = new HashSet<Role>();
            roles.add(anonym);
            User anounymuos = new User("Anonymus", null, roles);
            user = anounymuos;
        }
        return user;
    }

    @Override
    public void destroy() {
    }

    @Override
    public void init(FilterConfig filterConfig) throws ServletException {
    }

}

The following files are the models for users and roles:

public class User {

    private String username;
    private String password;
    private Set<Role> roles;
    private String currentHost;

    public User(String username, String password) {
        this.username = username;
        this.password = password;
    }

    public User(String username, String password, Set<Role> roles) {
        this(username, password);
        this.roles = roles;
    }

    public String getUsername() {
        return username;
    }

    public String getPassword() {
        return password;
    }

    private Set<Role> getRoles() {
        if (roles == null) {
            roles = new HashSet<Role>();
        }
        return roles;
    }

    public boolean isGranted(String resource){
        for (Role role : getRoles()) {
            if(role.isGranted(resource)){
                return true;
            }
        }
        return false;
    }

    public void addRole(Role role){
        getRoles().add(role);
    }

    public void removeRole(Role role){
        getRoles().remove(role);
    }

    public boolean isSameHost(String host){
        if(host != null){
            return host.equals(getCurrentHost());
        }
        return false;
//      return getCurrentHost() != null;
    }

    public void setCurrentHost(String currentHost) {
        this.currentHost = currentHost;
    }

    private String getCurrentHost() {
        return currentHost;
    }

    @Override
    public String toString() {
        return getUsername();
    }

}

public class Role {

    private String title;
    private Role superRole;
    private Set<String> grantedResources;

    public Role(String title) {
        this.title = title;
    }

    public Role(String title, Role superRole) {
        this(title);
        this.superRole = superRole;
    }

    public void grantResource(String resource){
        if(!isGrantedBySuperRole(resource)){
            getGrantedResources().add(resource);
        }
    }

    public void denyResource(String resource){
        getGrantedResources().remove(resource);
    }

    public boolean isGranted(String resource){
        return getGrantedResources().contains(resource) ? true : isGrantedBySuperRole(resource);
    }

    private boolean isGrantedBySuperRole(String resource){
        Role superRole = getSuperRole();
        if(superRole != null){
            return superRole.isGranted(resource);
        }
        return false;
    }

    private Role getSuperRole() {
        return superRole;
    }

    private Set<String> getGrantedResources() {
        if (grantedResources == null) {
            grantedResources = new HashSet<String>();
        }
        return grantedResources;
    }

    public String getTitle() {
        return title;
    }

}
A: 

I don't spot any security flaws in the code, but I would myself have preferred a prefix mapping for the filter, e.g. /secured/* or so instead of /* so that you don't need to check the URL everytime, e.g. for static resources and so on. Further on, do you actually have considered Java EE container managed authentication? That would take all the user and role management from your hands.

As to your concrete question:

If a user does a logout, the session will be destroyed and the user will be forwarded to the default page. Now it is possible to go back and see the last page with priviliges of the loged out user. A refresh solves this problem. Is this because of the browser's cache? Is there any way to prevent not logged on users from seeing restricted resources through the browser history?

Yes, you just need to instruct the client to not cache the resource in question. You can do that by either adding the following HTML meta headers to the page in question:

<meta http-equiv="Cache-Control" content="no-cache, no-store, must-revalidate">
<meta http-equiv="Pragma" content="no-cache">
<meta http-equiv="Expires" content="0">

or by doing the same programmatically in a Filter which is mapped on an url-pattern covering those pages:

response.setHeader("Cache-Control", "no-cache, no-store, must-revalidate"); // HTTP 1.1.
response.setHeader("Pragma", "no-cache"); // HTTP 1.0.
response.setDateHeader("Expires", 0); // Proxies.
BalusC