Skip to content

Commit

Permalink
Validate client ID for approvals
Browse files Browse the repository at this point in the history
  • Loading branch information
fhanik committed Apr 18, 2017
1 parent cba3074 commit 24c270c
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,27 @@
*******************************************************************************/
package org.cloudfoundry.identity.uaa.account;

import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.cloudfoundry.identity.uaa.approval.Approval;
import org.cloudfoundry.identity.uaa.approval.ApprovalsService;
import org.cloudfoundry.identity.uaa.approval.DescribedApproval;
import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
import org.cloudfoundry.identity.uaa.constants.OriginKeys;
import org.cloudfoundry.identity.uaa.approval.Approval;
import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.core.Authentication;
import org.springframework.security.oauth2.provider.ClientDetails;
import org.springframework.security.oauth2.provider.ClientDetailsService;
import org.springframework.security.oauth2.provider.NoSuchClientException;
import org.springframework.stereotype.Controller;
import org.springframework.ui.Model;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.servlet.View;
import org.springframework.web.servlet.view.RedirectView;

import java.util.ArrayList;
import java.util.Collection;
Expand All @@ -37,6 +43,8 @@
@Controller
public class ProfileController {

protected static Log logger = LogFactory.getLog(ProfileController.class);

private final ApprovalsService approvalsService;
private final ClientDetailsService clientDetailsService;

Expand Down Expand Up @@ -108,6 +116,13 @@ else if (null != delete) {
return "redirect:profile";
}

@ExceptionHandler
public View handleException(NoSuchClientException nsce) {
logger.debug("Unable to find client for approvals:"+nsce.getMessage());
return new RedirectView("profile?error_message_code=request.invalid_parameter", true);
}


private boolean isUaaManagedUser(Authentication authentication) {
if (authentication.getPrincipal() instanceof UaaPrincipal) {
UaaPrincipal principal = (UaaPrincipal) authentication.getPrincipal();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Cloud Foundry
* Cloud Foundry
* Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved.
*
* This product is licensed to you under the Apache License, Version 2.0 (the "License").
Expand All @@ -12,41 +12,45 @@
*******************************************************************************/
package org.cloudfoundry.identity.uaa.approval;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
import org.cloudfoundry.identity.uaa.web.ConvertingExceptionView;
import org.cloudfoundry.identity.uaa.web.ExceptionReport;
import org.cloudfoundry.identity.uaa.error.UaaException;
import org.cloudfoundry.identity.uaa.resources.ActionResult;
import org.cloudfoundry.identity.uaa.security.DefaultSecurityContextAccessor;
import org.cloudfoundry.identity.uaa.security.SecurityContextAccessor;
import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
import org.cloudfoundry.identity.uaa.util.UaaPagingUtils;
import org.cloudfoundry.identity.uaa.web.ConvertingExceptionView;
import org.cloudfoundry.identity.uaa.web.ExceptionReport;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.http.converter.HttpMessageConverter;
import org.springframework.security.access.AccessDeniedException;
import org.springframework.security.core.userdetails.UsernameNotFoundException;
import org.springframework.security.oauth2.provider.ClientDetails;
import org.springframework.security.oauth2.provider.ClientDetailsService;
import org.springframework.security.oauth2.provider.NoSuchClientException;
import org.springframework.security.oauth2.provider.client.BaseClientDetails;
import org.springframework.stereotype.Controller;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.*;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.ResponseBody;
import org.springframework.web.client.RestTemplate;
import org.springframework.web.servlet.View;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

@Controller
public class ApprovalsAdminEndpoints implements InitializingBean, ApprovalsControllerService {

Expand Down Expand Up @@ -168,6 +172,7 @@ public List<Approval> updateApprovals(@RequestBody Approval[] approvals) {
@ResponseBody
@Override
public List<Approval> updateClientApprovals(@PathVariable String clientId, @RequestBody Approval[] approvals) {
clientDetailsService.loadClientByClientId(clientId);
String currentUserId = getCurrentUserId();
logger.debug("Updating approvals for user: " + currentUserId);
approvalStore.revokeApprovals(String.format(USER_AND_CLIENT_FILTER_TEMPLATE, currentUserId, clientId));
Expand Down Expand Up @@ -202,10 +207,17 @@ private boolean isValidUser(String userId) {
public ActionResult revokeApprovals(@RequestParam(required = true) String clientId) {
String username = getCurrentUserId();
logger.debug("Revoking all existing approvals for user: " + username + " and client " + clientId);
clientDetailsService.loadClientByClientId(clientId);
approvalStore.revokeApprovals(String.format(USER_AND_CLIENT_FILTER_TEMPLATE, username, clientId));
return new ActionResult("ok", "Approvals of user " + username + " and client " + clientId + " revoked");
}

@ExceptionHandler
public View handleException(NoSuchClientException nsce) {
logger.debug("Client not found:"+nsce.getMessage());
return handleException(new UaaException(nsce.getMessage(), 404));
}

@ExceptionHandler
public View handleException(Exception t) {
UaaException e = t instanceof UaaException ? (UaaException) t : new UaaException("Unexpected error",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Cloud Foundry
* Cloud Foundry
* Copyright (c) [2009-2016] Pivotal Software, Inc. All Rights Reserved.
*
* This product is licensed to you under the Apache License, Version 2.0 (the "License").
Expand All @@ -12,30 +12,12 @@
*******************************************************************************/
package org.cloudfoundry.identity.uaa.oauth.approval;

import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.APPROVED;
import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.DENIED;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import com.fasterxml.jackson.core.type.TypeReference;
import org.cloudfoundry.identity.uaa.approval.Approval;
import org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus;
import org.cloudfoundry.identity.uaa.approval.ApprovalsAdminEndpoints;
import org.cloudfoundry.identity.uaa.approval.JdbcApprovalStore;
import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
import org.cloudfoundry.identity.uaa.error.UaaException;
import org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus;
import org.cloudfoundry.identity.uaa.resources.jdbc.JdbcPagingListFactory;
import org.cloudfoundry.identity.uaa.resources.jdbc.SimpleSearchQueryConverter;
import org.cloudfoundry.identity.uaa.security.SecurityContextAccessor;
Expand All @@ -48,14 +30,34 @@
import org.cloudfoundry.identity.uaa.util.JsonUtils;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.springframework.jdbc.core.JdbcTemplate;
import org.springframework.security.oauth2.provider.NoSuchClientException;
import org.springframework.security.oauth2.provider.client.BaseClientDetails;
import org.springframework.security.oauth2.provider.client.InMemoryClientDetailsService;

import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.APPROVED;
import static org.cloudfoundry.identity.uaa.approval.Approval.ApprovalStatus.DENIED;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class ApprovalsAdminEndpointsTests extends JdbcTestBase {
private UaaTestAccounts testAccounts = null;

private JdbcApprovalStore dao;

private UaaUserDatabase userDao = null;
Expand All @@ -64,6 +66,9 @@ public class ApprovalsAdminEndpointsTests extends JdbcTestBase {

private ApprovalsAdminEndpoints endpoints;

@Rule
public ExpectedException exception = ExpectedException.none();

@Before
public void initApprovalsAdminEndpointsTests() {
testAccounts = UaaTestAccounts.standard(null);
Expand Down Expand Up @@ -114,6 +119,21 @@ public void cleanupDataSource() throws Exception {
assertThat(jdbcTemplate.queryForObject("select count(*) from users", Integer.class), is(0));
}

@Test
public void validate_client_id_on_revoke() throws Exception {
exception.expect(NoSuchClientException.class);
exception.expectMessage("No client with requested id: invalid_id");
endpoints.revokeApprovals("invalid_id");
}

@Test
public void validate_client_id_on_update() throws Exception {
exception.expect(NoSuchClientException.class);
exception.expectMessage("No client with requested id: invalid_id");
endpoints.updateClientApprovals("invalid_id", new Approval[0]);
}


@Test
public void canGetApprovals() {
addApproval(marissa.getId(), "c1", "uaa.user", 6000, APPROVED);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.header;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrlPattern;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

Expand Down Expand Up @@ -128,6 +129,37 @@ public void test_oauth_authorize_without_csrf() throws Exception {
.andExpect(status().isFound()); //approval page no longer showing up
}

@Test
public void revoke() throws Exception {
test_oauth_authorize_without_csrf();
MockHttpSession session = getAuthenticatedSession(user1);
getMockMvc().perform(
post("/profile")
.with(cookieCsrf())
.param("delete","true")
.param("clientId", client1.getClientId())
.session(session)
)
.andExpect(status().isFound())
.andExpect(header().string("Location", "profile"));

}

@Test
public void revoke_invalid_client() throws Exception {
test_oauth_authorize_without_csrf();
MockHttpSession session = getAuthenticatedSession(user1);
getMockMvc().perform(
post("/profile")
.with(cookieCsrf())
.param("delete","true")
.param("clientId", "invalid_id")
.session(session)
)
.andExpect(status().isFound())
.andExpect(header().string("Location", "profile?error_message_code=request.invalid_parameter"));
}

@Test
public void test_get_approvals() throws Exception {
test_oauth_authorize_without_csrf();
Expand Down

0 comments on commit 24c270c

Please sign in to comment.