FriendsOfFlarum / terms

Ask your users to accept TOS and Privacy Policy
https://discuss.flarum.org/d/11714
MIT License
14 stars 10 forks source link

Registration possible without accepting TOS #3

Closed manurohr closed 6 years ago

manurohr commented 6 years ago

I have installed the extension flagrow terms using composer. I have tried to register a new account using german language and activated recaptcha by sijad. I see two tick boxes appearing on the modal and I don't click them. I pass the recaptcha test and click register button. Flarum redirects me to main page and tells me to check confirmation e-mail which I did. After confirming the link in e-mail I was able to create discussions. I do not see any yellow banner telling me to accept tos before using flarum in full capacity.

modal_tos

I expect an error appearing telling me to accept tos first before registering a new account.

Technical Details

Flarum 0.1.0-beta.7PHP 7.1.12 Flagrow Terms: 0.2.0 Browsers: tested in Chrome 66.0, Firefox 60.0.1, Safari 11.1 OS: macOs Sierra, 10.12.6

Logs: one php error found but i guess it's unrelated to this issue: [28-May-2018 08:34:49 UTC] PHP Notice: Undefined offset: 0 in /Applications/MAMP/htdocs/quickline/vendor/reflar/pretty-mail/src/Mailer.php on line 33

no nginx error, no flarum logs

I have tried php flarum cache:clear, also no effect. php debug is turned on.

Flarum Info

Flarum core 0.1.0-beta.7 (c6aeeeb3c1d5cf92c586c9f8ee43d2127d0fb933)
PHP 7.1.12
Loaded extensions: Core, date, libxml, openssl, pcre, sqlite3, zlib, bcmath, bz2, calendar, ctype, curl, dom, hash, fileinfo, filter, ftp, gd, SPL, iconv, intl, json, ldap, mbstring, session, standard, mysqlnd, PDO, pdo_mysql, pdo_sqlite, apcu, posix, readline, Reflection, mysqli, SimpleXML, soap, sockets, exif, tokenizer, wddx, xml, xmlreader, xmlrpc, xmlwriter, xsl, zip, apc, Phar, imap, mcrypt, gettext, pgsql, pdo_pgsql
EXT flagrow-analytics 0.6.0
EXT flarum-approval v0.1.0-beta.7
EXT flarum-bbcode v0.1.0-beta.5
EXT wiwatsrt-best-answer v0.1.0-beta.11
EXT datitisev-dashboard v0.1.0-beta.6.1
EXT cbmainz-de 0.7.1
EXT flarum-emoji v0.1.0-beta.6
EXT flarum-english v0.1.0-beta.7
EXT flagrow-bazaar 0.2.4
EXT flagrow-byobu 0.1.2
EXT flagrow-html-errors 0.1.1
EXT flagrow-impersonate 0.1.0
EXT flagrow-split 0.2
EXT flagrow-terms 0.2.0
EXT flagrow-upload 0.6.0
EXT flagrow-user-directory dev-develop (ba7961687c2dba73ebe0be02087789e956e231da)
EXT flagrow-users-list 0.1.2
EXT flarum-flags v0.1.0-beta.7
EXT sijad-google-analytics 0.1.1
EXT sijad-recaptcha 0.0.2
EXT flarum-likes v0.1.0-beta.6
EXT sijad-links 0.1.0-beta.6
EXT flarum-lock v0.1.0-beta.7
EXT flarum-markdown v0.1.0-beta.5
EXT xengine-markdown-editor 1.3.1
EXT flarum-mentions v0.1.0-beta.7
EXT sijad-pages 0.1.0-beta.3
EXT reflar-polls 0.1.1
EXT reflar-pretty-mail 0.1.0-beta.2
EXT reflar-reactions dev-develop (6099a08a61c6daf4b4b480d30f066f8140e2ec4f)
EXT flarum-sticky v0.1.0-beta.7
EXT flarum-subscriptions v0.1.0-beta.6
EXT flarum-suspend v0.1.0-beta.7
EXT flarum-tags dev-beta-7
Base URL: http://localhost:7888/quickline
Installation path: /Applications/MAMP/htdocs/quickline
luceos commented 6 years ago

@manurohr there's an issue with both the log in and register modals. The issue is that any extension modifying the body of the modal will negate the behavior of another extension. It's already a miracle you're seeing both changes, I expect @sijad to have injected the code using javascript and not mithril. In the end what is sent to the server is also important and there either of the extensions might override the payload added by the other.

Until beta 8 I suggest you decide which of these extensions you prefer and de-install the other.

@clarkwinkelmann another conflict extension to add?

sijad commented 6 years ago

@manurohr could you please try my PR #4? I can't test it right now.

manurohr commented 6 years ago

@sijad tried your pull request, no php nor flarum logs found. even removed your captcha-extension, had no effect on it.

clarkwinkelmann commented 6 years ago

I'm surprised because I did test this extension and confirmed the two work together without any issue (it's listed as compatible in the discussion)

@manurohr even removed your captcha-extension, had no effect on it.

You mean you still have the issue with @sijad 's extension removed ? Then there's likely another extension in there that's causing the issue. Did you try with all third-party extensions disabled ?

PS: I closed @sijad PR because I'm certain it does not make any difference (explanation in the PR thread)

EDIT: one obvious way to know if an extension is tampering with the terms extension is to look in the network tab while logging in (edit: registering). If the /login (edit: I meant /register) endpoint is called it's all good. But if another "register" endpoint is called, it's most likely that an extension replaced the register logic. Flagrow Terms only works if the default route (/register) is handling the register. So far I've only seen the Reflar User Management doing that, but other extensions could be doing the same.

clarkwinkelmann commented 6 years ago

Okay found the issue while talking to @manurohr on Discord.

It's not related to any other extension. It's an issue with the middleware and Flarum being in a subfolder.

Going to fix this right away :innocent:

clarkwinkelmann commented 6 years ago

Should be fixed in version 0.2.1, let me know if you still have issues :wink:

manurohr commented 6 years ago

Great! it worked 👍