Skip to content

app/Policies/OwnershipTrait contains a bug, i think #320

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
1 task
doctor-beat opened this issue Mar 12, 2024 · 9 comments
Closed
1 task

app/Policies/OwnershipTrait contains a bug, i think #320

doctor-beat opened this issue Mar 12, 2024 · 9 comments
Labels
bug Something isn't working fixed The issue is fixed (in a coming release)
Milestone

Comments

@doctor-beat
Copy link

Version

2FAuth – v5.0.4

Details & Steps to reproduce

app/Policies/OwnershipTrait line 16 contains a bug, i think. In my case the $item->user_id is a string with does not make it triple equal to $user->id

When i wrap the $item->user_id in a intval() the class starts working.

It appears to bethe same issue as described here: #305

Expectation

no errors when clicking the sfauth key.

Error & Logs

No response

Execution environment

No response

Containerization

  • Docker

Additional information

No response

@Bubka
Copy link
Owner

Bubka commented Mar 12, 2024

Hi,

what db are you using?
I can't reproduce using mysql.

edit: also tested with sqlite, can't reproduce either

@Bubka
Copy link
Owner

Bubka commented Mar 13, 2024

Can you please confirm you have this line in log files (see /2fauth/storage/logs/*.log:

User ID #x cannot view twofaccount ID #y

@doctor-beat
Copy link
Author

doctor-beat commented Mar 13, 2024

Hi,

  1. we are on mysql, both columns in db are in fact integer types
  2. php 8.2
  3. yes those are in the logs and are what triggered me to locate the issue:

[2024-03-12 16:38:55] production.NOTICE: User isOwner of with ID #integer vs twofaccount ID #string: [2024-03-12 16:38:55] production.NOTICE: User ID #3 cannot view twofaccount ID #3 [2024-03-12 16:40:45] production.NOTICE: User isOwner of with ID #integer vs twofaccount ID #string:

I have added an extra line of logging for personal debugging

@Bubka
Copy link
Owner

Bubka commented Mar 13, 2024

Ok, thx for the env setup.
I´m going to make more testing, I would like to understand why types are not consistent from one setup to another 🤔

@doctor-beat
Copy link
Author

yes, thanks. I i can be of any help let me know. I am a java//php dev as well.

@Bubka
Copy link
Owner

Bubka commented Mar 13, 2024

Well, I cannot reproduce, even with the exact same setup. What page do you load while debugging the trait?

@Bubka
Copy link
Owner

Bubka commented Mar 14, 2024

Could you please try this fix for me ?

Edit 2fauth_install_dir/app/Models/TwoFAccount.php.
Line 148, set the $casts like this:

    protected $casts = [
        'user_id' => 'integer',
    ];

Thx

@Bubka Bubka added the bug Something isn't working label Mar 14, 2024
@Bubka Bubka moved this from Todo to In Progress in 2FAuth backlog Mar 14, 2024
@Bubka Bubka added this to the 5.1.0 milestone Mar 14, 2024
@doctor-beat
Copy link
Author

doctor-beat commented Mar 14, 2024

That fixes the issue, triple equality is now comparing two integers

@Bubka
Copy link
Owner

Bubka commented Mar 14, 2024

Ok thanks 👍🏻

@Bubka Bubka added the fixed The issue is fixed (in a coming release) label Mar 14, 2024
@Bubka Bubka moved this from In Progress to Done in 2FAuth backlog Mar 14, 2024
@Bubka Bubka closed this as completed in e956959 Mar 15, 2024
@Bubka Bubka moved this from Done to Released in 2FAuth backlog Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed The issue is fixed (in a coming release)
Projects
Status: Released
Development

No branches or pull requests

2 participants