added CSRF protection to approval page

pull/604/head
Justin Richer 2014-05-13 09:27:02 -04:00
parent fcfbf1080f
commit a253ebc908
3 changed files with 40 additions and 24 deletions

View File

@ -237,6 +237,7 @@
</c:choose>"? </c:choose>"?
</h3> </h3>
<input id="user_oauth_approval" name="user_oauth_approval" value="true" type="hidden" /> <input id="user_oauth_approval" name="user_oauth_approval" value="true" type="hidden" />
<input name="csrf" value="${ csrf }" type="hidden" />
<input name="authorize" value="Authorize" type="submit" <input name="authorize" value="Authorize" type="submit"
onclick="$('#user_oauth_approval').attr('value',true)" class="btn btn-success btn-large" /> onclick="$('#user_oauth_approval').attr('value',true)" class="btn btn-success btn-large" />
&nbsp; &nbsp;

View File

@ -26,6 +26,7 @@ import java.util.LinkedHashSet;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.UUID;
import org.mitre.oauth2.model.ClientDetailsEntity; import org.mitre.oauth2.model.ClientDetailsEntity;
import org.mitre.oauth2.model.SystemScope; import org.mitre.oauth2.model.SystemScope;
@ -192,6 +193,10 @@ public class OAuthConfirmationController {
model.put("gras", false); model.put("gras", false);
} }
// inject a random value for CSRF purposes
String csrf = UUID.randomUUID().toString();
model.put("csrf", csrf);
authRequest.getExtensions().put("csrf", csrf);
return "approve"; return "approve";
} }

View File

@ -96,11 +96,21 @@ public class TofuUserApprovalHandler implements UserApprovalHandler {
return true; return true;
} else { } else {
// if not, check to see if the user has approved it // if not, check to see if the user has approved it
if (Boolean.parseBoolean(authorizationRequest.getApprovalParameters().get("user_oauth_approval"))) { // TODO: make parameter name configurable?
// TODO: make parameter name configurable? // check the value of the CSRF parameter
boolean approved = Boolean.parseBoolean(authorizationRequest.getApprovalParameters().get("user_oauth_approval"));
return userAuthentication.isAuthenticated() && approved; if (authorizationRequest.getExtensions().get("csrf") != null) {
if (authorizationRequest.getExtensions().get("csrf").equals(authorizationRequest.getApprovalParameters().get("csrf"))) {
// make sure the user is actually authenticated
return userAuthentication.isAuthenticated();
}
}
}
// if the above doesn't pass, it's not yet approved
return false;
} }
} }
@ -182,9 +192,9 @@ public class TofuUserApprovalHandler implements UserApprovalHandler {
ClientDetails client = clientDetailsService.loadClientByClientId(clientId); ClientDetails client = clientDetailsService.loadClientByClientId(clientId);
// This must be re-parsed here because SECOAUTH forces us to call things in a strange order // This must be re-parsed here because SECOAUTH forces us to call things in a strange order
boolean approved = Boolean.parseBoolean(authorizationRequest.getApprovalParameters().get("user_oauth_approval")); if (Boolean.parseBoolean(authorizationRequest.getApprovalParameters().get("user_oauth_approval"))
&& authorizationRequest.getExtensions().get("csrf") != null
if (approved) { && authorizationRequest.getExtensions().get("csrf").equals(authorizationRequest.getApprovalParameters().get("csrf"))) {
authorizationRequest.setApproved(true); authorizationRequest.setApproved(true);