Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2884 +/- ##
==========================================
- Coverage 78.29% 78.25% -0.04%
==========================================
Files 673 673
Lines 55130 55192 +62
Branches 728 728
==========================================
+ Hits 43164 43192 +28
- Misses 11888 11922 +34
Partials 78 78
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@ryan-pratt seems like the CLI is failing now with an auth error. Can you investigate? |
ryan-pratt
force-pushed
the
maint/login
branch
from
c0eb2e0 to
beacc5d
Compare
|
What is this telling us about other people running COSMOS in a CI/CD pipeline? Will this be a breaking change for them as well? |
I won't know the details until I can get it to actually work, but we knew it was technically a breaking change from the outset (removing the ability to slam the auth endpoints, which someone could definitely be doing) |
because I think the scripts section is hitting it too fast
ryan-pratt
force-pushed
the
maint/login
branch
2 times, most recently
from
9c54cb4 to
8b4b46b
Compare
|
|
Ok, I gave up and just rolled my own rate limiting for bad pw attempts only, so this is not breaking for CI. Everything's passing now. |
jmthomas
left a comment
jmthomas
left a comment
There was a problem hiding this comment.
So you were probably just rate limiting based on good logins. Yeah this is much better.
| # Check stored password hash | ||
| pw_hash = Store.get(PRIMARY_KEY) | ||
| raise "invalid password hash" unless pw_hash.start_with?("$argon2") # Catch users who didn't run the migration utility when upgrading to COSMOS 7 | ||
| raise "invalid password hash" if pw_hash.nil? || !pw_hash.start_with?("$argon2") # Catch users who didn't run the migration utility when upgrading to COSMOS 7 |
There was a problem hiding this comment.
Only if it's a fresh install that never had a password set. I actually don't know if this is a real check that needs to be there, but I was getting "undefined method start_with? for nil" while messing with unit tests
I was rate limiting every request, good or bad. I did that intentionally, but it's become too much of a headache to do it that way. |
2 participants
Added rate limiting for failed auth attempts for the user and service passwords. The user and service passwords have their own counters so that a badly behaving service doesn't lock out the user. Counters are semaphored to be thread safe and effectively handle a brute force attack.
I made the threshold configurable in .env to allow people to set how aggressive they want it. I intentionally made it so you can't turn it off, but you can set arbitrarily high limits.