Skip to content

Commit

Permalink
[KEYCLOAK-10779] - CSRF check to My Resources
Browse files Browse the repository at this point in the history
  • Loading branch information
@pedroigor @hmlnarik
pedroigor authored and hmlnarik committed Jul 17, 2019
1 parent e3b2243 commit dbaba6f
Show file tree
Hide file tree
Showing 5 changed files with 134 additions and 18 deletions.
29 changes: 26 additions & 3 deletions services/src/main/java/org/keycloak/services/resources/account/AccountFormService.java
View file
Expand Up @@ -715,7 +715,15 @@ public Response resourceDetailPage(@PathParam("resource_id") String resourceId)

@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);
Expand Down Expand Up @@ -823,7 +831,15 @@ public Response grantPermission(@PathParam("resource_id") String resourceId, @Fo

@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);
Expand Down Expand Up @@ -899,7 +915,14 @@ public Response shareResource(@PathParam("resource_id") String resourceId, @Form

@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();

Expand Down
44 changes: 29 additions & 15 deletions ...ests/base/src/main/java/org/keycloak/testsuite/adapter/page/PhotozClientAuthzTestApp.java
View file
Expand Up @@ -36,6 +36,7 @@

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;
Expand Down Expand Up @@ -209,6 +210,10 @@ public void accountMyResource(String name) {

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();
Expand All @@ -225,6 +230,10 @@ public void accountGrantRemoveScope(String name, String requester, String scope)

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();
Expand All @@ -233,35 +242,36 @@ public void accountRevokeResource(String name, String requester) {

public void accountShareResource(String name, String user) {
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 shareButton = driver.findElement(By.id("share-button"));
waitUntilElement(shareButton).is().clickable();
shareButton.click();
shareResource(user);
waitForPageToLoad();
}

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();
waitForPageToLoad();

shareResource(user);

waitForPageToLoad();
}

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)
waitUntilElement(userIdInput).attribute(UIUtils.VALUE_ATTR_NAME).contains(user);

WebElement shareButton = driver.findElement(By.id("share-button"));
waitUntilElement(shareButton).is().clickable();
shareButton.click();

waitForPageToLoad();
}

public void assertError() {
assertEquals("We're sorry...", driver.findElement(By.id("kc-page-title")).getText());
}

public void accountDenyResource(String name) {
Expand Down Expand Up @@ -302,4 +312,8 @@ public void navigateTo() {
public boolean isCurrent() {
return URLUtils.currentUrlStartsWith(toString());
}

public void executeScript(String script) {
testExecutor.executeScript(script);
}
}
73 changes: 73 additions & 0 deletions ...rg/keycloak/testsuite/adapter/example/authorization/AbstractPhotozExampleAdapterTest.java
View file
Expand Up @@ -677,6 +677,51 @@ public void testRequestResourceToOwner() throws Exception {
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 testOwnerSharingResource() throws Exception {
loginToClientPage(aliceUser);
Expand Down Expand Up @@ -720,6 +765,34 @@ public void testOwnerSharingResource() throws Exception {
clientPage.viewAlbum(ALICE_ALBUM_NAME, this::assertWasDenied);
}

@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);
}

private void importResourceServerSettings() throws FileNotFoundException {
ResourceServerRepresentation authSettings = loadJson(new FileInputStream(new File(TEST_APPS_HOME_DIR + "/photoz/photoz-restful-api-authz-service.json")), ResourceServerRepresentation.class);

Expand Down
3 changes: 3 additions & 0 deletions themes/src/main/resources/theme/base/account/resource-detail.ftl
View file
Expand Up @@ -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>
Expand Down Expand Up @@ -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??>
Expand Down Expand Up @@ -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>
Expand Down
3 changes: 3 additions & 0 deletions themes/src/main/resources/theme/base/account/resources.ftl
View file
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -334,6 +336,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>
Expand Down

0 comments on commit dbaba6f

Please sign in to comment.