View Issue Details

IDProjectCategoryView StatusLast Update
0001192Main CAcert Websitewebsite contentpublic2015-03-10 20:09
ReporterINOPIAEAssigned ToBenBE 
PrioritynormalSeverityminorReproducibilityhave not tried
Status closedResolutionfixed 
Product Version2013 Q3 
Target Version2013 Q4Fixed in Version2014 Q4 
Summary0001192: Check on log into the account if user aggreed to CCA, if not prompt him an acception form
DescriptionAfter 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
TagsNo tags attached.
Reviewed byNEOatNHNG, BenBE
Test Instructions

Relationships

related to 0000505 closedINOPIAE CCA agree mark 

Activities

INOPIAE

2013-09-14 16:13

updater   ~0004318

I added the fix to https://github.com/INOPIAE/CAcert/commit/9709a2ae6177f612ca06e2698041494fdafb7992

INOPIAE

2013-09-14 16:44

updater   ~0004319

Last edited: 2013-09-14 19:36

View 4 revisions

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')

INOPIAE

2013-09-14 21:26

updater   ~0004324

I applied a new fix https://github.com/INOPIAE/CAcert/commit/4b924244c859aba7daf8c6e04036784d49a767e4

Uli60

2013-10-01 21:17

updater   ~0004365

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

INOPIAE

2014-01-21 09:26

updater   ~0004531

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

Eva

2014-02-04 22:07

updater   ~0004561

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

Eva

2014-02-04 22:23

updater   ~0004563

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

NEOatNHNG

2014-02-05 01:54

administrator   ~0004565

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

INOPIAE

2014-02-22 09:35

updater   ~0004595

I pushed a new fix to https://github.com/INOPIAE/CAcert/tree/bug-1192

BenBE

2014-02-25 21:33

updater   ~0004605

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.

NEOatNHNG

2014-02-26 03:59

administrator   ~0004611

Last edited: 2014-02-26 04:00

View 2 revisions

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.

JensK

2014-03-22 14:49

reporter   ~0004676

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.

Eva

2014-04-15 21:18

updater   ~0004724

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

Eva

2014-04-15 21:31

updater   ~0004727

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

INOPIAE

2014-05-20 21:30

updater   ~0004785

for newsletter see https://wiki.cacert.org/PR/News/CCARollout/EN

INOPIAE

2014-09-30 20:03

updater   ~0005035

I pushed the fix with the explanation text to:
https://github.com/INOPIAE/CAcert/commit/b90ee5a97dcf351e22f43f26bf971c3f39fcdf85

Eva

2014-09-30 20:18

updater   ~0005038

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

INOPIAE

2014-10-14 20:01

updater   ~0005049

New fix pushed to https://github.com/INOPIAE/CAcert.git
b90ee5a..58d2ce2

Eva

2014-10-14 20:19

updater   ~0005050

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

NEOatNHNG

2014-10-16 20:07

administrator   ~0005054

Review OK. Still needs a second review.

BenBE

2014-11-04 08:13

updater   ~0005088

Second review OK.

Text preview sent to critical for inclusion of the translations.

I don't see two tests yet though.

wytze

2014-11-04 09:14

developer   ~0005090

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.

felixd

2014-11-04 18:17

updater   ~0005091

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

BenBE

2014-11-23 15:06

updater   ~0005120

Sent to crit for deployment.

wytze

2014-11-24 10:17

developer   ~0005123

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

Issue History

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 View Revisions
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 View Revisions
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 View Revisions
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 => dastrath
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 dastrath => 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 View Revisions
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