feat: Enables CORS and JWT configuration for WebApplications in CspApplication tag #973
Conversation
AshokThangavel
requested review from
isc-dchui,
isc-egabhart,
isc-eneil,
isc-jili,
isc-jlechtne,
isc-kiyer,
isc-pbarton and
isc-tleavitt
as code owners
|
(Unfortunately) CSPApplication has been deprecated in favor of WebApplication and CPFMerge, so we're really not looking to make any improvements to it. I'll let @isc-tleavitt and @isc-kiyer weigh in if they believe differently though. |
|
Hi @isc-dchui |
…corrected unittest classnames.
@AshokThangavel Thanks for the PR! Please remove any changes to CSPApplication. Since it is deprecated, we want to encourage moving to WebApplication. For tests added, please add them to WebApplication instead of CSPApplication. |
…d update CORS/JWT configuration
|
Hi @isc-kiyer |
isc-kiyer
left a comment
isc-kiyer
left a comment
There was a problem hiding this comment.
@AshokThangavel a few more comments (mostly regarding naming and removing redundancies)!
| JWTAuthEnabled="1" | ||
| JWTRefreshTokenTimeout="900" | ||
| CorsHeadersList="Access-Control-Allow-Origin, Access-Control-Allow-Methods, Access-Control-Allow-Headers, Access-Control-Allow-Credentials, Access-Control-Max-Age, Access-Control-Expose-Headers, Access-Control-Request-Method, Access-Control-Request-Headers" | ||
| PasswordAuthEnabled="1" |
There was a problem hiding this comment.
For WebApplication, these properties for auth are all encapsulated as part of AutheEnabled and not separate properties.
There was a problem hiding this comment.
@isc-kiyer
Could you please provide an example of how the authentication settings are represented when they are encapsulated under the AuthEnabled attribute in the element?
A sample XML snippet would be very helpful.
Thank you!
| CorsAllowlist="https://pm.intersystems.com" | ||
| CorsCredentialsAllowed="1" | ||
| CorsHeadersList="Access-Control-Allow-Origin, Access-Control-Allow-Methods, Access-Control-Allow-Headers, Access-Control-Allow-Credentials, Access-Control-Max-Age, Access-Control-Expose-Headers, Origin, Access-Control-Request-Method, Access-Control-Request-Headers" | ||
| PasswordAuthEnabled="1" Recurse="1" UnauthenticatedEnabled="0" UseCookies="2"/> |
There was a problem hiding this comment.
For WebApplication, these properties for auth are all encapsulated as part of AutheEnabled and not separate properties.
| JWTAccessTokenTimeout="60" | ||
| JWTAuthEnabled="1" | ||
| JWTRefreshTokenTimeout="900" | ||
| PasswordAuthEnabled="1" Recurse="1" UnauthenticatedEnabled="0" UseCookies="2"/> |
There was a problem hiding this comment.
For WebApplication, these properties for auth are all encapsulated as part of AutheEnabled and not separate properties.
| @@ -0,0 +1,72 @@ | |||
| /// This class validates that CORS headers and allowed origins are configured correctly, | |||
| /// and that JWT authentication is properly set up in the <b><WebApplication></b> configuration section. | |||
| Class Test.PM.Integration.ConfigCorsAndJWTInWebAppTest Extends Test.PM.Integration.Base | |||
There was a problem hiding this comment.
This test class captures the behavior so I don't think the unit tests are needed. Furthermore, in general, if unit testing a single feature, we prefer to keep tests in a single class and have multiple methods rather than splitting it across classes..
Lastly, I would name this Test.PM.Integration.ResourceProcessor.WebApplication so any other WebApplication related tests can be added here in future.
There was a problem hiding this comment.
Thank you for the feedback.
I’ve made the changes based on your suggestions:
- Renamed the integration test class to
Test.PM.Integration.ResourceProcessor.WebApplication - Removed separate
unit test classessince the existing test class already captures the required behavior.
Thank you!
…and remove unit test classes
3 participants
PR: Feature - Default CORS and JWT Configuration for Web Applications
This Pull Request enables default configuration settings for Cross-Origin Resource Sharing (CORS) and JSON Web Token (JWT) Authentication directly into the IPM
module.xml<WebApplication>resource definition. This enhancement enables the use of existing features through the package manager.Fixes #814
Summary of Feature Enhancements
This feature request introduces the following capabilities:
Support for loading CORS and JWT properties defined in the
<WebApplication>configuration sectionSupport for loading CORS and JWT properties generated through the IPM CLI (
Generate/gen) command<CSPApplication><WebApplication>element.Key Features & Rationale
CorsAllowlist,CorsCredentialsAllowed,CorsHeadersListJWTAuthEnabled,JWTAccessTokenTimeout,JWTRefreshTokenTimeoutCombined Configuration Example
The following attributes are now included in the
<WebApplication>tag by default, ensuring immediate security compliance for the created web application (/testcors):Detailed Test Results
Extensive unit and integration testing was performed to validate the correct application of these settings. All tests passed successfully, confirming the configuration integrity and non-interference between the two security features.
1. Unit Test Results (CORS Configuration Verification)
Class:
Test.PM.Unit.WebAppCorsTest2. Unit Test Results (JWT Configuration Verification)
Class:
Test.PM.Unit.WebAppJWTConfigTest3. Integration Test Results (Combined Validation)
Class:
Test.PM.Integration.ConfigCorsAndJWTInWebAppTestThis test confirms the successful and simultaneous application of all CORS and JWT settings during a single module load, demonstrating complete feature integration.
cors-rest-appsmodule successfully./testcorscreated scuccessfullyhttps://www.example.com,https://pm.intersystems.com1Access-Control-Allow-Origin, ...(Confirmed comprehensive list)/testcorscreated scuccessfully60)1)900)cors-rest-appsmodule successfully..947516 sec.Test.PM.Integration.CorsAndJWTTest passed