diff --git a/openid-connect-server/src/main/java/org/mitre/oauth2/web/RevocationEndpoint.java b/openid-connect-server/src/main/java/org/mitre/oauth2/web/RevocationEndpoint.java index cc9d04ad3..7c9dab091 100644 --- a/openid-connect-server/src/main/java/org/mitre/oauth2/web/RevocationEndpoint.java +++ b/openid-connect-server/src/main/java/org/mitre/oauth2/web/RevocationEndpoint.java @@ -16,17 +16,24 @@ *******************************************************************************/ package org.mitre.oauth2.web; -import java.security.Principal; +import static org.mitre.oauth2.web.AuthenticationUtilities.ensureOAuthScope; +import java.util.Collection; + +import org.mitre.oauth2.model.ClientDetailsEntity; import org.mitre.oauth2.model.OAuth2AccessTokenEntity; import org.mitre.oauth2.model.OAuth2RefreshTokenEntity; +import org.mitre.oauth2.service.ClientDetailsEntityService; import org.mitre.oauth2.service.OAuth2TokenEntityService; +import org.mitre.oauth2.service.SystemScopeService; import org.mitre.openid.connect.view.HttpCodeView; +import org.mitre.uma.model.ResourceSet; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.Authentication; import org.springframework.security.oauth2.common.exceptions.InvalidTokenException; import org.springframework.security.oauth2.provider.OAuth2Authentication; import org.springframework.security.oauth2.provider.OAuth2Request; @@ -38,7 +45,10 @@ import org.springframework.web.bind.annotation.RequestParam; @Controller public class RevocationEndpoint { @Autowired - OAuth2TokenEntityService tokenServices; + private ClientDetailsEntityService clientService; + + @Autowired + private OAuth2TokenEntityService tokenServices; /** * Logger for this class @@ -49,32 +59,53 @@ public class RevocationEndpoint { @PreAuthorize("hasRole('ROLE_ADMIN') or hasRole('ROLE_CLIENT')") @RequestMapping("/" + URL) - public String revoke(@RequestParam("token") String tokenValue, @RequestParam(value = "token_type_hint", required = false) String tokenType, Principal principal, Model model) { + public String revoke(@RequestParam("token") String tokenValue, @RequestParam(value = "token_type_hint", required = false) String tokenType, Authentication auth, Model model) { // This is the token as passed in from OAuth (in case we need it some day) //OAuth2AccessTokenEntity tok = tokenServices.getAccessToken((OAuth2Authentication) principal); - OAuth2Request authRequest = null; - if (principal instanceof OAuth2Authentication) { - // if the client is acting on its own behalf (the common case), pull out the client authorization request - authRequest = ((OAuth2Authentication) principal).getOAuth2Request(); + ClientDetailsEntity authClient = null; + + if (auth instanceof OAuth2Authentication) { + // the client authenticated with OAuth, do our UMA checks + ensureOAuthScope(auth, SystemScopeService.UMA_PROTECTION_SCOPE); + // get out the client that was issued the access token (not the token being revoked) + OAuth2Authentication o2a = (OAuth2Authentication) auth; + + String authClientId = o2a.getOAuth2Request().getClientId(); + authClient = clientService.loadClientByClientId(authClientId); + + // the owner is the user who authorized the token in the first place + String ownerId = o2a.getUserAuthentication().getName(); + + } else { + // the client authenticated directly, make sure it's got the right access + + String authClientId = auth.getName(); // direct authentication puts the client_id into the authentication's name field + authClient = clientService.loadClientByClientId(authClientId); + } try { // check and handle access tokens first OAuth2AccessTokenEntity accessToken = tokenServices.readAccessToken(tokenValue); - if (authRequest != null) { - // client acting on its own, make sure it owns the token - if (!accessToken.getClient().getClientId().equals(authRequest.getClientId())) { - // trying to revoke a token we don't own, throw a 403 - model.addAttribute(HttpCodeView.CODE, HttpStatus.FORBIDDEN); - return HttpCodeView.VIEWNAME; - } + + // client acting on its own, make sure it owns the token + if (!accessToken.getClient().getClientId().equals(authClient.getClientId())) { + // trying to revoke a token we don't own, throw a 403 + + logger.info("Client " + authClient.getClientId() + " tried to revoke a token owned by " + accessToken.getClient().getClientId()); + + model.addAttribute(HttpCodeView.CODE, HttpStatus.FORBIDDEN); + return HttpCodeView.VIEWNAME; } // if we got this far, we're allowed to do this tokenServices.revokeAccessToken(accessToken); + + logger.debug("Client " + authClient.getClientId() + " revoked access token " + tokenValue); + model.addAttribute(HttpCodeView.CODE, HttpStatus.OK); return HttpCodeView.VIEWNAME; @@ -84,17 +115,21 @@ public class RevocationEndpoint { try { OAuth2RefreshTokenEntity refreshToken = tokenServices.getRefreshToken(tokenValue); - if (authRequest != null) { - // client acting on its own, make sure it owns the token - if (!refreshToken.getClient().getClientId().equals(authRequest.getClientId())) { - // trying to revoke a token we don't own, throw a 403 - model.addAttribute(HttpCodeView.CODE, HttpStatus.FORBIDDEN); - return HttpCodeView.VIEWNAME; - } + // client acting on its own, make sure it owns the token + if (!refreshToken.getClient().getClientId().equals(authClient.getClientId())) { + // trying to revoke a token we don't own, throw a 403 + + logger.info("Client " + authClient.getClientId() + " tried to revoke a token owned by " + refreshToken.getClient().getClientId()); + + model.addAttribute(HttpCodeView.CODE, HttpStatus.FORBIDDEN); + return HttpCodeView.VIEWNAME; } // if we got this far, we're allowed to do this tokenServices.revokeRefreshToken(refreshToken); + + logger.debug("Client " + authClient.getClientId() + " revoked access token " + tokenValue); + model.addAttribute(HttpCodeView.CODE, HttpStatus.OK); return HttpCodeView.VIEWNAME; @@ -102,6 +137,8 @@ public class RevocationEndpoint { // neither token type was found, simply say "OK" and be on our way. + logger.debug("Failed to revoke token " + tokenValue); + model.addAttribute(HttpCodeView.CODE, HttpStatus.OK); return HttpCodeView.VIEWNAME; } diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/web/DataAPI.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/web/DataAPI.java index 39b4584b0..8f2667a3d 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/web/DataAPI.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/web/DataAPI.java @@ -111,12 +111,13 @@ public class DataAPI { } break; case END_OBJECT: - reader.endObject(); break; case END_DOCUMENT: break; } } + + reader.endObject(); return "httpCodeView"; }