Skip to content

chore: bypass slow password validation in integration tests#58610

Draft
icewind1991 wants to merge 2 commits intomasterfrom
integration-disable-slow-password-hashing
Draft

chore: bypass slow password validation in integration tests#58610
icewind1991 wants to merge 2 commits intomasterfrom
integration-disable-slow-password-hashing

Conversation

@icewind1991
Copy link
Member

From early testing it looks like we spend 50%-60% of the runtime of integration tests doing password validation 🙈

This enables the security_softening, which bypasses this by caching the password validation.

Signed-off-by: Robin Appelman <robin@icewind.nl>
@nickvergessen
Copy link
Member

From early testing it looks like we spend 50%-60% of the runtime of integration tests doing password validation

Did you also do:

config:system:set hashing_default_password --value=true --type=bool

locally when testing your improving? Otherwise that would explain the non-existing difference on CI :P

@@ -146,6 +153,7 @@ jobs:
mkdir data
./occ maintenance:install --verbose ${{ contains(matrix.test-suite,'ldap') && '--data-dir=/dev/shm/nc_int' || '' }} --database=sqlite --database-name=nextcloud --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass admin
./occ config:system:set hashing_default_password --value=true --type=boolean
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this should be moved to

if [ "$INSTALLED" == "true" ]; then
# Disable appstore to avoid spamming from CI
$OCC config:system:set appstoreenabled --value=false --type=boolean
# Disable bruteforce protection because the integration tests do trigger them
$OCC config:system:set auth.bruteforce.protection.enabled --value false --type bool
# Disable rate limit protection because the integration tests do trigger them
$OCC config:system:set ratelimit.protection.enabled --value false --type bool
# Allow local remote urls otherwise we can not share
$OCC config:system:set allow_local_remote_servers --value true --type bool
# Allow self signed certificates
$OCC config:system:set sharing.federation.allowSelfSignedCertificates --value true --type bool
# Allow creating users with dummy passwords
$OCC app:disable password_policy
else

so that people running it locally benefit as well

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
Copy link
Member Author

hashing_default_password doesn't seem to make much of a difference for me locally.

It seems to have just been a case of ACPu not being enabled before, the performance improvements seem to work now: 8m/15m for sharees_feautures/sharing_features compared to 13m/24m before

The various failures are probably a side effect of the added caching, will look into those

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.

2 participants