Skip to content
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

Version 2.6.2: Personal Results Download downloads another user's results where brewer.id !=brewer.uid (2.7.1 - fails with 403 response code) #1580

Open
graemecoates opened this issue Oct 14, 2024 · 12 comments
Assignees
Labels
bug confirmed Bug report confirmed in test environment in latest master commit Fix is in latest commit to master. Not in release.

Comments

@graemecoates
Copy link

Reporting About the Latest Version? Yes

Searched Issue Reports for Bug? Yes

Reviewed the Good To Know Issue Reports? Yes

Please review the Issue reports that are labeled Good to Know. Find one and click on the label to view them all.

Your Intallation's BCOE&M Info

  • Version: 2.6.2 /2.7.1.1
  • Installation URL: homebrewcompetition.wales
  • Hosted Installation? No

Describe the Bug

In 2.6.2, we have brewers where the id in the brewer table is not the same as the uid. This is due to there being user records, where no matching brewer record exist - hence the auto-increment ids become out of sync (presume this is because they never added an entry after registration?). In this case, when a user (A) with this situation attempts to download their own results in csv format, the user (brewer A) is given a file with the name of brewer B, where the user id of brewer B matches the brewer id of brewer A. (the URL is of the form id=&filter=). Changing the id and filter in the download URL to match the user id of brewer A allows it to work correctly (NB: the user A has access to both A and B results this way , and sees the name of brewer B in the filename downloaded.

In 2.7.1, the URL is rewritten and on this version results downloads fail completely with a 403 (using either id/uid in the URL params).

I've confirmed behaviour in a sandbox copy of the production site.

To Reproduce
Steps to reproduce the behavior:

  1. DB with some records where brewer.id<>brewer.uid
  2. Results entered for the brewer.
  3. Login as brewer and attempt to download own results
    4a. In 2.6.2, you get the results for the brewer with uid=brewerA id
    4b. In 2.7.1 it 403s.

Expected Behavior

User should get own results and none other. Should not 403. Should not see names of other brewers.

Server Environment

  • PHP Version: 8.2
  • MySQL Version: MariaDB 10.11.6-0+deb12u1
  • Other Info: Apache 2.4.62-1~deb12u2
@graemecoates graemecoates changed the title Version 2.6.2: Personal Results Download downloads another user's results where brewer.id !=brewer.uid (2.7.1 - fails with 403) Version 2.6.2: Personal Results Download downloads another user's results where brewer.id !=brewer.uid (2.7.1 - fails with 403 response code) Oct 14, 2024
@geoffhumphrey
Copy link
Owner

Hi @graemecoates - thanks for reporting.

Confirmed the bug 2.7.X. A fix will be in place in the next version.

As reported, for either version, I would suggest you run the Data Integrity function periodically. This will make sure that all ids in the users table match up with the uids in the brewer table.

You can run the function manually by going to Admin Dashboard > Data Management > Integrity > Clean-Up Data. Doing this should resolve the issue in 2.6.2.

For anything beyond 2.6.2, add the following line to /output/export.output.php AFTER line 9:

if ($section == "export-personal-results") $authorized = TRUE;
if ($authorized) data_integrity_check();

This will do two things - 1) it will allow user download of the Personal Results csv file, and 2) it will run the data integrity check function before running the export code.

geoffhumphrey added a commit that referenced this issue Oct 16, 2024
@geoffhumphrey geoffhumphrey self-assigned this Oct 16, 2024
@geoffhumphrey geoffhumphrey added bug in latest master commit Fix is in latest commit to master. Not in release. confirmed Bug report confirmed in test environment v2.7.2 labels Oct 16, 2024
@geoffhumphrey geoffhumphrey added this to the v2.7.2 Release milestone Oct 16, 2024
@graemecoates
Copy link
Author

I have run the process on a 2.6.2 install, and now we are in the situation that brewer A can only see brewer B's results. Is there still an underlying bug here?

@graemecoates
Copy link
Author

graemecoates commented Oct 17, 2024

I can also confirm that running the Clean up Data task doesn't resolve the issue on 2.7.1 either. It doesn't match the id/uids up either:

MariaDB [bcoem]> select id,uid from brewer where id=177 \G
*************************** 1. row ***************************
id: 177
uid: 183

@geoffhumphrey
Copy link
Owner

geoffhumphrey commented Oct 17, 2024

I'll have to investigate.

The current way the data integrity check works is to first check if there are any missing email addresses in the brewer table and update it accordingly with the record in the user table - however, this only works if the uid in the brewer table corresponds to the id in the user table.

This match up is necessary because the next part of the function uses email addresses as the key to match up records in the user and brewer tables.

The next part of the function loops through each record in the user table, querying the brewer table to see if there is a record whose brewerEmail is equal to the user_name in user table. If one is found, it then compares the record's id in the user table to the uid in the brewer table. If those are not equal, the brewer table record's uid is updated with the corresponding id number from the user table.

In my testing, this worked correctly, so in your case, it sounds like matching of records in the two tables is not taking place - which indicates that the email addresses do not match. Can you verify if this is the case? If so, you may need to manually update the records in the DB - and I'll try to figure out a root cause.

Without looking at any code as of yet, it may have to do with a user who already has entries in the system changing their email address associated with their user record, but that change did not matriculate into the brewer table.

@graemecoates
Copy link
Author

graemecoates commented Oct 18, 2024

If I understand:

select * from users u where not exists (select 1 from brewer b where b.brewerEmail=u.user_name and u.id=b.uid);

should return no records (which it does). The reverse also returns no rows. I don't think this is an issue in non-matching emails - I just think that when it's generating the link for the results download, it's using the wrong id to do so.

I could supply an anonymised version of the database if that's more helpful - if you have details on how best to supply this, can you let me know please?

@geoffhumphrey
Copy link
Owner

If you're willing to supply the anonymized DB, that would really help with trying to track down the issue. If you'd go to https://brewingcompetitions.com/contact and send a general inquiry, I'll get you a way to send the DB file.

@graemecoates
Copy link
Author

@geoffhumphrey I've tried to send you a message as above (you might have got it). BTW, your contact form throws an error on submit - see:

image

@geoffhumphrey
Copy link
Owner

Hi @graemecoates - unfortunately, I did not get an email. Upon investigating, the SMTP password I had configured was out of date. Would you mind sending again?

@graemecoates
Copy link
Author

@geoffhumphrey I'm still not sure if your contact form actually works - it throws a red error with no detail of what's wrong with the form, but no confirmation that it has sent....

image

@geoffhumphrey
Copy link
Owner

Oh, boy. This is what you get when you code without sufficient amounts of coffee in your system. Apologies. Please try again. I swear the third time will be the charm.

@graemecoates
Copy link
Author

Hi @geoffhumphrey - do you have any update on this at all? The competition is still in a position that it can't effectively distribute its results to brewers.

@geoffhumphrey
Copy link
Owner

Hi @graemecoates - my apologies. I was out of the country and was not able to download the file. I'm back home now and will take a look. In the meantime, it looks like you've published results, so at least your brewers know who placed in which categories. In the meantime, if you want to disable downloading of personal results, you can remove or comment out lines 39-49 of the /sections/brewer_entries_sec.php file. This will remove the download button above your user's entry list on their My Account page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug confirmed Bug report confirmed in test environment in latest master commit Fix is in latest commit to master. Not in release.
Projects
None yet
Development

No branches or pull requests

2 participants