[Sandstorm] Firefly III creates more than one user instead of using the main user

[jemaph]

Bug description
I am running Firefly III version 4.7.5.1 on Sandstorm

I've noticed since the lastest update that users with whom they've shared a Firefly III grain can't access the account's data, and are shown the interface as if nothing was present.

Steps to reproduce
Create a Firefly III grain on Sandstorm, share it to another user, the new user won't be able access the data created by the first and his profile will show an ID of 2 or more.
I managed to reproduce the bug on Sandstorm Oasis in demo mode

Extra Info
I've found the root cause as being an old version of app/Http/Middleware/Sandstorm.php being packaged inside the spk.
Our installed version comes from here
https://apps.sandstorm.io/app/uws252ya9mep4t77tevn85333xzsgrpgth8q4y1rhknn1hammw70
The spk package being https://app-index.sandstorm.io/packages/ed958b9fed3330f1e48a2b0603e82950
When I checked the contents of the file using spk unpack on the package that I've downloaded from there, I found out that it didn't handle multiple users gracefully.

This is a snippet taken from the file extracted

    public function handle(Request $request, Closure $next, $guard = null)
    {
        // is in Sandstorm environment?
        $sandstorm = 1 === (int)getenv('SANDSTORM');
        app('view')->share('SANDSTORM', $sandstorm);
        if (!$sandstorm) {
            return $next($request);
        }

        // we're in sandstorm! is user a guest?
        if (Auth::guard($guard)->guest()) {
            /** @var UserRepositoryInterface $repository */
            $repository = app(UserRepositoryInterface::class);
            $userId     = (string)$request->header('X-Sandstorm-User-Id');
            // catch anonymous:
            $userId = $userId === '' ? 'anonymous' : $userId;
            $email  = $userId . '@firefly';
            $user   = $repository->findByEmail($email) ?? $this->createUser($email);
            Log::debug(sprintf('Sandstorm user email is "%s"', $email));

            Auth::guard($guard)->login($user);
            $repository->attachRole($user, 'owner');
            app('view')->share('SANDSTORM_ANON', false);
        }

        return $next($request);
    }

I tried updating the file but it didn't work without overriding the checks on $count as the database now contains more than one user (making the latest code throw an exception saying Your Firefly III installation has more than one user, which is weird.)

[JC5]

Alright, I'm going to poll @ocdtrekkie here because I am not sure if I am following Sandstorm behavior.

In the old situation: multiple users could access a single administration (ie. a single grain). Firefly III would bundle everybody into a single Firefly III user (so Bob, Alice and others would all share user 1).

In the new situation, Firefly III leans on Sandstorm to tell it who's accessing the grain. Users are found or created. Bob gets 1, Alice gets 2, etc.

However since Firefly II does not allow multiple users to access the same data set, you've suddenly lost access.

I don't particularly mind breaking this behavior (and thus restoring the old function) but I could use Jacob's insight here.

I hope I can answer your question asap.

[ocdtrekkie]

The above-quoted function looks significantly different than your current code: https://github.com/firefly-iii/firefly-iii/blob/master/app/Http/Middleware/Sandstorm.php

The original poster suggests that somehow an old version of Sandstorm.php ended up in your package. Is this possible?

[ocdtrekkie]

I would definitely say the expected behavior of sharing a Firefly III grain with someone is that that person would be able to see their Firefly III data. Presumably if someone wanted their own Firefly III data, they'd make their own Firefly III grain.

[JC5]

No, the code is newer. It was greatly simplified to speed up Firefly III.

I can fix the behaviour and push a new version. Would be end of this month, to coincide with other fixes. Would that work?

[JC5]

Alright I've pushed a fix. The code can be replaced manually, if you want to. I'm going to test this but right now I'm getting 404's:

==> default: Box 'sandstorm/debian-jessie64' could not be found. Attempting to find and install...
    default: Box Provider: virtualbox
    default: Box Version: >= 0
==> default: Loading metadata for box 'sandstorm/debian-jessie64'
    default: URL: https://vagrantcloud.com/sandstorm/debian-jessie64
==> default: Adding box 'sandstorm/debian-jessie64' (v8.2.1) for provider: virtualbox
    default: Downloading: https://vagrantcloud.com/sandstorm/boxes/debian-jessie64/versions/8.2.1/providers/virtualbox.box
    default: Download redirected to host: atlas.hashicorp.com
An error occurred while downloading the remote file. The error
message, if any, is reproduced below. Please fix this error and try
again.

The requested URL returned error: 404 Not Found
Command failed with a non-zero exit status (1).

I had it before and it worked again after some time, so stay tuned!

[jemaph]

Thanks for the fix, it's fine if it's released at the end of the month.

[ocdtrekkie]

@JC5 The sandstorm/debian-jessie64 box no longer exists. I can send you a PR to update your .sandstorm directory a bit.

[JC5]

That's cool, just push it to develop, thanks! 😁

[ocdtrekkie]

Found a flaw in one of your modifications in .sandstorm as well, so I'm fixing that too. Just trying to test my changes.

[ocdtrekkie]

See #1569 for less sadness building for Sandstorm

[JC5]

👍 the build works, the fix works, and I'll submit some other updates as well.

[Ditti4]

So how can we fix this situation after user 2 was already created? Revoking access on Sandstorm doesn't seem to help. We're on version 4.7.5.3.

[JC5]

The system should fall back to the oldest user automatically.

[Ditti4]

Correct me if I'm wrong, but this looks like it tries to find a user with the given email first and only if it doesn't, it'll use the oldest user. So once you run into the issue and a separate user is created, then that user is always used instead of the oldest user.

[JC5]

You're right of course! I'll think of something to delete that user.