Conversation
Preserve validated protected-page URLs when anonymous users are sent to the login form, then redirect them back after successful authentication. Centralize resumable access checks, restrict forwarded URLs and query parameters through an allowlist, and retain existing authorization rules after login. Tighten access to payment claims, finance actions, and administrator member details. Update the login Playwright coverage for allowed, forbidden, and role-specific direct routes, and remove obsolete small-manager callback parameters. Modifiing GET requests are not changed to POST, this change would be too large.
Add a serial Playwright workflow covering member 4444 from creation through deletion. The workflow verifies: - Profile updates by club admin, administrator, manager, small manager, and member - Small-manager assignment - Member finance credit - Disable and enable behavior - Account creation and administrative locking - Account and member deletion Also add the member fixture and reusable helpers for profile updates, visibility changes, and deletion.
| $row = array(); | ||
| $row[] = $i++; | ||
| $row[] = '<a href="javascript:open_win(\'./view_adm_user_detail.php?id='.$zaznam['id'].'\',\'\')" class="adr_name">'.$zaznam['prijmeni'].'</a>'; | ||
| $row[] = IsLoggedAdmin() |
There was a problem hiding this comment.
This file is for small admin - why test for admin ?
There was a problem hiding this comment.
Here was the link to unsecured page available for anybody.
It was not possible to deduce the rights, the highest has been chosen.
The page title is "adminova stranka - editace clenu oddilu".
If small admin is appropriate, I have no problem with this.
There was a problem hiding this comment.
view_adm_user_detail.php is page primary for small admin. Sorry for ambiguous description
There was a problem hiding this comment.
Internal page, no reason to call login. Possible security risk
There was a problem hiding this comment.
This one is definitely not right. In current implementation anybody can change anybody.
How to reproduce:
Login in TST as tnov_5 or any unprivileged user
Change the URL to: https://members.eob.cz/tst/view_adm_user_detail.php?id=12
Or direct change with:
Change Libor
With RequirePageAccess the user is authentificated first, then authorized.
There was a problem hiding this comment.
I agree that this is not good, we missed this somewhere and the access rights must be higher.
Correct access rights are : small admin
@xkenia - please look at this file, it was added by you - what do you think about it ?
There was a problem hiding this comment.
This file need more attention. As @scoufal mentioned, it has security problems.
There was a problem hiding this comment.
This changes should be done only with Big Admin priviledge, imho.
|
My suggestion - use the file |
|
We should discuss what to do with "internal" pages in case of access without proper rights, whether to require login or write an error (as was the case originally). |
Only user race registration, claim and user finance page are allowed to be forwarded to after login.
|
|
||
| require_once ("./connect.inc.php"); | ||
| require_once ("./sess.inc.php"); | ||
| RequirePageAccess(IsLoggedFinance()); |
There was a problem hiding this comment.
No access right check here now ?
There was a problem hiding this comment.
A access check would be desirable, open question is if with login&forwarding (part of this PR) or just with check (rather a faster fix)
3 participants
Allow external links direct to specific page.
The possible redirections are whitelisted for non-modifying pages to reduce security risk.
Access to following pages is tightened: claim.php, fin_bank_orphan_assign.php, view_adm_user_detail.php
Primary redirection targets: