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

Allow configurable init parameter in ESAPIFilter for unauthorized requests #462

Closed
kwwall opened this issue Dec 21, 2018 · 3 comments
Closed
Assignees

Comments

@kwwall
Copy link
Contributor

kwwall commented Dec 21, 2018

The ESAPIFilter currently has a hard-coded page ("WEB-INF/index.jsp") that users are forwarded to if they are not authorized for the current requested page. The logic is:

			// check access to this URL
			if ( !ESAPI.accessController().isAuthorizedForURL(request.getRequestURI()) ) {
				request.setAttribute("message", "Unauthorized" );
				RequestDispatcher dispatcher = request.getRequestDispatcher("WEB-INF/index.jsp");
				dispatcher.forward(request, response);
				return;
			}

The hard-coded "WEB-INF/index.jsp" should be replaced by an init parameter that can be set in the web.xml when this JavaEE servlet filter is configured, but this should remain the default for backwards compatibility.

I will name this new init parameter "publicUnauthorizedLandingPage" (the parameter name emphasizing that that landing page itself should be probably be accessible be everyone, or at least all authenticated users)

@kwwall kwwall self-assigned this Dec 21, 2018
@jeremiahjstacey
Copy link
Collaborator

In the past we've discussed working to decouple from the singletons in the ESAPI baseline. The API here presents an opportunity to do that, I think.

The function javax.servlet.Filter.init(FilterConfig) appears to be the setup function for the Filter instance. From the FilterConfig reference we can allow the external webContainer to provide the unauthorized URL/Location for the environment. Similar to how the resourceDirectory is being used, if the value is not provided in the configuration then use the existing default of "WEB-INF/index.jsp".

Alternatively, the value could go into the properties and we could use Singletons to retrieve it from the ESAPI context. From previous discussions on the difficulty of refactoring those implementations I'm hesitant to suggest that route.

/** Redirect location for unauthorized access.*/
private String unauthRedirect = "WEB-INF/index.jsp";

public void init(FilterConfig filterConfig) {
		String path = filterConfig.getInitParameter("resourceDirectory");
		if ( path != null ) {
			ESAPI.securityConfiguration().setResourceDirectory( path );
		}
            
               String unAuthRedirectPath = filterConfig.getInitParameter("unauthRedirect");
               if (unAuthRedirectPath != null) {
                   //logger.info ("Configuring ESAPI Unauthorized Path from FilterConfig: ", unAuthRedirectPath );
                   unauthRedirect = unAuthRedirectPath;
              }
	}

Then reference the new class variable in place of the current hard-coded path.

RequestDispatcher dispatcher = request.getRequestDispatcher(unauthRedirect );

I believe this then requires that the end-user configure their environments to provide the filter with the needed information in order to override the default behavior and keeps the additional code and implementation scope for this correction limited to this class in ESAPI.

@kwwall
Copy link
Contributor Author

kwwall commented Dec 24, 2018

@jeremiahjstacey Yes; that exactly was my point for creating this GitHub issue and what I intend to do (or, actually have already done; just haven't submitted the PR yet). @simon0117 was the one who pointed out that "WEB-INF/index.jsp" was hard-coded. I have configured it so that if a user doesn't configure it in their web.xml, that it will still default to the same old value.

jeremiahjstacey pushed a commit that referenced this issue Jan 29, 2019
* Release notes to close issue #463

* Changes to replace deprecated method CryptoHelper.arrayCompare() with MessageDigest.isEqual() which is safe in JDK 7.

* Close issue #246

* Close issue #462

* Close issue #364 and general assertion cleanup.

* Close issue #417

* Close issue #465

* Add more details, especially regarding dependency updates to address CVEs.

* Add WARNINGS to javadoc as noted in comment for GitHub issue #233

* Update OWASP Dependency Check from release 2.1.0 to 4.0.1 (i.e., the latest version).

* General clean-up. Add paragraph to discuss CVE-2018-8088 and why ESAPI is not affected.

* Update to mention PR #467 and closing of issue #360

* Fix typo in path for configuration/esapi and add 2.2.0.0 release notes.

* Change release from 2.1.0.2-SNAPSHOT to 2.2.0.0-SNAPSHOT in prep for release. See GitHub issue #471

* Preparation for 2.2.0.0 release. See GitHub issue #471 for details.

* Try to clarify by example the git commands used.

* Added Jeremiah's PR #472

* * Reference issue 188 as being closed.
* Udate status of latest PRs.
* Added 'Basic ESAPI Facts' section.
@kwwall
Copy link
Contributor Author

kwwall commented Jan 30, 2019

Close via PR #464

@kwwall kwwall closed this as completed Jan 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants