"Share board" adds user twice, causes server-side constraint violation and client crash

[eliasp]

Describe the bug
I found a certain constraint violation (details below) in my server logs over and over again.
It turned out to be caused by the Android Client trying to add a user twice to a board's ACL.
While trying to reproduce the issue a dialog with a client stacktrace appeared on 2 occasions (also below).

Steps to reproduce the behavior:

1. Go to 'Share board' modal
2. Add another user and grant it the "manage" permission
3. Go back to the board and refresh it by pulling down
4. Open the 'Share board' modal again
5. Sometimes, user is displayed twice, error on server is logged. Sometimes it seems the sync happens immediately and the user shows up a 2nd time even without closing the modal first and triggering a manual sync.

Expected behavior

• a user can only be added once to 'Share board'
• the app doesn't cause a server-side 500/constraint violation

Screenshots

Versions

• Nextcloud: 23.0.1
• Nextcloud Deck: 1.6.0
• Nextcloud Android: 3.19.0
• Nextcloud Android Deck: 1.20.0

Smartphone (please complete the following information):

• Device: OnePlus Nord
• Android-Version: 11
• App-Store:
  • Google Play Store
  • Google Play Store (Beta channel)
  • F-Droid
  • Huawei AppGallery

Stacktrace

• a ~500 line App debug log can be provided privately on request - I'll keep it around for some time

• App Stacktrace shown at least twice when trying to reproduce the issue:

  App Version: 1.20.0
  App Version Code: 1020000
  Server App Version: 1.6.0
  App Flavor: fdroid
  
  Files App Version Code: 30190090
  
  ---
  
  OS Version: 4.19.113-perf+(2104202034)
  OS API Level: 30
  Device: Nord
  Manufacturer: OnePlus
  Model (and Product): AC2003 (Nord_EEA)
  
  ---
  
  com.nextcloud.android.sso.exceptions.NextcloudHttpRequestFailedException: HTTP request failed with HTTP status-code: 403
  	at com.nextcloud.android.sso.api.AidlNetworkRequest.performNetworkRequestV2(AidlNetworkRequest.java:188)
  	at com.nextcloud.android.sso.api.NextcloudAPI.performNetworkRequestV2(NextcloudAPI.java:199)
  	at com.nextcloud.android.sso.api.NextcloudAPI.lambda$performRequestObservableV2$1$com-nextcloud-android-sso-api-NextcloudAPI(NextcloudAPI.java:129)
  	at com.nextcloud.android.sso.api.NextcloudAPI$$ExternalSyntheticLambda2.subscribe(Unknown Source:6)
  	at io.reactivex.internal.operators.observable.ObservableFromPublisher.subscribeActual(ObservableFromPublisher.java:31)
  	at io.reactivex.Observable.subscribe(Observable.java:12284)
  	at io.reactivex.internal.operators.observable.ObservableMap.subscribeActual(ObservableMap.java:32)
  	at io.reactivex.Observable.subscribe(Observable.java:12284)
  	at io.reactivex.internal.operators.observable.ObservableSubscribeOn$SubscribeTask.run(ObservableSubscribeOn.java:96)
  	at io.reactivex.internal.schedulers.ScheduledDirectTask.call(ScheduledDirectTask.java:38)
  	at io.reactivex.internal.schedulers.ScheduledDirectTask.call(ScheduledDirectTask.java:26)
  	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
  	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
  	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
  	at java.lang.Thread.run(Thread.java:923)
  Caused by: java.lang.IllegalStateException: {"status":403,"message":"Permission denied"}
  
  	at com.nextcloud.android.sso.InputStreamBinder.processRequestV2(InputStreamBinder.java:455)
  	at com.nextcloud.android.sso.InputStreamBinder.performNextcloudRequestAndBodyStreamV2(InputStreamBinder.java:128)
  	at com.nextcloud.android.sso.InputStreamBinder.performNextcloudRequestV2(InputStreamBinder.java:111)
  	at com.nextcloud.android.sso.aidl.IInputStreamService$Stub.onTransact(IInputStreamService.java:158)
  	at android.os.Binder.execTransactInternal(Binder.java:1165)
  	at android.os.Binder.execTransact(Binder.java:1134)
  

• Server stacktrace due to constraint violation:

  {
    "File": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/3rdparty/doctrine/dbal/src/Driver/PDO/Exception.php",
    "Line": 26,
    "Previous": {
      "Exception": "PDOException",
      "Message": "SQLSTATE[23000]: Integrity constraint violation: 19 UNIQUE constraint failed: oc_deck_board_acl.board_id, oc_deck_board_acl.type, oc_deck_board_acl.participant",
      "Code": "23000",
      "Trace": [
        {
          "file": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/3rdparty/doctrine/dbal/src/Driver/PDO/Statement.php",
          "line": 92,
          "function": "execute",
          "class": "PDOStatement",
          "type": "->"
        },
        {
          "file": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/3rdparty/doctrine/dbal/src/Statement.php",
          "line": 189,
          "function": "execute",
          "class": "Doctrine\\DBAL\\Driver\\PDO\\Statement",
          "type": "->"
        },
        {
          "file": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/lib/private/DB/PreparedStatement.php",
          "line": 87,
          "function": "execute",
          "class": "Doctrine\\DBAL\\Statement",
          "type": "->"
        },
        {
          "file": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/lib/public/AppFramework/Db/Mapper.php",
          "line": 252,
          "function": "execute",
          "class": "OC\\DB\\PreparedStatement",
          "type": "->"
        },
        {
          "file": "/var/lib/nextcloud/store-apps/deck/lib/Db/DeckMapper.php",
          "line": 50,
          "function": "execute",
          "class": "OCP\\AppFramework\\Db\\Mapper",
          "type": "->"
        },
        {
          "file": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/lib/public/AppFramework/Db/Mapper.php",
          "line": 127,
          "function": "execute",
          "class": "OCA\\Deck\\Db\\DeckMapper",
          "type": "->"
        },
        {
          "file": "/var/lib/nextcloud/store-apps/deck/lib/Service/BoardService.php",
          "line": 516,
          "function": "insert",
          "class": "OCP\\AppFramework\\Db\\Mapper",
          "type": "->"
        },
        {
          "file": "/var/lib/nextcloud/store-apps/deck/lib/Controller/BoardApiController.php",
          "line": 161,
          "function": "addAcl",
          "class": "OCA\\Deck\\Service\\BoardService",
          "type": "->"
        },
        {
          "file": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/lib/private/AppFramework/Http/Dispatcher.php",
          "line": 217,
          "function": "addAcl",
          "class": "OCA\\Deck\\Controller\\BoardApiController",
          "type": "->"
        },
        {
          "file": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/lib/private/AppFramework/Http/Dispatcher.php",
          "line": 126,
          "function": "executeController",
          "class": "OC\\AppFramework\\Http\\Dispatcher",
          "type": "->"
        },
        {
          "file": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/lib/private/AppFramework/App.php",
          "line": 157,
          "function": "dispatch",
          "class": "OC\\AppFramework\\Http\\Dispatcher",
          "type": "->"
        },
        {
          "file": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/lib/private/Route/Router.php",
          "line": 302,
          "function": "main",
          "class": "OC\\AppFramework\\App",
          "type": "::"
        },
        {
          "file": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/lib/base.php",
          "line": 1006,
          "function": "match",
          "class": "OC\\Route\\Router",
          "type": "->"
        },
        {
          "file": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/index.php",
          "line": 36,
          "function": "handleRequest",
          "class": "OC",
          "type": "::"
        }
      ],
      "File": "/nix/store/55b4l2f8d5ww1m9pb3l402qiigj7rlxl-nextcloud-23.0.1/3rdparty/doctrine/dbal/src/Driver/PDO/Statement.php",
      "Line": 92
    }

[desperateCoder]

Hi @eliasp,
first of all: thanks for your really detailed and nicely explained report! We're really not used to that kind of issues! 😅

I tried to reproduce it on my instance, but wasn't able to get any error out of it yet. This might be because of my pre-historic version of Nextcloud, I should update here...

I'll dive deeper into this as soon as I find some time for that and keep you updated!

@stefan-niedermann I tried your NC instance as well, are you up to date? I wasn't able to reproduce it on your server as well...

[eliasp]

Any data I could provide from Android (does it maintain a local SQLite DB?) of the App in this erroneous state that could help debug this further?

[desperateCoder]

Any data I could provide

I think I got this without sniffing your private data, but thanks for the offer!

does it maintain a local SQLite DB?

It does. And I'm kind of ashamed of my past self, that I didn't have the same constraint on the table as the server. I guess back then when we started, a ton of things were different and we had a shitload to do, so this fell off at some point. And that's the possible fix I'm thinking of: throw away current shares from local DB, add the constraint and let the sync recover the truth from server to local DB.

After spending the half day updating from NC 20 to 23 (ain't easy when you have a shitty webhoster), updating my android studio and the corresponding emulators, I still wasn't able to reproduce it. I really have no idea how you could even run into this... The UI isn't offering an already existing user in the dialog, at least for me. Once someone is added, I won't find this user to create a new share.

The technical side is easily fixable as mentioned above, but what could be the root cause? I'd ask for a video, but there surely is a reason for blurring the users out in your screenshots... Could you maybe create a dummy user and try the same thing with it while recording?

Another thing i find interesting: {"status":403,"message":"Permission denied"} -> could you please check if you have share permissions in the web-UI?

Sorry for the wall of text, but I warned you: "I'll keep you updated"!

[eliasp]

And that's the possible fix I'm thinking of: throw away current shares from local DB, add the constraint and let the sync recover the truth from server to local DB.

Sounds reasonable, but I feel like there's still a race condition in the App logic itself leading to this state in the 1st place (although this would result now in a local constraint violation when persisting it).

The UI isn't offering an already existing user in the dialog, at least for me. Once someone is added, I won't find this user to create a new share.

Same for me. The 2nd instance of the same user is added "automatically": either it pops up instantly (is there an automatic sync in the background?) when having added the 1st one or after a forced sync.

Another thing i find interesting: {"status":403,"message":"Permission denied"} -> could you please check if you have share permissions in the web-UI?

I do, as I'm the creator/admin of this board/instance.
I was wondering about the 403 as well, as this didn't make any sense to me… will try to reproduce and grab the corresponding server-side logs for the 403 to get a better understanding what parameters are submitted which result in this 403.