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 #169

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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,78 @@
/**
* 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.web.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_MODULE_ID = "coreapps";

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.

I wish the privilege did not have coreapps in its name because the legacyui module should not be aware of any module.


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

Choose a reason for hiding this comment

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

What does this method do ?

Copy link
Contributor Author

@jnsereko jnsereko Jul 30, 2021

Choose a reason for hiding this comment

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

Hey @sherrif10, I think It generally does nothing other than obeying java laws.
As far as I know, javax.servelt.Filter class is an interface so when implementing it, I have to implement all its methods otherwise the java-compiler will complain


}

/**
* @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;
Collection<Module> modules = ModuleFactory.getLoadedModules();
if (modules.contains(ModuleFactory.getModuleById(COREAPPS_MODULE_ID))) {
Copy link
Contributor Author

@jnsereko jnsereko Aug 3, 2021

Choose a reason for hiding this comment

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

hello @isears @sherrif10 @dkayiwa

@dkayiwa, won't this line cater for those using the legacyui module without reference application modules.

Copy link
Member

Choose a reason for hiding this comment

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

Of course it would. But it is not a good practise for the legacyui module to be aware of any other modules.

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 + "\"");
httpRes.sendRedirect(httpReq.getContextPath() + "/login.htm");
isears marked this conversation as resolved.
Show resolved Hide resolved
}
}
chain.doFilter(req, res);
}

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

}
}
4 changes: 4 additions & 0 deletions omod/src/main/java/org/openmrs/web/WebComponentRegistrar.java
Expand Up @@ -17,6 +17,7 @@

import org.directwebremoting.servlet.EfficientShutdownServletContextAttributeListener;
import org.openmrs.module.web.filter.AdminPageFilter;
import org.openmrs.module.web.filter.AdminAuthorisationFilter;
import org.openmrs.module.web.filter.ForcePasswordChangeFilter;
import org.openmrs.module.web.filter.RedirectAfterLoginFilter;
import org.springframework.stereotype.Component;
Expand Down Expand Up @@ -47,6 +48,9 @@ public void setServletContext(ServletContext servletContext) {
filter = servletContext.addFilter("adminPageFilter", new AdminPageFilter());
filter.addMappingForUrlPatterns(EnumSet.of(DispatcherType.REQUEST), true, "/admin");

filter = servletContext.addFilter("adminAuthorisationFilter", new AdminAuthorisationFilter());
filter.addMappingForUrlPatterns(EnumSet.of(DispatcherType.REQUEST), true, "/admin/*");

servletContext.addListener(new SessionListener());
/*
* EfficientShutdownServletContextAttributeListener is used instead of
Expand Down