diff --git a/openid-connect-client/src/main/java/org/mitre/oauth2/introspectingfilter/AuthorizationRequestImpl.java b/openid-connect-client/src/main/java/org/mitre/oauth2/introspectingfilter/AuthorizationRequestImpl.java deleted file mode 100644 index 701b4d8f6..000000000 --- a/openid-connect-client/src/main/java/org/mitre/oauth2/introspectingfilter/AuthorizationRequestImpl.java +++ /dev/null @@ -1,83 +0,0 @@ -package org.mitre.oauth2.introspectingfilter; - -import com.google.gson.JsonElement; -import com.google.gson.JsonObject; -import java.util.Collection; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; -import org.springframework.security.core.GrantedAuthority; -import org.springframework.security.oauth2.provider.AuthorizationRequest; - -public class AuthorizationRequestImpl implements AuthorizationRequest { - - private JsonObject token; - private String clientId; - private Set scopes = null; - - public AuthorizationRequestImpl(JsonObject token) { - this.token = token; - clientId = token.get("client_id").getAsString(); - scopes = new HashSet(); - for (JsonElement e : token.get("scope").getAsJsonArray()) { - scopes.add(e.getAsString()); - } - } - - @Override - public Map getAuthorizationParameters() { - return null; - } - - @Override - public Map getApprovalParameters() { - return null; - } - - @Override - public String getClientId() { - return clientId; - } - - @Override - public Set getScope() { - - return scopes; - } - - @Override - public Set getResourceIds() { - return null; - } - - @Override - public Collection getAuthorities() { - return null; - } - - @Override - public boolean isApproved() { - return true; - } - - @Override - public boolean isDenied() { - return false; - } - - @Override - public String getState() { - return null; - } - - @Override - public String getRedirectUri() { - return null; - } - - @Override - public Set getResponseTypes() { - return null; - } - -} diff --git a/openid-connect-client/src/main/java/org/mitre/oauth2/introspectingfilter/IntrospectingTokenService.java b/openid-connect-client/src/main/java/org/mitre/oauth2/introspectingfilter/IntrospectingTokenService.java index dc77f41b5..41e0c99ed 100644 --- a/openid-connect-client/src/main/java/org/mitre/oauth2/introspectingfilter/IntrospectingTokenService.java +++ b/openid-connect-client/src/main/java/org/mitre/oauth2/introspectingfilter/IntrospectingTokenService.java @@ -1,27 +1,16 @@ package org.mitre.oauth2.introspectingfilter; -import com.google.common.collect.Sets; -import com.google.gson.JsonElement; -import com.google.gson.JsonObject; -import com.google.gson.JsonParser; -import java.text.DateFormat; -import java.text.ParseException; -import java.text.SimpleDateFormat; -import java.util.Collection; import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Set; -import java.util.logging.Level; -import java.util.logging.Logger; + import org.slf4j.LoggerFactory; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; -import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.oauth2.common.OAuth2AccessToken; -import org.springframework.security.oauth2.common.OAuth2RefreshToken; import org.springframework.security.oauth2.provider.AuthorizationRequest; import org.springframework.security.oauth2.provider.OAuth2Authentication; import org.springframework.security.oauth2.provider.token.ResourceServerTokenServices; @@ -31,6 +20,10 @@ import org.springframework.util.MultiValueMap; import org.springframework.web.client.RestClientException; import org.springframework.web.client.RestTemplate; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParser; + public class IntrospectingTokenService implements ResourceServerTokenServices { @@ -87,7 +80,15 @@ public class IntrospectingTokenService implements ResourceServerTokenServices { } private AuthorizationRequest createAuthRequest(final JsonObject token) { - AuthorizationRequest authReq = new AuthorizationRequestImpl(token); + + clientId = token.get("client_id").getAsString(); + Set scopes = new HashSet(); + for (JsonElement e : token.get("scope").getAsJsonArray()) { + scopes.add(e.getAsString()); + } + AuthorizationRequest authReq = new AuthorizationRequest(); + authReq.setScope(scopes); + authReq.setClientId(clientId); return authReq; } diff --git a/openid-connect-server/src/main/java/org/mitre/oauth2/token/ChainedTokenGranter.java b/openid-connect-server/src/main/java/org/mitre/oauth2/token/ChainedTokenGranter.java index f4e81b0b0..3de4fad23 100644 --- a/openid-connect-server/src/main/java/org/mitre/oauth2/token/ChainedTokenGranter.java +++ b/openid-connect-server/src/main/java/org/mitre/oauth2/token/ChainedTokenGranter.java @@ -15,7 +15,6 @@ import org.springframework.security.core.AuthenticationException; import org.springframework.security.oauth2.common.exceptions.InvalidScopeException; import org.springframework.security.oauth2.common.exceptions.InvalidTokenException; import org.springframework.security.oauth2.provider.AuthorizationRequest; -import org.springframework.security.oauth2.provider.AuthorizationRequestManager; import org.springframework.security.oauth2.provider.OAuth2Authentication; import org.springframework.security.oauth2.provider.token.AbstractTokenGranter; import org.springframework.stereotype.Component; @@ -30,9 +29,6 @@ import com.google.common.collect.Sets; public class ChainedTokenGranter extends AbstractTokenGranter { private static final String grantType = "urn:ietf:params:oauth:grant_type:redelegate"; - - @Autowired - private static AuthorizationRequestManager authorizationRequestManager; // keep down-cast versions so we can get to the right queries private OAuth2TokenEntityService tokenServices; @@ -44,7 +40,7 @@ public class ChainedTokenGranter extends AbstractTokenGranter { */ @Autowired public ChainedTokenGranter(OAuth2TokenEntityService tokenServices, ClientDetailsEntityService clientDetailsService) { - super(tokenServices, clientDetailsService, grantType, authorizationRequestManager); + super(tokenServices, clientDetailsService, grantType); this.tokenServices = tokenServices; } @@ -76,21 +72,20 @@ public class ChainedTokenGranter extends AbstractTokenGranter { if (approvedScopes.containsAll(requestedScopes)) { // build an appropriate auth request to hand to the token services layer - AuthorizationRequest outgoingAuthRequest = authorizationRequestManager.createFromExisting(authorizationRequest); - outgoingAuthRequest.setApproved(true); + authorizationRequest.setApproved(true); if (requestedScopes.isEmpty()) { // if there are no scopes, inherit the original scopes from the token - outgoingAuthRequest.setScope(approvedScopes); + authorizationRequest.setScope(approvedScopes); } else { // if scopes were asked for, give only the subset of scopes requested // this allows safe downscoping - outgoingAuthRequest.setScope(Sets.intersection(requestedScopes, approvedScopes)); + authorizationRequest.setScope(Sets.intersection(requestedScopes, approvedScopes)); } // NOTE: don't revoke the existing access token // create a new access token - OAuth2Authentication authentication = new OAuth2Authentication(outgoingAuthRequest, incomingToken.getAuthenticationHolder().getAuthentication().getUserAuthentication()); + OAuth2Authentication authentication = new OAuth2Authentication(authorizationRequest, incomingToken.getAuthenticationHolder().getAuthentication().getUserAuthentication()); return authentication; diff --git a/openid-connect-server/src/main/java/org/mitre/oauth2/token/JwtAssertionTokenGranter.java b/openid-connect-server/src/main/java/org/mitre/oauth2/token/JwtAssertionTokenGranter.java index b3fc7d884..47df3d97e 100644 --- a/openid-connect-server/src/main/java/org/mitre/oauth2/token/JwtAssertionTokenGranter.java +++ b/openid-connect-server/src/main/java/org/mitre/oauth2/token/JwtAssertionTokenGranter.java @@ -18,7 +18,6 @@ import org.springframework.security.oauth2.common.OAuth2AccessToken; import org.springframework.security.oauth2.common.exceptions.InvalidClientException; import org.springframework.security.oauth2.common.exceptions.InvalidTokenException; import org.springframework.security.oauth2.provider.AuthorizationRequest; -import org.springframework.security.oauth2.provider.AuthorizationRequestManager; import org.springframework.security.oauth2.provider.token.AbstractTokenGranter; import org.springframework.stereotype.Component; @@ -46,12 +45,9 @@ public class JwtAssertionTokenGranter extends AbstractTokenGranter { @Autowired private ConfigurationPropertiesBean config; - @Autowired - private static AuthorizationRequestManager authorizationRequestManager; - @Autowired public JwtAssertionTokenGranter(OAuth2TokenEntityService tokenServices, ClientDetailsEntityService clientDetailsService) { - super(tokenServices, clientDetailsService, grantType, authorizationRequestManager); + super(tokenServices, clientDetailsService, grantType); this.tokenServices = tokenServices; } diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/ConnectAuthorizationRequest.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/ConnectAuthorizationRequest.java deleted file mode 100644 index af8537be9..000000000 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/ConnectAuthorizationRequest.java +++ /dev/null @@ -1,195 +0,0 @@ -package org.mitre.openid.connect; - -import java.util.Collection; -import java.util.Map; -import java.util.Set; - -import org.mitre.openid.connect.model.ApprovedSite; -import org.springframework.security.core.GrantedAuthority; -import org.springframework.security.oauth2.provider.AuthorizationRequest; - -import com.google.common.collect.Maps; -import com.google.common.collect.Sets; - -public class ConnectAuthorizationRequest implements AuthorizationRequest { - - //SECOAUTH interface parameters - private Map authorizationParameters; - private Map approvalParameters; - private String clientId; - private Set scope; - private Set resourceIds; - private Collection authorities; - private boolean approved = false; - private String state; - private String redirectUri; - private Set responseTypes; - - //Extra parameters - private ApprovedSite approvedSite; //See issue 230 - - /** - * Default constructor. Initialize maps & sets. - */ - public ConnectAuthorizationRequest() { - authorizationParameters = Maps.newHashMap(); - approvalParameters = Maps.newHashMap(); - scope = Sets.newHashSet(); - resourceIds = Sets.newHashSet(); - authorities = Sets.newHashSet(); - responseTypes = Sets.newHashSet(); - } - - /** - * Constructor. - * - * @param authorizationParameters - * @param approvalParameters - * @param clientId - * @param scope - * @param resourceIds - * @param authorities - * @param approved - * @param state - * @param redirectUri - * @param responseTypes - */ - public ConnectAuthorizationRequest(Map authorizationParameters, Map approvalParameters, String clientId, Set scope, Set resourceIds, - Collection authorities, boolean approved, String state, String redirectUri, Set responseTypes) { - this.authorizationParameters = authorizationParameters; - this.approvalParameters = approvalParameters; - this.clientId = clientId; - this.scope = scope; - this.resourceIds = resourceIds; - this.authorities = authorities; - this.approved = approved; - this.state = state; - this.redirectUri = redirectUri; - this.responseTypes = responseTypes; - } - - @Override - public Map getAuthorizationParameters() { - return authorizationParameters; - } - - @Override - public void setAuthorizationParameters(Map authorizationParameters) { - this.authorizationParameters = authorizationParameters; - } - - @Override - public Map getApprovalParameters() { - return approvalParameters; - } - - @Override - public void setApprovalParameters(Map approvalParameters) { - this.approvalParameters = approvalParameters; - } - - @Override - public String getClientId() { - return clientId; - } - - @Override - public void setClientId(String clientId) { - this.clientId = clientId; - } - - @Override - public Set getScope() { - return scope; - } - - @Override - public void setScope(Set scope) { - this.scope = scope; - } - - @Override - public Set getResourceIds() { - return resourceIds; - } - - @Override - public void setResourceIds(Set resourceIds) { - this.resourceIds = resourceIds; - } - - @Override - public Collection getAuthorities() { - return authorities; - } - - @Override - public void setAuthorities(Collection authorities) { - this.authorities = authorities; - } - - @Override - public boolean isApproved() { - return approved; - } - - @Override - public void setApproved(boolean approved) { - this.approved = approved; - } - - @Override - public boolean isDenied() { - return !approved; - } - - @Override - public void setDenied(boolean denied) { - this.approved = !denied; - } - - @Override - public String getState() { - return state; - } - - @Override - public void setState(String state) { - this.state = state; - } - - @Override - public String getRedirectUri() { - return redirectUri; - } - - @Override - public void setRedirectUri(String redirectUri) { - this.redirectUri = redirectUri; - } - - @Override - public Set getResponseTypes() { - return responseTypes; - } - - @Override - public void setResponseTypes(Set responseTypes) { - this.responseTypes = responseTypes; - } - - /** - * @return the approvedSite - */ - public ApprovedSite getApprovedSite() { - return approvedSite; - } - - /** - * @param approvedSite the approvedSite to set - */ - public void setApprovedSite(ApprovedSite approvedSite) { - this.approvedSite = approvedSite; - } - -} diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/ConnectAuthorizationRequestManager.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/ConnectAuthorizationRequestManager.java index 22c7eea8d..bcf5b990a 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/ConnectAuthorizationRequestManager.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/ConnectAuthorizationRequestManager.java @@ -27,7 +27,6 @@ import org.springframework.security.oauth2.common.util.OAuth2Utils; import org.springframework.security.oauth2.provider.AuthorizationRequest; import org.springframework.security.oauth2.provider.AuthorizationRequestManager; import org.springframework.security.oauth2.provider.ClientDetails; -import org.springframework.security.oauth2.provider.DefaultAuthorizationRequest; import org.springframework.stereotype.Component; import com.google.common.base.Strings; @@ -72,13 +71,21 @@ public class ConnectAuthorizationRequestManager implements AuthorizationRequestM Map parameters = processRequestObject(inputParams); String clientId = parameters.get("client_id"); - if (clientId == null) { - throw new InvalidClientException("A client id must be provided"); + ClientDetails client = null; + + if (clientId != null) { + client = clientDetailsService.loadClientByClientId(clientId); } - ClientDetails client = clientDetailsService.loadClientByClientId(clientId); String requestNonce = parameters.get("nonce"); + AuthorizationRequest request = new AuthorizationRequest(inputParams, Collections. emptyMap(), + inputParams.get(AuthorizationRequest.CLIENT_ID), + OAuth2Utils.parseParameterList(inputParams.get(AuthorizationRequest.SCOPE)), null, + null, false, inputParams.get(AuthorizationRequest.STATE), + inputParams.get(AuthorizationRequest.REDIRECT_URI), + OAuth2Utils.parseParameterList(inputParams.get(AuthorizationRequest.RESPONSE_TYPE))); + //Only process if the user is authenticated. If the user is not authenticated yet, this //code will be called a second time once the user is redirected from the login page back //to the auth endpoint. @@ -91,24 +98,21 @@ public class ConnectAuthorizationRequestManager implements AuthorizationRequestM nonceService.save(nonce); } else { - throw new NonceReuseException(client.getClientId(), requestNonce); + throw new NonceReuseException(client == null ? "unidentified client" : client.getClientId(), requestNonce); } } Set scopes = OAuth2Utils.parseParameterList(parameters.get("scope")); - if ((scopes == null || scopes.isEmpty())) { + if ((scopes == null || scopes.isEmpty()) && client != null) { //TODO: do we want to allow default scoping at all? Set clientScopes = client.getScope(); scopes = clientScopes; } - ConnectAuthorizationRequest request = new ConnectAuthorizationRequest(); - request.setApprovalParameters(parameters); - request.setClientId(clientId); request.setScope(scopes); + return request; - } /** @@ -234,20 +238,4 @@ public class ConnectAuthorizationRequestManager implements AuthorizationRequestM } } - @Override - public AuthorizationRequest createFromExisting(AuthorizationRequest original) { - ConnectAuthorizationRequest copy - = new ConnectAuthorizationRequest(original.getAuthorizationParameters(), original.getApprovalParameters(), - original.getClientId(), original.getScope(), original.getResourceIds(), - original.getAuthorities(),original.isApproved(), original.getState(), - original.getRedirectUri(), original.getResponseTypes()); - - //If original is a ConnectAuthorizationRequest, preserve extra properties - if (original instanceof ConnectAuthorizationRequest) { - copy.setApprovedSite(((ConnectAuthorizationRequest) original).getApprovedSite()); - } - - return copy; - } - } diff --git a/openid-connect-server/src/main/java/org/mitre/openid/connect/token/TofuUserApprovalHandler.java b/openid-connect-server/src/main/java/org/mitre/openid/connect/token/TofuUserApprovalHandler.java index f82b88cc7..6eabfab76 100644 --- a/openid-connect-server/src/main/java/org/mitre/openid/connect/token/TofuUserApprovalHandler.java +++ b/openid-connect-server/src/main/java/org/mitre/openid/connect/token/TofuUserApprovalHandler.java @@ -64,17 +64,7 @@ public class TofuUserApprovalHandler implements UserApprovalHandler { private ClientDetailsService clientDetailsService; - /** - * Check if the user has already stored a positive approval decision for this site; or if the - * site is whitelisted, approve it automatically. - * - * Otherwise, return false so that the user will see the approval page and can make their own decision. - * - * @param authorizationRequest the incoming authorization request - * @param userAuthentication the Principal representing the currently-logged-in user - * - * @return true if the site is approved, false otherwise - */ + @Override public boolean isApproved(AuthorizationRequest authorizationRequest, Authentication userAuthentication) { @@ -92,39 +82,29 @@ public class TofuUserApprovalHandler implements UserApprovalHandler { } } - - /** - * Check whether the requested scope set is a proper subset of the allowed scopes. - * - * @param requestedScopes - * @param allowedScopes - * @return - */ - private boolean scopesMatch(Set requestedScopes, Set allowedScopes) { - - for (String scope : requestedScopes) { - - if (!allowedScopes.contains(scope)) { - return false; //throw new InvalidScopeException("Invalid scope: " + scope, allowedScopes); - } - } - - return true; - } /** - * Pre-process the authorization request during the approval stage, check against whitelist, approved sites, and stuff. + * Check if the user has already stored a positive approval decision for this site; or if the + * site is whitelisted, approve it automatically. + * + * Otherwise the user will be directed to the approval page and can make their own decision. + * + * @param authorizationRequest the incoming authorization request + * @param userAuthentication the Principal representing the currently-logged-in user + * + * @return the updated AuthorizationRequest */ @Override - public AuthorizationRequest updateBeforeApproval(AuthorizationRequest authorizationRequest, Authentication userAuthentication) { + public AuthorizationRequest checkForPreApproval(AuthorizationRequest authorizationRequest, Authentication userAuthentication) { + //First, check database to see if the user identified by the userAuthentication has stored an approval decision //getName may not be filled in? TODO: investigate String userId = userAuthentication.getName(); String clientId = authorizationRequest.getClientId(); - ClientDetails client = clientDetailsService.loadClientByClientId(clientId); //lookup ApprovedSites by userId and clientId + boolean alreadyApproved = false; Collection aps = approvedSiteService.getByClientIdAndUserId(clientId, userId); for (ApprovedSite ap : aps) { @@ -137,28 +117,40 @@ public class TofuUserApprovalHandler implements UserApprovalHandler { ap.setAccessDate(new Date()); approvedSiteService.save(ap); - authorizationRequest.setApproved(true); - - return authorizationRequest; + authorizationRequest.setApproved(true); + alreadyApproved = true; } } } - WhitelistedSite ws = whitelistedSiteService.getByClientId(clientId); - if (ws != null && scopesMatch(authorizationRequest.getScope(), ws.getAllowedScopes())) { - - //Create an approved site - approvedSiteService.createApprovedSite(clientId, userId, null, ws.getAllowedScopes(), ws); - - authorizationRequest.setApproved(true); - - return authorizationRequest; + if (!alreadyApproved) { + WhitelistedSite ws = whitelistedSiteService.getByClientId(clientId); + if (ws != null && scopesMatch(authorizationRequest.getScope(), ws.getAllowedScopes())) { + + //Create an approved site + approvedSiteService.createApprovedSite(clientId, userId, null, ws.getAllowedScopes(), ws); + authorizationRequest.setApproved(true); + } } + return authorizationRequest; + + } + + + @Override + public AuthorizationRequest updateAfterApproval(AuthorizationRequest authorizationRequest, Authentication userAuthentication) { + + String userId = userAuthentication.getName(); + String clientId = authorizationRequest.getClientId(); + ClientDetails client = clientDetailsService.loadClientByClientId(clientId); + // This must be re-parsed here because SECOAUTH forces us to call things in a strange order boolean approved = Boolean.parseBoolean(authorizationRequest.getApprovalParameters().get("user_oauth_approval")); - if (approved && !authorizationRequest.getApprovalParameters().isEmpty()) { + if (approved) { + + authorizationRequest.setApproved(true); // process scopes from user input Set allowedScopes = Sets.newHashSet(); @@ -200,12 +192,28 @@ public class TofuUserApprovalHandler implements UserApprovalHandler { approvedSiteService.createApprovedSite(clientId, userId, timeout, allowedScopes, null); } - // TODO: should we set approved here? It gets called later via the isApproved method in this class... - - return authorizationRequest; } return authorizationRequest; } + /** + * Check whether the requested scope set is a proper subset of the allowed scopes. + * + * @param requestedScopes + * @param allowedScopes + * @return + */ + private boolean scopesMatch(Set requestedScopes, Set allowedScopes) { + + for (String scope : requestedScopes) { + + if (!allowedScopes.contains(scope)) { + return false; //throw new InvalidScopeException("Invalid scope: " + scope, allowedScopes); + } + } + + return true; + } + } diff --git a/openid-connect-server/src/main/webapp/WEB-INF/server-config.xml b/openid-connect-server/src/main/webapp/WEB-INF/server-config.xml index 300fabb26..03ee04b2b 100644 --- a/openid-connect-server/src/main/webapp/WEB-INF/server-config.xml +++ b/openid-connect-server/src/main/webapp/WEB-INF/server-config.xml @@ -13,7 +13,7 @@ http://www.springframework.org/schema/util http://www.springframework.org/schema/util/spring-util-3.1.xsd"> - + diff --git a/spring-security-oauth b/spring-security-oauth index a063a7e0f..2c0d469e5 160000 --- a/spring-security-oauth +++ b/spring-security-oauth @@ -1 +1 @@ -Subproject commit a063a7e0f2e622d93f5facf474e9c1d0c8e37603 +Subproject commit 2c0d469e50982baf15f1202561f00d54baa36c4e