Delete client tokens when client revoke is invoked

Remove client token self revocation as not needed.

[#152093534] https://www.pivotaltracker.com/story/show/152093534

server/src/main/java/org/cloudfoundry/identity/uaa/oauth/TokenRevocationEndpoint.java

@@ -16,6 +16,7 @@
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.cloudfoundry.identity.uaa.audit.event.SystemDeletable;
 import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
 import org.cloudfoundry.identity.uaa.oauth.client.ClientConstants;
 import org.cloudfoundry.identity.uaa.oauth.event.TokenRevocationEvent;
@@ -102,6 +103,7 @@ public ResponseEntity<Void> revokeTokensForClient(@PathVariable String clientId)
         clientDetailsService.updateClientDetails(client, zoneId);
         eventPublisher.publishEvent(new TokenRevocationEvent(null, clientId, zoneId, SecurityContextHolder.getContext().getAuthentication()));
         logger.debug("Tokens revoked for client: " + clientId);
+        ((SystemDeletable)tokenProvisioning).deleteByClient(clientId, zoneId);
         return new ResponseEntity<>(OK);
     }
 

server/src/main/java/org/cloudfoundry/identity/uaa/oauth/token/JdbcRevocableTokenProvisioning.java

@@ -58,7 +58,7 @@ public class JdbcRevocableTokenProvisioning implements RevocableTokenProvisionin
     protected long expirationCheckInterval = 30000; //30 seconds
     private long maxExpirationRuntime = 2500l;
 
-    protected JdbcRevocableTokenProvisioning(JdbcTemplate jdbcTemplate, LimitSqlAdapter limitSqlAdapter) {
+    public JdbcRevocableTokenProvisioning(JdbcTemplate jdbcTemplate, LimitSqlAdapter limitSqlAdapter) {
         this.rowMapper =  new RevocableTokenRowMapper();
         this.template = jdbcTemplate;
         this.limitSqlAdapter = limitSqlAdapter;

server/src/test/java/org/cloudfoundry/identity/uaa/oauth/TokenRevocationEndpointTests.java

@@ -0,0 +1,123 @@
+/*
+ * ****************************************************************************
+ *     Cloud Foundry
+ *     Copyright (c) [2009-2017] Pivotal Software, Inc. All Rights Reserved.
+ *
+ *     This product is licensed to you under the Apache License, Version 2.0 (the "License").
+ *     You may not use this product except in compliance with the License.
+ *
+ *     This product includes a number of subcomponents with
+ *     separate copyright notices and license terms. Your use of these
+ *     subcomponents is subject to the terms and conditions of the
+ *     subcomponent's license, as noted in the LICENSE file.
+ * ****************************************************************************
+ */
+
+package org.cloudfoundry.identity.uaa.oauth;
+
+import org.cloudfoundry.identity.uaa.authentication.UaaAuthentication;
+import org.cloudfoundry.identity.uaa.authentication.UaaAuthenticationDetails;
+import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
+import org.cloudfoundry.identity.uaa.constants.OriginKeys;
+import org.cloudfoundry.identity.uaa.oauth.token.JdbcRevocableTokenProvisioning;
+import org.cloudfoundry.identity.uaa.oauth.token.RevocableToken;
+import org.cloudfoundry.identity.uaa.resources.jdbc.JdbcPagingListFactory;
+import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning;
+import org.cloudfoundry.identity.uaa.scim.jdbc.JdbcScimUserProvisioning;
+import org.cloudfoundry.identity.uaa.test.JdbcTestBase;
+import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
+import org.cloudfoundry.identity.uaa.zone.MultitenantJdbcClientDetailsService;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.springframework.context.ApplicationEventPublisher;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.security.oauth2.common.util.RandomValueStringGenerator;
+import org.springframework.security.oauth2.provider.ClientDetails;
+import org.springframework.security.oauth2.provider.OAuth2Request;
+import org.springframework.security.oauth2.provider.client.BaseClientDetails;
+
+import java.util.Collections;
+
+import static org.cloudfoundry.identity.uaa.oauth.client.ClientConstants.TOKEN_SALT;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+
+public class TokenRevocationEndpointTests extends JdbcTestBase {
+
+    private TokenRevocationEndpoint endpoint;
+    private RandomValueStringGenerator generator;
+    private BaseClientDetails client;
+    private ApplicationEventPublisher publisher;
+    private MultitenantJdbcClientDetailsService clientService;
+
+    @Before
+    public void setupForTokenRevocation() throws Exception {
+        String zoneId = IdentityZoneHolder.get().getId();
+        generator = new RandomValueStringGenerator();
+        String clientId = generator.generate().toLowerCase();
+        client = new BaseClientDetails(clientId, "", "some.scopes", "client_credentials", "authorities");
+        client.addAdditionalInformation(TOKEN_SALT, "pre-salt");
+        clientService = spy(new MultitenantJdbcClientDetailsService(jdbcTemplate));
+        clientService.addClientDetails(client, zoneId);
+
+        ScimUserProvisioning userProvisioning = new JdbcScimUserProvisioning(
+            jdbcTemplate,
+            new JdbcPagingListFactory(jdbcTemplate, limitSqlAdapter)
+        );
+        JdbcRevocableTokenProvisioning provisioning = spy(new JdbcRevocableTokenProvisioning(jdbcTemplate, limitSqlAdapter));
+        endpoint = spy(new TokenRevocationEndpoint(clientService, userProvisioning, provisioning));
+        publisher = mock(ApplicationEventPublisher.class);
+        endpoint.setApplicationEventPublisher(publisher);
+
+        SecurityContextHolder.getContext().setAuthentication(
+            new UaaOauth2Authentication(
+                "token-value",
+                zoneId,
+                mock(OAuth2Request.class),
+                new UaaAuthentication(
+                    new UaaPrincipal("id", "username", "username@test.com", OriginKeys.UAA, "", zoneId),
+                    Collections.emptyList(),
+                    mock(UaaAuthenticationDetails.class)
+                )
+            )
+        );
+
+        provisioning.create(
+            new RevocableToken()
+                .setClientId(client.getClientId())
+                .setTokenId("token-id")
+                .setUserId(null)
+                .setResponseType(RevocableToken.TokenType.ACCESS_TOKEN)
+                .setValue("value")
+                .setIssuedAt(System.currentTimeMillis()),
+            zoneId
+        );
+    }
+
+    @After
+    public void cleanup() throws Exception {
+        SecurityContextHolder.clearContext();
+        IdentityZoneHolder.clear();
+    }
+
+    @Test
+    public void revokeTokensForClient() throws Exception {
+        assertEquals("pre-salt", getClient().getAdditionalInformation().get(TOKEN_SALT));
+        assertEquals(1, clientTokenCount());
+        endpoint.revokeTokensForClient(client.getClientId());
+        assertNotEquals("pre-salt", getClient().getAdditionalInformation().get(TOKEN_SALT));
+        assertEquals(0, clientTokenCount());
+    }
+
+    public ClientDetails getClient() {
+        return clientService.loadClientByClientId(client.getClientId());
+    }
+
+    public int clientTokenCount() {
+        return jdbcTemplate.queryForObject("select count(*) from revocable_tokens where client_id = ?", Integer.class, client.getClientId());
+    }
+
+}

uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml

@@ -63,7 +63,7 @@
            authentication-manager-ref="emptyAuthenticationManager"
            entry-point-ref="oauthAuthenticationEntryPoint"
            xmlns="http://www.springframework.org/schema/security" use-expressions="true">
-        <intercept-url pattern="/oauth/token/revoke/client/**" access="#oauth2.hasScope('uaa.admin') or @self.isClientTokenRevocationForSelf(request, 4)" />
+        <intercept-url pattern="/oauth/token/revoke/client/**" access="#oauth2.hasScope('tokens.revoke')" />
         <intercept-url pattern="/oauth/token/revoke/user/**/client/**" access="#oauth2.hasScope('uaa.admin') or #oauth2.hasScope('tokens.revoke') or (@self.isUserTokenRevocationForSelf(request, 4) and @self.isClientTokenRevocationForSelf(request, 6))" />
         <intercept-url pattern="/oauth/token/revoke/user/**" access="#oauth2.hasScope('uaa.admin') or (#oauth2.hasScope('tokens.revoke') and @self.isUserTokenRevocationForSelf(request, 4))" />
         <intercept-url pattern="/oauth/token/revoke/**" access="#oauth2.hasScope('tokens.revoke') or @self.isTokenRevocationForSelf(request, 3)" method="DELETE"/>

uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/token/TokenRevocationEndpointMockMvcTest.java

@@ -96,7 +96,7 @@ public void revokeOwnJWToken() throws Exception {
     }
 
     @Test
-    public void revokeOtherClientToken() throws Exception {
+    public void revokeOtherClientTokenByJti() throws Exception {
         String revokerClientId = generator.generate();
         String resourceClientId = generator.generate();
 
@@ -152,6 +152,72 @@ public void revokeOtherClientToken() throws Exception {
         }
     }
 
+    @Test
+    public void revokeOtherClientTokenByClientId_tokensDotRevoke() throws Exception {
+        revokeOtherClientTokenByClientId("tokens.revoke");
+    }
+
+    @Test
+    public void revokeOtherClientTokenByClientId_uaaDotAdmin() throws Exception {
+        revokeOtherClientTokenByClientId("uaa.admin");
+    }
+
+    public void revokeOtherClientTokenByClientId(String scope) throws Exception {
+        String revokerClientId = generator.generate();
+        String resourceClientId = generator.generate();
+
+        BaseClientDetails revokerClient =
+            setUpClients(revokerClientId,
+                         scope,
+                         "openid",
+                         "client_credentials,password",
+                         true
+            );
+
+
+        BaseClientDetails targetClient =
+            setUpClients(resourceClientId,
+                         "uaa.none",
+                         "openid",
+                         "client_credentials,password",
+                         true
+            );
+
+
+        //this is the token we will revoke
+        String revokeAccessToken =
+            getClientCredentialsOAuthAccessToken(
+                getMockMvc(),
+                revokerClient.getClientId(),
+                SECRET,
+                scope,
+                null,
+                false
+            );
+
+        String tokenToBeRevoked =
+            getClientCredentialsOAuthAccessToken(
+                getMockMvc(),
+                resourceClientId,
+                SECRET,
+                null,
+                null,
+                true
+            );
+
+        getMockMvc().perform(delete("/oauth/token/revoke/client/" + resourceClientId)
+                                 .header("Authorization", "Bearer " + revokeAccessToken))
+            .andExpect(status().isOk());
+
+
+        try {
+            tokenProvisioning.retrieve(tokenToBeRevoked, IdentityZoneHolder.get().getId());
+            fail("Token should have been deleted");
+        } catch (EmptyResultDataAccessException e) {
+            //expected
+        }
+    }
+
     @Test
     public void revokeOtherClientTokenForbidden() throws Exception {
         String resourceClientId = generator.generate();