Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MM-893: Solved Authentication Bypass to System Administration #426

Merged
merged 2 commits into from Aug 7, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,73 @@
/**
* This Source Code Form is subject to the terms of the Mozilla Public License,
* v. 2.0. If a copy of the MPL was not distributed with this file, You can
* obtain one at http://mozilla.org/MPL/2.0/. OpenMRS is also distributed under
* the terms of the Healthcare Disclaimer located at http://openmrs.org/license.
*
* Copyright (C) OpenMRS Inc. OpenMRS is a registered trademark and the OpenMRS
* graphic logo is a trademark of OpenMRS Inc.
*/
package org.openmrs.module.coreapps.filter;

import java.io.IOException;
import java.util.Collection;

import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.openmrs.User;
import org.openmrs.api.context.Context;
import org.openmrs.module.Module;
import org.openmrs.module.ModuleFactory;
import org.openmrs.web.WebConstants;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* This filter checks if an authenticated user trying to access administrative functions has the
* <STRONG>System Administration<STRONG> privelege. It will intercept any requests with *admin/* in
* its url. Unauthorised user will be redirected to the home page.
*/
public class AdminAuthorisationFilter implements Filter {

private static final Logger log = LoggerFactory.getLogger(AdminAuthorisationFilter.class);

private static final String COREAPPS_SYSTEM_ADMINISTRATOR_PRIVELEGE = "App: coreapps.systemAdministration";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small thing: we should probably put this in the CoreAppsConstants file like the other privileges.


/**
* @see Filter#init(FilterConfig)
*/
public void init(FilterConfig filterConfig) throws ServletException {

}

/**
* @see javax.servlet.Filter#doFilter(javax.servlet.ServletRequest,
* javax.servlet.ServletResponse, javax.servlet.FilterChain)
*/
public void doFilter(ServletRequest req, ServletResponse res, FilterChain chain) throws IOException, ServletException {
HttpServletRequest httpReq = (HttpServletRequest) req;
User user = Context.getAuthenticatedUser();
if (user != null && !user.hasPrivilege(COREAPPS_SYSTEM_ADMINISTRATOR_PRIVELEGE)) {
httpReq.getSession().setAttribute(WebConstants.DENIED_PAGE, httpReq.getRequestURI());
HttpServletResponse httpRes = (HttpServletResponse) res;
log.info("User " + user + " has no privilage \"" + COREAPPS_SYSTEM_ADMINISTRATOR_PRIVELEGE + "\"");
jnsereko marked this conversation as resolved.
Show resolved Hide resolved
httpRes.sendRedirect(httpReq.getContextPath() + "/login.htm");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After calling sendRedirect() it's best to have a return so that we don't continue running this request through the filter chain.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ibacher I really don't know how much to thank you. I think I have added the request correctly.

}
chain.doFilter(req, res);
}

/**
* @see Filter#destroy()
*/
public void destroy() {

}
}
10 changes: 10 additions & 0 deletions omod/src/main/resources/config.xml
Expand Up @@ -86,6 +86,16 @@

</mappingFiles>

<filter>
<filter-name>${project.parent.artifactId}AdminAuthorisationFilter</filter-name>
<filter-class>${project.parent.groupId}.${project.parent.artifactId}.filter.AdminAuthorisationFilter</filter-class>
</filter>

<filter-mapping>
<filter-name>${project.parent.artifactId}AdminAuthorisationFilter</filter-name>
<url-pattern>/admin/*</url-pattern>
</filter-mapping>

<!-- Internationalization -->
<!-- All message codes should start with ${project.parent.artifactId}. -->
<messages>
Expand Down