Skip to content
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

[Accelerator 4] Adding consent enforcement and error handling implementation #130

Merged
merged 15 commits into from
Nov 11, 2024

Conversation

Ashi1993
Copy link
Contributor

@Ashi1993 Ashi1993 commented Oct 2, 2024

Adding consent enforcement implementation

Explain in a few lines the purpose of this pull request

Issue link: required

Doc Issue: Optional, link issue from documentation repository

Applicable Labels: Spec, product, version, type (specify requested labels)


Development Checklist

  1. Build complete solution with pull request in place.
  2. Ran checkstyle plugin with pull request in place.
  3. Ran Findbugs plugin with pull request in place.
  4. Ran FindSecurityBugs plugin and verified report.
  5. Formatted code according to WSO2 code style.
  6. Have you verified the PR doesn't commit any keys, passwords, tokens, usernames, or other secrets?
  7. Migration scripts written (if applicable).
  8. Have you followed secure coding standards in WSO2 Secure Engineering Guidelines?

Testing Checklist

  1. Written unit tests.
  2. Verified tests in multiple database environments (if applicable).
  3. Tested with BI enabled (if applicable).

@Ashi1993 Ashi1993 changed the title Adding consent enforcement implementation [Accelerator 4] Adding consent enforcement implementation Oct 15, 2024
@Ashi1993 Ashi1993 changed the title [Accelerator 4] Adding consent enforcement implementation [Accelerator 4] Adding consent enforcement and error handling implementation Oct 24, 2024
@@ -25,7 +25,7 @@
<RequestRouter>org.wso2.financial.services.accelerator.gateway.executor.core.DefaultRequestRouter</RequestRouter>
{% endif %}
<FinancialServicesGatewayExecutors>
{% for type in financial_services.gateway.openbanking_gateway_executors.type %}
{% for type in financial_services.gateway.gateway_executors.type %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{% for type in financial_services.gateway.gateway_executors.type %}

Do we need gateway twice here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1e13ec5

@@ -203,6 +203,7 @@
com.fasterxml.jackson.core;version="${jackson.databinding.version}",
org.owasp.encoder;version="${encoder.wso2.version}"
</Import-Package>
<DynamicImport-Package>*</DynamicImport-Package>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline had to add this to solve dependency not identifying issue.

this.status = ResponseStatus.FOUND;
this.payload = createDefaultErrorObject(errorRedirectURI, error.toString(), errorDescription, state);
this.payload = createDefaultErrorObject(errorURI, error.toString(), errorDescription, state);
}
}

public JSONObject createDefaultErrorObject(URI redirectURI, String errorCode, String errorMessage, String state) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public JSONObject createDefaultErrorObject(URI redirectURI, String errorCode, String errorMessage, String state) {
public JSONObject createDefaultErrorObject(URI redirectURI, String errorCode, String errorDescription, String state) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1e13ec5

username = OAuth2Util.resolveUsernameFromUserId(ConsentExtensionConstants.TENANT_DOMAIN, userID);
}
} catch (UserStoreException e) {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we add a debug log here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1e13ec5

// From date is earlier than To date
return toDate.isAfter(fromDate);
// From date is equal or earlier than To date
return toDate.isEqual(fromDate) || toDate.isAfter(fromDate);
} catch (DateTimeParseException e) {
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we add a debug log here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1e13ec5

return;
}

boolean isValid = (boolean) jsonResponse.get(IS_VALID);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getBoolean() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1e13ec5

JSONArray errorList = new JSONArray();
for (FSExecutorError error : errors) {
JSONObject errorObj = new JSONObject();
errorObj.put("code", error.getCode());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constants?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1e13ec5

return errorList;
}

private boolean isAnyClientErrors(HashSet<String> statusCodes) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private boolean isAnyClientErrors(HashSet<String> statusCodes) {
private boolean isAnyClientErrors(Set<String> statusCodes) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1e13ec5

Comment on lines 174 to 179
for (String statusCode : statusCodes) {
if (statusCode.startsWith("4")) {
return true;
}
}
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stream?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1e13ec5

"consentInformation": {
"key": "value"
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missed EOF

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1e13ec5

@VimukthiRajapaksha VimukthiRajapaksha merged commit e0fc714 into wso2:main Nov 11, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants