Skip to content

Commit

Permalink
[#146] Issue with the token-per-page support for REST endpoint contai…
Browse files Browse the repository at this point in the history 
…ning path parameters (part 1)

* change the logic to return and accept JSON payload for tokens, so that extending the Token Transfer Object will not need manual parsing

[#139] Is it really worth making a new request to retrieve the master token to be stored in a variable?
* The POST method in JavaScriptServlet can be used exclusively for querying page tokens because:
** the JavaScript logic only invokes it if the token-per-page functionality is enabled, so there is no need to return the master token in that case.
** if used token rotation is enabled, the new master token is returned in the response header
  • Loading branch information
@forgedhallpass
forgedhallpass committed Oct 6, 2020
1 parent bb65071 commit 3e19915
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 100 deletions.
4 changes: 0 additions & 4 deletions csrfguard/src/main/java/org/owasp/csrfguard/CsrfGuard.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,6 @@ public List<IAction> getActions() {
return config().getActions();
}

public String getJavascriptSourceFile() {
return config().getJavascriptSourceFile();
}

/**
* @return if inject
* @see ConfigurationProvider#isJavascriptInjectFormAttributes()
Expand Down
46 changes: 13 additions & 33 deletions csrfguard/src/main/java/org/owasp/csrfguard/servlet/JavaScriptServlet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@
import org.owasp.csrfguard.CsrfGuardServletContextListener;
import org.owasp.csrfguard.log.LogLevel;
import org.owasp.csrfguard.session.LogicalSession;
import org.owasp.csrfguard.token.service.TokenService;
import org.owasp.csrfguard.token.storage.LogicalSessionExtractor;
import org.owasp.csrfguard.token.transferobject.TokenTO;
import org.owasp.csrfguard.util.CsrfGuardUtils;

import javax.servlet.ServletConfig;
Expand All @@ -46,12 +46,12 @@
import java.io.IOException;
import java.net.MalformedURLException;
import java.net.URL;
import java.nio.charset.Charset;
import java.util.HashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

public final class JavaScriptServlet extends HttpServlet {

Expand Down Expand Up @@ -94,7 +94,7 @@ public final class JavaScriptServlet extends HttpServlet {
private static final String TOKENS_PER_PAGE_IDENTIFIER = "'%TOKENS_PER_PAGE%'";

/* MIME Type constants */
private static final String TEXT_PLAIN_MIME_TYPE = "text/plain";
private static final String JSON_MIME_TYPE = "application/json";
private static final String JAVASCRIPT_MIME_TYPE = "text/javascript";

/**
Expand Down Expand Up @@ -147,44 +147,28 @@ public void doPost(final HttpServletRequest request, final HttpServletResponse r
// TODO pass the logical session downstream, see whether the null check can be done from here
final LogicalSession logicalSession = csrfGuard.getLogicalSessionExtractor().extract(request);
if (Objects.isNull(logicalSession)) {
writeMasterToken(request, response);
throw new IllegalStateException("This should not happen. A logical session should already exist at this point.");
} else {
final Map<String, String> pageTokens = csrfGuard.getTokenService().getPageTokens(logicalSession.getKey());
writePageTokens(response, pageTokens);
final TokenTO tokenTO = new TokenTO(pageTokens);
writeTokens(response, tokenTO);
}
} else {
writeMasterToken(request, response);
response.sendError(400, "This endpoint should not be invoked if the Token-Per-Page functionality is disabled!");
}
} else {
response.sendError(403, "Master token missing from the request.");
}
}

private static void writeMasterToken(final HttpServletRequest request, final HttpServletResponse response) throws IOException {
response.setContentType(TEXT_PLAIN_MIME_TYPE);
private static void writeTokens(final HttpServletResponse response, final TokenTO tokenTO) throws IOException {
final String jsonTokenTO = tokenTO.toString();

final CsrfGuard csrfGuard = CsrfGuard.getInstance();
final TokenService tokenService = csrfGuard.getTokenService();
final LogicalSession logicalSession = csrfGuard.getLogicalSessionExtractor().extract(request);

if (Objects.isNull(logicalSession)) {
throw new IllegalStateException("This should not happen");
} else {
final String masterTokenNameAndValue = delimitTokenFromValue(csrfGuard.getTokenName(), tokenService.getMasterToken(logicalSession.getKey()));

response.getWriter().write(masterTokenNameAndValue);
}
}

private static void writePageTokens(final HttpServletResponse response, final Map<String, String> pageTokens) throws IOException {
final String aggregatedPageTokens = pageTokens.entrySet().stream()
.map(e -> delimitTokenFromValue(e.getKey(), e.getValue()))
.collect(Collectors.joining(","));

response.setContentType(TEXT_PLAIN_MIME_TYPE);
response.setContentLength(aggregatedPageTokens.length());
response.setContentType(JSON_MIME_TYPE);
response.setContentLength(jsonTokenTO.length());
response.setCharacterEncoding(Charset.defaultCharset().displayName());

response.getWriter().write(aggregatedPageTokens);
response.getWriter().write(jsonTokenTO);
}

private static void writeJavaScript(final HttpServletRequest request, final HttpServletResponse response) throws IOException {
Expand Down Expand Up @@ -239,10 +223,6 @@ private static String parseDomain(final StringBuffer url) {
}
}

private static String delimitTokenFromValue(final String tokenName, final String tokenValue) {
return tokenName + ':' + tokenValue;
}

private void writeJavaScript(final CsrfGuard csrfGuard, final HttpServletRequest request, final HttpServletResponse response) throws IOException {
final String refererHeader = request.getHeader("referer");

Expand Down
96 changes: 33 additions & 63 deletions csrfguard/src/main/resources/csrfguard.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -488,43 +488,15 @@ if (owaspCSRFGuardScriptHasLoaded !== true) {
injectToElements(all, tokenName, tokenValue, pageTokens);
}

function parsePageTokens(response) {
var name = '';
var value = '';
var nameContext = true;

let pageTokens = {};
for (var i = 0; i < response.length; i++) {
var character = response.charAt(i);

if (character === ':') {
nameContext = false;
} else if (character !== ',') {
if (nameContext === true) {
name += character;
} else {
value += character;
}
}

if (character === ',' || (i + 1) >= response.length) {
pageTokens[name] = value;
name = '';
value = '';
nameContext = true;
}
}

return pageTokens;
}

/**
* obtain array of page specific tokens
*/
function requestPageTokens(tokenName, tokenValue, callback) {
var xhr = window.XMLHttpRequest ? new window.XMLHttpRequest : new window.ActiveXObject('Microsoft.XMLHTTP');
const xhr = window.XMLHttpRequest ? new window.XMLHttpRequest : new window.ActiveXObject('Microsoft.XMLHTTP');

xhr.open('POST', '%SERVLET_PATH%');

/* if AJAX is enabled, the token header will be automatically added, no need to set it again */
if ('%INJECT_XHR%' !== true) {
if (tokenName !== undefined && tokenValue !== undefined) {
xhr.setRequestHeader(tokenName, tokenValue);
Expand All @@ -534,7 +506,7 @@ if (owaspCSRFGuardScriptHasLoaded !== true) {
xhr.onreadystatechange = function () {
if (xhr.readyState === 4) {
if (xhr.status === 200) {
let pageTokens = parsePageTokens(xhr.responseText);
let pageTokens = JSON.parse(xhr.responseText)['pageTokens'];
callback.call(this, pageTokens);
} else {
alert(xhr.status + ': CSRF check failed');
Expand Down Expand Up @@ -615,32 +587,30 @@ if (owaspCSRFGuardScriptHasLoaded !== true) {
}

XMLHttpRequest.prototype.onsend = function (data) {
if ('%INJECT_XHR%') {
addEvent(this, 'readystatechange', function () {
if (this.readyState === 4) {
let tokenResponseHeader = this.getResponseHeader(tokenName);
if (tokenResponseHeader != undefined) {
try {
let tokenTO = window.JSON.parse(tokenResponseHeader)

let newMasterToken = tokenTO['masterToken'];
if (newMasterToken !== undefined) {
masterTokenValue = newMasterToken;
}

let newPageTokens = tokenTO['pageTokens'];
if (newPageTokens !== undefined) {
Object.keys(newPageTokens).forEach(key => pageTokenWrapper.pageTokens[key] = newPageTokens[key]); // TODO do not use arrow functions because IE does not support it
}

injectTokens(tokenName, masterTokenValue, pageTokenWrapper.pageTokens);
} catch (e) {
console.error("Error while updating tokens from response header.")
addEvent(this, 'readystatechange', function () {
if (this.readyState === 4) {
let tokenResponseHeader = this.getResponseHeader(tokenName);
if (tokenResponseHeader != undefined) {
try {
let tokenTO = JSON.parse(tokenResponseHeader)

let newMasterToken = tokenTO['masterToken'];
if (newMasterToken !== undefined) {
masterTokenValue = newMasterToken;
}

let newPageTokens = tokenTO['pageTokens'];
if (newPageTokens !== undefined) {
Object.keys(newPageTokens).forEach(key => pageTokenWrapper.pageTokens[key] = newPageTokens[key]); // TODO do not use arrow functions because IE does not support it
}

injectTokens(tokenName, masterTokenValue, pageTokenWrapper.pageTokens);
} catch (e) {
console.error("Error while updating tokens from response header.")
}
}
}, false)
}
}
});

var computePageToken = function(modifiedUrl) {
let result = null;
Expand Down Expand Up @@ -704,17 +674,17 @@ if (owaspCSRFGuardScriptHasLoaded !== true) {
};
}

let pageTokenRequestCallback = function (receivedPageTokens) {
pageTokenWrapper.pageTokens = receivedPageTokens;
if ('%TOKENS_PER_PAGE%') {
let pageTokenRequestCallback = function (receivedPageTokens) {
pageTokenWrapper.pageTokens = receivedPageTokens;

pageTokenWrapper.pageTokensLoaded = true;
pageTokenWrapper.pageTokensLoaded = true;

if (isLoadedWrapper.isDomContentLoaded) {
injectTokens(tokenName, masterTokenValue, receivedPageTokens);
}
};
if (isLoadedWrapper.isDomContentLoaded) {
injectTokens(tokenName, masterTokenValue, receivedPageTokens);
}
};

if ('%TOKENS_PER_PAGE%') {
requestPageTokens(tokenName, masterTokenValue, pageTokenRequestCallback);
} else {
/* update nodes in DOM after load */
Expand Down

0 comments on commit 3e19915

Please sign in to comment.