"Normal reasons" for FORM_INVALID which now also apply to login forms.

General discussion of development ideas and the approaches taken in the 3.x branch of phpBB. The current feature release of phpBB 3 is 3.3/Proteus.
Forum rules
Please do not post support questions regarding installing, updating, or upgrading phpBB 3.3.x. If you need support for phpBB 3.3.x please visit the 3.3.x Support Forum on phpbb.com.

If you have questions regarding writing extensions please post in Extension Writers Discussion to receive proper guidance from our staff and community.
Post Reply
User avatar
EA117
Registered User
Posts: 21
Joined: Tue Apr 30, 2019 12:54 pm

"Normal reasons" for FORM_INVALID which now also apply to login forms.

Post by EA117 »

While poking at one of the users who still have "invalid form" during login, even with 3.2.7: https://www.phpbb.com/community/viewtop ... #p15248941

Edit: Second customer encountered same problem in https://www.phpbb.com/community/viewtop ... &t=2511881, and same workaround was necessary for them.

Thus far, we've been able to confirm that the issue is happening because the login form received as part of index.php has a current time stamp but a "wrong" session ID. I say "wrong" because it's not the session ID that phpBB then has and uses during all subsequent steps, start with the POST ucp.php?mode=login that fails with "invalid form."

With appropriate debug logging we've confirmed that "the session ID is different" is in fact the reason why the login fails with "invalid form". The subsequent re-attempt using the login form re-presented during the failure is no longer the login form from index.php, and has also been built with the "correct" current session ID; and so this second login attempt succeeds when POSTed.

I'm thinking "caching" is still the best reason to have "an index page containing a login form with a current time stamp but the wrong session ID"; e.g. that rendering of the index page & login form was made for someone else and not "your session", but the cache delivered the previously-built page containing the previous page's session ID. Someone who has a better grasp (or any grasp, really, since that would still be better than mine) of session ID mechanics might have additional logical reasons for "the session ID changed between the time of GET index.php and then trying to POST the login form contained in that index.php page."

If there is "something we do on login form templates to prevent them from being cached", but which is something we haven't been doing for the index.php template -- and have been "getting away with it" until now, because we hadn't been using add_form_token() or check_form_token() on index.php's login form -- maybe that would be relevant to this discussion too.

The other reason I wanted to post here was because of something that bugged me when investigating the code on this. phpBB ACP General tab, Server Configuration, Security settings has a setting for "Tie forms to guest sessions". The code that seemingly is intended to react to this is right in the add_form_token() and check_form_token() that I've been looking at, but is written as:

Code: Select all

$token_sid = ($user->data['user_id'] == ANONYMOUS && !empty($config['form_token_sid_guests'])) ? $user->session_id : '';
Which to me doesn't look like a test of "is form_token_sid_guests enabled or disabled", but is rather "does form_token_sid_guests exist in the schema." i.e. "!empty()" is going to be true regardless of whether the setting is enabled or disabled, correct?

So I could have suggested to the current user to "try turning off Tie forms to guest sessions", but I didn't think that would actually have any effect. Is there a reason I'm missing for why "!empty()" is actually the intended and required test here?
Last edited by EA117 on Thu May 16, 2019 3:43 pm, edited 2 times in total.

User avatar
mrgoldy
Former Team Member
Posts: 64
Joined: Fri Dec 18, 2015 9:41 pm
Location: The Netherlands
Contact:

Re: "Normal reasons" for FORM_INVALID which now apply to login forms.

Post by mrgoldy »

Haven't looked into this issue, also not with 3.2.6
But for your specific question on the !empty().

All variables retrieved through $config['setting'] are returned as strings.
All booleans stored in the config table are set to '1' (true) or '0' (false).
!empty('1'); returns true
!empty('0'); returns false
phpBB Studio Proud member of the Studio!

User avatar
EA117
Registered User
Posts: 21
Joined: Tue Apr 30, 2019 12:54 pm

Re: "Normal reasons" for FORM_INVALID which now apply to login forms.

Post by EA117 »

posey wrote: Sun May 05, 2019 10:10 pm !empty('1'); returns true
!empty('0'); returns false
Thank you; exactly what I was failing to realize; the "or if the value equals FALSE" condition of "empty()".

I will update the user to confirm that this setting should have an effect, by essentially making all logged-out user session sids "blank". So the sid in a cached document from someone else's guest session (if a cached document is what I'm getting) would still be "the correct sid" for my guest session, too.

Separately, top post edited just to try and give a clearer description on what's been observed with the index.php login form in this user's case.

User avatar
EA117
Registered User
Posts: 21
Joined: Tue Apr 30, 2019 12:54 pm

Re: "Normal reasons" for FORM_INVALID which now also apply to login forms.

Post by EA117 »

Right now I'm focusing on situations where "the sid is appended to the URL even though a cookie-based sid is also available." Because I feel like I can duplicate this user's "the anonymous session sid when building the login form on the index page is different than the anonymous session sid phpBB then uses when that login form is then posted to ucp.php?mode=login" -- if I force a sid into the URL. And I see the sid getting into the URL many times, even when my expectation with the browser settings I have is that it "should" always be available and retrieved from the cookie.

So there is something (or more likely, many things) I just don't understand there for why the sid parameter gets into the URL anyway, and will keep studying.

User avatar
3Di
Registered User
Posts: 951
Joined: Tue Nov 01, 2005 9:50 pm
Location: Milano 🇮🇹 Frankfurt 🇩🇪
Contact:

Re: "Normal reasons" for FORM_INVALID which now also apply to login forms.

Post by 3Di »

EA117 wrote: Wed May 08, 2019 12:22 am I just don't understand there for why the sid parameter gets into the URL anyway, and will keep studying.
Try forum_root/ucp.php?mode=delete_cookies - should do. (delete cookies at pagefooter)
🆓 Free support for our extensions also provided here: phpBB Studio
🚀 Looking for a specific feature or alternative option? We will rock you!
Please PM me only to request paid works. Thx. Want to compensate me for my interest? Donate
My development's activity º PhpStorm's proud user º Extensions, Scripts, MOD porting, Update/Upgrades

Post Reply