[KEYCLOAK-10779] - CSRF check to My Resources

(cherry picked from commit dbaba6f1b8c043da4a37c906dc0d1700956a0869)
This commit is contained in:
Pedro Igor 2019-07-15 10:01:00 -03:00 committed by Bruno Oliveira da Silva
parent 97811fdd51
commit e12c245355
5 changed files with 132 additions and 16 deletions

View file

@ -723,7 +723,15 @@ public class AccountFormService extends AbstractSecuredLocalService {
@Path("resource/{resource_id}/grant")
@POST
public Response grantPermission(@PathParam("resource_id") String resourceId, @FormParam("action") String action, @FormParam("permission_id") String[] permissionId, @FormParam("requester") String requester) {
public Response grantPermission(@PathParam("resource_id") String resourceId, @FormParam("action") String action, @FormParam("permission_id") String[] permissionId, @FormParam("requester") String requester, MultivaluedMap<String, String> formData) {
if (auth == null) {
return login("resource");
}
auth.require(AccountRoles.MANAGE_ACCOUNT);
csrfCheck(formData);
AuthorizationProvider authorization = session.getProvider(AuthorizationProvider.class);
PermissionTicketStore ticketStore = authorization.getStoreFactory().getPermissionTicketStore();
Resource resource = authorization.getStoreFactory().getResourceStore().findById(resourceId, null);
@ -837,7 +845,15 @@ public class AccountFormService extends AbstractSecuredLocalService {
@Path("resource/{resource_id}/share")
@POST
public Response shareResource(@PathParam("resource_id") String resourceId, @FormParam("user_id") String[] userIds, @FormParam("scope_id") String[] scopes) {
public Response shareResource(@PathParam("resource_id") String resourceId, @FormParam("user_id") String[] userIds, @FormParam("scope_id") String[] scopes, MultivaluedMap<String, String> formData) {
if (auth == null) {
return login("resource");
}
auth.require(AccountRoles.MANAGE_ACCOUNT);
csrfCheck(formData);
AuthorizationProvider authorization = session.getProvider(AuthorizationProvider.class);
PermissionTicketStore ticketStore = authorization.getStoreFactory().getPermissionTicketStore();
Resource resource = authorization.getStoreFactory().getResourceStore().findById(resourceId, null);
@ -915,7 +931,14 @@ public class AccountFormService extends AbstractSecuredLocalService {
@Path("resource")
@POST
public Response processResourceActions(@FormParam("resource_id") String[] resourceIds, @FormParam("action") String action) {
public Response processResourceActions(@FormParam("resource_id") String[] resourceIds, @FormParam("action") String action, MultivaluedMap<String, String> formData) {
if (auth == null) {
return login("resource");
}
auth.require(AccountRoles.MANAGE_ACCOUNT);
csrfCheck(formData);
AuthorizationProvider authorization = session.getProvider(AuthorizationProvider.class);
PermissionTicketStore ticketStore = authorization.getStoreFactory().getPermissionTicketStore();

View file

@ -36,6 +36,7 @@ import org.openqa.selenium.support.FindBy;
import java.net.URL;
import static org.junit.Assert.assertEquals;
import static org.keycloak.testsuite.util.UIUtils.clickLink;
import static org.keycloak.testsuite.util.WaitUtils.pause;
import static org.keycloak.testsuite.util.WaitUtils.waitForPageToLoad;
@ -206,6 +207,10 @@ public class PhotozClientAuthzTestApp extends AbstractPageWithInjectedUrl {
public void accountGrantResource(String name, String requester) {
accountMyResources();
grantResource(name, requester);
}
public void grantResource(String name, String requester) {
WebElement grantResource = driver.findElement(By.id("grant-" + name + "-" + requester));
waitUntilElement(grantResource).is().clickable();
grantResource.click();
@ -220,6 +225,10 @@ public class PhotozClientAuthzTestApp extends AbstractPageWithInjectedUrl {
public void accountRevokeResource(String name, String requester) {
accountMyResource(name);
revokeResource(name, requester);
}
public void revokeResource(String name, String requester) {
WebElement revokeResource = driver.findElement(By.id("revoke-" + name + "-" + requester));
waitUntilElement(revokeResource).is().clickable();
revokeResource.click();
@ -227,6 +236,20 @@ public class PhotozClientAuthzTestApp extends AbstractPageWithInjectedUrl {
public void accountShareResource(String name, String user) {
accountMyResource(name);
shareResource(user);
}
public void accountShareRemoveScope(String name, String user, String scope) {
accountMyResource(name);
WebElement shareRemoveScope = driver.findElement(By.id("share-remove-scope-" + name + "-" + scope));
waitUntilElement(shareRemoveScope).is().clickable();
shareRemoveScope.click();
shareResource(user);
}
public void shareResource(String user) {
WebElement userIdInput = driver.findElement(By.id("user_id"));
UIUtils.setTextInputValue(userIdInput, user);
pause(200); // We need to wait a bit for the form to "accept" the input (otherwise it registers the input as empty)
@ -237,21 +260,8 @@ public class PhotozClientAuthzTestApp extends AbstractPageWithInjectedUrl {
shareButton.click();
}
public void accountShareRemoveScope(String name, String user, String scope) {
accountMyResource(name);
WebElement userIdInput = driver.findElement(By.id("user_id"));
UIUtils.setTextInputValue(userIdInput, user);
pause(200); // We need to wait a bit for the form to "accept" the input (otherwise it registers the input as empty)
waitUntilElement(userIdInput).attribute(UIUtils.VALUE_ATTR_NAME).contains(user);
WebElement shareRemoveScope = driver.findElement(By.id("share-remove-scope-" + name + "-" + scope));
waitUntilElement(shareRemoveScope).is().clickable();
shareRemoveScope.click();
WebElement shareButton = driver.findElement(By.id("share-button"));
waitUntilElement(shareButton).is().clickable();
shareButton.click();
public void assertError() {
assertEquals("We're sorry...", driver.findElement(By.id("kc-page-title")).getText());
}
public void accountDenyResource(String name) {
@ -292,4 +302,8 @@ public class PhotozClientAuthzTestApp extends AbstractPageWithInjectedUrl {
public boolean isCurrent() {
return URLUtils.currentUrlStartsWith(toString());
}
public void executeScript(String script) {
testExecutor.executeScript(script);
}
}

View file

@ -117,4 +117,77 @@ public abstract class AbstractPhotozAccountResourcesAdapterTest extends Abstract
clientPage.viewAlbum(ALICE_ALBUM_NAME, this::assertWasNotDenied);
clientPage.deleteAlbum(ALICE_ALBUM_NAME, this::assertWasDenied);
}
@Test
public void testCsrfGrantAccess() throws Exception {
loginToClientPage(aliceUser);
clientPage.createAlbum(ALICE_ALBUM_NAME, true);
loginToClientPage(jdoeUser);
clientPage.viewAlbum(ALICE_ALBUM_NAME, this::assertWasDenied);
clientPage.deleteAlbum(ALICE_ALBUM_NAME, this::assertWasDenied);
loginToClientPage(aliceUser);
clientPage.viewAlbum(ALICE_ALBUM_NAME, this::assertWasNotDenied);
clientPage.accountMyResources();
clientPage.executeScript("document.forms[0].stateChecker.value = 'invalid'");
clientPage.grantResource(ALICE_ALBUM_NAME, "jdoe");
clientPage.assertError();
}
@Test
public void testCsrfRevokeResource() throws Exception {
loginToClientPage(aliceUser);
clientPage.createAlbum(ALICE_ALBUM_NAME, true);
loginToClientPage(jdoeUser);
clientPage.viewAlbum(ALICE_ALBUM_NAME, this::assertWasDenied);
clientPage.deleteAlbum(ALICE_ALBUM_NAME, this::assertWasDenied);
loginToClientPage(aliceUser);
clientPage.viewAlbum(ALICE_ALBUM_NAME, this::assertWasNotDenied);
clientPage.accountGrantResource(ALICE_ALBUM_NAME, "jdoe");
clientPage.navigateTo();
testExecutor.init(defaultArguments(), this::assertInitNotAuth)
.login(this::assertOnTestAppUrl)
.init(defaultArguments(), this::assertSuccessfullyLoggedIn);
loginToClientPage(aliceUser);
clientPage.viewAlbum(ALICE_ALBUM_NAME, this::assertWasNotDenied);
clientPage.accountMyResource(ALICE_ALBUM_NAME);
clientPage.executeScript("document.forms[0].stateChecker.value = 'invalid'");
clientPage.revokeResource(ALICE_ALBUM_NAME, "jdoe");
clientPage.assertError();
}
@Test
public void testCrfCheckSharingResource() throws Exception {
loginToClientPage(aliceUser);
clientPage.createAlbum(ALICE_ALBUM_NAME, true);
clientPage.accountMyResource(ALICE_ALBUM_NAME);
clientPage.executeScript("document.forms['shareForm'].stateChecker.value = 'invalid'");
clientPage.shareResource("jdoe");
clientPage.assertError();
clientPage.navigateTo();
testExecutor.init(defaultArguments(), this::assertInitNotAuth)
.login()
.init(defaultArguments(), this::assertSuccessfullyLoggedIn);
loginToClientPage(aliceUser);
clientPage.accountMyResource(ALICE_ALBUM_NAME);
clientPage.shareResource("jdoe");
clientPage.navigateTo();
testExecutor.init(defaultArguments(), this::assertInitNotAuth)
.login(this::assertOnTestAppUrl)
.init(defaultArguments(), this::assertSuccessfullyLoggedIn);
loginToClientPage(jdoeUser);
clientPage.viewAlbum(ALICE_ALBUM_NAME, this::assertWasNotDenied);
clientPage.deleteAlbum(ALICE_ALBUM_NAME, this::assertWasNotDenied);
}
}

View file

@ -121,6 +121,7 @@
<form action="${url.getResourceGrant(authorization.resource.id)}" name="revokeForm-${authorization.resource.id}-${permission.requester.username}" method="post">
<input type="hidden" name="action" value="revoke">
<input type="hidden" name="requester" value="${permission.requester.username}">
<input type="hidden" id="stateChecker" name="stateChecker" value="${stateChecker}">
<tr>
<td>
<#if permission.requester.email??>${permission.requester.email}<#else>${permission.requester.username}</#if>
@ -188,6 +189,7 @@
<form action="${url.getResourceGrant(authorization.resource.id)}" name="revokePolicyForm-${authorization.resource.id}-${permission.id}" method="post">
<input type="hidden" name="action" value="revokePolicy">
<input type="hidden" name="permission_id" value="${permission.id}"/>
<input type="hidden" id="stateChecker" name="stateChecker" value="${stateChecker}">
<tr>
<td>
<#if permission.description??>
@ -239,6 +241,7 @@
<div class="row">
<div class="col-md-10">
<form action="${url.getResourceShare(authorization.resource.id)}" name="shareForm" method="post">
<input type="hidden" id="stateChecker" name="stateChecker" value="${stateChecker}">
<div class="col-sm-3 col-md-3">
<label for="password" class="control-label">${msg("username")} or ${msg("email")} </label> <span class="required">*</span>
</div>

View file

@ -136,6 +136,7 @@
<form action="${url.getResourceGrant(resource.id)}" name="approveForm-${resource.id}-${permission.requester.username}" method="post">
<input type="hidden" name="action" value="grant">
<input type="hidden" name="requester" value="${permission.requester.username}">
<input type="hidden" id="stateChecker" name="stateChecker" value="${stateChecker}">
<tr>
<td>
<#if resource.displayName??>${resource.displayName}<#else>${resource.name}</#if>
@ -234,6 +235,7 @@
<div class="col-md-12">
<form action="${url.resourceUrl}" name="shareForm" method="post">
<input type="hidden" name="action" value="cancel"/>
<input type="hidden" id="stateChecker" name="stateChecker" value="${stateChecker}">
<table class="table table-striped table-bordered">
<thead>
<tr>
@ -331,6 +333,7 @@
<div class="col-md-12">
<form action="${url.resourceUrl}" name="waitingApprovalForm" method="post">
<input type="hidden" name="action" value="cancelRequest"/>
<input type="hidden" id="stateChecker" name="stateChecker" value="${stateChecker}">
<table class="table table-striped table-bordered">
<thead>
<tr>