Repository: lldap/lldap (language: rust)

Issue:
[BUG] creating new users in parallel using GraphQL API and sqlite as database can result in `error returned from database: (code: 5) database is locked`
**Describe the bug**

Creating new users in parallel using GraphQL API and sqlite as database can result in `error returned from database: (code: 5) database is locked`. The affected user isn't created.


**To Reproduce**

The following bash script starts `lldap:latest` in a Docker container and then proceeds to create 50 users at once, it yields the error reliably on my machine, usually several times:

```
#!/usr/bin/env bash

set -eo pipefail

function auth() {
  local lldap_http_host="$1"
  local password="$2"
  local response
  response=$(curl \
    --silent \
    --request POST \
    --url "${lldap_http_host}/auth/simple/login" \
    --header 'Content-Type: application/json' \
    --data "{ \"username\" : \"admin\", \"password\" : \"${password}\" }")
  echo "$response" | jq --raw-output '.token'
}

function create_user() {
  local lldap_http_host="$1"
  local token="$2"
  local username="$3"
  local request
  request="$(cat <<EOF
{
  "query" : "mutation CreateUser(\$user: CreateUserInput!) {createUser(user: \$user) {id creationDate}}",
  "operationName" : "CreateUser",
  "variables" : {
    "user" : {
        "id": "${username}",
        "email": "${username}@mail.test"
    }
  }
}
EOF
)"
  local response
  response=$(curl \
    --silent \
    --request POST \
    --url "${lldap_http_host}/api/graphql" \
    --header "Authorization: Bearer ${token}" \
    --header 'Content-Type: application/json' \
    --data "$request")
  if [[ ! "$response" == '{"data":{"createUser":{"id":"testuser'* ]]
  then
    >&2 echo "Unexpected response: ${response}"
  fi
}

password=$(tr -dc A-Za-z0-9 </dev/urandom | head -c 18; echo)
cnt_id=$(docker run \
    --detach \
    --rm \
    --env "LLDAP_LDAP_USER_PASS=${password}" \
    lldap/lldap:latest)

function on_exit {
    docker stop "$cnt_id" > /dev/null || true
}

trap on_exit EXIT

docker logs -f "$cnt_id" &
sleep 3 # need to wait for lldap to properly start up

cnt_ip=$(docker inspect -f '{{range.NetworkSettings.Networks}}{{.IPAddress}}{{end}}' "$cnt_id")
lldap_http_host="http://${cnt_ip}:17170"
token=$(auth "$lldap_http_host" "$password")

for i in $(seq 1 50)
do
  create_user "$lldap_http_host" "$token" "testuser${i}" &
done
```


**Expected behavior**

At least for me it's:
- not expected that LLDAP doesn't simply wait for the lock
- not expected that LLDAP passes database errors unfiltered to the client, these can have security implications


**Logs**

Example lldap log output from above script:

```
2024-10-29T15:44:56.019184062+00:00  ERROR    🚨 [error]:  | error: Database error: `Execution Error: error returned from database: (code: 5) database is locked`
2024-10-29T15:44:56.019348453+00:00  ERROR    🚨 [error]:  | error: Database error: `Execution Error: error returned from database: (code: 5) database is locked`
2024-10-29T15:44:56.019863107+00:00  ERROR    🚨 [error]:  | error: Database error: `Execution Error: error returned from database: (code: 5) database is locked`
```



Discussion / hints:
Thanks for the bug and the reproduction! This is something that I expected
the ORM to handle, but it might take a bit more massaging to make it
happen. I haven't really tested contention for LLDAP.

On another note, how is displaying the database error a potential security
issue? Do you have a specific scenario in mind, or a recommendation from
some security experts?

On Tue, 29 Oct 2024, 17:27 Ansgar Tasler, ***@***.***> wrote:

> *Describe the bug*
>
> Creating new users in parallel using GraphQL API and sqlite as database
> can result in error returned from database: (code: 5) database is locked.
>
> *To Reproduce*
>
> The following bash script starts lldap:latest in a Docker container and
> then proceeds to create 50 users at once, it yields the error reliably on
> my machine, usually several times:
>
> #!/usr/bin/env bash
>
> set -eo pipefail
>
> function auth() {
>   local lldap_http_host="$1"
>   local password="$2"
>   local response
>   response=$(curl \
>     --silent \
>     --request POST \
>     --url "${lldap_http_host}/auth/simple/login" \
>     --header 'Content-Type: application/json' \
>     --data "{ \"username\" : \"admin\", \"password\" : \"${password}\" }")
>   echo "$response" | jq --raw-output '.token'
> }
>
> function create_user() {
>   local lldap_http_host="$1"
>   local token="$2"
>   local username="$3"
>   local request
>   request="$(cat <<EOF
> {
>   "query" : "mutation CreateUser(\$user: CreateUserInput!) {createUser(user: \$user) {id creationDate}}",
>   "operationName" : "CreateUser",
>   "variables" : {
>     "user" : {
>         "id": "${username}",
>         "email": ***@***.***"
>     }
>   }
> }
> EOF
> )"
>   local response
>   response=$(curl \
>     --silent \
>     --request POST \
>     --url "${lldap_http_host}/api/graphql" \
>     --header "Authorization: Bearer ${token}" \
>     --header 'Content-Type: application/json' \
>     --data "$request")
>   if [[ ! "$response" == '{"data":{"createUser":{"id":"testuser'* ]]
>   then
>     >&2 echo "Unexpected response: ${response}"
>   fi
> }
>
> password=$(tr -dc A-Za-z0-9 </dev/urandom | head -c 18; echo)
> cnt_id=$(docker run \
>     --detach \
>     --rm \
>     --env "LLDAP_LDAP_USER_PASS=${password}" \
>     lldap/lldap:latest)
>
> function on_exit {
>     docker stop "$cnt_id" > /dev/null || true
> }
>
> trap on_exit EXIT
>
> docker logs -f "$cnt_id" &
> sleep 3 # need to wait for lldap to properly start up
>
> cnt_ip=$(docker inspect -f '{{range.NetworkSettings.Networks}}{{.IPAddress}}{{end}}' "$cnt_id")
> lldap_http_host="http://${cnt_ip}:17170"
> token=$(auth "$lldap_http_host" "$password")
>
> for i in $(seq 1 50)
> do
>   create_user "$lldap_http_host" "$token" "testuser${i}" &
> done
>
> *Expected behavior*
>
> At least for me it's:
>
>    - not expected that LLDAP doesn't simply wait for the lock
>    - not expected that LLDAP passes database errors unfiltered to the
>    client, these can have security implications
>
> *Logs*
>
> Example lldap log output from above script:
>
> 2024-10-29T15:44:56.019184062+00:00  ERROR    🚨 [error]:  | error: Database error: `Execution Error: error returned from database: (code: 5) database is locked`
> 2024-10-29T15:44:56.019348453+00:00  ERROR    🚨 [error]:  | error: Database error: `Execution Error: error returned from database: (code: 5) database is locked`
> 2024-10-29T15:44:56.019863107+00:00  ERROR    🚨 [error]:  | error: Database error: `Execution Error: error returned from database: (code: 5) database is locked`
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/lldap/lldap/issues/1021>, or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAGCPWKGJUZO4YI5GP525ETZ56ZN3AVCNFSM6AAAAABQ2IFSU6VHI2DSMVQWIX3LMV43ASLTON2WKOZSGYZDCNZWHA4DSNA>
> .
> You are receiving this because you are subscribed to this thread.Message
> ID: ***@***.***>
>

Relevant: 
https://github.com/launchbadge/sqlx/issues/459
https://github.com/launchbadge/sqlx/issues/451

We can probably use a pool of a single worker for our use case.

Fix patch:
diff --git a/server/src/infra/database_string.rs b/server/src/infra/database_string.rs
index 12d1b8ab..67b165f4 100644
--- a/server/src/infra/database_string.rs
+++ b/server/src/infra/database_string.rs
@@ -17,6 +17,12 @@ impl From<&str> for DatabaseUrl {
     }
 }
 
+impl DatabaseUrl {
+    pub fn db_type(&self) -> &str {
+        self.0.scheme()
+    }
+}
+
 impl std::fmt::Debug for DatabaseUrl {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         if self.0.password().is_some() {
diff --git a/server/src/main.rs b/server/src/main.rs
index 833ff059..6f42473c 100644
--- a/server/src/main.rs
+++ b/server/src/main.rs
@@ -82,9 +82,14 @@ async fn ensure_group_exists(handler: &SqlBackendHandler, group_name: &str) -> R
 
 async fn setup_sql_tables(database_url: &DatabaseUrl) -> Result<DatabaseConnection> {
     let sql_pool = {
+        let num_connections = if database_url.db_type() == "sqlite" {
+            1
+        } else {
+            5
+        };
         let mut sql_opt = sea_orm::ConnectOptions::new(database_url.to_string());
         sql_opt
-            .max_connections(5)
+            .max_connections(num_connections)
             .sqlx_logging(true)
             .sqlx_logging_level(log::LevelFilter::Debug);
         Database::connect(sql_opt).await?


Test patch: