View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0000966 | Main CAcert Website | organisational section | public | 2011-08-02 21:48 | 2013-01-15 17:39 |
Reporter | alex | Assigned To | Ted | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Platform | Test CAcert Website | OS | N/A | OS Version | Test |
Fixed in Version | 2011 Q4 | ||||
Summary | 0000966: Delete Admin for [organization] deletes admin even though cancel button is pressed | ||||
Description | Answering the question "Are you really sure you want to remove A. B. from administering this organisation?" with Cancel-Button user is still removed from organization. | ||||
Steps To Reproduce | Having one organization with more than one org assurer, one main and one additional. 1. Org Assurer -> View 2. shows Org Administrator 3. pressing "Delete" on the additional Administrator a) 4. responding question "Delete Admin for [organization] - Are you really sure you want to remove [org admin] from administering this organisation?" with Cancel button. 5. Additional admin is still deleted. b) 4. responding question "Delete Admin for [organization] - Are you really sure you want to remove [org admin] from administering this organisation?" with Delete button. 5. Additional admin is also deleted. Workaround: Deleting additional admin can only be ommited if neither button is pressed but any other item in main menu structure. | ||||
Tags | No tags attached. | ||||
Reviewed by | Ted, NEOatNHNG | ||||
Test Instructions | |||||
|
Created fix, created git branch bug-966, merged into master, installed on testserver |
|
Tested: - Login as user with Org Admin flag set - Org Assurer -> View Organisations -> List Admins of an org - Click "Delete" of one Admin, then "Cancel" Admin is not deleted ==> OK - Click "Delete" of one Admin then "Delete" Admin is deleted ==> OK |
|
Logged in as OA - Click "Delete" of one Admin, then "Cancel" Admin is not deleted ==> OK - Click "Delete" of one Admin then "Delete" Admin is deleted ==> OK Logged in as Org Admin - Click "Delete" of one Admin, then "Cancel" Admin is not deleted ==> OK - Click "Delete" of one Admin then "Delete" Admin is not deleted ==> false |
|
login with orgadmin@ 1. Org Assurer -> View 2. shows Org Administrator 3. pressing "Delete" on the additional Administrator 4. responding question "Delete Admin for [organization] - Are you really sure you want to remove [org admin] from administering this organisation?" with Cancel button. (returns to overview ->) 5. Additional admin is still visible => ok 6. responding question "Delete Admin for [organization] - Are you really sure you want to remove [org admin] from administering this organisation?" with Delete button. 7. Additional admin is NOT deleted - is still visible ... => ok 8. add admin 9. adding new email 10. new admin added => ok |
|
login with orgassurer@ 1. Org Assurer -> View 2. shows 0 (!) Org Administrator (no org linked to org-assurer account) 3. Org Assurer -> View Organisations 4. select from an organisation Admin (x) 5. shows Admins from domain https://cacert1.it-sls.de/account.php?id=32&orgid=xxx 6. select delete for 2nd admin 7. click cancel 8. lists both admins => ok as expected 9. select delete for 2nd admin 10. click delete 11. lists only remaining admin => ok as expected |
|
state w/o bugfix testscenario w/o bugfix login with orgadmin@ Org Assurer - View select admin list delete 2nd admin delete button account removed from list |
|
State without bug fix Logged in as Org Admin - Click "Delete" of one Admin, then "Cancel" Admin is deleted ==> false - Click "Delete" of one Admin then "Delete" Admin is deleted ==> Ok |
|
/pages/account/34.php sets process variable to delete or cancel (language specific) /pages/account.php doesn't take care about the cancel action request bug fix only checks for english version "Delete" instead of language specific _("Delete") this problem also relates to other areas eg domains this problem needs general review |
|
see report https://bugs.cacert.org/view.php?id=966#c2273 |
|
a one-liner to insert into /includes/account.php if ($process == _("Cancel")) { // General reset CANCEL process requests $process = ""; } solves probably the overall problem with the not working "Cancel" all cancel requests are set to process "", so no action will be triggered within account.php The id and oldid settings still keep intact, so the procedure follows the last process path As far I've tested this workaround, this works for the Org Assurer -> delete OrgAdmin but I did not found another test scenario yet |
|
|
|
/includes/account.php doesn't take care on any "Cancel" request processes A quick fix is to reset the $process variable to empty before the process is handled later within account.php within each section. This fixes the Org Assurer problem deleting an Org Admin, but maybe this fix also fixes other related bugs not yet known and not yet reported with buggy "Cancel" requests. The trick: don't touch the $id and $oldid settings, so the process flow still continues with the section where the $id or $oldid references to. eg Org Assurer area The query line can be expanded with other "stop" process requests. Current setting is for all languages of "Cancel" requests submitted by forms. |
|
I have corrected the form elements themselves and committed the blacklisting of _("Cancel") proposed by Uli. Needs testing and second review |
|
1. login as Org Assurer a) Org Assurer + View Orgs + Select one Org to delete + Press delete + confirm with Cancel (31) Org kept in list => ok 2. login as Org Assurer a) Org Assurer + View Orgs + Select "Admins" of one Org + Press delete + confirm with Cancel (34) Admin kept in list => ok b) Org Assurer + View Orgs + Select "Admins" of one Org + select an admin - Press delete + confirm with Delete (34) Admin deleted from list => ok 3. login as Org Assurer a) Org Assurer + View Orgs + Select "Domains" of one Org + Select one domain to delete + Press delete + confirm with Cancel (30) Domain kept in list => ok b) Org Assurer + View Orgs + Select "Domains" of one Org + Select one domain to delete + Press delete + confirm with Delete (30) Domain revoked from list comment "also revokes all certs" => ok Cert Revocation verification: login as Org Admin Org Client Certs - View all certs are still listed, including last created cert (see under 4) with 2 emails: E = org.client3@<2nd-domain>.de E = org.client3@wiamail.de => ??? Verification 2 - login as Org Assurer Org Assurer - View Orgs - select wiamail wiamail domains listed -> 1 (-> ok, 2nd revoked) strange that client cert with secondary 2nd-domain email address gets not revoked, but this seems to be another bug (-> revoke certs routine) 4. login as Org Admin a) Org Client Certs + New + add email1 + Another email + enter email2 (16) client cert created cert lists only 1 of 2 email addresses Owner => E = org.client1@wiamail.de Alternate name => E-Mail-Adresse: org.client1@wiamail.de 2nd email address that was entered E = org.client1@avintec.de not listed in the cert => ??? Verification: login as Org Assurer select view org select wiamail view listed domains -> 1 -> wiamail.de ok, thats the fault, avintec.de is not listed and so therefor not added to client cert => ok b) adding 2nd domain into domain list (-> success) c) Login as Org Admin Org Client Certs + New + add email3 + Another email + enter email4 (16) client cert created client cert lists 2 email addresses E = org.client3@<2nd-domain>.de E = org.client3@wiamail.de => ok |
|
login in as OrgAssurer in all cases Deleting Organisation Question if delete => ok Cancel => Organisation deleted => false Delete => Organisation delete => ok Deleting domain Question if delete => ok Cancel => nothing happens => ok Delete => Organisation delete => ok Deleting OrgAdmin Question if delete => ok Cancel => nothing happens => ok Delete => OrgAdmin delete => ok |
|
reports https://bugs.cacert.org/view.php?id=966#c2301 and https://bugs.cacert.org/view.php?id=966#c2307 clashes in one point: from https://bugs.cacert.org/view.php?id=966#c2301 1. login as Org Assurer a) Org Assurer + View Orgs + Select one Org to delete + Press delete + confirm with Cancel (31) Org kept in list => ok from https://bugs.cacert.org/view.php?id=966#c2307 login in as OrgAssurer in all cases Deleting Organisation Question if delete => ok Cancel => Organisation deleted => false <=== ?????? Delete => Organisation delete => ok the affected link looks like: https://cacert1.it-sls.de/account.php?id=31&orgid=255 the problem: someone from the software assessors badly fixed /pages/account/31.php (this wasn't in the original patch !) line 32 from: <td class="DataTD" colspan="2"> <input type="submit" name="process" value="<?=_("Cancel")?>"> <input type="submit" name="process" value="<?=_("Delete")?>"> </td> to: <td class="DataTD" colspan="2"> <input type="submit" name="cancel" value="<?=_("Cancel")?>"> <input type="submit" name="process" value="<?=_("Delete")?>"> </td> to name the input field "cancel" instead of "process" is invalid (never checked by /includes/account.php) so the cancel button pressed deletes the organisation see http://git-cacert.it-sls.de/cgi-bin/gitweb.cgi?p=cacert-devel.git;a=blobdiff;f=pages/account/31.php;h=9f3d27e73427872bb4e472ecc5145eb88b8ccf1b;hb=a6e492df7f1b33ad6cd07a5b885d15dd06b4e361;hpb=32038d25e6997a4d5366c4829931279dabe608cb so to reset the /pages/account/31.php fix to its original state from repository cacert commit ce4bfbaf0c2babb5bba2568d3b8712e1615aa651 Author: Michael T<C3><A4>nzer <neo@nhng.de> Date: Sun Aug 21 02:07:40 2011 +0200 |
|
read note https://bugs.cacert.org/view.php?id=966#c2359 for the fix (to remove the not made patch in /pages/account/31.php) |
|
potential problems with 3 other cases: pages/account/30.php (delete certificates issued under org domain) pages/account/16.php (Another Email) pages/account/31.php (delete organisation) pages/account/34.php (remove OrgAdmin from org) |
|
34 - remove orgadmin works 16 - doesn't work as expected another email turns into input of 2nd email, but email will be not added to the client cert 30 - doesn't validate for revocation press delete - deletes the client cert without validation so at least 16.php, 30.php and 31.php needs to be set back to state before fix ?!? (16) orgassurer -> checked org and domains, includes 2 domains new cert -> adding 1st email -> ok "another email" adding 2nd email -> ok "next" cert contains 2 email addresses #1 and 0000002 -> ok so 30.php + 31.php remains to be a problem |
|
<input type="submit" name="cancel" value="<?=_("Cancel")?>"> <input type="submit" name="process" value="<?=_("Delete")?>"> fixed to <input type="submit" name="process" value="<?=_("Cancel")?>"> <input type="submit" name="process" value="<?=_("Delete")?>"> testing delete org -> cancel -> org deleted :-P tested 2 times addtl. fix by ted in account.php prevents functioning a) account.php repaired b) 31.php reset to name="cancel" |
|
I have (hopefully) refixed the bug and pushed the changes to the test server. Needs review and retest |
|
delete org "Bug 934 GmbH, Nordrhein Westfalen DE" sure ? cancel -> org remains delete org "Test GbR, NRW DE" sure ? cancel -> org remains delete org "Bug 934 GmbH, Nordrhein Westfalen DE" with id=256 sure ? delete -> org removed from list Ok |
|
30.php orgassurer - view orgs - select DOMAIN(S) of one org -> domain list of one org 2 domains -> press delete of one domain -> sure ? cancel or delete -> cancel -> returns to list with 2 domains => ok 2 domains -> press delete of one domain -> sure ? cancel or delete -> cancel -> returns to list with 2 domains => ok 2 domains -> press delete of one domain -> sure ? cancel or delete -> delete -> returns to list with 1 domains domain deleted => ok |
|
login in as OrgAssurer in all cases Deleting Organisation Question if delete => ok Cancel => nothing happens => ok Delete => Organisation delete => ok Deleting domain Question if delete => ok Cancel => nothing happens => ok Delete => Organisation delete => ok Deleting OrgAdmin Question if delete => ok Cancel => nothing happens => ok Delete => OrgAdmin delete => ok |
|
Login as OrgAssurer (OrgAdmin flag set) View Organisations -> Admins ---------------------------- - Click "Delete" then "Cancel" Admin is not deleted ==> OK - Click "Delete" then "Delete" Admin is deleted ==> probably OK, but if Admin was "Master" (Master Account=Yes) then organisation remains without master. Is this intended? View Organisations -> Domains ----------------------------- - Click "Delete" then "Cancel" Domain is not deleted ==> OK - Click "Delete" then "Delete" Domain is deleted ==> OK View Organisations ------------------ - Click "Delete" then "Cancel" Organization is not deleted ==> OK |
|
Login as OrgAdmin OrgAssurer -> View -> Organisations -> Admins (2) - Click "Delete" of one Admin, then "Cancel" Admin is not deleted ==> OK - Click "Delete" of one Admin then "Delete" Admin is deleted ==> OK OrgAssurer -> View - Lists besides the organisation and their admins also domains which were given to the organisation. - These domains cannot be any more under "Domains" from the OrgAdmin Account. Why? How can the OrgAdmin from the company handle his domains? - If this is desired behaviour everything is fine but for me it's not clear yet how to behave as OrgAdmin ;-) |
|
Testcase as described in https://wiki.cacert.org/Software/CurrentTest 4. (as Org Admin) Org Client Certs - New - add email1 - Another email - enter email2 (16) After pressing "Next" either: - works fine with domains listed in organisational accounts - displays error message "I couldn't match any emails against your organisational account." when using a domain not listed in organisational account ==> OK |
|
https://bugs.cacert.org/view.php?id=966#c2474 > Login as OrgAssurer (OrgAdmin flag set) - View Organisations -> Admins > - Click "Delete" then "Delete" > Admin is deleted ==> probably OK, but if Admin was "Master" (Master > Account=Yes) then organisation remains without master. Is this intended? yes, this is as expected. Org Assurer who sets MASTER flag to an Org Admin should not be changed automaticly by a system process if not processed by an Org Assurer |
|
delete org tested in https://bugs.cacert.org/view.php?id=966#c2367 delete domain tested in https://bugs.cacert.org/view.php?id=966#c2368 now test with delete admin login as orgassurer (orgadmin flag set) org assurer - view orgs select admin(2) of one domain press https://cacert1.it-sls.de/account.php?id=34&orgid=234&memid=171053 delete confirm? -> cancel page returns to overview, 2 admins listed => ok press https://cacert1.it-sls.de/account.php?id=34&orgid=234&memid=171053 delete confirm? -> delete page returns to overview, 1 admin listed, 1 deleted => ok => all ok test 4: login as orgadmin add org client cert, class3, one email: E = bug966.user387@wiamail.de CN = Bug966 User387 OU = HQ O = Wiamail C = DE => ok add org client cert, class3, multiple emails: user 388, first round, first email -> ok -> another email 2nd round, enter 2nd email -> ok -> another email returns to form with 1st email, 2nd email not in list class3 selection not preset, once checked at step 1 probably this patch enterferes with patch 0000824 (0000824: Organisation User Certificates: Need UI improvement for proper production usage) that adds csr pasting, for org client cert, that adds enable login checkbox so probably bug#824 should be disabled for continue testing bug 966 |
|
after removing bug#824 from testserver (testserver reset did happen 2011-09-27), old fixed procedure by bug 0000966 for org.client.certs is still active on testserver: re test 4: login as orgadmin Org client cert email bug966.user11@wiamail.de class3 another email email bug966.user11@fra.avintec.de class3 next click install view cert: CN Bug966 User11 => ok O Wiamail => ok OU Sales => ok SerNo 10:8B => ok valid 4.10.2011 - 3.10.2012 (1 year) => ok cert content - owner: E = bug966.user11@fra.avintec.de E = bug966.user11@wiamail.de CN = Bug966 User11 OU = Sales O = Wiamail C = DE => ok alternate name: E-Mail-Adresse: bug966.user11@wiamail.de E-Mail-Adresse: bug966.user11@fra.avintec.de => ok Org client cert has settings from last user => reset ? email1 bug966.user12@wiamail.de email2 bug966.user12@fra.avintec.de email3 bug966.user12-alternate3@wiamail.de name Bug966 User12 class1 next click install view cert - all ok details owner: E = bug966.user12-alternate3@wiamail.de E = bug966.user12@fra.avintec.de E = bug966.user12@wiamail.de CN = Bug966 User12 OU = Finance & Controlling O = Wiamail C = DE => ok alternate name: E-Mail-Adresse: bug966.user12@wiamail.de E-Mail-Adresse: bug966.user12@fra.avintec.de E-Mail-Adresse: bug966.user12-alternate3@wiamail.de => ok next round: 4 email fields, 2 from previous run cleared email bug966.user13@wiamail.de class3 - next check client cert - details owner: E = bug966.user13@wiamail.de CN = Bug966 User13 OU = Finance & Controlling O = Wiamail C = DE addtl. email fields cleared => ok alternate name: email: E-Mail-Adresse: bug966.user13@wiamail.de addtl. email fields cleared => ok next round - 2 email fields (1 email used in previous run) email: bug966.user14@wiamail.de class1 next owner and alternate name: 1 email => ok leaving email addresses and names and department fields from run to run is not optimal, but is ok here. more or less email addresses are handled correct in a bulk adding certs procedure, the name from prev run make some sense to remember which one has been passed and which one needs to be used next => ok to me |
|
Don't get the relationship between the bug and Ulis tests, so cancelled own testing. |
|
Reminder sent to: egal, edgarwahn, Sourcerer, Ted This fix only needs a second review. |
|
Did review, handing off to critical admins |
|
The patch has been installed on the production server on October 21, 2011. See also: https://lists.cacert.org/wws/arc/cacert-systemlog/2011-10/msg00011.html |
|
More than 3 month fixed and no complaints |
Date Modified | Username | Field | Change |
---|---|---|---|
2011-08-02 21:48 | alex | New Issue | |
2011-08-02 22:08 | Ted | Note Added: 0002255 | |
2011-08-02 22:08 | Ted | Assigned To | => Ted |
2011-08-02 22:08 | Ted | Status | new => needs review & testing |
2011-08-02 22:10 | Ted | Source_changeset_attached | => cacert-devel master fb256669 |
2011-08-02 22:12 | Ted | Note Added: 0002258 | |
2011-08-02 22:15 | Ted | Note Edited: 0002255 | |
2011-08-02 22:37 | INOPIAE | Note Added: 0002266 | |
2011-08-02 22:46 | Uli60 | Note Added: 0002267 | |
2011-08-02 22:51 | INOPIAE | Note Edited: 0002266 | |
2011-08-02 22:57 | Uli60 | Note Added: 0002269 | |
2011-08-02 23:01 | INOPIAE | Note Edited: 0002266 | |
2011-08-02 23:05 | Uli60 | Note Edited: 0002269 | |
2011-08-02 23:08 | Uli60 | Note Edited: 0002267 | |
2011-08-02 23:23 | Uli60 | Note Added: 0002271 | |
2011-08-02 23:23 | Uli60 | Note Edited: 0002271 | |
2011-08-02 23:25 | INOPIAE | Note Added: 0002272 | |
2011-08-02 23:54 | Uli60 | Note Added: 0002273 | |
2011-08-02 23:55 | Uli60 | Note Added: 0002274 | |
2011-08-02 23:55 | Uli60 | Status | needs review & testing => needs work |
2011-08-03 22:34 | Uli60 | Relationship added | related to 0000938 |
2011-08-10 00:17 | Uli60 | Note Added: 0002287 | |
2011-08-10 00:24 | Uli60 | File Added: account-966-2287.php | |
2011-08-10 00:34 | Uli60 | Note Added: 0002288 | |
2011-08-10 00:34 | Uli60 | Status | needs work => fix available |
2011-08-16 22:40 | NEOatNHNG | Source_changeset_attached | => cacert-devel master 27bb8872 |
2011-08-16 22:40 | NEOatNHNG | Source_changeset_attached | => cacert-devel master a6e492df |
2011-08-16 22:40 | NEOatNHNG | Source_changeset_attached | => cacert-devel master 32038d25 |
2011-08-16 23:25 | NEOatNHNG | Note Added: 0002300 | |
2011-08-16 23:25 | NEOatNHNG | Status | fix available => needs review & testing |
2011-08-16 23:26 | NEOatNHNG | Project | test.cacert.org => Main CAcert Website |
2011-08-16 23:26 | NEOatNHNG | Category | cacert1.it-sls.de => General |
2011-08-16 23:27 | NEOatNHNG | Reviewed by | => NEOatNHNG |
2011-08-16 23:27 | NEOatNHNG | Category | General => organisational section |
2011-08-17 10:45 | Uli60 | Note Added: 0002301 | |
2011-08-17 10:57 | Uli60 | Note Edited: 0002301 | |
2011-08-17 11:03 | Uli60 | Note Edited: 0002301 | |
2011-08-17 11:09 | Uli60 | Note Edited: 0002301 | |
2011-08-17 11:23 | Uli60 | Note Edited: 0002301 | |
2011-08-19 05:46 | INOPIAE | Note Added: 0002307 | |
2011-08-30 14:42 | Uli60 | Note Added: 0002359 | |
2011-08-30 14:57 | Uli60 | Note Edited: 0002359 | |
2011-08-30 14:58 | Uli60 | Note Added: 0002360 | |
2011-08-30 14:58 | Uli60 | Assigned To | Ted => Uli60 |
2011-08-30 14:58 | Uli60 | Status | needs review & testing => fix available |
2011-08-30 15:08 | Uli60 | Note Edited: 0002359 | |
2011-08-30 15:11 | Uli60 | Note Edited: 0002359 | |
2011-08-30 15:12 | Uli60 | Note Added: 0002361 | |
2011-08-30 15:14 | Uli60 | Note Edited: 0002361 | |
2011-08-30 15:17 | Uli60 | Note Edited: 0002361 | |
2011-08-30 15:30 | Uli60 | Note Added: 0002362 | |
2011-08-30 19:27 | Uli60 | Note Edited: 0002362 | |
2011-08-30 19:33 | Uli60 | Note Edited: 0002362 | |
2011-08-30 21:25 | Uli60 | Note Added: 0002365 | |
2011-08-30 21:33 | NEOatNHNG | Note Added: 0002366 | |
2011-08-30 21:33 | NEOatNHNG | Status | fix available => needs review & testing |
2011-08-30 21:35 | NEOatNHNG | Source_changeset_attached | => cacert-devel master a58d70fb |
2011-08-30 21:35 | NEOatNHNG | Source_changeset_attached | => cacert-devel master fba0e788 |
2011-08-30 21:35 | Uli60 | Note Added: 0002367 | |
2011-08-30 22:02 | Uli60 | Note Added: 0002368 | |
2011-08-31 05:31 | INOPIAE | Note Added: 0002379 | |
2011-09-15 16:45 | Uli60 | Relationship replaced | has duplicate 0000938 |
2011-09-20 21:46 | alex | Note Added: 0002474 | |
2011-09-20 22:09 | alex | Note Added: 0002475 | |
2011-09-20 22:19 | alex | Note Added: 0002476 | |
2011-09-21 11:15 | Uli60 | Note Added: 0002481 | |
2011-09-22 22:22 | Uli60 | Note Added: 0002491 | |
2011-09-22 22:25 | Uli60 | Note Edited: 0002491 | |
2011-09-22 22:40 | Uli60 | Note Edited: 0002491 | |
2011-09-22 22:48 | Uli60 | Note Edited: 0002491 | |
2011-09-27 23:12 | NEOatNHNG | Source_changeset_attached | => cacert-devel testserver 11545f0b |
2011-10-04 14:46 | Uli60 | Note Added: 0002570 | |
2011-10-04 14:56 | Uli60 | Note Edited: 0002570 | |
2011-10-04 14:58 | Uli60 | Note Edited: 0002570 | |
2011-10-06 18:54 | illuminat | Note Added: 0002578 | |
2011-10-20 17:26 | NEOatNHNG | Assigned To | Uli60 => Ted |
2011-10-20 17:26 | NEOatNHNG | Status | needs review & testing => needs review |
2011-10-20 17:28 | NEOatNHNG | Note Added: 0002612 | |
2011-10-21 19:40 | Ted | Reviewed by | NEOatNHNG => Ted, NEOatNHNG |
2011-10-21 19:40 | Ted | Note Added: 0002625 | |
2011-10-21 19:40 | Ted | Status | needs review => ready to deploy |
2011-10-21 20:05 | wytze | Note Added: 0002626 | |
2011-10-21 20:05 | wytze | Status | ready to deploy => solved? |
2011-10-21 20:05 | wytze | Resolution | open => fixed |
2011-11-20 00:35 | NEOatNHNG | Source_changeset_attached | => cacert-devel release d9330116 |
2012-12-21 05:21 | Werner Dworak | Note Added: 0003526 | |
2012-12-21 05:21 | Werner Dworak | Status | solved? => closed |
2013-01-15 17:39 | Werner Dworak | Fixed in Version | => 2011 Q4 |