Media entity owner access check

Prepared a quick fix.

Requires a first review before updating the tests.

The Needs Review Queue Bot → tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

I feel we have to have test coverage for the described issue (have not looked yet though)

@smustgrave Sure. You are right, we need. But first I want to hear the thoughts of other people about the issue.

No I mean I'm surprised we already don't but looking at the test failures maybe we do MediaAccessTest

@smustgrave
To be honest, I was also surprised when I caught this bug.
In the kernel, a non-strict condition is used everywhere: $account->id() == $entity->getOwnerId(), so everything works fine.
But the media checkAccess uses a strict condition.

Also will add an explanation for everyone why I set the Needs review status.
Atm I fixed the issue in the following way:
$is_owner = ($account->id() && $entity->getOwnerId() && $account->id() === (int) $entity->getOwnerId());
This is a solution that generally complies with the strict typing that is gradually being implemented in PHP.

But perhaps here it would be worth using a legacy approach that is more common in the core
$account->id() == $entity->getOwnerId())

Ah yes, the string|int|null user ID is definitely troublesome. We either cast to int for both values, or change the strict comparison. I don't have a string opinion on it either way. I feel casting to int is slightly nicer, but not sure if that could lead to other unforeseen issues.

So either:
$is_owner = $account->id() && (int) $account->id() === (int) $entity->getOwnerId();

Or simply just do:
$is_owner = $account->id() && $account->id() == $entity->getOwnerId();

The node modules contains this by the way, so maybe to non-strict comparison would be more in line with that:

$uid = $node->getOwnerId();
if (... && $account->isAuthenticated() && $account->id() == $uid)

Thanks @seanB for taking a look!

Simply apply the changes suggested by @seanB over #13

Used $is_owner = $account->id() && (int) $account->id() === (int) $entity->getOwnerId();
to check the access.
Please review and suggest if any other changes required

Moving the issue to needs review

Will need a test case for the issue though