View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001192 | Main CAcert Website | website content | public | 2013-07-16 22:03 | 2015-03-10 20:09 |
| Reporter | INOPIAE | Assigned To | BenBE | ||
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | closed | Resolution | fixed | ||
| Product Version | 2013 Q3 | ||||
| Target Version | 2013 Q4 | Fixed in Version | 2014 Q4 | ||
| Summary | 0001192: Check on log into the account if user aggreed to CCA, if not prompt him an acception form | ||||
| Description | After the CCA agreement is recorded at account creation, entering an assurance and certificate creation, we need to check if the user already aggreeded to CCA. If the user did not accept the CCA so far the user should be promted to accept the CCA or to ask for account deletions | ||||
| Tags | No tags attached. | ||||
| Reviewed by | NEOatNHNG, BenBE | ||||
| Test Instructions | |||||
|
|
I added the fix to https://github.com/INOPIAE/CAcert/commit/9709a2ae6177f612ca06e2698041494fdafb7992 |
|
|
I create 100 test accounts with x.cca-test@acme.com for testing purposes with x = 1 to 100. The SA should delete the CCA acceptance of these accounts with this sql-statement: Delete from `user_agreements` WHERE `document`='CCA' and `user_agreements`.`memid` in (Select `users`.`id` from `users` WHERE `users`.`email` Like '%cca-test@acme.com') |
|
|
I applied a new fix https://github.com/INOPIAE/CAcert/commit/4b924244c859aba7daf8c6e04036784d49a767e4 |
|
|
please describe a testscenario, how to a testuser can prepare a test environment so the testuser can trigger a non-agreed CCA popup at logon procedure from my PoV untestable scenario I've tried with several old accounts created in the early months of testserver building, that potentialy didn't yet undergo a monitored CCA agreement logging but did not find any that triggers a forced CCA agreement in logon procedure new accounts cannot be created without accepting CCA ... so in next step a logon to account never can trigger missing CCA agreement dead end |
|
|
I pushed a new fix to https://github.com/INOPIAE/CAcert/commit/39eefaff8d0c74f34a8cebd085f6f15dcfeced1e For Testing: Try to login with a user with CCA acceptance with password Try to login with a user with CCA acceptance with certificate Try to login with a user without CCA acceptance with password Try to login with a user without CCA acceptance with certificate Some accounts without CCA agreement x.continous@acme.com Password as usual |
|
|
Tried to login with PW with an acc without CCA acceptance. Got a page with the option to accept CCA or to not accept CCA. I chose accept CCA. -> Got to the normal first page when logged into the account. -> ok Tried to login with 66.continous@acme.com agian. -> Could login normaly and came to the normal page after login. -> ok Tried to login with PW with an acc without CCA acceptance. (73.continous@acme.com) Got a page with the option to accept CCA or to not accept CCA. I chose to not accept CCA. -> I got to the login page again. -> ok Tried to login with 73.continous@acme.com again. -> Got again to the page with the question if I want to accept CCA. -> ok Tried to login with an account that did accept CCA before the patch with PW. -> Got to the normal first page after login. -> ok Tried to login with an account that did accept CCA before the patch with a certificate. -> Got to the normal first page after login. -> ok Tried to login with an account without CCA acceptance with a certificate. -> Got again to the page with the question if I want to accept CCA. -> Did not accept CCA -> came back to same page -> would be nice to get back to startpage or something else but -> ok (something else would be nicer) Tried again to login with certificate on same account -> same result -> ok Tried again to login with certificate on same account. Accepted CCA. -> got to normal first page. -> ok Tried again to login with certificate on same account. -> got to normal first page. -> ok Beside of actual text and the possibility to get a better behavior when not accepting CCA on a certificate login: => ok |
|
|
Additional test: Tried to login with an account without CCA that has a valid certificate - with a pw. -> Got to the page with the question if I want to accept CCA. Did not accept CCA. -> Got back to login-page Tried to login with certificate on same account. -> Got to the page with the question if I want to accept CCA. Did not accept CCA. -> stayed on the page. repeated the process with same result (pw, certificate, pw, certificate, always without acceptance of CCA) => ok |
|
|
Testing & Review discovered major issues: - When coming from the password dialogue (index oldid=4) the redirection to the CCA dialogue should be done if necessary before unpacking $_SESSION['_config']['oldlocation'] - When the password is authenticated the log in state is set to logged in no matter what. So even if the user was properly directed to the CCA dialogue the user could still change the URL to something else without clicking accept or deny. A possible solution might be to check for cca acceptance in the loggedin.php. But as this is executed on every page view cca acceptance should probably be cached in the session so it doesn't have to hit the database |
|
|
I pushed a new fix to https://github.com/INOPIAE/CAcert/tree/bug-1192 |
|
|
In includes/loggedin.php lines 165ff there are two changes that should be made: 1. Don't check against true/false explicitely, i.e. $var['value'] instead of $var['value'] == true, also put (numeric) constants before the equal operator. 2. The RFC defines the Location header with a capital L. In pages/index/52.php line 23: 3. Unnecessarily many spaces in translation string and before closing tag. In www/index.php line 167: 4. see remark on 2. 5. Same on line 341. |
|
|
From my review: Bigger problem is in the /includes/index.php:360 if the page is directly called via https://cacert1.it-sls.de/index.php?id=52 and acknowledged then the $_SESSION['profile'] is still unset and causes errors. I don't know whether one could exploit this to set this value so that it doesn't cause errors but $_SESSION['profile']['loggedin'] is set to true. Making it possible to work around authentication. Concrete exploit: go to https://cacert1.it-sls.de/account.php there you are redirected to https://cacert1.it-sls.de/index.php?id=4 but before that happens a basic $_SESSION['profile'] is set up with $_SESSION['profile']['loggedin']=0. Now go to https://cacert1.it-sls.de/index.php?id=52 without entering your password et voilà: enjoy a completely anonymous account. I have fixed that issue in a more recent commit. Changes are on the test server. Please test and do a second review. |
|
|
Retest: PW login with an account that didn't accept the CCA before: Redirect to page asking whether I accept the CCA => OK Chose not to accept: Redirect back to the login page => OK Logged in again, chose to accept: Login successful => OK Logged out and back in with the same account: Login successful without having to accept the CCA again => OK (Note: The page prompting for CCA acceptance still shows "#### Explanation why #### Please replace me ####") Tried the exploit described by NEOatNHNG above: https://cacert1.it-sls.de/index.php?id=52 shows the CCA acceptance prompt, but accepting redirects back to the login page => OK I cannot test certificate login since I don't have access to any accounts with valid certificates that didn't accept the CCa yet. |
|
|
Started at: https://cacert1.it-sls.de/account.php -> got redirected to: https://cacert1.it-sls.de/index.php?id=4 changed the address manually to https://cacert1.it-sls.de/index.php?id=52 accepted CCA -> got redirected to https://cacert1.it-sls.de/index.php?id=4 => ok |
|
|
Tried to log into an account without CCA acceptance with client certificate -> directed to https://cacert1.it-sls.de/index.php?id=52 Did not accept CCA -> stayed on https://cacert1.it-sls.de/index.php?id=52 repeated this -> same result looked at CCA and went back -> https://cacert1.it-sls.de/index.php?id=52 accepted CCA -> question for certificate -> got into account logged out, tried to log in again with certificate -> could log in normally -> ok tried to log into an account without CCA acceptance with PW -> directed to https://cacert1.it-sls.de/index.php?id=52 Did not accept CCA -> redirected to https://cacert1.it-sls.de/index.php?id=4 repeated this -> same result accepted CCA -> question for certificate -> got into account logged out, tried to log in again with PW -> could log in normally logged out, tried to log in again with certificate -> could log in normally -> => ok |
|
|
for newsletter see https://wiki.cacert.org/PR/News/CCARollout/EN |
|
|
I pushed the fix with the explanation text to: https://github.com/INOPIAE/CAcert/commit/b90ee5a97dcf351e22f43f26bf971c3f39fcdf85 |
|
|
When I log in with an account that has not accepted the CCA, I got a text describing the procedure. However it is centered and by this not optimal to read as such a text should be. I would highly prefer to have it left bound. -> ok but could be improved |
|
|
New fix pushed to https://github.com/INOPIAE/CAcert.git b90ee5a..58d2ce2 |
|
|
The text is ok, but I really find it quite hard to read with centered formatting. Pleeeeeeeeeeeeaaaaase consider to do make it leftbound. -> ok but can be improved |
|
|
Review OK. Still needs a second review. |
|
|
Second review OK. Text preview sent to critical for inclusion of the translations. I don't see two tests yet though. |
|
|
The 3 files have been placed in a new directory /home/cacert/www/www/bug1192, and have been picked up by the 'make upload' command which transfers texts to be translated to the translations server. They have not been added to CVS and thus not to a new tarball either. I have checked that invocation of any of these files through the web interface properly results in a white screen without further ado, by courtesy of the die() call at the start. |
|
|
I logged in as "100.cca-test@acme.com" on the testing server and got the CCA-acceptance notice. 1. Once I accepted it, I wasn't shown it again on a second login. 2. When I declined it (with another account), I was redirected back to the login page and trying to login again resulted in the same behavior. 3. I understood the text. PASSED |
|
|
Sent to crit for deployment. |
|
|
The fix has been installed on the production server on November 24, 2014. See also: https://lists.cacert.org/wws/arc/cacert-systemlog/2014-11/msg00015.html |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2013-07-16 22:03 | INOPIAE | New Issue | |
| 2013-07-16 22:03 | INOPIAE | Assigned To | => INOPIAE |
| 2013-07-16 22:03 | INOPIAE | Relationship added | related to 0000505 |
| 2013-09-14 16:13 | INOPIAE | Note Added: 0004318 | |
| 2013-09-14 16:13 | INOPIAE | Assigned To | INOPIAE => BenBE |
| 2013-09-14 16:13 | INOPIAE | Status | new => fix available |
| 2013-09-14 16:44 | INOPIAE | Note Added: 0004319 | |
| 2013-09-14 16:57 | INOPIAE | Note Edited: 0004319 | |
| 2013-09-14 17:10 | BenBE | Source_changeset_attached | => cacert-devel testserver-stable b8afc4a6 |
| 2013-09-14 17:10 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable 9709a2ae |
| 2013-09-14 17:28 | INOPIAE | Note Edited: 0004319 | |
| 2013-09-14 17:42 | BenBE | Reviewed by | => BenBE |
| 2013-09-14 17:42 | BenBE | Status | fix available => needs review & testing |
| 2013-09-14 19:36 | INOPIAE | Note Edited: 0004319 | |
| 2013-09-14 21:26 | INOPIAE | Note Added: 0004324 | |
| 2013-09-14 21:40 | BenBE | Source_changeset_attached | => cacert-devel testserver-stable 20a580d5 |
| 2013-09-14 21:40 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable 4b924244 |
| 2013-09-14 23:50 | BenBE | Source_changeset_attached | => cacert-devel testserver-stable 87c40b72 |
| 2013-09-14 23:50 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable 62d40806 |
| 2013-09-21 06:00 | BenBE | Assigned To | BenBE => egal |
| 2013-10-01 19:55 | BenBE | Source_changeset_attached | => cacert-devel testserver-stable 7f2c3c37 |
| 2013-10-01 19:55 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable b7fa75ca |
| 2013-10-01 19:55 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable 89dd7d96 |
| 2013-10-01 19:55 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable 4fc4ad15 |
| 2013-10-01 19:55 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable c55a67e1 |
| 2013-10-01 19:55 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable bacbfe51 |
| 2013-10-01 19:55 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable 59acdb94 |
| 2013-10-01 19:55 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable 63c09680 |
| 2013-10-01 21:17 | Uli60 | Note Added: 0004365 | |
| 2014-01-21 09:26 | INOPIAE | Note Added: 0004531 | |
| 2014-01-28 21:15 | BenBE | Source_changeset_attached | => cacert-devel testserver-stable 110664e2 |
| 2014-01-28 21:15 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable 39eefaff |
| 2014-01-29 00:40 | NEOatNHNG | Source_changeset_attached | => cacert-devel testserver-stable 8f01f433 |
| 2014-01-29 00:40 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable 7277e4da |
| 2014-01-29 00:41 | NEOatNHNG | Reviewed by | BenBE => |
| 2014-02-04 22:07 | Eva | Note Added: 0004561 | |
| 2014-02-04 22:23 | Eva | Note Added: 0004563 | |
| 2014-02-05 01:54 | NEOatNHNG | Note Added: 0004565 | |
| 2014-02-05 01:55 | NEOatNHNG | Assigned To | egal => INOPIAE |
| 2014-02-05 01:55 | NEOatNHNG | Status | needs review & testing => needs work |
| 2014-02-22 09:35 | INOPIAE | Note Added: 0004595 | |
| 2014-02-22 09:35 | INOPIAE | Assigned To | INOPIAE => BenBE |
| 2014-02-22 09:35 | INOPIAE | Status | needs work => fix available |
| 2014-02-25 21:33 | BenBE | Note Added: 0004605 | |
| 2014-02-26 03:55 | NEOatNHNG | Source_changeset_attached | => cacert-devel testserver-stable 0316d513 |
| 2014-02-26 03:55 | NEOatNHNG | Source_changeset_attached | => cacert-devel testserver-stable e96e2ab4 |
| 2014-02-26 03:55 | NEOatNHNG | Source_changeset_attached | => cacert-devel testserver-stable 942f0c74 |
| 2014-02-26 03:55 | NEOatNHNG | Source_changeset_attached | => cacert-devel testserver-stable f8ce545d |
| 2014-02-26 03:55 | NEOatNHNG | Source_changeset_attached | => cacert-devel testserver-stable 4c796ed2 |
| 2014-02-26 03:55 | NEOatNHNG | Source_changeset_attached | => cacert-devel testserver-stable e8e106e6 |
| 2014-02-26 03:55 | Source_changeset_attached | => cacert-devel testserver-stable 139ea302 | |
| 2014-02-26 03:55 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable 83e83861 |
| 2014-02-26 03:59 | NEOatNHNG | Reviewed by | => NEOatNHNG |
| 2014-02-26 03:59 | NEOatNHNG | Note Added: 0004611 | |
| 2014-02-26 03:59 | NEOatNHNG | Status | fix available => needs review & testing |
| 2014-02-26 04:00 | NEOatNHNG | Note Edited: 0004611 | |
| 2014-03-15 12:01 | INOPIAE | Summary | Check on log into the account if user aggreed to CCA, if not promt him an acception form => Check on log into the account if user aggreed to CCA, if not prompt him an acception form |
| 2014-03-22 14:49 | JensK | Note Added: 0004676 | |
| 2014-04-15 21:18 | Eva | Note Added: 0004724 | |
| 2014-04-15 21:31 | Eva | Note Added: 0004727 | |
| 2014-05-20 21:30 | INOPIAE | Note Added: 0004785 | |
| 2014-09-30 20:03 | INOPIAE | Note Added: 0005035 | |
| 2014-09-30 20:15 | BenBE | Source_changeset_attached | => cacert-devel testserver-stable 7a4bc4f9 |
| 2014-09-30 20:15 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable b90ee5a9 |
| 2014-09-30 20:18 | Eva | Note Added: 0005038 | |
| 2014-10-14 20:01 | INOPIAE | Note Added: 0005049 | |
| 2014-10-14 20:05 | BenBE | Source_changeset_attached | => cacert-devel testserver-stable 2bb611d7 |
| 2014-10-14 20:05 | INOPIAE | Source_changeset_attached | => cacert-devel testserver-stable 58d2ce2b |
| 2014-10-14 20:19 | Eva | Note Added: 0005050 | |
| 2014-10-16 20:07 | NEOatNHNG | Note Added: 0005054 | |
| 2014-11-04 08:13 | BenBE | Reviewed by | NEOatNHNG => NEOatNHNG, BenBE |
| 2014-11-04 08:13 | BenBE | Note Added: 0005088 | |
| 2014-11-04 08:13 | BenBE | Status | needs review & testing => needs testing |
| 2014-11-04 09:14 | wytze | Note Added: 0005090 | |
| 2014-11-04 18:17 | felixd | Note Added: 0005091 | |
| 2014-11-04 18:42 | BenBE | Status | needs testing => ready to deploy |
| 2014-11-23 15:06 | BenBE | Note Added: 0005120 | |
| 2014-11-23 16:00 | BenBE | Source_changeset_attached | => cacert-devel release 7d6bbbd7 |
| 2014-11-24 10:17 | wytze | Note Added: 0005123 | |
| 2014-11-24 10:17 | wytze | Status | ready to deploy => solved? |
| 2014-11-24 10:17 | wytze | Fixed in Version | => 2014 Q4 |
| 2014-11-24 10:17 | wytze | Resolution | open => fixed |
| 2015-03-10 20:09 | INOPIAE | Status | solved? => closed |
