Skip to content

Conversation

@tvdijen
Copy link
Contributor

@tvdijen tvdijen commented Feb 9, 2025

Closes #37

@pradtke
Copy link
Contributor

pradtke commented Feb 14, 2025

@tvdijen I think there is an incompatibility between SSP 2.3.5 and simplesamlphp/assert 1.8, at least when I try to use that version of assert I get a stack trace about Utils\Time calling an assert method that does not exist.

I tried updating the composer.json to mark simplesamlphp/assert:1.8 as conflict, which then installs assert:1.7.0 which resolves that issue. What do you suggest as path forward? I assume simplesamlphp/assert:1.8 will be a problem for anyone who installs it with the current release of SSP 2.3.

BadMethodCallException: Assertion named `validDuration` does not exists.

/Users/patrick/Desktop/cirrus/ssp-modules/simplesamlphp-module-ratelimit/vendor/simplesamlphp/assert/src/Assert.php:379
/Users/patrick/Desktop/cirrus/ssp-modules/simplesamlphp-module-ratelimit/vendor/simplesamlphp/simplesamlphp/src/SimpleSAML/Utils/Time.php:94
/Users/patrick/Desktop/cirrus/ssp-modules/simplesamlphp-module-ratelimit/src/Limiters/UserPassBaseLimiter.php:35
/Users/patrick/Desktop/cirrus/ssp-modules/simplesamlphp-module-ratelimit/tests/src/Limiters/UsernameLimiterTest.php:28
/Users/patrick/Desktop/cirrus/ssp-modules/simplesamlphp-module-ratelimit/tests/src/Limiters/UsernameLimiterTest.php:20

@tvdijen
Copy link
Contributor Author

tvdijen commented Feb 14, 2025

We've fixed that yesterday, but haven't tagged 2.3.6 just yet

@pradtke
Copy link
Contributor

pradtke commented Feb 14, 2025

Thanks, and I could use some composer advice. Should I make the min version for this module 2.3.6 so that it aligns with the v1.8.1 assert package in this module's composer.json? Or relax the min version of assert so this package can still be installed in SSP 2.3.5 and earlier?

@tvdijen
Copy link
Contributor Author

tvdijen commented Feb 14, 2025

I wouldn't change a thing.. The issue is in SSP and has to be fixed there. I'll try and do a 2.3.6-release this weekend.

@tvdijen
Copy link
Contributor Author

tvdijen commented Feb 17, 2025

Tagged SSP v2.3.6
@pradtke Would you mind re-running the tests?

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.32%. Comparing base (a0cbc32) to head (2623c13).
Report is 21 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #38      +/-   ##
============================================
- Coverage     95.37%   90.32%   -5.05%     
- Complexity       73       87      +14     
============================================
  Files             6        8       +2     
  Lines           216      310      +94     
============================================
+ Hits            206      280      +74     
- Misses           10       30      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pradtke pradtke merged commit f598544 into cirrusidentity:master May 5, 2025
11 checks passed
@tvdijen tvdijen deleted the patch-4 branch May 6, 2025 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

composer dependency requirements not allowing to get newer xml-common to satisfy the CVE

3 participants