-
Notifications
You must be signed in to change notification settings - Fork 374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add FAPI validations for DCR #2178
Merged
janakamarasena
merged 43 commits into
wso2-extensions:master
from
SachiniSiriwardene:master_DCR_Validations
Oct 25, 2023
Merged
Changes from 34 commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
fbfbc9b
add FAPI validations
SachiniSiriwardene 27a8849
Add FAPI based dcr validations and PPID based validations
SachiniSiriwardene 1974e9b
Remove changes
SachiniSiriwardene f83dc46
Remove changes
SachiniSiriwardene 3912da3
remove * imports
SachiniSiriwardene 435809a
Merge branch 'master' of https://github.com/wso2-extensions/identity-…
SachiniSiriwardene 2290f39
add changes for error handling
SachiniSiriwardene 3e5943b
validate encryption algorithm and method
SachiniSiriwardene 2224506
change config name
SachiniSiriwardene 6ee33bb
fix pr comments
SachiniSiriwardene 2190b37
change method signature for checking fapi compliant property
SachiniSiriwardene 2e64b9e
add fapi compliant property to service provider
SachiniSiriwardene 6dafaf8
add unit tests for signature validation
SachiniSiriwardene aacab81
Merge branch 'master' of https://github.com/wso2-extensions/identity-…
SachiniSiriwardene 6b66a41
fix pr comments
SachiniSiriwardene 54a629e
refactor error handling
SachiniSiriwardene 6a3a108
Merge branch 'master' of https://github.com/wso2-extensions/identity-…
SachiniSiriwardene 5f0f8d4
check FAPI validation enabled for DCR
SachiniSiriwardene 1bf041a
refactor code
SachiniSiriwardene 63f8978
refactor code
SachiniSiriwardene 5bfc80f
Merge branch 'master' of https://github.com/wso2-extensions/identity-…
SachiniSiriwardene df951e1
Merge branch 'master' of https://github.com/wso2-extensions/identity-…
SachiniSiriwardene b7afd8d
add changes to send ssa in response
SachiniSiriwardene 1bb49c5
remove unwanted imports
SachiniSiriwardene ed71ba9
Merge branch 'master' of https://github.com/wso2-extensions/identity-…
SachiniSiriwardene b02c765
Merge branch 'master' of https://github.com/wso2-extensions/identity-…
SachiniSiriwardene cdc11bc
add method to filter signature algorithms
SachiniSiriwardene 54bde5b
Merge branch 'master' of https://github.com/wso2-extensions/identity-…
SachiniSiriwardene ab47967
Merge branch 'master' of https://github.com/wso2-extensions/identity-…
SachiniSiriwardene ce77254
add method to filter signature algorithms
SachiniSiriwardene 45be6e1
modify to send correct error code
SachiniSiriwardene 1dd335c
Merge branch 'master' of https://github.com/wso2-extensions/identity-…
SachiniSiriwardene 4df315c
refactor code
SachiniSiriwardene d030cee
Merge branch 'master' of https://github.com/wso2-extensions/identity-…
SachiniSiriwardene a418767
address pr comments
SachiniSiriwardene feb4731
address pr comments
SachiniSiriwardene f6b06f0
change variable name
SachiniSiriwardene fda251e
add pr review changes
SachiniSiriwardene ec5f3e4
refactor code
SachiniSiriwardene 12f4e99
address pr comments
SachiniSiriwardene e4cfcc7
add default subject type if empty
SachiniSiriwardene ed3a3d5
remove null check
SachiniSiriwardene c7a9144
Merge branch 'master' of https://github.com/wso2-extensions/identity-…
SachiniSiriwardene File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
package org.wso2.carbon.identity.oauth.dcr.service; | ||
|
||
import com.google.gson.Gson; | ||
import com.nimbusds.jwt.SignedJWT; | ||
import org.apache.commons.lang.ArrayUtils; | ||
import org.apache.commons.lang.StringUtils; | ||
import org.apache.commons.logging.Log; | ||
|
@@ -34,7 +35,9 @@ | |
import org.wso2.carbon.identity.application.common.util.IdentityApplicationConstants; | ||
import org.wso2.carbon.identity.application.mgt.ApplicationManagementService; | ||
import org.wso2.carbon.identity.application.mgt.ApplicationMgtUtil; | ||
import org.wso2.carbon.identity.core.util.IdentityUtil; | ||
import org.wso2.carbon.identity.oauth.IdentityOAuthAdminException; | ||
import org.wso2.carbon.identity.oauth.IdentityOAuthClientException; | ||
import org.wso2.carbon.identity.oauth.OAuthAdminService; | ||
import org.wso2.carbon.identity.oauth.common.OAuthConstants; | ||
import org.wso2.carbon.identity.oauth.common.exception.InvalidOAuthClientException; | ||
|
@@ -52,11 +55,15 @@ | |
import org.wso2.carbon.identity.oauth.dcr.util.ErrorCodes; | ||
import org.wso2.carbon.identity.oauth.dto.OAuthConsumerAppDTO; | ||
import org.wso2.carbon.identity.oauth2.IdentityOAuth2Exception; | ||
import org.wso2.carbon.identity.oauth2.util.JWTSignatureValidationUtils; | ||
import org.wso2.carbon.identity.oauth2.util.OAuth2Util; | ||
|
||
import java.text.ParseException; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import java.util.HashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.regex.Pattern; | ||
|
||
import static org.wso2.carbon.identity.oauth.Error.INVALID_OAUTH_CLIENT; | ||
|
@@ -74,6 +81,8 @@ public class DCRMService { | |
private static final String GRANT_TYPE_SEPARATOR = " "; | ||
private static final String APP_DISPLAY_NAME = "DisplayName"; | ||
private static Pattern clientIdRegexPattern = null; | ||
private static final String SSA_VALIDATION_JWKS = "OAuth.DCRM.SoftwareStatementJWKS"; | ||
|
||
|
||
/** | ||
* Get OAuth2/OIDC application information with client_id. | ||
|
@@ -292,8 +301,7 @@ public Application updateApplication(ApplicationUpdateRequest updateRequest, Str | |
appDTO.setIdTokenEncryptionMethod(updateRequest.getIdTokenEncryptionMethod()); | ||
} | ||
if (updateRequest.getRequestObjectSignatureAlgorithm() != null) { | ||
appDTO.setRequestObjectSignatureValidationEnabled | ||
janakamarasena marked this conversation as resolved.
Show resolved
Hide resolved
|
||
(updateRequest.isRequireSignedRequestObject()); | ||
appDTO.setRequestObjectSignatureAlgorithm(updateRequest.getRequestObjectSignatureAlgorithm()); | ||
} | ||
if (updateRequest.getTlsClientAuthSubjectDN() != null) { | ||
appDTO.setTlsClientAuthSubjectDN(updateRequest.getTlsClientAuthSubjectDN()); | ||
|
@@ -315,14 +323,18 @@ public Application updateApplication(ApplicationUpdateRequest updateRequest, Str | |
appDTO.setPkceSupportPlain(updateRequest.isExtPkceSupportPlain()); | ||
appDTO.setBypassClientCredentials(updateRequest.isExtPublicClient()); | ||
oAuthAdminService.updateConsumerApplication(appDTO); | ||
} catch (IdentityOAuthClientException e) { | ||
throw new DCRMClientException(DCRMConstants.ErrorCodes.INVALID_CLIENT_METADATA, e.getMessage(), e); | ||
} catch (IdentityOAuthAdminException e) { | ||
throw DCRMUtils.generateServerException( | ||
DCRMConstants.ErrorMessages.FAILED_TO_UPDATE_APPLICATION, clientId, e); | ||
} | ||
OAuthConsumerAppDTO oAuthConsumerAppDTO = getApplicationById(clientId); | ||
// Setting the jwksURI to be sent in the response. | ||
oAuthConsumerAppDTO.setJwksURI(updateRequest.getJwksURI()); | ||
return buildResponse(oAuthConsumerAppDTO); | ||
Application application = buildResponse(oAuthConsumerAppDTO); | ||
application.setSoftwareStatement(updateRequest.getSoftwareStatement()); | ||
return application; | ||
} | ||
|
||
/** | ||
|
@@ -414,6 +426,15 @@ private Application createOAuthApplication(ApplicationRegistrationRequest regist | |
throw DCRMUtils.generateClientException(DCRMConstants.ErrorMessages.CONFLICT_EXISTING_CLIENT_ID, | ||
registrationRequest.getConsumerKey()); | ||
} | ||
// Validate software statement assertion signature. | ||
if (StringUtils.isNotEmpty(registrationRequest.getSoftwareStatement())) { | ||
try { | ||
validateSSASignature(registrationRequest.getSoftwareStatement()); | ||
} catch (IdentityOAuth2Exception e) { | ||
throw new DCRMClientException(DCRMConstants.ErrorCodes.INVALID_SOFTWARE_STATEMENT, | ||
janakamarasena marked this conversation as resolved.
Show resolved
Hide resolved
|
||
DCRMConstants.ErrorMessages.SIGNATURE_VALIDATION_FAILED.getMessage(), e); | ||
} | ||
} | ||
|
||
// Create a service provider. | ||
ServiceProvider serviceProvider = createServiceProvider(applicationOwner, tenantDomain, spName, templateName, | ||
|
@@ -448,7 +469,9 @@ private Application createOAuthApplication(ApplicationRegistrationRequest regist | |
deleteApplication(createdApp.getOauthConsumerKey()); | ||
throw ex; | ||
} | ||
return buildResponse(createdApp); | ||
Application application = buildResponse(createdApp); | ||
application.setSoftwareStatement(registrationRequest.getSoftwareStatement()); | ||
return application; | ||
} | ||
|
||
private Application buildResponse(OAuthConsumerAppDTO createdApp) { | ||
|
@@ -603,6 +626,8 @@ private OAuthConsumerAppDTO createOAuthApp(ApplicationRegistrationRequest regist | |
OAuthConsumerAppDTO createdApp; | ||
try { | ||
createdApp = oAuthAdminService.registerAndRetrieveOAuthApplicationData(oAuthConsumerApp); | ||
} catch (IdentityOAuthClientException e) { | ||
throw new DCRMClientException(DCRMConstants.ErrorCodes.INVALID_CLIENT_METADATA, e.getMessage(), e); | ||
} catch (IdentityOAuthAdminException e) { | ||
throw DCRMUtils.generateServerException( | ||
DCRMConstants.ErrorMessages.FAILED_TO_REGISTER_APPLICATION, spName, e); | ||
|
@@ -630,6 +655,19 @@ private ServiceProvider createServiceProvider(String applicationOwner, String te | |
sp.setDescription("Service Provider for application " + spName); | ||
sp.setManagementApp(isManagementApp); | ||
|
||
Map<String, Object> spProperties = new HashMap<>(); | ||
boolean enableFAPI = Boolean.parseBoolean(IdentityUtil.getProperty(OAuthConstants.ENABLE_FAPI)); | ||
if (enableFAPI) { | ||
boolean enableFAPIDCR = Boolean.parseBoolean(IdentityUtil.getProperty( | ||
OAuthConstants.ENABLE_FAPI_VALIDATION)); | ||
if (enableFAPIDCR) { | ||
// Add FAPI conformant application nad isThirdParty property to the service provider. | ||
spProperties.put(OAuthConstants.IS_FAPI_CONFORMANT_APP, true); | ||
} | ||
} | ||
spProperties.put(OAuthConstants.IS_THIRD_PARTY_APP, true); | ||
addSPProperties(spProperties, sp); | ||
|
||
createServiceProvider(sp, tenantDomain, applicationOwner, templateName); | ||
|
||
// Get created service provider. | ||
|
@@ -925,4 +963,48 @@ private ServiceProvider cloneServiceProvider(ServiceProvider serviceProvider) { | |
ServiceProvider clonedServiceProvider = gson.fromJson(gson.toJson(serviceProvider), ServiceProvider.class); | ||
return clonedServiceProvider; | ||
} | ||
|
||
/** | ||
* Validate SSA signature using jwks_uri. | ||
* @param softwareStatement Software Statement | ||
* @throws DCRMClientException | ||
* @throws IdentityOAuth2Exception | ||
*/ | ||
private void validateSSASignature(String softwareStatement) throws DCRMClientException, IdentityOAuth2Exception { | ||
|
||
SignedJWT signedJWT = null; | ||
try { | ||
signedJWT = SignedJWT.parse(softwareStatement); | ||
} catch (ParseException e) { | ||
throw new DCRMClientException(DCRMConstants.ErrorCodes.INVALID_SOFTWARE_STATEMENT, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. e is missing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed in 6ee33bb |
||
DCRMConstants.ErrorMessages.SIGNATURE_VALIDATION_FAILED.getMessage(), e); | ||
} | ||
String jwksURL = IdentityUtil.getProperty(SSA_VALIDATION_JWKS); | ||
janakamarasena marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (!JWTSignatureValidationUtils.validateUsingJWKSUri(signedJWT, jwksURL)) { | ||
throw new DCRMClientException(DCRMConstants.ErrorCodes.INVALID_SOFTWARE_STATEMENT, | ||
DCRMConstants.ErrorMessages.SIGNATURE_VALIDATION_FAILED.getMessage()); | ||
} | ||
} | ||
|
||
/** | ||
* Add the properties to the service provider. | ||
* @param spProperties Map of property name and values to be added. | ||
* @param serviceProvider ServiceProvider object. | ||
*/ | ||
private void addSPProperties(Map<String, Object> spProperties, ServiceProvider serviceProvider) { | ||
|
||
ServiceProviderProperty[] serviceProviderProperties = serviceProvider.getSpProperties(); | ||
for (Map.Entry<String, Object> entry : spProperties.entrySet()) { | ||
boolean propertyExists = Arrays.stream(serviceProviderProperties) | ||
.anyMatch(property -> property.getName().equals(entry.getKey())); | ||
if (!propertyExists) { | ||
ServiceProviderProperty serviceProviderProperty = new ServiceProviderProperty(); | ||
serviceProviderProperty.setName(entry.getKey()); | ||
serviceProviderProperty.setValue(entry.getValue().toString()); | ||
serviceProviderProperties = (ServiceProviderProperty[]) ArrayUtils.add(serviceProviderProperties, | ||
serviceProviderProperty); | ||
} | ||
} | ||
serviceProvider.setSpProperties(serviceProviderProperties); | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we change the constant name to hint that the fapi validations are enabled for DCRM?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed in a418767