Skip to content

Commit

Permalink
Call oauth/token/revoke endpoint with opaque token
Browse files Browse the repository at this point in the history 
[#117586873] https://www.pivotaltracker.com/story/show/117586873

Signed-off-by: Madhura Bhave <mbhave@pivotal.io>
  • Loading branch information
@Priyata25 @mbhave
Priyata25 authored and mbhave committed Apr 15, 2016
1 parent 224dc67 commit 39b4767
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 75 deletions.
53 changes: 4 additions & 49 deletions server/src/main/java/org/cloudfoundry/identity/uaa/oauth/TokenRevocationEndpoint.java
View file
Expand Up @@ -14,23 +14,16 @@

package org.cloudfoundry.identity.uaa.oauth;

import com.fasterxml.jackson.core.type.TypeReference;
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.oauth.jwt.Jwt;
import org.cloudfoundry.identity.uaa.oauth.jwt.JwtHelper;
import org.cloudfoundry.identity.uaa.oauth.token.RevocableToken;
import org.cloudfoundry.identity.uaa.oauth.token.RevocableTokenProvisioning;
import org.cloudfoundry.identity.uaa.scim.ScimUser;
import org.cloudfoundry.identity.uaa.scim.ScimUserProvisioning;
import org.cloudfoundry.identity.uaa.scim.exception.ScimResourceNotFoundException;
import org.cloudfoundry.identity.uaa.util.JsonUtils;
import org.cloudfoundry.identity.uaa.zone.MultitenantJdbcClientDetailsService;
import org.springframework.dao.EmptyResultDataAccessException;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.security.oauth2.common.exceptions.InsufficientScopeException;
import org.springframework.security.oauth2.common.exceptions.InvalidTokenException;
import org.springframework.security.oauth2.common.exceptions.OAuth2Exception;
import org.springframework.security.oauth2.common.util.RandomValueStringGenerator;
Expand All @@ -39,16 +32,11 @@
import org.springframework.security.oauth2.provider.error.DefaultWebResponseExceptionTranslator;
import org.springframework.security.oauth2.provider.error.WebResponseExceptionTranslator;
import org.springframework.stereotype.Controller;
import org.springframework.util.StringUtils;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;

import javax.servlet.http.HttpServletRequest;
import java.util.List;
import java.util.Map;

import static org.springframework.http.HttpStatus.OK;

@Controller
Expand Down Expand Up @@ -88,41 +76,13 @@ public ResponseEntity<Void> revokeTokensForClient(@PathVariable String clientId)
}

@RequestMapping(value = "/oauth/token/revoke/{tokenId}", method = RequestMethod.DELETE)
public ResponseEntity<Void> revokeTokenById(@PathVariable String tokenId,
HttpServletRequest request) {
public ResponseEntity<Void> revokeTokenById(@PathVariable String tokenId) {
logger.debug("Revoking token");
String authorization = request.getHeader("Authorization");
String requestToken = authorization.split(" ")[1];
Jwt jwt = JwtHelper.decode(requestToken);
Map<String, Object> claims = JsonUtils.readValue(jwt.getClaims(), new TypeReference<Map<String, Object>>() {});

RevocableToken token = tokenProvisioning.retrieve(tokenId);

List<String> scopes = (List<String>) claims.get("scope");

if (!scopes.contains("tokens.revoke") && !scopes.contains("uaa.admin")) {
String userId = (String) claims.get("user_id");
String clientId = (String) claims.get("client_id");
if (StringUtils.hasText(userId)) {
if (token.getUserId().equals(userId)) {
tokenProvisioning.delete(tokenId, -1);
logger.debug("Revoked user token with ID: " + tokenId);
return new ResponseEntity<>(OK);
}
} else if (StringUtils.hasText(clientId)) {
if (token.getClientId().equals(clientId)) {
tokenProvisioning.delete(tokenId, -1);
logger.debug("Revoked client token with ID: " + tokenId);
return new ResponseEntity<>(OK);
}
}
} else {
tokenProvisioning.delete(tokenId, -1);
logger.debug("Revoked token with ID: " + tokenId);
return new ResponseEntity<>(OK);
}
tokenProvisioning.delete(tokenId, -1);

throw new InsufficientScopeException("Cannot revoke others' tokens unless you have either `tokens.revoke` or `uaa.admin`");
logger.debug("Revoked token with ID: " + tokenId);
return new ResponseEntity<>(OK);
}

@ExceptionHandler({ScimResourceNotFoundException.class, NoSuchClientException.class, EmptyResultDataAccessException.class})
Expand All @@ -136,9 +96,4 @@ public int getHttpErrorCode() {
};
return exceptionTranslator.translate(e404);
}

@ExceptionHandler(InsufficientScopeException.class)
public ResponseEntity<InsufficientScopeException> handleInvalidClientDetails(InsufficientScopeException e) {
return new ResponseEntity<>(e, HttpStatus.FORBIDDEN);
}
}
40 changes: 35 additions & 5 deletions ...dentity/uaa/security/IsUserSelfCheck.java → ...ry/identity/uaa/security/IsSelfCheck.java
View file
Expand Up @@ -16,6 +16,8 @@


import org.cloudfoundry.identity.uaa.authentication.UaaPrincipal;
import org.cloudfoundry.identity.uaa.oauth.token.RevocableToken;
import org.cloudfoundry.identity.uaa.oauth.token.RevocableTokenProvisioning;
import org.cloudfoundry.identity.uaa.util.UaaUrlUtils;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.context.SecurityContextHolder;
Expand All @@ -24,9 +26,15 @@

import javax.servlet.http.HttpServletRequest;

public class IsUserSelfCheck {
public class IsSelfCheck {

public boolean isSelf(HttpServletRequest request, int pathParameterIndex) {
private final RevocableTokenProvisioning tokenProvisioning;

public IsSelfCheck(RevocableTokenProvisioning tokenProvisioning) {
this.tokenProvisioning = tokenProvisioning;
}

public boolean isUserSelf(HttpServletRequest request, int pathParameterIndex) {
String pathInfo = UaaUrlUtils.getRequestPath(request);
if (!StringUtils.hasText(pathInfo)) {
return false;
Expand All @@ -37,15 +45,15 @@ public boolean isSelf(HttpServletRequest request, int pathParameterIndex) {
return false;
}

String idFromAuth = extractIdFromAuthentication(SecurityContextHolder.getContext().getAuthentication());
String idFromAuth = extractIdFromAuthentication(SecurityContextHolder.getContext().getAuthentication(), false);
if (idFromAuth==null) {
return false;
}

return idFromAuth.equals(idFromUrl);
}

protected String extractIdFromAuthentication(Authentication authentication) {
protected String extractIdFromAuthentication(Authentication authentication, boolean clientAuthenticationAllowed) {
if (authentication==null) {
return null;
}
Expand All @@ -55,7 +63,7 @@ protected String extractIdFromAuthentication(Authentication authentication) {
if (authentication instanceof OAuth2Authentication) {
OAuth2Authentication a = (OAuth2Authentication)authentication;
if (a.isClientOnly()) {
return null;
return clientAuthenticationAllowed ? a.getOAuth2Request().getClientId() : null;
} else {
if (a.getUserAuthentication().getPrincipal() instanceof UaaPrincipal) {
return ((UaaPrincipal)a.getUserAuthentication().getPrincipal()).getId();
Expand All @@ -69,4 +77,26 @@ protected String extractIdFromUrl(int pathParameterIndex, String pathInfo) {
return UaaUrlUtils.extractPathVariableFromUrl(pathParameterIndex, pathInfo);
}

public boolean isTokenRevocationForSelf(HttpServletRequest request) {

String pathInfo = UaaUrlUtils.getRequestPath(request);
if (!StringUtils.hasText(pathInfo)) {
return false;
}

String tokenId = extractIdFromUrl(3, pathInfo);
if (tokenId==null) {
return false;
}

String idFromAuth = extractIdFromAuthentication(SecurityContextHolder.getContext().getAuthentication(), true);
if (idFromAuth==null) {
return false;
}

RevocableToken revocableToken = tokenProvisioning.retrieve(tokenId);
String subjectId = revocableToken.getUserId() != null ? revocableToken.getUserId() : revocableToken.getClientId();

return idFromAuth.equals(subjectId);
}
}
74 changes: 64 additions & 10 deletions ...oauth/expression/IsUserSelfCheckTest.java → ...uaa/oauth/expression/IsSelfCheckTest.java
View file
Expand Up @@ -18,12 +18,15 @@
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.security.IsUserSelfCheck;
import org.cloudfoundry.identity.uaa.oauth.token.RevocableToken;
import org.cloudfoundry.identity.uaa.oauth.token.RevocableTokenProvisioning;
import org.cloudfoundry.identity.uaa.security.IsSelfCheck;
import org.cloudfoundry.identity.uaa.util.UaaStringUtils;
import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;
import org.springframework.mock.web.MockHttpServletRequest;
import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
Expand All @@ -38,14 +41,16 @@
import java.util.List;

import static org.junit.Assert.*;
import static org.mockito.Mockito.when;

public class IsUserSelfCheckTest {
public class IsSelfCheckTest {

private IsUserSelfCheck bean;
private IsSelfCheck bean;
private UaaAuthentication authentication;
private String id;
private MockHttpServletRequest request;
private UaaPrincipal principal;
private RevocableTokenProvisioning tokenProvisioning;

@Before
public void getBean() {
Expand All @@ -54,7 +59,8 @@ public void getBean() {
request.setRemoteAddr("127.0.0.1");
principal = new UaaPrincipal(id, "username","username@email.org", OriginKeys.UAA, null, IdentityZoneHolder.get().getId());
authentication = new UaaAuthentication(principal, Collections.<GrantedAuthority>emptyList(), new UaaAuthenticationDetails(request));
bean = new IsUserSelfCheck();
tokenProvisioning = Mockito.mock(RevocableTokenProvisioning.class);
bean = new IsSelfCheck(tokenProvisioning);
}

@After
Expand All @@ -66,14 +72,14 @@ public void clearContext() {
public void testSelfCheckLastUaaAuth() {
SecurityContextHolder.getContext().setAuthentication(authentication);
request.setPathInfo("/Users/"+id);
assertTrue(bean.isSelf(request, 1));
assertTrue(bean.isUserSelf(request, 1));
}

@Test
public void testSelfCheckSecondUaaAuth() {
SecurityContextHolder.getContext().setAuthentication(authentication);
request.setPathInfo("/Users/" + id + "/verify");
assertTrue(bean.isSelf(request,1));
assertTrue(bean.isUserSelf(request,1));
}

@Test
Expand All @@ -88,10 +94,10 @@ public void testSelfCheck_TokenAuth() {
SecurityContextHolder.getContext().setAuthentication(new OAuth2Authentication(authorizationRequest.createOAuth2Request(), userAuthentication));

request.setPathInfo("/Users/" + id + "/verify");
assertTrue(bean.isSelf(request, 1));
assertTrue(bean.isUserSelf(request, 1));

request.setPathInfo("/Users/"+id);
assertTrue(bean.isSelf(request, 1));
assertTrue(bean.isUserSelf(request, 1));
}

@Test
Expand All @@ -106,10 +112,58 @@ public void testSelfCheck_Token_ClientAuth_Fails() {
SecurityContextHolder.getContext().setAuthentication(new OAuth2Authentication(authorizationRequest.createOAuth2Request(), userAuthentication));

request.setPathInfo("/Users/" + id + "/verify");
assertFalse(bean.isSelf(request, 1));
assertFalse(bean.isUserSelf(request, 1));

request.setPathInfo("/Users/"+id);
assertFalse(bean.isSelf(request, 1));
assertFalse(bean.isUserSelf(request, 1));
}

@Test
public void testSelfUserToken() throws Exception {
RevocableToken revocableToken = new RevocableToken();
revocableToken.setUserId(id);

String tokenId = "my-token-id";
when(tokenProvisioning.retrieve(tokenId)).thenReturn(revocableToken);

SecurityContextHolder.getContext().setAuthentication(authentication);
request.setPathInfo("/oauth/token/revoke/" + tokenId);

assertTrue(bean.isTokenRevocationForSelf(request));
}

@Test
public void testSelfClientToken() throws Exception {
BaseClientDetails client = new BaseClientDetails();
String clientId = "admin";
List<SimpleGrantedAuthority> authorities = new LinkedList<>();
authorities.add(new SimpleGrantedAuthority("zones." + IdentityZoneHolder.get().getId() + ".admin"));
client.setAuthorities(authorities);
AuthorizationRequest authorizationRequest = new AuthorizationRequest(clientId, UaaStringUtils.getStringsFromAuthorities(authorities));
authorizationRequest.setResourceIdsAndAuthoritiesFromClientDetails(client);
SecurityContextHolder.getContext().setAuthentication(new OAuth2Authentication(authorizationRequest.createOAuth2Request(), null));

RevocableToken revocableToken = new RevocableToken();
revocableToken.setClientId(clientId);

String tokenId = "my-token-id";
when(tokenProvisioning.retrieve(tokenId)).thenReturn(revocableToken);
request.setPathInfo("/oauth/token/revoke/" + tokenId);

assertTrue(bean.isTokenRevocationForSelf(request));
}

@Test
public void testNotSelfToken() throws Exception {
RevocableToken revocableToken = new RevocableToken();
revocableToken.setUserId("other_user_id");

String tokenId = "my-token-id";
when(tokenProvisioning.retrieve(tokenId)).thenReturn(revocableToken);

SecurityContextHolder.getContext().setAuthentication(authentication);
request.setPathInfo("/oauth/token/revoke/" + tokenId);

assertFalse(bean.isTokenRevocationForSelf(request));
}
}
1 change: 1 addition & 0 deletions uaa/src/main/webapp/WEB-INF/spring/oauth-endpoints.xml
View file
Expand Up @@ -55,6 +55,7 @@
xmlns="http://www.springframework.org/schema/security" use-expressions="true">
<intercept-url pattern="/oauth/token/revoke/client/**" access="#oauth2.hasScope('uaa.admin')" />
<intercept-url pattern="/oauth/token/revoke/user/**" access="#oauth2.hasScope('uaa.admin')" />
<intercept-url pattern="/**" access="#oauth2.hasScope('tokens.revoke') or @self.isTokenRevocationForSelf(request)" />
<custom-filter ref="resourceAgnosticAuthenticationFilter" position="PRE_AUTH_FILTER" />
<access-denied-handler ref="oauthAccessDeniedHandler" />
<expression-handler ref="oauthWebExpressionHandler" />
Expand Down
8 changes: 5 additions & 3 deletions uaa/src/main/webapp/WEB-INF/spring/scim-endpoints.xml
View file
Expand Up @@ -196,16 +196,18 @@
<csrf disabled="true"/>
</http>

<bean name="self" class="org.cloudfoundry.identity.uaa.security.IsUserSelfCheck"/>
<bean name="self" class="org.cloudfoundry.identity.uaa.security.IsSelfCheck">
<constructor-arg name="tokenProvisioning" ref="revocableTokenProvisioning"/>
</bean>

<http name="scimUsers" pattern="/Users/**" create-session="stateless" authentication-manager-ref="emptyAuthenticationManager"
entry-point-ref="oauthAuthenticationEntryPoint"
xmlns="http://www.springframework.org/schema/security" use-expressions="true">
<intercept-url pattern="/Users/*/verify-link" access="#oauth2.hasAnyScope('scim.create') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="GET" />
<intercept-url pattern="/Users/*/verify" access="#oauth2.hasAnyScope('scim.write','scim.create') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="GET" />
<intercept-url pattern="/Users/**" access="#oauth2.hasAnyScope('scim.read') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin') or @self.isSelf(request,1)" method="GET" /> <!-- add self logic -->
<intercept-url pattern="/Users/**" access="#oauth2.hasAnyScope('scim.read') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin') or @self.isUserSelf(request,1)" method="GET" /> <!-- add self logic -->
<intercept-url pattern="/Users/*" access="#oauth2.hasAnyScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="DELETE" />
<intercept-url pattern="/Users/*" access="#oauth2.hasAnyScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin') or @self.isSelf(request,1)" method="PUT" /> <!-- add self logic -->
<intercept-url pattern="/Users/*" access="#oauth2.hasAnyScope('scim.write') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin') or @self.isUserSelf(request,1)" method="PUT" /> <!-- add self logic -->
<intercept-url pattern="/Users" access="#oauth2.hasAnyScope('scim.write','scim.create') or #oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')" method="POST" />
<intercept-url pattern="/**" access="ROLE_NONEXISTENT" />
<expression-handler ref="oauthWebExpressionHandler" />
Expand Down

0 comments on commit 39b4767

Please sign in to comment.