Modified page as per issue: bug-1550. #5

bmc · 2023-08-22T17:08:23Z

bmc commented

2023-08-22 17:08:23 +00:00

Following the request and notes in Bug-1550, implemented the following.

bmc added 1 commit 2023-08-22 17:08:24 +00:00

Modified page as per issue: bug-1550. c2f3370013

bmc requested review from jandd 2023-08-22 17:08:50 +00:00

jandd requested review from ted 2023-08-23 11:54:33 +00:00

jandd reviewed 2023-08-23 11:58:20 +00:00

pages/account/3.php

					
				@ -103,4 +31,0 @@

				  </tr>

				<?

				if($_SESSION['profile']['points'] >= 50)

jandd commented

2023-08-23 11:58:20 +00:00

Is this the only occurrence of the 50 points check? Have you checked whether CommModule/client.pl or at least the place where the INSERT of the CSR occurs make sure that it is not possible to create certificates with the full name of the user?

bmc commented

2023-08-25 16:03:17 +00:00

My initial checks showed that the rest of the system continued to apply the 50 point check, but I will re-check as you have suggested.

bmc commented

2023-08-26 21:09:32 +00:00

I tried to reply to this, but got a CSRF error. Trying again.

I did not change any code in the system outside of page/account/3.php, and all of my examination of other code appears to show that nothing in the tests for 50 points has changed.

I tried to reply to this, but got a CSRF error. Trying again. I did not change any code in the system outside of page/account/3.php, and all of my examination of other code appears to show that nothing in the tests for 50 points has changed.

bmc marked this conversation as resolved

jandd reviewed 2023-08-23 11:59:22 +00:00

jandd left a comment

Advice for next time: please keep whitespace changes to a minimum

jandd commented

2024-09-30 09:33:15 +00:00

@bmc could you please resolve the merge conflict with the current version in the main branch? The large set of whitespace changes makes it hard to review this PR properly.

If the intended change (as discussed in #1550) is the only thing that needs to change it may be sufficient to make the "rootcert" form field into a hidden field with the value 2 and don't display a selection at all. This would be a 2 line change against the current code instead of the large change this PR contains currently.

@bmc could you please resolve the merge conflict with the current version in the main branch? The large set of whitespace changes makes it hard to review this PR properly. If the intended change (as discussed in #1550) is the only thing that needs to change it may be sufficient to make the "rootcert" form field into a hidden field with the value 2 and don't display a selection at all. This would be a 2 line change against the current code instead of the large change this PR contains currently.

bmc was assigned by jandd

2024-09-30 09:33:26 +00:00

bmc removed review request for ted 2024-10-07 00:17:40 +00:00

bmc removed review request for jandd 2024-10-07 00:17:41 +00:00

bmc commented

2024-10-07 00:20:12 +00:00

I have replaced this branch with fix/bug-1550a implementing the change as requested by JanDD and therefore, this PR can be discarded.

bmc closed this pull request

2024-10-07 00:20:13 +00:00

jandd deleted branch fix/bug-1550

2024-10-16 15:07:55 +00:00