-
Notifications
You must be signed in to change notification settings - Fork 541
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 DCR configurations #4977
Add DCR configurations #4977
Conversation
<SoftwareStatementJWKS></SoftwareStatementJWKS> | ||
{% endif %} | ||
{% if oauth.dcr.enable_fapi_validation%} | ||
<EnableFAPIValidation>{{oauth.dcr.enable_fapi_validation}}</EnableFAPIValidation> |
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 rename this to something like. EnableFAPIEnforcement
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
@@ -975,6 +978,12 @@ | |||
--> | |||
<DCRM> | |||
<ApplicationRolePermissionRequiredToView>{{oauth.dcrm.application_role_permission_required_to_view}}</ApplicationRolePermissionRequiredToView> | |||
{% if oauth.dcr.ssa_jkws%} | |||
<SoftwareStatementJWKS></SoftwareStatementJWKS> |
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.
missing templating
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
@@ -975,6 +978,12 @@ | |||
--> | |||
<DCRM> | |||
<ApplicationRolePermissionRequiredToView>{{oauth.dcrm.application_role_permission_required_to_view}}</ApplicationRolePermissionRequiredToView> | |||
{% if oauth.dcr.ssa_jkws%} |
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.
{% if oauth.dcr.ssa_jkws%} | |
{% if oauth.dcr.ssa_jkws is defined %} |
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
…ework into master_DCRConf � Conflicts: � features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/identity.xml.j2 � features/identity-core/org.wso2.carbon.identity.core.server.feature/resources/org.wso2.carbon.identity.core.server.feature.default.json
<SoftwareStatementJWKS>{{oauth.dcr.ssa_jkws}}</SoftwareStatementJWKS> | ||
{% endif %} | ||
{% if oauth.dcr.enable_fapi_validation%} | ||
<EnableFAPIEnforcement>{{oauth.dcr.enable_fapi_validation}}</EnableFAPIEnforcement> |
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.
lets rename this to enable_fapi_enforcement
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.
since we are adding defaults no need to check is defined. Lets remove is defined checks for configs with default values
@@ -240,6 +240,8 @@ | |||
"oauth.oidc.fapi.allowed_client_authentication_methods": ["private_key_jwt", "tls_client_auth"], | |||
"oauth.oidc.fapi.allowed_signature_algorithms": ["PS256", "ES256"], | |||
|
|||
"oauth.oidc.fapi.enable_validation": false, |
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.
let's set this to true by default.
@@ -885,6 +885,13 @@ | |||
<AllowedSignatureAlgorithm>{{algorithm}}</AllowedSignatureAlgorithm> | |||
{% endfor %} | |||
</AllowedSignatureAlgorithms> | |||
{% if oauth.oidc.fapi.enable_validation is defined %} | |||
<EnableFAPIValidation>{{oauth.oidc.fapi.enable_validation}}</EnableFAPIValidation> |
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.
lets move this config to the top of the FAPI configs
PR builder started |
PR builder completed |
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.
Approving the pull request based on the successful pr build https://github.com/wso2/product-is/actions/runs/6491648776
Proposed changes in this pull request
[List all changes you want to add here. If you fixed an issue, please
add a reference to that issue as well.]
When should this PR be merged
[Please describe any preconditions that need to be addressed before we
can merge this pull request.]
Follow up actions
[List any possible follow-up actions here; for instance, testing data
migrations, software that we need to install on staging and production
environments.]
Checklist (for reviewing)
General
Functionality
Code
Tests
Security
Documentation