Skip to content

Commit

Permalink
Add code improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
@kalanika
kalanika committed Oct 22, 2021
1 parent b951216 commit eb59bee
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 42 deletions.
4 changes: 2 additions & 2 deletions component/org.wso2.carbon.identity.dpop/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@
org.wso2.carbon.identity.oauth2.*;
version="${identity.inbound.auth.oauth.imp.pkg.version}",
org.wso2.carbon.identity.auth.service.*;
version="${identity.carbon.auth.rest.version}",
version="${identity.carbon.auth.rest.imp.pkg.version}",
org.wso2.carbon.identity.base;
version="${carbon.identity.package.import.version.range}",
org.wso2.carbon.identity.core.*;
Expand All @@ -135,7 +135,7 @@
version="${org.wso2.carbon.database.utils.version.range}",
org.apache.axiom.om.*; version="${axiom.osgi.version.range}",
org.wso2.carbon.idp.mgt;
version="${carbon.identity.version}",
version="${carbon.identity.framework.imp.pkg.version.range}",
org.wso2.carbon.context.*; version="${carbon.kernel.imp.pkg.version.range}",
net.minidev.json.*; version="${net.minidev.json.imp.pkg.version.range}"
</Import-Package>
Expand Down
9 changes: 6 additions & 3 deletions ...bon.identity.dpop/src/main/java/org/wso2/carbon/identity/dpop/constant/DPoPConstants.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
public class DPoPConstants {

public static final String OAUTH_CONFIG_ELEMENT = "OAuth";
public static final String DPOP_CONFIG_ELEMENT = "DPoPConfig";
public static final String DPOP_CONFIG_ELEMENT = "DPoP";
public static final String DPOP_ENABLED = "Enable";
public static final String HEADER_VALIDITY = "OAuth.DPoPConfig.HeaderValidity";
public static final String HEADER_VALIDITY = "OAuth.DPoP.HeaderValidity";
public static final int DEFAULT_HEADER_VALIDITY = 60000;
public static final String DPOP_ISSUED_AT = "iat";
public static final String DPOP_HTTP_URI = "htu";
Expand All @@ -35,6 +35,8 @@ public class DPoPConstants {
public static final String DPOP_TOKEN_TYPE = "DPoP";
public static final String INVALID_DPOP_PROOF = "invalid_dpop_proof";
public static final String INVALID_DPOP_ERROR = "Invalid DPoP Proof";
public static final String INVALID_CLIENT = "invalid_client";
public static final String INVALID_CLIENT_ERROR = "Invalid Client";
public static final String ECDSA_ENCRYPTION = "EC";
public static final String RSA_ENCRYPTION = "RSA";
public static final String HTTP_METHOD ="httpMethod";
Expand All @@ -54,6 +56,7 @@ public static class SQLQueries {
public static final String RETRIEVE_TOKEN_BINDING_BY_REFRESH_TOKEN =
"SELECT BINDING.TOKEN_BINDING_TYPE,BINDING.TOKEN_BINDING_VALUE,BINDING.TOKEN_BINDING_REF " +
"FROM IDN_OAUTH2_ACCESS_TOKEN TOKEN LEFT JOIN IDN_OAUTH2_TOKEN_BINDING BINDING ON " +
"TOKEN.TOKEN_ID=BINDING.TOKEN_ID WHERE TOKEN.REFRESH_TOKEN = ?";
"TOKEN.TOKEN_ID=BINDING.TOKEN_ID WHERE TOKEN.REFRESH_TOKEN = ? " +
"AND BINDING.TOKEN_BINDING_TYPE = ?";
}
}
6 changes: 5 additions & 1 deletion ...dentity.dpop/src/main/java/org/wso2/carbon/identity/dpop/dao/DPoPTokenManagerDAOImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,11 @@ private TokenBinding getBindingFromRefreshToken(String refreshToken,boolean isTo

return tokenBinding;
},
preparedStatement -> preparedStatement.setString(1, finalRefreshToken));
preparedStatement -> {
int parameterIndex = 0;
preparedStatement.setString(++parameterIndex, finalRefreshToken);
preparedStatement.setString(++parameterIndex, DPoPConstants.DPOP_TOKEN_TYPE);
});

return tokenBindingList.isEmpty() ? null : tokenBindingList.get(0);
} catch (DataAccessException e) {
Expand Down
70 changes: 40 additions & 30 deletions ...rc/main/java/org/wso2/carbon/identity/dpop/listener/OauthDPoPInterceptorHandlerProxy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public void onPreTokenIssue(OAuth2AccessTokenReqDTO tokenReqDTO, OAuthTokenReqMe
}
}
} catch (InvalidOAuthClientException e) {
throw new IdentityOAuth2ClientException("invalid_client", "Invalid Client");
throw new IdentityOAuth2ClientException(DPoPConstants.INVALID_CLIENT, DPoPConstants.INVALID_CLIENT_ERROR);
}
}

Expand All @@ -100,39 +100,49 @@ public void onPreTokenRenewal(OAuth2AccessTokenReqDTO tokenReqDTO, OAuthTokenReq
log.debug(String.format("DPoP proxy intercepted the token renewal request from the client : %s.",
consumerKey));
}
try {
String tokenBindingType = DPoPHeaderValidator.getApplicationBindingType(tokenReqDTO.getClientId());
TokenBinding tokenBinding =
tokenBindingTypeManagerDao.getTokenBinding(tokenReqDTO.getRefreshToken(), OAuth2Util.isHashEnabled());
if (tokenBinding != null) {
if (!DPoPConstants.DPOP_TOKEN_TYPE.equals(tokenBindingType)) {
if (log.isDebugEnabled()) {
log.debug(String.format("DPoP based token binding is not enabled for the " +
"application Id : %s.", consumerKey));
}
throw new IdentityOAuth2ClientException(DPoPConstants.INVALID_CLIENT,
DPoPConstants.INVALID_CLIENT_ERROR);
}

// Check if the Refresh token is of DPoP type.
boolean isDPoPBinding = false;
TokenBinding tokenBinding =
tokenBindingTypeManagerDao.getTokenBinding(tokenReqDTO.getRefreshToken(), OAuth2Util.isHashEnabled());
if (tokenBinding != null &&
StringUtils.equalsIgnoreCase(DPoPConstants.DPOP_TOKEN_TYPE, tokenBinding.getBindingType())) {
isDPoPBinding = true;
}

String dPoPProof = DPoPHeaderValidator.getDPoPHeader(tokReqMsgCtx);
if (isDPoPBinding && StringUtils.isNotBlank(dPoPProof)) {
String dPoPProof = DPoPHeaderValidator.getDPoPHeader(tokReqMsgCtx);
if (StringUtils.isBlank(dPoPProof)) {
if (log.isDebugEnabled()) {
log.debug(String.format("Renewal request received without the DPoP proof from the " +
"application Id: %s.", consumerKey));
}
throw new IdentityOAuth2ClientException(DPoPConstants.INVALID_DPOP_PROOF,
"DPoP proof is required.");
}

if (!DPoPHeaderValidator.isValidDPoP(dPoPProof, tokenReqDTO, tokReqMsgCtx)) {
if (log.isDebugEnabled()) {
log.debug(String.format("DPoP proof validation failed for the application Id : %s.", consumerKey));
if (!DPoPHeaderValidator.isValidDPoP(dPoPProof, tokenReqDTO, tokReqMsgCtx)) {
if (log.isDebugEnabled()) {
log.debug(String.format("DPoP proof validation failed for the application Id : %s.",
consumerKey));
}
throw new IdentityOAuth2ClientException(DPoPConstants.INVALID_DPOP_PROOF,
DPoPConstants.INVALID_DPOP_ERROR);
}
if (!tokReqMsgCtx.getTokenBinding().getBindingValue().equalsIgnoreCase(tokenBinding.getBindingValue())) {
if (log.isDebugEnabled()) {
log.debug("DPoP proof thumbprint value of the public key is not equal to binding value from" +
" the refresh token.");
}
throw new IdentityOAuth2ClientException(DPoPConstants.INVALID_DPOP_PROOF,
DPoPConstants.INVALID_DPOP_ERROR);
}
throw new IdentityOAuth2ClientException(DPoPConstants.INVALID_DPOP_PROOF,
DPoPConstants.INVALID_DPOP_ERROR);
}
if (!tokReqMsgCtx.getTokenBinding().getBindingValue().equalsIgnoreCase(tokenBinding.getBindingValue())) {
log.error("DPoP proof thumbprint value of the public key is not equal to binding value from" +
" the refresh token.");
throw new IdentityOAuth2ClientException(DPoPConstants.INVALID_DPOP_PROOF,
DPoPConstants.INVALID_DPOP_ERROR);
}
} else if (isDPoPBinding) {
if (log.isDebugEnabled()) {
log.debug(String.format("Renewal request received without the DPoP proof from the application Id: %s.",
consumerKey));
}

throw new IdentityOAuth2ClientException(DPoPConstants.INVALID_DPOP_PROOF, "DPoP proof is required.");
} catch (InvalidOAuthClientException e) {
throw new IdentityOAuth2ClientException(DPoPConstants.INVALID_CLIENT, DPoPConstants.INVALID_CLIENT_ERROR);
}
}

Expand Down
14 changes: 12 additions & 2 deletions ...tity.dpop/src/main/java/org/wso2/carbon/identity/dpop/validators/DPoPHeaderValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import java.sql.Timestamp;
import java.text.ParseException;
import java.util.Date;

import javax.servlet.http.HttpServletRequest;

/**
Expand Down Expand Up @@ -261,8 +262,17 @@ private static boolean checkHTTPURI(String httpUrl, JWTClaimsSet jwtClaimsSet) t
private static int getDPoPValidityPeriod() {

String validityPeriod = IdentityUtil.getProperty(DPoPConstants.HEADER_VALIDITY);
return StringUtils.isNotBlank(validityPeriod) ? Integer.parseInt(validityPeriod.trim()) * 1000
: DPoPConstants.DEFAULT_HEADER_VALIDITY;
if (StringUtils.isNotBlank(validityPeriod)) {
if (Integer.parseInt(validityPeriod.trim()) < 0) {
if (log.isDebugEnabled()) {
log.debug("Configured dpop validity period is set to a negative value.Hence the default validity " +
"period will be used.");
}
return DPoPConstants.DEFAULT_HEADER_VALIDITY;
}
return Integer.parseInt(validityPeriod.trim()) * 1000;
}
return DPoPConstants.DEFAULT_HEADER_VALIDITY;
}

private static void setCnFValue(OAuthTokenReqMessageContext tokReqMsgCtx, String tokenBindingValue) {
Expand Down
2 changes: 1 addition & 1 deletion ...o2/carbon/identity/oauth2/token/handler/clientauth/mutualtls/utils/MutualTLSUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void testGetThumbPrint() throws Exception {
X509Certificate Cert = (X509Certificate) factory.generateCertificate(
new ByteArrayInputStream(DatatypeConverter.parseBase64Binary(CERTIFICATE_CONTENT)));
assertEquals(MutualTLSUtil.getThumbPrint(Cert, null),
"OTE2OWI4MzQ0MTQ5ZDMzMTk3ZmI2NjNjOGYyNjZhNTZhYzgxZWU5Zg");
"YTJkZTg5OGQ3NWUwMTQ2N2UwYTcwMGE1ZTFmMTcyMjE5ZGUwMDBiMDE2ZWVhOWI0NjY1OWQ4YTZlZjQ3YzJmMQ");

}

Expand Down
8 changes: 5 additions & 3 deletions pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,13 +388,15 @@

<properties>

<carbon.identity.version>5.20.66</carbon.identity.version>
<identity.inbound.auth.oauth.version>6.2.18</identity.inbound.auth.oauth.version>
<carbon.identity.version>5.20.234</carbon.identity.version>
<carbon.identity.framework.imp.pkg.version.range>[5.17.5, 6.0.0)
</carbon.identity.framework.imp.pkg.version.range>
<identity.inbound.auth.oauth.version>6.7.70</identity.inbound.auth.oauth.version>
<org.wso2.carbon.database.utils.version>2.0.7</org.wso2.carbon.database.utils.version>
<org.wso2.carbon.database.utils.version.range>[2.0.0,2.1.0)</org.wso2.carbon.database.utils.version.range>

<identity.inbound.auth.oauth.imp.pkg.version>[6.2.18,7.0.0)</identity.inbound.auth.oauth.imp.pkg.version>
<identity.carbon.auth.rest.version>1.4.0</identity.carbon.auth.rest.version>
<identity.carbon.auth.rest.version>1.4.47</identity.carbon.auth.rest.version>
<identity.carbon.auth.rest.imp.pkg.version>[1.0.0,2.0.0)</identity.carbon.auth.rest.imp.pkg.version>

<carbon.kernel.version>4.6.0</carbon.kernel.version>
Expand Down

0 comments on commit eb59bee

Please sign in to comment.