From c1dfe24b3aa39cf227f0e415fabcd958cb2129db Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 15:40:33 +0000 Subject: [PATCH 01/10] Initial plan From ab511cf5bfdcd424bbb586d9d5a2614da3e1233f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 15:48:59 +0000 Subject: [PATCH 02/10] Add security warnings to sample credentials and create SECURITY.md Co-authored-by: thomasturrell <1552612+thomasturrell@users.noreply.github.com> --- README.md | 6 + SECURITY.md | 130 ++++++++++++++++++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ .../src/main/resources/application.properties | 8 ++ 31 files changed, 368 insertions(+) create mode 100644 SECURITY.md diff --git a/README.md b/README.md index fa1c59bf..4ebef054 100644 --- a/README.md +++ b/README.md @@ -489,6 +489,12 @@ public ResponseEntity> postStatements( ``` +## Security + +For security best practices and credential management guidelines, see [SECURITY.md](SECURITY.md). + +**Important**: The sample applications contain hardcoded credentials for demonstration purposes only. Never use these credentials in production. See [SECURITY.md](SECURITY.md) for secure configuration approaches. + ## Contributing We welcome contributions! Please see [CONTRIBUTING.md](CONTRIBUTING.md) for: diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..fcdcd056 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,130 @@ +# Security Policy + +## Reporting a Vulnerability + +If you discover a security vulnerability in xapi-java, please report it to the project maintainers. Please do not create a public GitHub issue for security vulnerabilities. + +## Security Best Practices + +### Credential Management + +The xAPI client library supports multiple authentication methods for connecting to Learning Record Stores (LRS): + +#### Configuration Properties + +The library accepts the following configuration properties: + +- `xapi.client.username` - Username for basic authentication +- `xapi.client.password` - Password for basic authentication +- `xapi.client.authorization` - Full authorization header value (takes precedence over username/password) + +#### Security Recommendations + +**DO NOT** hardcode credentials in application.properties files: + +```properties +# ❌ NEVER DO THIS IN PRODUCTION +xapi.client.username = admin +xapi.client.password = password +``` + +**DO** use one of these secure approaches: + +1. **Environment Variables** (Recommended) + ```properties + xapi.client.username = ${XAPI_USERNAME} + xapi.client.password = ${XAPI_PASSWORD} + ``` + +2. **External Configuration** + - Use Spring Cloud Config Server + - Use Kubernetes Secrets + - Use AWS Secrets Manager, Azure Key Vault, or similar services + +3. **Authorization Header with Tokens** + ```properties + xapi.client.authorization = ${XAPI_AUTH_TOKEN} + ``` + +4. **System Properties** + ```bash + java -jar myapp.jar \ + --xapi.client.username=${XAPI_USERNAME} \ + --xapi.client.password=${XAPI_PASSWORD} + ``` + +### Sample Applications + +The sample applications in the `samples/` directory contain hardcoded credentials for demonstration purposes only: + +```properties +xapi.client.username = admin +xapi.client.password = password +``` + +**These are example credentials and MUST NOT be used in production environments.** + +Before running samples against a real LRS: +1. Copy the `application.properties` file +2. Replace the credentials with your actual LRS credentials +3. Add the properties file to `.gitignore` to prevent accidental commits +4. Or use environment variables to override the properties + +### HTTPS Usage + +The xAPI client library uses Spring WebClient which supports HTTPS by default. Always use HTTPS URLs when connecting to production LRS endpoints: + +```properties +# ✅ Good - HTTPS +xapi.client.baseUrl = https://lrs.example.com/xapi/ + +# ❌ Bad - HTTP (only for local development) +xapi.client.baseUrl = http://localhost:8080/xapi/ +``` + +### Dependency Security + +This project uses: +- Dependabot for automated dependency updates +- GitHub Actions for CI/CD security scanning +- SonarCloud for static code analysis + +Regular dependency updates help maintain security. Review and merge Dependabot PRs promptly. + +## Security Hotspots Review + +### Hardcoded Credentials (Addressed) + +**Issue**: Sample application.properties files contained hardcoded credentials without security warnings. + +**Resolution**: Added prominent security warnings to all sample configuration files explaining that credentials are for demo purposes only and providing guidance on secure credential management. + +**Status**: ✅ Resolved - Demo credentials are clearly marked with security warnings + +### Additional Findings + +No additional security hotspots requiring immediate action were identified during the review. The codebase follows security best practices: + +- Uses Spring Security's built-in authentication mechanisms +- Employs HTTPS for external communications +- Validates all xAPI data against the specification +- Uses parameterized queries (no SQL injection vulnerabilities) +- No use of insecure cryptographic functions +- No dynamic class loading or unsafe deserialization + +## Security Scan Results + +This repository is regularly scanned using: +- **SonarCloud**: For code quality and security analysis +- **GitHub CodeQL**: For security vulnerability detection +- **Dependabot**: For dependency vulnerability alerts + +## Supported Versions + +Security updates are provided for the latest stable release. Users are encouraged to upgrade to the latest version to receive security fixes. + +## Additional Resources + +- [xAPI Specification Security Considerations](https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-About.md#def-security) +- [Spring Boot Security Documentation](https://docs.spring.io/spring-boot/docs/current/reference/html/features.html#features.security) +- [OWASP Top 10](https://owasp.org/www-project-top-ten/) diff --git a/samples/delete-activity-profile/src/main/resources/application.properties b/samples/delete-activity-profile/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/delete-activity-profile/src/main/resources/application.properties +++ b/samples/delete-activity-profile/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/delete-agent-profile/src/main/resources/application.properties b/samples/delete-agent-profile/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/delete-agent-profile/src/main/resources/application.properties +++ b/samples/delete-agent-profile/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/delete-state/src/main/resources/application.properties b/samples/delete-state/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/delete-state/src/main/resources/application.properties +++ b/samples/delete-state/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/delete-states/src/main/resources/application.properties b/samples/delete-states/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/delete-states/src/main/resources/application.properties +++ b/samples/delete-states/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-about/src/main/resources/application.properties b/samples/get-about/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/get-about/src/main/resources/application.properties +++ b/samples/get-about/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-activity-profile/src/main/resources/application.properties b/samples/get-activity-profile/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/get-activity-profile/src/main/resources/application.properties +++ b/samples/get-activity-profile/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-activity-profiles/src/main/resources/application.properties b/samples/get-activity-profiles/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/get-activity-profiles/src/main/resources/application.properties +++ b/samples/get-activity-profiles/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-activity/src/main/resources/application.properties b/samples/get-activity/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/get-activity/src/main/resources/application.properties +++ b/samples/get-activity/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-agent-profile/src/main/resources/application.properties b/samples/get-agent-profile/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/get-agent-profile/src/main/resources/application.properties +++ b/samples/get-agent-profile/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-agent-profiles/src/main/resources/application.properties b/samples/get-agent-profiles/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/get-agent-profiles/src/main/resources/application.properties +++ b/samples/get-agent-profiles/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-agents/src/main/resources/application.properties b/samples/get-agents/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/get-agents/src/main/resources/application.properties +++ b/samples/get-agents/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-more-statements/src/main/resources/application.properties b/samples/get-more-statements/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/get-more-statements/src/main/resources/application.properties +++ b/samples/get-more-statements/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-state/src/main/resources/application.properties b/samples/get-state/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/get-state/src/main/resources/application.properties +++ b/samples/get-state/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-statement-iterator/src/main/resources/application.properties b/samples/get-statement-iterator/src/main/resources/application.properties index 379e502a..88c11c85 100644 --- a/samples/get-statement-iterator/src/main/resources/application.properties +++ b/samples/get-statement-iterator/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-statement-with-attachment/src/main/resources/application.properties b/samples/get-statement-with-attachment/src/main/resources/application.properties index 379e502a..88c11c85 100644 --- a/samples/get-statement-with-attachment/src/main/resources/application.properties +++ b/samples/get-statement-with-attachment/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-statement/src/main/resources/application.properties b/samples/get-statement/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/get-statement/src/main/resources/application.properties +++ b/samples/get-statement/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-statements/src/main/resources/application.properties b/samples/get-statements/src/main/resources/application.properties index 379e502a..88c11c85 100644 --- a/samples/get-statements/src/main/resources/application.properties +++ b/samples/get-statements/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-states/src/main/resources/application.properties b/samples/get-states/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/get-states/src/main/resources/application.properties +++ b/samples/get-states/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-voided-statement/src/main/resources/application.properties b/samples/get-voided-statement/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/get-voided-statement/src/main/resources/application.properties +++ b/samples/get-voided-statement/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-activity-profile/src/main/resources/application.properties b/samples/post-activity-profile/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/post-activity-profile/src/main/resources/application.properties +++ b/samples/post-activity-profile/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-agent-profile/src/main/resources/application.properties b/samples/post-agent-profile/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/post-agent-profile/src/main/resources/application.properties +++ b/samples/post-agent-profile/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-signed-statement/src/main/resources/application.properties b/samples/post-signed-statement/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/post-signed-statement/src/main/resources/application.properties +++ b/samples/post-signed-statement/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-state/src/main/resources/application.properties b/samples/post-state/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/post-state/src/main/resources/application.properties +++ b/samples/post-state/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-statement-with-attachment/src/main/resources/application.properties b/samples/post-statement-with-attachment/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/post-statement-with-attachment/src/main/resources/application.properties +++ b/samples/post-statement-with-attachment/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-statement/src/main/resources/application.properties b/samples/post-statement/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/post-statement/src/main/resources/application.properties +++ b/samples/post-statement/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-statements/src/main/resources/application.properties b/samples/post-statements/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/post-statements/src/main/resources/application.properties +++ b/samples/post-statements/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/put-activity-profile/src/main/resources/application.properties b/samples/put-activity-profile/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/put-activity-profile/src/main/resources/application.properties +++ b/samples/put-activity-profile/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/put-agent-profile/src/main/resources/application.properties b/samples/put-agent-profile/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/put-agent-profile/src/main/resources/application.properties +++ b/samples/put-agent-profile/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/put-state/src/main/resources/application.properties b/samples/put-state/src/main/resources/application.properties index de20217a..d0a53844 100644 --- a/samples/put-state/src/main/resources/application.properties +++ b/samples/put-state/src/main/resources/application.properties @@ -1,3 +1,11 @@ +# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. +# DO NOT use these credentials in production environments. +# For production use: +# - Store credentials in environment variables or secure configuration management +# - Use the xapi.client.authorization property with a secure token +# - Consider using Spring Cloud Config or similar secure configuration solutions +# - Never commit real credentials to version control + xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ From 87853cde75eac0fc89b2f3846c1b35ac9b9bc24c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 15:59:44 +0000 Subject: [PATCH 03/10] Fix ReDoS vulnerability in ISO 8601 duration regex validation Co-authored-by: thomasturrell <1552612+thomasturrell@users.noreply.github.com> --- .../java/dev/learning/xapi/model/Result.java | 5 +- .../model/ResultDurationValidationTests.java | 209 ++++++++++++++++++ 2 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 xapi-model/src/test/java/dev/learning/xapi/model/ResultDurationValidationTests.java diff --git a/xapi-model/src/main/java/dev/learning/xapi/model/Result.java b/xapi-model/src/main/java/dev/learning/xapi/model/Result.java index d6336624..ee770028 100644 --- a/xapi-model/src/main/java/dev/learning/xapi/model/Result.java +++ b/xapi-model/src/main/java/dev/learning/xapi/model/Result.java @@ -42,10 +42,11 @@ public class Result { /** Period of time over which the Statement occurred. */ // Java Duration does not store ISO 8601:2004 durations. + // Using possessive quantifiers on digits to prevent ReDoS (Regular Expression Denial of Service) @Pattern( regexp = - "^(P\\d+W)?$|^P(?!$)(\\d+Y)?(\\d+M)?" // NOSONAR - + "(\\d+D)?(T(?=\\d)(\\d+H)?(\\d+M)?(\\d*\\.?\\d+S)?)?$", // NOSONAR + "^P\\d++W$|^P(?!$)(\\d++Y)?(\\d++M)?" + + "(\\d++D)?(T(?=\\d)(\\d++H)?(\\d++M)?((?:\\d++\\.\\d++|\\d++)S)?)?$", flags = Pattern.Flag.CASE_INSENSITIVE, message = "Must be a valid ISO 8601:2004 duration format.") private String duration; diff --git a/xapi-model/src/test/java/dev/learning/xapi/model/ResultDurationValidationTests.java b/xapi-model/src/test/java/dev/learning/xapi/model/ResultDurationValidationTests.java new file mode 100644 index 00000000..0fd48302 --- /dev/null +++ b/xapi-model/src/test/java/dev/learning/xapi/model/ResultDurationValidationTests.java @@ -0,0 +1,209 @@ +/* + * Copyright 2016-2025 Berry Cloud Ltd. All rights reserved. + */ + +package dev.learning.xapi.model; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.not; + +import jakarta.validation.ConstraintViolation; +import jakarta.validation.Validation; +import jakarta.validation.Validator; +import java.util.Set; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Result Duration Validation Tests. + * + *

Tests for ISO 8601:2004 duration format validation to ensure the regex pattern correctly + * validates durations and is not vulnerable to ReDoS (Regular Expression Denial of Service) + * attacks. + * + * @author GitHub Copilot + */ +@DisplayName("Result Duration Validation Tests") +class ResultDurationValidationTests { + + private final Validator validator = Validation.buildDefaultValidatorFactory().getValidator(); + + @ParameterizedTest + @ValueSource( + strings = { + // Week format + "P1W", + "P52W", + "P104W", + // Day format + "P1D", + "P365D", + // Time format + "PT1H", + "PT30M", + "PT45S", + "PT1.5S", + "PT0.5S", + // Combined date format + "P1Y", + "P1M", + "P1Y2M", + "P1Y2M3D", + // Combined date and time format + "P1DT1H", + "P1DT1H30M", + "P1DT1H30M45S", + "P1Y2M3DT4H5M6S", + "P1Y2M3DT4H5M6.7S", + // Minimal valid formats + "PT0S", + "PT1S", + "P0D", + // Real-world examples + "PT8H", + "P90D", + "P2Y", + "PT15M30S" + }) + @DisplayName("When Duration Is Valid Then Validation Passes") + void whenDurationIsValidThenValidationPasses(String duration) { + + // Given Result With Valid Duration + final var result = Result.builder().duration(duration).build(); + + // When Validating Result + final Set> violations = validator.validate(result); + + // Then Validation Passes + assertThat(violations, empty()); + } + + @ParameterizedTest + @ValueSource( + strings = { + // Invalid formats + "", + "T1H", + "1D", + "PD", + "PT", + "P1", + "1Y2M", + // Invalid time without T + "P1H", + "P1M30S", + // Invalid mixing weeks with other units + "P1W1D", + "P1W1Y", + "P1WT1H", + // Invalid decimal placement + "P1.5D", + "P1.5Y", + "PT1.5H", + "PT1.5M", + // Missing P prefix + "1Y2M3D", + "T1H30M", + // Invalid order + "P1D1Y", + "PT1S1M", + "PT1M1H", + // Double separators + "P1Y2M3DTT1H", + "PP1D", + // Negative values + "P-1D", + "PT-1H" + }) + @DisplayName("When Duration Is Invalid Then Validation Fails") + void whenDurationIsInvalidThenValidationFails(String duration) { + + // Given Result With Invalid Duration + final var result = Result.builder().duration(duration).build(); + + // When Validating Result + final Set> violations = validator.validate(result); + + // Then Validation Fails + assertThat(violations, not(empty())); + assertThat(violations, hasSize(1)); + } + + @Test + @DisplayName("When Duration Is Null Then Validation Passes") + void whenDurationIsNullThenValidationPasses() { + + // Given Result With Null Duration + final var result = Result.builder().duration(null).build(); + + // When Validating Result + final Set> violations = validator.validate(result); + + // Then Validation Passes + assertThat(violations, empty()); + } + + @Test + @DisplayName("When Duration Has Many Digits Then Validation Completes Quickly") + void whenDurationHasManyDigitsThenValidationCompletesQuickly() { + + // Given Result With Long But Valid Duration + final var result = Result.builder().duration("P99999Y99999M99999DT99999H99999M99999S").build(); + + // When Validating Result + final long startTime = System.nanoTime(); + final Set> violations = validator.validate(result); + final long endTime = System.nanoTime(); + final long durationMs = (endTime - startTime) / 1_000_000; + + // Then Validation Passes And Completes In Reasonable Time + assertThat(violations, empty()); + assertThat("Validation should complete in less than 100ms", durationMs < 100); + } + + @Test + @DisplayName( + "When Duration Is Adversarial Input Then Validation Completes Quickly Without ReDoS") + void whenDurationIsAdversarialInputThenValidationCompletesQuicklyWithoutReDoS() { + + // Given Result With Adversarial Input That Could Cause ReDoS + // This input is designed to trigger exponential backtracking in vulnerable regex patterns + final var adversarialInput = "P" + "9".repeat(50) + "!"; + final var result = Result.builder().duration(adversarialInput).build(); + + // When Validating Result + final long startTime = System.nanoTime(); + final Set> violations = validator.validate(result); + final long endTime = System.nanoTime(); + final long durationMs = (endTime - startTime) / 1_000_000; + + // Then Validation Fails Quickly (not vulnerable to ReDoS) + assertThat(violations, not(empty())); + assertThat("Validation should complete in less than 100ms even with adversarial input", + durationMs < 100); + } + + @Test + @DisplayName("When Duration Has Multiple Optional Groups Then Validation Completes Quickly") + void whenDurationHasMultipleOptionalGroupsThenValidationCompletesQuickly() { + + // Given Result With Input That Tests Multiple Optional Groups + // This tests the pattern with input that doesn't match but exercises optional groups + final var testInput = "PYMDTHMS"; + final var result = Result.builder().duration(testInput).build(); + + // When Validating Result + final long startTime = System.nanoTime(); + final Set> violations = validator.validate(result); + final long endTime = System.nanoTime(); + final long durationMs = (endTime - startTime) / 1_000_000; + + // Then Validation Fails Quickly Without Excessive Backtracking + assertThat(violations, not(empty())); + assertThat("Validation should complete in less than 100ms", durationMs < 100); + } +} From 90ebc338f00d256398c836c77d198277b5e8d4ca Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 16:03:34 +0000 Subject: [PATCH 04/10] Address code review comments - update author and add comments Co-authored-by: thomasturrell <1552612+thomasturrell@users.noreply.github.com> --- .../learning/xapi/model/ResultDurationValidationTests.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/xapi-model/src/test/java/dev/learning/xapi/model/ResultDurationValidationTests.java b/xapi-model/src/test/java/dev/learning/xapi/model/ResultDurationValidationTests.java index 0fd48302..75ad0df8 100644 --- a/xapi-model/src/test/java/dev/learning/xapi/model/ResultDurationValidationTests.java +++ b/xapi-model/src/test/java/dev/learning/xapi/model/ResultDurationValidationTests.java @@ -25,7 +25,7 @@ * validates durations and is not vulnerable to ReDoS (Regular Expression Denial of Service) * attacks. * - * @author GitHub Copilot + * @author Berry Cloud */ @DisplayName("Result Duration Validation Tests") class ResultDurationValidationTests { @@ -162,6 +162,7 @@ void whenDurationHasManyDigitsThenValidationCompletesQuickly() { // Then Validation Passes And Completes In Reasonable Time assertThat(violations, empty()); + // Validation should complete quickly - 100ms is generous for a regex match assertThat("Validation should complete in less than 100ms", durationMs < 100); } @@ -183,6 +184,7 @@ void whenDurationIsAdversarialInputThenValidationCompletesQuicklyWithoutReDoS() // Then Validation Fails Quickly (not vulnerable to ReDoS) assertThat(violations, not(empty())); + // Validation should complete quickly even with adversarial input - 100ms is generous assertThat("Validation should complete in less than 100ms even with adversarial input", durationMs < 100); } @@ -204,6 +206,7 @@ void whenDurationHasMultipleOptionalGroupsThenValidationCompletesQuickly() { // Then Validation Fails Quickly Without Excessive Backtracking assertThat(violations, not(empty())); + // Validation should complete quickly - 100ms is generous for a regex match assertThat("Validation should complete in less than 100ms", durationMs < 100); } } From 4a9fc8fb44821e49bc73038d92e1705e29cf19f0 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 24 Nov 2025 16:19:50 +0000 Subject: [PATCH 05/10] Revert security warnings from sample application.properties files and remove SECURITY.md Co-authored-by: thomasturrell <1552612+thomasturrell@users.noreply.github.com> --- README.md | 6 - SECURITY.md | 130 ------------------ .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- .../src/main/resources/application.properties | 8 -- 31 files changed, 368 deletions(-) delete mode 100644 SECURITY.md diff --git a/README.md b/README.md index 4ebef054..fa1c59bf 100644 --- a/README.md +++ b/README.md @@ -489,12 +489,6 @@ public ResponseEntity> postStatements( ``` -## Security - -For security best practices and credential management guidelines, see [SECURITY.md](SECURITY.md). - -**Important**: The sample applications contain hardcoded credentials for demonstration purposes only. Never use these credentials in production. See [SECURITY.md](SECURITY.md) for secure configuration approaches. - ## Contributing We welcome contributions! Please see [CONTRIBUTING.md](CONTRIBUTING.md) for: diff --git a/SECURITY.md b/SECURITY.md deleted file mode 100644 index fcdcd056..00000000 --- a/SECURITY.md +++ /dev/null @@ -1,130 +0,0 @@ -# Security Policy - -## Reporting a Vulnerability - -If you discover a security vulnerability in xapi-java, please report it to the project maintainers. Please do not create a public GitHub issue for security vulnerabilities. - -## Security Best Practices - -### Credential Management - -The xAPI client library supports multiple authentication methods for connecting to Learning Record Stores (LRS): - -#### Configuration Properties - -The library accepts the following configuration properties: - -- `xapi.client.username` - Username for basic authentication -- `xapi.client.password` - Password for basic authentication -- `xapi.client.authorization` - Full authorization header value (takes precedence over username/password) - -#### Security Recommendations - -**DO NOT** hardcode credentials in application.properties files: - -```properties -# ❌ NEVER DO THIS IN PRODUCTION -xapi.client.username = admin -xapi.client.password = password -``` - -**DO** use one of these secure approaches: - -1. **Environment Variables** (Recommended) - ```properties - xapi.client.username = ${XAPI_USERNAME} - xapi.client.password = ${XAPI_PASSWORD} - ``` - -2. **External Configuration** - - Use Spring Cloud Config Server - - Use Kubernetes Secrets - - Use AWS Secrets Manager, Azure Key Vault, or similar services - -3. **Authorization Header with Tokens** - ```properties - xapi.client.authorization = ${XAPI_AUTH_TOKEN} - ``` - -4. **System Properties** - ```bash - java -jar myapp.jar \ - --xapi.client.username=${XAPI_USERNAME} \ - --xapi.client.password=${XAPI_PASSWORD} - ``` - -### Sample Applications - -The sample applications in the `samples/` directory contain hardcoded credentials for demonstration purposes only: - -```properties -xapi.client.username = admin -xapi.client.password = password -``` - -**These are example credentials and MUST NOT be used in production environments.** - -Before running samples against a real LRS: -1. Copy the `application.properties` file -2. Replace the credentials with your actual LRS credentials -3. Add the properties file to `.gitignore` to prevent accidental commits -4. Or use environment variables to override the properties - -### HTTPS Usage - -The xAPI client library uses Spring WebClient which supports HTTPS by default. Always use HTTPS URLs when connecting to production LRS endpoints: - -```properties -# ✅ Good - HTTPS -xapi.client.baseUrl = https://lrs.example.com/xapi/ - -# ❌ Bad - HTTP (only for local development) -xapi.client.baseUrl = http://localhost:8080/xapi/ -``` - -### Dependency Security - -This project uses: -- Dependabot for automated dependency updates -- GitHub Actions for CI/CD security scanning -- SonarCloud for static code analysis - -Regular dependency updates help maintain security. Review and merge Dependabot PRs promptly. - -## Security Hotspots Review - -### Hardcoded Credentials (Addressed) - -**Issue**: Sample application.properties files contained hardcoded credentials without security warnings. - -**Resolution**: Added prominent security warnings to all sample configuration files explaining that credentials are for demo purposes only and providing guidance on secure credential management. - -**Status**: ✅ Resolved - Demo credentials are clearly marked with security warnings - -### Additional Findings - -No additional security hotspots requiring immediate action were identified during the review. The codebase follows security best practices: - -- Uses Spring Security's built-in authentication mechanisms -- Employs HTTPS for external communications -- Validates all xAPI data against the specification -- Uses parameterized queries (no SQL injection vulnerabilities) -- No use of insecure cryptographic functions -- No dynamic class loading or unsafe deserialization - -## Security Scan Results - -This repository is regularly scanned using: -- **SonarCloud**: For code quality and security analysis -- **GitHub CodeQL**: For security vulnerability detection -- **Dependabot**: For dependency vulnerability alerts - -## Supported Versions - -Security updates are provided for the latest stable release. Users are encouraged to upgrade to the latest version to receive security fixes. - -## Additional Resources - -- [xAPI Specification Security Considerations](https://github.com/adlnet/xAPI-Spec/blob/master/xAPI-About.md#def-security) -- [Spring Boot Security Documentation](https://docs.spring.io/spring-boot/docs/current/reference/html/features.html#features.security) -- [OWASP Top 10](https://owasp.org/www-project-top-ten/) diff --git a/samples/delete-activity-profile/src/main/resources/application.properties b/samples/delete-activity-profile/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/delete-activity-profile/src/main/resources/application.properties +++ b/samples/delete-activity-profile/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/delete-agent-profile/src/main/resources/application.properties b/samples/delete-agent-profile/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/delete-agent-profile/src/main/resources/application.properties +++ b/samples/delete-agent-profile/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/delete-state/src/main/resources/application.properties b/samples/delete-state/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/delete-state/src/main/resources/application.properties +++ b/samples/delete-state/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/delete-states/src/main/resources/application.properties b/samples/delete-states/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/delete-states/src/main/resources/application.properties +++ b/samples/delete-states/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-about/src/main/resources/application.properties b/samples/get-about/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/get-about/src/main/resources/application.properties +++ b/samples/get-about/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-activity-profile/src/main/resources/application.properties b/samples/get-activity-profile/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/get-activity-profile/src/main/resources/application.properties +++ b/samples/get-activity-profile/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-activity-profiles/src/main/resources/application.properties b/samples/get-activity-profiles/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/get-activity-profiles/src/main/resources/application.properties +++ b/samples/get-activity-profiles/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-activity/src/main/resources/application.properties b/samples/get-activity/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/get-activity/src/main/resources/application.properties +++ b/samples/get-activity/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-agent-profile/src/main/resources/application.properties b/samples/get-agent-profile/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/get-agent-profile/src/main/resources/application.properties +++ b/samples/get-agent-profile/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-agent-profiles/src/main/resources/application.properties b/samples/get-agent-profiles/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/get-agent-profiles/src/main/resources/application.properties +++ b/samples/get-agent-profiles/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-agents/src/main/resources/application.properties b/samples/get-agents/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/get-agents/src/main/resources/application.properties +++ b/samples/get-agents/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-more-statements/src/main/resources/application.properties b/samples/get-more-statements/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/get-more-statements/src/main/resources/application.properties +++ b/samples/get-more-statements/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-state/src/main/resources/application.properties b/samples/get-state/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/get-state/src/main/resources/application.properties +++ b/samples/get-state/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-statement-iterator/src/main/resources/application.properties b/samples/get-statement-iterator/src/main/resources/application.properties index 88c11c85..379e502a 100644 --- a/samples/get-statement-iterator/src/main/resources/application.properties +++ b/samples/get-statement-iterator/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-statement-with-attachment/src/main/resources/application.properties b/samples/get-statement-with-attachment/src/main/resources/application.properties index 88c11c85..379e502a 100644 --- a/samples/get-statement-with-attachment/src/main/resources/application.properties +++ b/samples/get-statement-with-attachment/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-statement/src/main/resources/application.properties b/samples/get-statement/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/get-statement/src/main/resources/application.properties +++ b/samples/get-statement/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-statements/src/main/resources/application.properties b/samples/get-statements/src/main/resources/application.properties index 88c11c85..379e502a 100644 --- a/samples/get-statements/src/main/resources/application.properties +++ b/samples/get-statements/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-states/src/main/resources/application.properties b/samples/get-states/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/get-states/src/main/resources/application.properties +++ b/samples/get-states/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/get-voided-statement/src/main/resources/application.properties b/samples/get-voided-statement/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/get-voided-statement/src/main/resources/application.properties +++ b/samples/get-voided-statement/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-activity-profile/src/main/resources/application.properties b/samples/post-activity-profile/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/post-activity-profile/src/main/resources/application.properties +++ b/samples/post-activity-profile/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-agent-profile/src/main/resources/application.properties b/samples/post-agent-profile/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/post-agent-profile/src/main/resources/application.properties +++ b/samples/post-agent-profile/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-signed-statement/src/main/resources/application.properties b/samples/post-signed-statement/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/post-signed-statement/src/main/resources/application.properties +++ b/samples/post-signed-statement/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-state/src/main/resources/application.properties b/samples/post-state/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/post-state/src/main/resources/application.properties +++ b/samples/post-state/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-statement-with-attachment/src/main/resources/application.properties b/samples/post-statement-with-attachment/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/post-statement-with-attachment/src/main/resources/application.properties +++ b/samples/post-statement-with-attachment/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-statement/src/main/resources/application.properties b/samples/post-statement/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/post-statement/src/main/resources/application.properties +++ b/samples/post-statement/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/post-statements/src/main/resources/application.properties b/samples/post-statements/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/post-statements/src/main/resources/application.properties +++ b/samples/post-statements/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/put-activity-profile/src/main/resources/application.properties b/samples/put-activity-profile/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/put-activity-profile/src/main/resources/application.properties +++ b/samples/put-activity-profile/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/put-agent-profile/src/main/resources/application.properties b/samples/put-agent-profile/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/put-agent-profile/src/main/resources/application.properties +++ b/samples/put-agent-profile/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ diff --git a/samples/put-state/src/main/resources/application.properties b/samples/put-state/src/main/resources/application.properties index d0a53844..de20217a 100644 --- a/samples/put-state/src/main/resources/application.properties +++ b/samples/put-state/src/main/resources/application.properties @@ -1,11 +1,3 @@ -# SECURITY WARNING: These credentials are FOR DEMO/TESTING PURPOSES ONLY. -# DO NOT use these credentials in production environments. -# For production use: -# - Store credentials in environment variables or secure configuration management -# - Use the xapi.client.authorization property with a secure token -# - Consider using Spring Cloud Config or similar secure configuration solutions -# - Never commit real credentials to version control - xapi.client.username = admin xapi.client.password = password xapi.client.baseUrl = https://example.com/xapi/ From 2807dde657c3837a4cbf6599f43e6edc66bc2d55 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 03:37:17 +0000 Subject: [PATCH 06/10] Move duration validation tests to existing ResultTests.java Co-authored-by: thomasturrell <1552612+thomasturrell@users.noreply.github.com> --- .../model/ResultDurationValidationTests.java | 212 ------------------ .../dev/learning/xapi/model/ResultTests.java | 191 ++++++++++++++++ 2 files changed, 191 insertions(+), 212 deletions(-) delete mode 100644 xapi-model/src/test/java/dev/learning/xapi/model/ResultDurationValidationTests.java diff --git a/xapi-model/src/test/java/dev/learning/xapi/model/ResultDurationValidationTests.java b/xapi-model/src/test/java/dev/learning/xapi/model/ResultDurationValidationTests.java deleted file mode 100644 index 75ad0df8..00000000 --- a/xapi-model/src/test/java/dev/learning/xapi/model/ResultDurationValidationTests.java +++ /dev/null @@ -1,212 +0,0 @@ -/* - * Copyright 2016-2025 Berry Cloud Ltd. All rights reserved. - */ - -package dev.learning.xapi.model; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.empty; -import static org.hamcrest.Matchers.hasSize; -import static org.hamcrest.Matchers.not; - -import jakarta.validation.ConstraintViolation; -import jakarta.validation.Validation; -import jakarta.validation.Validator; -import java.util.Set; -import org.junit.jupiter.api.DisplayName; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; - -/** - * Result Duration Validation Tests. - * - *

Tests for ISO 8601:2004 duration format validation to ensure the regex pattern correctly - * validates durations and is not vulnerable to ReDoS (Regular Expression Denial of Service) - * attacks. - * - * @author Berry Cloud - */ -@DisplayName("Result Duration Validation Tests") -class ResultDurationValidationTests { - - private final Validator validator = Validation.buildDefaultValidatorFactory().getValidator(); - - @ParameterizedTest - @ValueSource( - strings = { - // Week format - "P1W", - "P52W", - "P104W", - // Day format - "P1D", - "P365D", - // Time format - "PT1H", - "PT30M", - "PT45S", - "PT1.5S", - "PT0.5S", - // Combined date format - "P1Y", - "P1M", - "P1Y2M", - "P1Y2M3D", - // Combined date and time format - "P1DT1H", - "P1DT1H30M", - "P1DT1H30M45S", - "P1Y2M3DT4H5M6S", - "P1Y2M3DT4H5M6.7S", - // Minimal valid formats - "PT0S", - "PT1S", - "P0D", - // Real-world examples - "PT8H", - "P90D", - "P2Y", - "PT15M30S" - }) - @DisplayName("When Duration Is Valid Then Validation Passes") - void whenDurationIsValidThenValidationPasses(String duration) { - - // Given Result With Valid Duration - final var result = Result.builder().duration(duration).build(); - - // When Validating Result - final Set> violations = validator.validate(result); - - // Then Validation Passes - assertThat(violations, empty()); - } - - @ParameterizedTest - @ValueSource( - strings = { - // Invalid formats - "", - "T1H", - "1D", - "PD", - "PT", - "P1", - "1Y2M", - // Invalid time without T - "P1H", - "P1M30S", - // Invalid mixing weeks with other units - "P1W1D", - "P1W1Y", - "P1WT1H", - // Invalid decimal placement - "P1.5D", - "P1.5Y", - "PT1.5H", - "PT1.5M", - // Missing P prefix - "1Y2M3D", - "T1H30M", - // Invalid order - "P1D1Y", - "PT1S1M", - "PT1M1H", - // Double separators - "P1Y2M3DTT1H", - "PP1D", - // Negative values - "P-1D", - "PT-1H" - }) - @DisplayName("When Duration Is Invalid Then Validation Fails") - void whenDurationIsInvalidThenValidationFails(String duration) { - - // Given Result With Invalid Duration - final var result = Result.builder().duration(duration).build(); - - // When Validating Result - final Set> violations = validator.validate(result); - - // Then Validation Fails - assertThat(violations, not(empty())); - assertThat(violations, hasSize(1)); - } - - @Test - @DisplayName("When Duration Is Null Then Validation Passes") - void whenDurationIsNullThenValidationPasses() { - - // Given Result With Null Duration - final var result = Result.builder().duration(null).build(); - - // When Validating Result - final Set> violations = validator.validate(result); - - // Then Validation Passes - assertThat(violations, empty()); - } - - @Test - @DisplayName("When Duration Has Many Digits Then Validation Completes Quickly") - void whenDurationHasManyDigitsThenValidationCompletesQuickly() { - - // Given Result With Long But Valid Duration - final var result = Result.builder().duration("P99999Y99999M99999DT99999H99999M99999S").build(); - - // When Validating Result - final long startTime = System.nanoTime(); - final Set> violations = validator.validate(result); - final long endTime = System.nanoTime(); - final long durationMs = (endTime - startTime) / 1_000_000; - - // Then Validation Passes And Completes In Reasonable Time - assertThat(violations, empty()); - // Validation should complete quickly - 100ms is generous for a regex match - assertThat("Validation should complete in less than 100ms", durationMs < 100); - } - - @Test - @DisplayName( - "When Duration Is Adversarial Input Then Validation Completes Quickly Without ReDoS") - void whenDurationIsAdversarialInputThenValidationCompletesQuicklyWithoutReDoS() { - - // Given Result With Adversarial Input That Could Cause ReDoS - // This input is designed to trigger exponential backtracking in vulnerable regex patterns - final var adversarialInput = "P" + "9".repeat(50) + "!"; - final var result = Result.builder().duration(adversarialInput).build(); - - // When Validating Result - final long startTime = System.nanoTime(); - final Set> violations = validator.validate(result); - final long endTime = System.nanoTime(); - final long durationMs = (endTime - startTime) / 1_000_000; - - // Then Validation Fails Quickly (not vulnerable to ReDoS) - assertThat(violations, not(empty())); - // Validation should complete quickly even with adversarial input - 100ms is generous - assertThat("Validation should complete in less than 100ms even with adversarial input", - durationMs < 100); - } - - @Test - @DisplayName("When Duration Has Multiple Optional Groups Then Validation Completes Quickly") - void whenDurationHasMultipleOptionalGroupsThenValidationCompletesQuickly() { - - // Given Result With Input That Tests Multiple Optional Groups - // This tests the pattern with input that doesn't match but exercises optional groups - final var testInput = "PYMDTHMS"; - final var result = Result.builder().duration(testInput).build(); - - // When Validating Result - final long startTime = System.nanoTime(); - final Set> violations = validator.validate(result); - final long endTime = System.nanoTime(); - final long durationMs = (endTime - startTime) / 1_000_000; - - // Then Validation Fails Quickly Without Excessive Backtracking - assertThat(violations, not(empty())); - // Validation should complete quickly - 100ms is generous for a regex match - assertThat("Validation should complete in less than 100ms", durationMs < 100); - } -} diff --git a/xapi-model/src/test/java/dev/learning/xapi/model/ResultTests.java b/xapi-model/src/test/java/dev/learning/xapi/model/ResultTests.java index e5f79c7c..fca6dcc3 100644 --- a/xapi-model/src/test/java/dev/learning/xapi/model/ResultTests.java +++ b/xapi-model/src/test/java/dev/learning/xapi/model/ResultTests.java @@ -5,13 +5,22 @@ package dev.learning.xapi.model; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import com.fasterxml.jackson.databind.ObjectMapper; +import jakarta.validation.ConstraintViolation; +import jakarta.validation.Validation; +import jakarta.validation.Validator; import java.io.IOException; +import java.util.Set; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.util.ResourceUtils; /** @@ -25,6 +34,8 @@ class ResultTests { private final ObjectMapper objectMapper = new ObjectMapper().findAndRegisterModules(); + private final Validator validator = Validation.buildDefaultValidatorFactory().getValidator(); + @Test void whenDeserializingResultThenResultIsInstanceOfResult() throws Exception { @@ -139,4 +150,184 @@ void whenCallingToStringThenResultIsExpected() { "Result(score=Score(scaled=1.0, raw=1.0, min=0.0, max=5.0), success=true, completion=true, " + "response=test, duration=P1D, extensions=null)")); } + + // Duration Validation Tests + + @ParameterizedTest + @ValueSource( + strings = { + // Week format + "P1W", + "P52W", + "P104W", + // Day format + "P1D", + "P365D", + // Time format + "PT1H", + "PT30M", + "PT45S", + "PT1.5S", + "PT0.5S", + // Combined date format + "P1Y", + "P1M", + "P1Y2M", + "P1Y2M3D", + // Combined date and time format + "P1DT1H", + "P1DT1H30M", + "P1DT1H30M45S", + "P1Y2M3DT4H5M6S", + "P1Y2M3DT4H5M6.7S", + // Minimal valid formats + "PT0S", + "PT1S", + "P0D", + // Real-world examples + "PT8H", + "P90D", + "P2Y", + "PT15M30S" + }) + @DisplayName("When Duration Is Valid Then Validation Passes") + void whenDurationIsValidThenValidationPasses(String duration) { + + // Given Result With Valid Duration + final var result = Result.builder().duration(duration).build(); + + // When Validating Result + final Set> violations = validator.validate(result); + + // Then Validation Passes + assertThat(violations, empty()); + } + + @ParameterizedTest + @ValueSource( + strings = { + // Invalid formats + "", + "T1H", + "1D", + "PD", + "PT", + "P1", + "1Y2M", + // Invalid time without T + "P1H", + "P1M30S", + // Invalid mixing weeks with other units + "P1W1D", + "P1W1Y", + "P1WT1H", + // Invalid decimal placement + "P1.5D", + "P1.5Y", + "PT1.5H", + "PT1.5M", + // Missing P prefix + "1Y2M3D", + "T1H30M", + // Invalid order + "P1D1Y", + "PT1S1M", + "PT1M1H", + // Double separators + "P1Y2M3DTT1H", + "PP1D", + // Negative values + "P-1D", + "PT-1H" + }) + @DisplayName("When Duration Is Invalid Then Validation Fails") + void whenDurationIsInvalidThenValidationFails(String duration) { + + // Given Result With Invalid Duration + final var result = Result.builder().duration(duration).build(); + + // When Validating Result + final Set> violations = validator.validate(result); + + // Then Validation Fails + assertThat(violations, not(empty())); + assertThat(violations, hasSize(1)); + } + + @Test + @DisplayName("When Duration Is Null Then Validation Passes") + void whenDurationIsNullThenValidationPasses() { + + // Given Result With Null Duration + final var result = Result.builder().duration(null).build(); + + // When Validating Result + final Set> violations = validator.validate(result); + + // Then Validation Passes + assertThat(violations, empty()); + } + + @Test + @DisplayName("When Duration Has Many Digits Then Validation Completes Quickly") + void whenDurationHasManyDigitsThenValidationCompletesQuickly() { + + // Given Result With Long But Valid Duration + final var result = Result.builder().duration("P99999Y99999M99999DT99999H99999M99999S").build(); + + // When Validating Result + final long startTime = System.nanoTime(); + final Set> violations = validator.validate(result); + final long endTime = System.nanoTime(); + final long durationMs = (endTime - startTime) / 1_000_000; + + // Then Validation Passes And Completes In Reasonable Time + assertThat(violations, empty()); + // Validation should complete quickly - 500ms is generous for a regex match + assertThat("Validation should complete in less than 500ms", durationMs < 500); + } + + @Test + @DisplayName("When Duration Is Adversarial Input Then Validation Completes Quickly Without ReDoS") + void whenDurationIsAdversarialInputThenValidationCompletesQuicklyWithoutReDoS() { + + // Given Result With Adversarial Input That Could Cause ReDoS + // This input is designed to trigger exponential backtracking in vulnerable regex patterns + final var adversarialInput = "P" + "9".repeat(50) + "!"; + final var result = Result.builder().duration(adversarialInput).build(); + + // When Validating Result + final long startTime = System.nanoTime(); + final Set> violations = validator.validate(result); + final long endTime = System.nanoTime(); + final long durationMs = (endTime - startTime) / 1_000_000; + + // Then Validation Fails Quickly (not vulnerable to ReDoS) + assertThat(violations, not(empty())); + // Validation should complete quickly even with adversarial input - 500ms is generous + assertThat( + "Validation should complete in less than 500ms even with adversarial input", + durationMs < 500); + } + + @Test + @DisplayName("When Duration Has Multiple Optional Groups Then Validation Completes Quickly") + void whenDurationHasMultipleOptionalGroupsThenValidationCompletesQuickly() { + + // Given Result With Input That Tests Multiple Optional Groups + // This tests the pattern with input that doesn't match but exercises optional groups + final var testInput = "PYMDTHMS"; + final var result = Result.builder().duration(testInput).build(); + + // When Validating Result + final long startTime = System.nanoTime(); + final Set> violations = validator.validate(result); + final long endTime = System.nanoTime(); + final long durationMs = (endTime - startTime) / 1_000_000; + + // Then Validation Fails Quickly Without Excessive Backtracking + assertThat(violations, not(empty())); + // Validation should complete quickly - 500ms is generous for a regex match + assertThat("Validation should complete in less than 500ms", durationMs < 500); + } } From 335150c1ad14b7e1574314aee68843f6f8063993 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 03:55:39 +0000 Subject: [PATCH 07/10] Replace regex with custom DurationValidator to address SonarCloud code smell Co-authored-by: thomasturrell <1552612+thomasturrell@users.noreply.github.com> --- .../java/dev/learning/xapi/model/Result.java | 12 +- .../validation/constraints/ValidDuration.java | 64 ++++++ .../validators/DurationValidator.java | 214 ++++++++++++++++++ 3 files changed, 280 insertions(+), 10 deletions(-) create mode 100644 xapi-model/src/main/java/dev/learning/xapi/model/validation/constraints/ValidDuration.java create mode 100644 xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java diff --git a/xapi-model/src/main/java/dev/learning/xapi/model/Result.java b/xapi-model/src/main/java/dev/learning/xapi/model/Result.java index ee770028..6da6efc2 100644 --- a/xapi-model/src/main/java/dev/learning/xapi/model/Result.java +++ b/xapi-model/src/main/java/dev/learning/xapi/model/Result.java @@ -8,8 +8,8 @@ import com.fasterxml.jackson.annotation.JsonInclude.Include; import dev.learning.xapi.model.validation.constraints.HasScheme; import dev.learning.xapi.model.validation.constraints.VaildScore; +import dev.learning.xapi.model.validation.constraints.ValidDuration; import jakarta.validation.Valid; -import jakarta.validation.constraints.Pattern; import java.net.URI; import java.util.LinkedHashMap; import java.util.function.Consumer; @@ -41,15 +41,7 @@ public class Result { private String response; /** Period of time over which the Statement occurred. */ - // Java Duration does not store ISO 8601:2004 durations. - // Using possessive quantifiers on digits to prevent ReDoS (Regular Expression Denial of Service) - @Pattern( - regexp = - "^P\\d++W$|^P(?!$)(\\d++Y)?(\\d++M)?" - + "(\\d++D)?(T(?=\\d)(\\d++H)?(\\d++M)?((?:\\d++\\.\\d++|\\d++)S)?)?$", - flags = Pattern.Flag.CASE_INSENSITIVE, - message = "Must be a valid ISO 8601:2004 duration format.") - private String duration; + @ValidDuration private String duration; private LinkedHashMap<@HasScheme URI, Object> extensions; diff --git a/xapi-model/src/main/java/dev/learning/xapi/model/validation/constraints/ValidDuration.java b/xapi-model/src/main/java/dev/learning/xapi/model/validation/constraints/ValidDuration.java new file mode 100644 index 00000000..f3b3a3fc --- /dev/null +++ b/xapi-model/src/main/java/dev/learning/xapi/model/validation/constraints/ValidDuration.java @@ -0,0 +1,64 @@ +/* + * Copyright 2016-2025 Berry Cloud Ltd. All rights reserved. + */ + +package dev.learning.xapi.model.validation.constraints; + +import static java.lang.annotation.ElementType.ANNOTATION_TYPE; +import static java.lang.annotation.ElementType.CONSTRUCTOR; +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.PARAMETER; +import static java.lang.annotation.ElementType.TYPE_USE; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +import dev.learning.xapi.model.validation.internal.validators.DurationValidator; +import jakarta.validation.Constraint; +import jakarta.validation.Payload; +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +/** + * The annotated element must be a valid ISO 8601:2004 duration format. + * + *

Accepts formats like: + * + *

+ * + * @author Berry Cloud + * @see xAPI + * Result + */ +@Documented +@Constraint(validatedBy = {DurationValidator.class}) +@Target({METHOD, FIELD, ANNOTATION_TYPE, CONSTRUCTOR, PARAMETER, TYPE_USE}) +@Retention(RUNTIME) +public @interface ValidDuration { + + /** + * Error Message. + * + * @return the error message + */ + String message() default "Must be a valid ISO 8601:2004 duration format."; + + /** + * Groups. + * + * @return the validation groups + */ + Class[] groups() default {}; + + /** + * Payload. + * + * @return the payload + */ + Class[] payload() default {}; +} diff --git a/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java b/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java new file mode 100644 index 00000000..dab0ceb0 --- /dev/null +++ b/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java @@ -0,0 +1,214 @@ +/* + * Copyright 2016-2025 Berry Cloud Ltd. All rights reserved. + */ + +package dev.learning.xapi.model.validation.internal.validators; + +import dev.learning.xapi.model.validation.constraints.ValidDuration; +import jakarta.validation.ConstraintValidator; +import jakarta.validation.ConstraintValidatorContext; +import java.util.regex.Pattern; + +/** + * Validates ISO 8601:2004 duration format strings. + * + *

This validator uses a programmatic approach to avoid complex regex patterns that could be + * flagged as code smells. The validation is broken down into logical parts: + * + *

+ * + *

Uses possessive quantifiers (++) to prevent ReDoS attacks. + * + * @author Berry Cloud + */ +public class DurationValidator implements ConstraintValidator { + + // Simple patterns using possessive quantifiers to prevent ReDoS + private static final Pattern WEEK_PATTERN = + Pattern.compile("^P\\d++W$", Pattern.CASE_INSENSITIVE); + private static final Pattern DIGITS_PATTERN = Pattern.compile("^\\d++$"); + private static final Pattern DECIMAL_PATTERN = Pattern.compile("^\\d++\\.\\d++$"); + + @Override + public boolean isValid(String value, ConstraintValidatorContext context) { + if (value == null) { + return true; + } + + // Check for week format first (P[n]W) + if (WEEK_PATTERN.matcher(value).matches()) { + return true; + } + + // Must start with P + if (!value.toUpperCase().startsWith("P")) { + return false; + } + + // Remove P prefix for processing + String remaining = value.substring(1); + + // Empty after P is invalid + if (remaining.isEmpty()) { + return false; + } + + // Check if there's a time component (T separator) + int timeIndex = remaining.toUpperCase().indexOf('T'); + + String datePart; + String timePart; + + if (timeIndex >= 0) { + datePart = remaining.substring(0, timeIndex); + timePart = remaining.substring(timeIndex + 1); + + // T must be followed by time components + if (timePart.isEmpty()) { + return false; + } + } else { + datePart = remaining; + timePart = null; + } + + // Validate date part (Y, M, D components) + if (!datePart.isEmpty() && !isValidDatePart(datePart.toUpperCase())) { + return false; + } + + // Validate time part (H, M, S components) + if (timePart != null && !isValidTimePart(timePart.toUpperCase())) { + return false; + } + + // Must have at least one component + return !datePart.isEmpty() || timePart != null; + } + + /** + * Validates the date part of the duration (Y, M, D components). + * + *

Components must appear in order: Years, Months, Days. + */ + private boolean isValidDatePart(String datePart) { + if (datePart.isEmpty()) { + return true; + } + + int pos = 0; + + // Check for Years + int yearIndex = datePart.indexOf('Y'); + if (yearIndex >= 0) { + if (yearIndex == 0) { + return false; // No digits before Y + } + String digits = datePart.substring(pos, yearIndex); + if (!isValidDigits(digits)) { + return false; + } + pos = yearIndex + 1; + } + + // Check for Months + int monthIndex = datePart.indexOf('M', pos); + if (monthIndex >= 0) { + if (monthIndex == pos) { + return false; // No digits before M + } + String digits = datePart.substring(pos, monthIndex); + if (!isValidDigits(digits)) { + return false; + } + pos = monthIndex + 1; + } + + // Check for Days + int dayIndex = datePart.indexOf('D', pos); + if (dayIndex >= 0) { + if (dayIndex == pos) { + return false; // No digits before D + } + String digits = datePart.substring(pos, dayIndex); + if (!isValidDigits(digits)) { + return false; + } + pos = dayIndex + 1; + } + + // Nothing should remain after processing + return pos == datePart.length(); + } + + /** + * Validates the time part of the duration (H, M, S components). + * + *

Components must appear in order: Hours, Minutes, Seconds. Only seconds can have decimals. + */ + private boolean isValidTimePart(String timePart) { + if (timePart.isEmpty()) { + return false; // T must be followed by components + } + + int pos = 0; + + // Check for Hours + int hourIndex = timePart.indexOf('H'); + if (hourIndex >= 0) { + if (hourIndex == 0) { + return false; // No digits before H + } + String digits = timePart.substring(pos, hourIndex); + if (!isValidDigits(digits)) { + return false; + } + pos = hourIndex + 1; + } + + // Check for Minutes + int minuteIndex = timePart.indexOf('M', pos); + if (minuteIndex >= 0) { + if (minuteIndex == pos) { + return false; // No digits before M + } + String digits = timePart.substring(pos, minuteIndex); + if (!isValidDigits(digits)) { + return false; + } + pos = minuteIndex + 1; + } + + // Check for Seconds (can be decimal) + int secondIndex = timePart.indexOf('S', pos); + if (secondIndex >= 0) { + if (secondIndex == pos) { + return false; // No digits before S + } + String digits = timePart.substring(pos, secondIndex); + if (!isValidDigitsOrDecimal(digits)) { + return false; + } + pos = secondIndex + 1; + } + + // Nothing should remain after processing and at least one component must be present + return pos == timePart.length() && timePart.length() > 0; + } + + /** Checks if the string contains only digits. */ + private boolean isValidDigits(String value) { + return !value.isEmpty() && DIGITS_PATTERN.matcher(value).matches(); + } + + /** Checks if the string contains digits or a decimal number. */ + private boolean isValidDigitsOrDecimal(String value) { + if (value.isEmpty()) { + return false; + } + return DIGITS_PATTERN.matcher(value).matches() || DECIMAL_PATTERN.matcher(value).matches(); + } +} From cf48db33f627a7b6217f4d854cbfce6321153883 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 04:16:45 +0000 Subject: [PATCH 08/10] Simplify DurationValidator - use programmatic approach without regex Co-authored-by: thomasturrell <1552612+thomasturrell@users.noreply.github.com> --- .../validators/DurationValidator.java | 207 +++++------------- 1 file changed, 52 insertions(+), 155 deletions(-) diff --git a/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java b/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java index dab0ceb0..8fb9dc41 100644 --- a/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java +++ b/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java @@ -7,208 +7,105 @@ import dev.learning.xapi.model.validation.constraints.ValidDuration; import jakarta.validation.ConstraintValidator; import jakarta.validation.ConstraintValidatorContext; -import java.util.regex.Pattern; /** * Validates ISO 8601:2004 duration format strings. * - *

This validator uses a programmatic approach to avoid complex regex patterns that could be - * flagged as code smells. The validation is broken down into logical parts: - * - *

- * - *

Uses possessive quantifiers (++) to prevent ReDoS attacks. + *

Supports formats: P[n]W, P[n]Y[n]M[n]DT[n]H[n]M[n]S and variations. * * @author Berry Cloud */ public class DurationValidator implements ConstraintValidator { - // Simple patterns using possessive quantifiers to prevent ReDoS - private static final Pattern WEEK_PATTERN = - Pattern.compile("^P\\d++W$", Pattern.CASE_INSENSITIVE); - private static final Pattern DIGITS_PATTERN = Pattern.compile("^\\d++$"); - private static final Pattern DECIMAL_PATTERN = Pattern.compile("^\\d++\\.\\d++$"); - @Override public boolean isValid(String value, ConstraintValidatorContext context) { if (value == null) { return true; } - // Check for week format first (P[n]W) - if (WEEK_PATTERN.matcher(value).matches()) { - return true; - } + String upper = value.toUpperCase(); // Must start with P - if (!value.toUpperCase().startsWith("P")) { + if (!upper.startsWith("P") || upper.length() < 2) { return false; } - // Remove P prefix for processing - String remaining = value.substring(1); + String rest = upper.substring(1); - // Empty after P is invalid - if (remaining.isEmpty()) { - return false; + // Week format: P[n]W + if (rest.endsWith("W") && rest.length() > 1) { + return isDigits(rest.substring(0, rest.length() - 1)); } - // Check if there's a time component (T separator) - int timeIndex = remaining.toUpperCase().indexOf('T'); + // Split by T to get date and time parts + int tpos = rest.indexOf('T'); + String datePart = tpos >= 0 ? rest.substring(0, tpos) : rest; + String timePart = tpos >= 0 ? rest.substring(tpos + 1) : ""; - String datePart; - String timePart; - - if (timeIndex >= 0) { - datePart = remaining.substring(0, timeIndex); - timePart = remaining.substring(timeIndex + 1); - - // T must be followed by time components - if (timePart.isEmpty()) { - return false; - } - } else { - datePart = remaining; - timePart = null; + // Must have at least one component + if (datePart.isEmpty() && timePart.isEmpty()) { + return false; } - // Validate date part (Y, M, D components) - if (!datePart.isEmpty() && !isValidDatePart(datePart.toUpperCase())) { + // Validate date part (Y, M, D in order) + if (!datePart.isEmpty() && !isValidPart(datePart, "YMD", false)) { return false; } - // Validate time part (H, M, S components) - if (timePart != null && !isValidTimePart(timePart.toUpperCase())) { + // Validate time part (H, M, S in order, S can be decimal) + if (!timePart.isEmpty() && !isValidPart(timePart, "HMS", true)) { return false; } - // Must have at least one component - return !datePart.isEmpty() || timePart != null; + return true; } - /** - * Validates the date part of the duration (Y, M, D components). - * - *

Components must appear in order: Years, Months, Days. - */ - private boolean isValidDatePart(String datePart) { - if (datePart.isEmpty()) { - return true; - } - + private boolean isValidPart(String part, String designators, boolean allowDecimalLast) { int pos = 0; - - // Check for Years - int yearIndex = datePart.indexOf('Y'); - if (yearIndex >= 0) { - if (yearIndex == 0) { - return false; // No digits before Y - } - String digits = datePart.substring(pos, yearIndex); - if (!isValidDigits(digits)) { - return false; - } - pos = yearIndex + 1; - } - - // Check for Months - int monthIndex = datePart.indexOf('M', pos); - if (monthIndex >= 0) { - if (monthIndex == pos) { - return false; // No digits before M - } - String digits = datePart.substring(pos, monthIndex); - if (!isValidDigits(digits)) { - return false; + boolean found = false; + + for (int i = 0; i < designators.length(); i++) { + char designator = designators.charAt(i); + int idx = part.indexOf(designator, pos); + if (idx > pos) { + String digits = part.substring(pos, idx); + boolean isLast = i == designators.length() - 1; + if (!(isLast && allowDecimalLast ? isDigitsOrDecimal(digits) : isDigits(digits))) { + return false; + } + pos = idx + 1; + found = true; + } else if (idx == pos) { + return false; // Designator without preceding digits } - pos = monthIndex + 1; } - // Check for Days - int dayIndex = datePart.indexOf('D', pos); - if (dayIndex >= 0) { - if (dayIndex == pos) { - return false; // No digits before D - } - String digits = datePart.substring(pos, dayIndex); - if (!isValidDigits(digits)) { - return false; - } - pos = dayIndex + 1; - } - - // Nothing should remain after processing - return pos == datePart.length(); + return found && pos == part.length(); } - /** - * Validates the time part of the duration (H, M, S components). - * - *

Components must appear in order: Hours, Minutes, Seconds. Only seconds can have decimals. - */ - private boolean isValidTimePart(String timePart) { - if (timePart.isEmpty()) { - return false; // T must be followed by components - } - - int pos = 0; - - // Check for Hours - int hourIndex = timePart.indexOf('H'); - if (hourIndex >= 0) { - if (hourIndex == 0) { - return false; // No digits before H - } - String digits = timePart.substring(pos, hourIndex); - if (!isValidDigits(digits)) { - return false; - } - pos = hourIndex + 1; - } - - // Check for Minutes - int minuteIndex = timePart.indexOf('M', pos); - if (minuteIndex >= 0) { - if (minuteIndex == pos) { - return false; // No digits before M - } - String digits = timePart.substring(pos, minuteIndex); - if (!isValidDigits(digits)) { - return false; - } - pos = minuteIndex + 1; + private boolean isDigits(String s) { + if (s.isEmpty()) { + return false; } - - // Check for Seconds (can be decimal) - int secondIndex = timePart.indexOf('S', pos); - if (secondIndex >= 0) { - if (secondIndex == pos) { - return false; // No digits before S - } - String digits = timePart.substring(pos, secondIndex); - if (!isValidDigitsOrDecimal(digits)) { + for (int i = 0; i < s.length(); i++) { + if (!Character.isDigit(s.charAt(i))) { return false; } - pos = secondIndex + 1; } - - // Nothing should remain after processing and at least one component must be present - return pos == timePart.length() && timePart.length() > 0; + return true; } - /** Checks if the string contains only digits. */ - private boolean isValidDigits(String value) { - return !value.isEmpty() && DIGITS_PATTERN.matcher(value).matches(); - } - - /** Checks if the string contains digits or a decimal number. */ - private boolean isValidDigitsOrDecimal(String value) { - if (value.isEmpty()) { + private boolean isDigitsOrDecimal(String s) { + if (s.isEmpty()) { return false; } - return DIGITS_PATTERN.matcher(value).matches() || DECIMAL_PATTERN.matcher(value).matches(); + int dotPos = s.indexOf('.'); + if (dotPos < 0) { + return isDigits(s); + } + return dotPos > 0 + && dotPos < s.length() - 1 + && isDigits(s.substring(0, dotPos)) + && isDigits(s.substring(dotPos + 1)); } } From 839e2caac03054c40d2f4cd1e411c6e5bf17c88f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 04:52:28 +0000 Subject: [PATCH 09/10] Simplify DurationValidator using simple regex patterns for date/time parts Co-authored-by: thomasturrell <1552612+thomasturrell@users.noreply.github.com> --- .../validators/DurationValidator.java | 86 ++++++------------- 1 file changed, 25 insertions(+), 61 deletions(-) diff --git a/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java b/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java index 8fb9dc41..780a2d3e 100644 --- a/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java +++ b/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java @@ -7,6 +7,8 @@ import dev.learning.xapi.model.validation.constraints.ValidDuration; import jakarta.validation.ConstraintValidator; import jakarta.validation.ConstraintValidatorContext; +import java.util.regex.Matcher; +import java.util.regex.Pattern; /** * Validates ISO 8601:2004 duration format strings. @@ -17,28 +19,33 @@ */ public class DurationValidator implements ConstraintValidator { + // Simple patterns - each validates a single component type + private static final Pattern WEEK = Pattern.compile("^\\d+W$", Pattern.CASE_INSENSITIVE); + private static final Pattern DATE = + Pattern.compile("^(\\d+Y)?(\\d+M)?(\\d+D)?$", Pattern.CASE_INSENSITIVE); + private static final Pattern TIME = + Pattern.compile("^(\\d+H)?(\\d+M)?((\\d+\\.\\d+|\\d+)S)?$", Pattern.CASE_INSENSITIVE); + @Override public boolean isValid(String value, ConstraintValidatorContext context) { if (value == null) { return true; } - String upper = value.toUpperCase(); - - // Must start with P - if (!upper.startsWith("P") || upper.length() < 2) { + // Must start with P and have at least one component + if (!value.toUpperCase().startsWith("P") || value.length() < 2) { return false; } - String rest = upper.substring(1); + String rest = value.substring(1); // Week format: P[n]W - if (rest.endsWith("W") && rest.length() > 1) { - return isDigits(rest.substring(0, rest.length() - 1)); + if (WEEK.matcher(rest).matches()) { + return true; } // Split by T to get date and time parts - int tpos = rest.indexOf('T'); + int tpos = rest.toUpperCase().indexOf('T'); String datePart = tpos >= 0 ? rest.substring(0, tpos) : rest; String timePart = tpos >= 0 ? rest.substring(tpos + 1) : ""; @@ -47,65 +54,22 @@ public boolean isValid(String value, ConstraintValidatorContext context) { return false; } - // Validate date part (Y, M, D in order) - if (!datePart.isEmpty() && !isValidPart(datePart, "YMD", false)) { - return false; - } - - // Validate time part (H, M, S in order, S can be decimal) - if (!timePart.isEmpty() && !isValidPart(timePart, "HMS", true)) { - return false; - } - - return true; - } - - private boolean isValidPart(String part, String designators, boolean allowDecimalLast) { - int pos = 0; - boolean found = false; - - for (int i = 0; i < designators.length(); i++) { - char designator = designators.charAt(i); - int idx = part.indexOf(designator, pos); - if (idx > pos) { - String digits = part.substring(pos, idx); - boolean isLast = i == designators.length() - 1; - if (!(isLast && allowDecimalLast ? isDigitsOrDecimal(digits) : isDigits(digits))) { - return false; - } - pos = idx + 1; - found = true; - } else if (idx == pos) { - return false; // Designator without preceding digits + // Validate date part - must match pattern and have at least one component + if (!datePart.isEmpty()) { + Matcher m = DATE.matcher(datePart); + if (!m.matches() || (m.group(1) == null && m.group(2) == null && m.group(3) == null)) { + return false; } } - return found && pos == part.length(); - } - - private boolean isDigits(String s) { - if (s.isEmpty()) { - return false; - } - for (int i = 0; i < s.length(); i++) { - if (!Character.isDigit(s.charAt(i))) { + // Validate time part - must match pattern and have at least one component + if (!timePart.isEmpty()) { + Matcher m = TIME.matcher(timePart); + if (!m.matches() || (m.group(1) == null && m.group(2) == null && m.group(3) == null)) { return false; } } - return true; - } - private boolean isDigitsOrDecimal(String s) { - if (s.isEmpty()) { - return false; - } - int dotPos = s.indexOf('.'); - if (dotPos < 0) { - return isDigits(s); - } - return dotPos > 0 - && dotPos < s.length() - 1 - && isDigits(s.substring(0, dotPos)) - && isDigits(s.substring(dotPos + 1)); + return true; } } From d63fa5c3aad9ab97e982622a51c3518f3ea54f3e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 25 Nov 2025 05:10:12 +0000 Subject: [PATCH 10/10] Reduce cognitive complexity by extracting validation into private methods Co-authored-by: thomasturrell <1552612+thomasturrell@users.noreply.github.com> --- .../validators/DurationValidator.java | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java b/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java index 780a2d3e..81ce2cb5 100644 --- a/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java +++ b/xapi-model/src/main/java/dev/learning/xapi/model/validation/internal/validators/DurationValidator.java @@ -32,44 +32,40 @@ public boolean isValid(String value, ConstraintValidatorContext context) { return true; } - // Must start with P and have at least one component if (!value.toUpperCase().startsWith("P") || value.length() < 2) { return false; } String rest = value.substring(1); - // Week format: P[n]W if (WEEK.matcher(rest).matches()) { return true; } - // Split by T to get date and time parts int tpos = rest.toUpperCase().indexOf('T'); String datePart = tpos >= 0 ? rest.substring(0, tpos) : rest; String timePart = tpos >= 0 ? rest.substring(tpos + 1) : ""; - // Must have at least one component if (datePart.isEmpty() && timePart.isEmpty()) { return false; } - // Validate date part - must match pattern and have at least one component - if (!datePart.isEmpty()) { - Matcher m = DATE.matcher(datePart); - if (!m.matches() || (m.group(1) == null && m.group(2) == null && m.group(3) == null)) { - return false; - } - } + return isValidDatePart(datePart) && isValidTimePart(timePart); + } - // Validate time part - must match pattern and have at least one component - if (!timePart.isEmpty()) { - Matcher m = TIME.matcher(timePart); - if (!m.matches() || (m.group(1) == null && m.group(2) == null && m.group(3) == null)) { - return false; - } + private boolean isValidDatePart(String datePart) { + if (datePart.isEmpty()) { + return true; } + Matcher m = DATE.matcher(datePart); + return m.matches() && (m.group(1) != null || m.group(2) != null || m.group(3) != null); + } - return true; + private boolean isValidTimePart(String timePart) { + if (timePart.isEmpty()) { + return true; + } + Matcher m = TIME.matcher(timePart); + return m.matches() && (m.group(1) != null || m.group(2) != null || m.group(3) != null); } }