Summary [ 166.110s] 1453 tests run: 1453 passed (1 slow), 15 skipped
Above is before this commit, below is after.
Summary [ 140.410s] 1453 tests run: 1453 passed, 15 skipped
The mechanism here is the same as for the metastore tests. By skipping fsync we can run the tests much faster, and fsync doesn't matter in tests. Unless you're testing postgres itself I guess
Replace sleeps with the existing polling function instead. Exposes that the stress test isn't much of a stress test, it probably needs more data or something, but if it's heavy it should maybe not be part of `just test`
`just test` doesn't use docker, but the cleanup was still pruning containers over and over. This switches to only cleaning up containers created by the tests.
test-unit was building a second copy of the code with different features, and then the main test command ran the same tests anyway, so we just built and ran the tests twice
Locally shaves a minute off of a full build+test, on this branch (so both runs include the fsync change etc)
We're at [ 173.444s] overall for `just test` now
Build optimized versions of bcrypt and blowfish to speed them up, referencing things like https://github.com/oxidecomputer/omicron/blob/main/Cargo.toml#L981-L982
Password hashing is not a dominating cost, so this doesn't have a significant impact on the overall time, but a lot of tests like `**tranquil-pds::legacy_2fa** **test_legacy_2fa_code_consumed_after_use**` speed up. It goes from 5.6s to 2.7s on my machine.
Flushing to disk all the time ensures no partial states when writing to disk during awkwardnesses like crashes in prod risking real user data. That's good!
We're also doing it while running tests. Less good! Well, at least not important!
Comparing a clean test run (`just test --no-fail-fast`) and a run on this branch, on my MacOS M4 Pro, I get:
`**Summary** [ 517.723s] **1424** tests run: **1405** **passed** (**7** **slow**), **19** **failed**, **15** **skipped**`
before and
`**Summary** [ 326.369s] **1424** tests run: **1405** **passed**, **19** **failed**, **15** **skipped**`
after.
The reason I run `--no-fail-fast` is that a bunch of tests always fail and time out and stuff
`rpc:*?aud=did:web:api.bsky.app#bsky_appview` fails in the permissions check even though it's a valid scope. This makes it resolve correctly.
Also this commit fixes a bug where `rpc` as a bare scope is interpreted as all methods for all services, but it's really an invalid scope. Can drop it if it's a problem, but it just looks like a bug in the implementation rather than something intended?
Note that the parser treats `rpc:*` as "allow everything for no services", but aud is actually required, so it's not a valid scope? Didn't mess with it though because delegation uses it and that part of the code treats it as allow everything for every service. We should maybe split the scope parsers so we have one for delegated scopes and one for regular?
However, to avoid the same bug as bare `rpc`, I changed the enforcement step in "regular" OAuth to disallow `rpc:*` (this matches the existing behavior on main, no actual change, just a little acknowledgement).
Updates the default runner debian image sha to the multi-arch image instead of amd64-only. Also symlinks .containerignore -> .dockerignore since docker refuses to read .containerignore.
With that one can build and push multi-arch, like so:
```
docker buildx create --name tranquil-multiarch --driver docker-container --bootstrap
docker buildx build \
--builder tranquil-multiarch \
--platform linux/amd64,linux/arm64 \
-t atcr.io/tranquil.farm/tranquil-pds:0.6.7 \
--push \
.
docker buildx imagetools create \
-t atcr.io/tranquil.farm/tranquil-pds:latest \
atcr.io/tranquil.farm/tranquil-pds:0.6.7
```
if one were so inclined, that is.
This improves mobile behavior for the inputs without affecting other things.
Two inputs set to email, rest to URL pre recommendation in https://tangled.org/tranquil.farm/tranquil-pds/issues/140
Also disables autocapitalize, autocorrect, and spellcheck, while we're at it. All really annoying when typing in handles on a mobile phone.
My last PR introduced a migration to blobs, changing them from being unique per cid to being unique per cid+user. This is because two users can upload the same blob, get the same cid, and then previously in tranquil the second user would never actually get their ownership recorded, meaning listBlobs was missing the blob, and a migration _off_ of tranquil would leave them sans blob https://tangled.org/tranquil.farm/tranquil-pds/pulls/261/round/3
This PR attempts to add a job to repair existing instances where blobs have been deduplicated and not attributed to each owning user. It does this by pulling all records for each user, and then in batches crawling the records to find blob references. For each blob reference, it checks whether that user has its ownership recorded, and if not, repairs it by adding the entry.
To avoid re-running this potentially heavy job on every startup, I've used the config table to store a little record of whether it has been run.
This also means we can clean up the last blob operations that were per cid rather than user+cid 🪓
We should be setting ourselves up to be able to go garbage collect blobs safely so deleting the logo blobs manually won't matter anyway.
The current tranquil database design only allows each blob to be owned by one account. This means that if a second account also has that blob, tranquil skips associated the blob with the account. That works fine a lot of the time, since blobs are looked up by cid and the blob exists. However, it can lead to loss of data under certain scenarios.
One example is where I upload a blob, the blob already exists in my instance so insertion is skipped (postgres requires cid to be unique in blobs, fjall only allows one owner per blob cid), I then decide to migrate off tranquil, the blob does not come with me since it is not mine.
Another example is where an account is deleted. If a blob was uploaded for account a, then account b uploads the same blob tranquil skips storing it since it exists. Then I delete account a, now account b's blob is missing.
I accidentally stumbled upon this when I migrated my account to my own tranuil instance and list blobs now lists 2 fewer blobs than before, two images that had been uploaded by accounts already on the PDS.
ps I found record_blobs a bit confusing, at first it looked like a blob ownership table, but then it turns out to just be used for migrations!
This PR makes the blob primary key be cid+user for postgres, and updates the queries to account for there being multiple "blobs" with the same cid. For queries that just care about the blob existing, it doesn't matter "whose" blob it is, so limit 1.
Most of the work is on the metastore side. Adds ref_count to track how many are referencing the blob since we can't just check for other rows. Instead of storing blobs directly, we now store a per account cid, and the blob reference itself is shared and keyed by cid only. This means some of these operations now require updating two places, so they're done in `batch`es.
With the new layout get_blob_value becomes simpler, all blob data is a single "table" or whatever it's called, so we just grab it using cid instead of looking it up for the user.
Migrates blobs rather than maintaining two different versions of the tables, although it seems like that could be supported.
I removed a test that asserted the old behavior, and added a reasonable (?) set of new tests that assert the new behavior, including a parity test.
This adds security alerts section to the communication settings, with one new item: a toggle for enabling/disabling legacy login alerts. It's enabled by default. Disabling it means that you no longer get emails when logging in through a non-2FA enabled login flow, like createSession.
The reason I tackled this is that I have a bot account that automatically refreshes its session, using username and app password, and I'm close to having 100 of these emails in my inbox. I also tried to just disable TOTP but I wasn't allowed (I fixed that in a different PR).
English and Swedish translations were me, the rest were MyMemory. I imagine someone can improve on them after this is merged!
This removes the `require_legacy_session_mfa` call on the TOTP disable path. It looks at the last session, which isn't really relevant, and we're getting a fresh TOTP from the form anyway, which should be enough proof to allow disabling TOTP.
The way I bumped into this was having a "bot account" log in with app password on some app on a schdule, triggering lots of warning emails. So I tried to disable TOTP to stop the flood and wasn't allowed.
Running tranquil with `just run-dev` using `pds.test` seems to be broken due to a recent change that blocks requests to private ips. Inside the compose network `pds.test` resolves to the traefik container's private IP. So it can't make requests to stuff like `https://pds.test/oauth-client-metadata.json` or the local plc.
Introduces a new flag, default off, that allows connecting to private IPs, set to true for the dev compose.
Johanna LarssonandTangled
LewisandTangled
juneandTangled
Louis EscherandTangled
nelind
Anastasia CalicoandTangled
TrezyandTangled
LunaandTangled
xan.lolandTangled
SteffoandTangled