Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review69367 --- forward port, for interested ones - https://git.reviewboard.kde.org/r/120876/ - Hrvoje Senjan On Oct. 10, 2014, 1:30 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 10, 2014, 1:30 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 28, 2014, 11:05 p.m., Hrvoje Senjan wrote: forward port, for interested ones - https://git.reviewboard.kde.org/r/120876/ Thank you, Hrvoje, you are a gentleman and a scholar... and I am happy to have been able to help... :-) - Ian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review69367 --- On Oct. 9, 2014, 11:30 p.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 9, 2014, 11:30 p.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68183 --- A simplified patch for Dr Konqi went in for review about 20 hours ago. There are now about 4 hours till the KDE 4.14.2 deadline and there has been no feedback re the new patch, but it does follow previous reviewers' suggestions. So I propose to commit this code and thus fix https://bugs.kde.org/show_bug.cgi?id=337742 and also protect Dr Konqi from token-based security being discontinued in the future in Bugzilla software. - Ian Wadham On Oct. 9, 2014, 12:06 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 9, 2014, 12:06 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Okt. 9, 2014, 8:06 nachm., Ian Wadham wrote: A simplified patch for Dr Konqi went in for review about 20 hours ago. There are now about 4 hours till the KDE 4.14.2 deadline and there has been no feedback re the new patch, but it does follow previous reviewers' suggestions. So I propose to commit this code and thus fix https://bugs.kde.org/show_bug.cgi?id=337742 and also protect Dr Konqi from token-based security being discontinued in the future in Bugzilla software. No functional objections from my side (some coding style, but I don't maintain that code and it's not kdelibs) It would be really great if anyone with a record on DrKonqi could have commented, but fact is that DrKonqi is broken right now and could hardly break more. And a broken bug report tool (that shows it's brokeness *after* the user took the effort to report a bug) makes KDE look really bad. If Albert (or his release team hat) doesn't veto, you should push it - as alternative, Albert (representing the release team) could offer to pick it between tag and release. - Thomas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68183 --- On Okt. 9, 2014, 12:06 vorm., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Okt. 9, 2014, 12:06 vorm.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On oct. 7, 2014, 1:13 p.m., Thomas Lübking wrote: My 2¢ Bugzilla will require an update anyway and that means at some point it'll be (then silently) broken in KDE SC4 again and somebody has to step up and fix it with another patch. In the meantime we've diverging codebases for KDE 4 5 - meh. I agree with Albert that this patch looks a bit scaringly complex (at least compared to Frédéric's patch), but believe that the complexity can be vastly reduced and like a forward compatible and 4+5 common patch better. Albert Astals Cid wrote: You have a point here, if it's possible that Frédéric's patch gets broken in the timeframe we still have users around using kde-runtime4 then that would be a good reason to use this patch. I'd appreciate an assesment on how much more future-proof this patch is versus Frédéric's one. Thomas Lübking wrote: Afaiu it will break when the bugzilla server upgrades to 5.0 (the token security model will be dropped) but I could not find a schedule for future bugzilla releases (nor know about bugs.kde.org update policy) - Ben? If users around using kde-runtime4 is the critical condition, this seems a likely threat, though (given eg. RHEL lifetimes - RHEL7 extended support ends 2027 ;-) Ben Cooksley wrote: bugs.kde.org is updated when it becomes necessary (security issues) or when someone gets around to deploying the latest release. There isn't really a schedule as such. Based on the above comment, i'd suggest making Dr Konqi as capable as possible - although do remember that we probably don't want to receive bug reports from extremely old versions of our software, even if RHEL is supporting it. Ian Wadham wrote: @Albert: I had to cherry-pick Revision 681446e1 from master into KDE/4.14 today. This was committed to master over 2 weeks ago, but I did not realise then that it had to go into KDE/4.14 too. It fixes a bug in the backtrace formatting on all platforms, makes sure the Dr Konqi window is on top of the crashed app's window on all platforms and has a workaround for a crash caused by KCookieJar not being found on Apple OS X. The third item has to go into the repository first, because the patch for this present review (which avoids using cookies) affects the same area of code. Sorry for the noise. cherry-pick Revision 681446e1 In which repo? - Albert --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68051 --- On oct. 9, 2014, 12:06 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated oct. 9, 2014, 12:06 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On oct. 9, 2014, 8:06 p.m., Ian Wadham wrote: A simplified patch for Dr Konqi went in for review about 20 hours ago. There are now about 4 hours till the KDE 4.14.2 deadline and there has been no feedback re the new patch, but it does follow previous reviewers' suggestions. So I propose to commit this code and thus fix https://bugs.kde.org/show_bug.cgi?id=337742 and also protect Dr Konqi from token-based security being discontinued in the future in Bugzilla software. Thomas Lübking wrote: No functional objections from my side (some coding style, but I don't maintain that code and it's not kdelibs) It would be really great if anyone with a record on DrKonqi could have commented, but fact is that DrKonqi is broken right now and could hardly break more. And a broken bug report tool (that shows it's brokeness *after* the user took the effort to report a bug) makes KDE look really bad. If Albert (or his release team hat) doesn't veto, you should push it - as alternative, Albert (representing the release team) could offer to pick it between tag and release. Just commit it :) - Albert --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68183 --- On oct. 9, 2014, 12:06 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated oct. 9, 2014, 12:06 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68187 --- Ship it! Ship It! - Albert Astals Cid On oct. 9, 2014, 12:06 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated oct. 9, 2014, 12:06 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 7, 2014, 1:13 p.m., Thomas Lübking wrote: My 2¢ Bugzilla will require an update anyway and that means at some point it'll be (then silently) broken in KDE SC4 again and somebody has to step up and fix it with another patch. In the meantime we've diverging codebases for KDE 4 5 - meh. I agree with Albert that this patch looks a bit scaringly complex (at least compared to Frédéric's patch), but believe that the complexity can be vastly reduced and like a forward compatible and 4+5 common patch better. Albert Astals Cid wrote: You have a point here, if it's possible that Frédéric's patch gets broken in the timeframe we still have users around using kde-runtime4 then that would be a good reason to use this patch. I'd appreciate an assesment on how much more future-proof this patch is versus Frédéric's one. Thomas Lübking wrote: Afaiu it will break when the bugzilla server upgrades to 5.0 (the token security model will be dropped) but I could not find a schedule for future bugzilla releases (nor know about bugs.kde.org update policy) - Ben? If users around using kde-runtime4 is the critical condition, this seems a likely threat, though (given eg. RHEL lifetimes - RHEL7 extended support ends 2027 ;-) Ben Cooksley wrote: bugs.kde.org is updated when it becomes necessary (security issues) or when someone gets around to deploying the latest release. There isn't really a schedule as such. Based on the above comment, i'd suggest making Dr Konqi as capable as possible - although do remember that we probably don't want to receive bug reports from extremely old versions of our software, even if RHEL is supporting it. Ian Wadham wrote: @Albert: I had to cherry-pick Revision 681446e1 from master into KDE/4.14 today. This was committed to master over 2 weeks ago, but I did not realise then that it had to go into KDE/4.14 too. It fixes a bug in the backtrace formatting on all platforms, makes sure the Dr Konqi window is on top of the crashed app's window on all platforms and has a workaround for a crash caused by KCookieJar not being found on Apple OS X. The third item has to go into the repository first, because the patch for this present review (which avoids using cookies) affects the same area of code. Sorry for the noise. Albert Astals Cid wrote: cherry-pick Revision 681446e1 In which repo? That fix is 681446e1 in the (KDE 4) kde-runtime repo, KDE/14.4 branch, and it is 25ec1c8d in kde-runtime, master branch. I cherry-picked it from master to KDE/14.4 in my local repo, then I pushed it to origin KDE/14.4. - Ian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68051 --- On Oct. 9, 2014, 12:06 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 9, 2014, 12:06 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 9, 2014, 11:30 p.m.) Status -- This change has been marked as submitted. Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 9, 2014, 12:06 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Changes --- Simplified BugzillaManager::setFeaturesForVersion() by using the KDE_MAKE_VERSION macro. Removed the commented-out testing code. Re-tested with a range of version numbers, past, present, future, incomplete and having more tha 3 parts. Performed end-to-end tests of crash-report submission, including new bug reports, attachment to previous bugs and rejection of duplicates. This patch will be committed to KDE 4.14 and master later today, if there are no objections. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs (updated) - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 7, 2014, 1:13 p.m., Thomas Lübking wrote:
drkonqi/bugzillalib.cpp, line 81
https://git.reviewboard.kde.org/r/120431/diff/4/?file=316623#file316623line81
The patch largely consists of hand-crafted version handling.
replacing this by int version = KDE_MAKE_VERSION(major, minor,
release); and simple integer metricts for comparism should considerably
lower complexity (thus make the patch easier to be accepted ;-)
Yes, I know it's crap to write a lot of code and remove it afterwards.
Ian Wadham wrote:
This is exactly the kind of advice I hope for in a review. I did look to
see, several weeks ago, if there were some version-compare methods in KDE or
Qt or on Google, but did not find any. I also considered something like
version = (major * 100 + minor * 1000 + release), but thought it would be
rather a 1960s style kludge... ;-) I have lived and worked through all the
trouble those YYMMDD strings caused (Y2K and all that). Still, I suppose the
parts of Bugzilla versions are unlikely to go past 255...
I have no objection to deleting code and replacing it with something
shorter and simpler. I guess I would still need the code to convert a QString
version (from Bugzilla and the bugs.kde.org database) to a single
KDE_MAKE_VERSION integer, or is there something else in KDE/Qt to do that?
What *does* worry me is 5-minutes-to-midnight re-programming, i.e. I
would have to make and test all changes on Thursday, just hours before Albert
starts tagging KDE 4.14.2. In my experience, that could be riskier than
committing my patch as it stands.
But either way, we at least have a month to fix any problems before KDE
4.14.3, the last release of KDE 4. Then there is always KDE 14.12 and Dr
Konqi is an app, not a library...
@Albert and Thomas: Please let me know what you would like me to commit
on Thursday: my patch as it stands, my patch simplified as Thomas suggests or
Frédéric's patch.
Albert Astals Cid wrote:
I would prefer the simplified version. If you really feel you're going to
break the code, i'll take a commitment to do the simplified version for 4.14.3
Thomas Lübking wrote:
I guess I would still need the code to convert a QString version (from
Bugzilla and the bugs.kde.org database) to a single KDE_MAKE_VERSION integer,
or is there something else in KDE/Qt to do that?
You'll have to split the received string into 3 integers (major, minor,
release) for the KDE_MAKE_VERSION macro.
I found no spec for it. Is it really more than 1.2.3?
In case: the present number splitting just collects the first 3 numbers -
you'd rather have to isolate [0-9]*.[0-9]*.[0-9]* first.
If not:
```cpp
QStringList l = string.split(QLatin1Char('1'));
if (l.count() == 3)
version = KDE_MAKE_VERSION(l.at(0).toUInt(), l.at(1).toUInt(),
l.at(2).toUInt());
else
kDebug() sth's severely fucked up here;
fi
Also added a couple of lines to pad out the Bugzilla version number if it has
3 parts and a kWarning() if it has 3 parts.
On Oct. 7, 2014, 1:13 p.m., Thomas Lübking wrote:
drkonqi/bugzillalib.cpp, line 125
https://git.reviewboard.kde.org/r/120431/diff/4/?file=316623#file316623line125
I'll frankly call this over-engineered (though likely just as result of
the handcrafted version stuff):
m_security = UseCookies;
m_otherStuff = OldDefault;
if (version KDE_MAKE_VERSION(4,4,3)) {
m_security = UseTokens;
}
if (version KDE_MAKE_VERSION(4,4,4)) {
m_otherStuff = NewStuff;
}
if (version KDE_MAKE_VERSION(5,0,0)) {
m_security = UsePasswords;
}
...
This adds better connection between version requirement and impact.
Thanks, Thomas, the simplified patch uses the above approach. But, ahem, there
are three logic errors in the above... :-) Need to use = in the comparisons...
On Oct. 7, 2014, 1:13 p.m., Thomas Lübking wrote:
drkonqi/bugzillalib.cpp, line 159
https://git.reviewboard.kde.org/r/120431/diff/4/?file=316623#file316623line159
this apprently either enfores a strict numeric order in the changes
array or verify your comparism algo.
It should not abort outside debug builds (and I don't see why it should
abort for changes order at all)
The logic needs to go in ascending order of versions and the same in your
KDE_MAKE_VERSIONS approach. Essentially it fast-forwards through the versions
until the right internal settings (of m_security, etc.) are reached, simulating
the changes that occur in the real Bugzilla. If you do them out of order, the
last change could be the winner...
- Ian
---
This is an automatically generated e-mail. To reply, visit:
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 7, 2014, 1:13 p.m., Thomas Lübking wrote: My 2¢ Bugzilla will require an update anyway and that means at some point it'll be (then silently) broken in KDE SC4 again and somebody has to step up and fix it with another patch. In the meantime we've diverging codebases for KDE 4 5 - meh. I agree with Albert that this patch looks a bit scaringly complex (at least compared to Frédéric's patch), but believe that the complexity can be vastly reduced and like a forward compatible and 4+5 common patch better. Albert Astals Cid wrote: You have a point here, if it's possible that Frédéric's patch gets broken in the timeframe we still have users around using kde-runtime4 then that would be a good reason to use this patch. I'd appreciate an assesment on how much more future-proof this patch is versus Frédéric's one. Thomas Lübking wrote: Afaiu it will break when the bugzilla server upgrades to 5.0 (the token security model will be dropped) but I could not find a schedule for future bugzilla releases (nor know about bugs.kde.org update policy) - Ben? If users around using kde-runtime4 is the critical condition, this seems a likely threat, though (given eg. RHEL lifetimes - RHEL7 extended support ends 2027 ;-) Ben Cooksley wrote: bugs.kde.org is updated when it becomes necessary (security issues) or when someone gets around to deploying the latest release. There isn't really a schedule as such. Based on the above comment, i'd suggest making Dr Konqi as capable as possible - although do remember that we probably don't want to receive bug reports from extremely old versions of our software, even if RHEL is supporting it. @Albert: I had to cherry-pick Revision 681446e1 from master into KDE/4.14 today. This was committed to master over 2 weeks ago, but I did not realise then that it had to go into KDE/4.14 too. It fixes a bug in the backtrace formatting on all platforms, makes sure the Dr Konqi window is on top of the crashed app's window on all platforms and has a workaround for a crash caused by KCookieJar not being found on Apple OS X. The third item has to go into the repository first, because the patch for this present review (which avoids using cookies) affects the same area of code. Sorry for the noise. - Ian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68051 --- On Oct. 9, 2014, 12:06 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 9, 2014, 12:06 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 5, 2014, 7:43 a.m., Ben Cooksley wrote: As this is needed to restore the functionality of Dr Konqi, can someone familiar with the codebase please review it so we can get this in? Ian Wadham wrote: Perhaps I am the person most familiar with the codebase of Dr Konqi, having worked on it for a few months now. So, if nobody else steps forward within the next 24 hours, I will do my own Ship It and commit to KDE 4 kde-runtime master in time for Thursday's close of the KDE 4.14.2 bugfix release. If KDE core developers want the Dr Konqi bugs fixed in KF5, they will have to forward-port this change and my earlier changes themselves. I cannot do that. I work on Apple OS X and we do not have KF5 and Qt 5 building there yet. I do not propose to address Thomas's comments. I have more important things to do. Albert Astals Cid wrote: With my release team hat: - Sure commit to master if you can't find someone else to review and you *know* your code is right and you're going to fix it when/if it break - No, don't commit to KDE/4.14 this is a huge change and I don't feel like it is guaranteed to go in, you can be a good guy and review https://git.reviewboard.kde.org/r/120376/ since it seems that one fixes the immediate problems people are having, no? (you say you're the one that knows the code more yet have not reviewed it, why?) - Your obsession to not contribute to KF5 based versions will byte you again when you decide to move to KF5, you should really rethink it. Because we do have KF5 and Qt5 building on MacOsX according to at least one of the members of the MacOSX team, no? Marko Käning wrote: I wouldn not phrase it an obsession to not to contribute to KF5. ;) In fact, it has been pointed out multiple times on KDE-MAC, in pm as well as in various RRs, that the MacOSX team at the moment mainly tries to get KDE4 into a working state, which is why Ian wants to push this forward. And yes, we have KF5 on Qt5 in a state where my OSX/CI(/Jenkins) systems are able to build and install many projects successfully. BUT, unfortunately, this does NOT mean that I am able to RUN every of these apps successfully, as the OSX/CI system's specifics (being that all frameworks, libs and apps live in their own install roots) in conjunction with a (still missing) working QStandardPaths patch lead to the problem that most of the apps can't find their config and data files at runtime at this point in time. :( As I am *alone* on KF5, I haven't managed to proceed with the QStandardPaths issue, since many other things kept me far too busy (like the inclusion of new projects on OSX/CI, dealing with Jenkins master [also for Linux], tending project dependencies, creating a KDE4.13 branch on our macports-kde git repo, testing KDE4 applications, etc...). Eventually I conclude herewith that the MacOSX team: - does contribute directly to Qt5/KF5 big time - althought it is only me ATM, - does indirectly contribute to Qt5/KF5, as many RRs can be modified easily for inclusion into KF5, as it has happened already for e.g. https://git.reviewboard.kde.org/r/119847/ and https://git.reviewboard.kde.org/r/119848/ - believes that 1st priority should be to get KDE4 in good shape on OSX, which is why Nicolas wants to release KDE 4.13.3 this week and will proceed with 4.14.x right afterwards, - needs decent user feedback with valuable backtraces which is why a non-dysfunctional DrKonqi is required on all OSX versions, hence this RR. Thomas Lübking wrote: Screw OSX ;-) Afaiu DrKonqui is dysfunctional due to bugzilla server changes atm. (bug #337742) what means either this or review https://git.reviewboard.kde.org/r/120376/ more or less *has* to go into KDE SC4 - regardless on whether it's required for exotic OS' or not. @Ian, please attach Jekyll Wu, George Kiagiadakis, Matthias Fuchs and Dario Andres Rodriguez, they've been the main submitters to bugzillalib. Feel free to cancel anything I may commit, Albert, but you could be doing both yourself and KDE software (4 and 5) a disservice. Bug https://bugs.kde.org/show_bug.cgi?id=337742 will remain... I only said perhaps in my statement about the codebase, hoping someone more knowledgeable would step up. If you will read the Description of this review (Note 2) and the dialog in https://git.reviewboard.kde.org/r/120376/ you will find that I have already had some input to the other review and given it some thought. Last week I was expecting a core developer who knows Dr Konqi to review both patches, but nobody did. This week I have a course to prepare, for presentation tomorrow, so I have no time, and KDE 4.14.2 closes on Thursday. I do not even know whether Frédéric has commit privileges. One swallow does
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 7, 2014, 6:31 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, and Darío Andrés Rodríguez. Changes --- Hi Dario, Are you not on kde-core-devel? Are you able to review this stuff as a matter of urgency? Seems you are the one who knows most about Dr Konqi... Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 5, 2014, 7:43 a.m., Ben Cooksley wrote: As this is needed to restore the functionality of Dr Konqi, can someone familiar with the codebase please review it so we can get this in? Ian Wadham wrote: Perhaps I am the person most familiar with the codebase of Dr Konqi, having worked on it for a few months now. So, if nobody else steps forward within the next 24 hours, I will do my own Ship It and commit to KDE 4 kde-runtime master in time for Thursday's close of the KDE 4.14.2 bugfix release. If KDE core developers want the Dr Konqi bugs fixed in KF5, they will have to forward-port this change and my earlier changes themselves. I cannot do that. I work on Apple OS X and we do not have KF5 and Qt 5 building there yet. I do not propose to address Thomas's comments. I have more important things to do. Albert Astals Cid wrote: With my release team hat: - Sure commit to master if you can't find someone else to review and you *know* your code is right and you're going to fix it when/if it break - No, don't commit to KDE/4.14 this is a huge change and I don't feel like it is guaranteed to go in, you can be a good guy and review https://git.reviewboard.kde.org/r/120376/ since it seems that one fixes the immediate problems people are having, no? (you say you're the one that knows the code more yet have not reviewed it, why?) - Your obsession to not contribute to KF5 based versions will byte you again when you decide to move to KF5, you should really rethink it. Because we do have KF5 and Qt5 building on MacOsX according to at least one of the members of the MacOSX team, no? Marko Käning wrote: I wouldn not phrase it an obsession to not to contribute to KF5. ;) In fact, it has been pointed out multiple times on KDE-MAC, in pm as well as in various RRs, that the MacOSX team at the moment mainly tries to get KDE4 into a working state, which is why Ian wants to push this forward. And yes, we have KF5 on Qt5 in a state where my OSX/CI(/Jenkins) systems are able to build and install many projects successfully. BUT, unfortunately, this does NOT mean that I am able to RUN every of these apps successfully, as the OSX/CI system's specifics (being that all frameworks, libs and apps live in their own install roots) in conjunction with a (still missing) working QStandardPaths patch lead to the problem that most of the apps can't find their config and data files at runtime at this point in time. :( As I am *alone* on KF5, I haven't managed to proceed with the QStandardPaths issue, since many other things kept me far too busy (like the inclusion of new projects on OSX/CI, dealing with Jenkins master [also for Linux], tending project dependencies, creating a KDE4.13 branch on our macports-kde git repo, testing KDE4 applications, etc...). Eventually I conclude herewith that the MacOSX team: - does contribute directly to Qt5/KF5 big time - althought it is only me ATM, - does indirectly contribute to Qt5/KF5, as many RRs can be modified easily for inclusion into KF5, as it has happened already for e.g. https://git.reviewboard.kde.org/r/119847/ and https://git.reviewboard.kde.org/r/119848/ - believes that 1st priority should be to get KDE4 in good shape on OSX, which is why Nicolas wants to release KDE 4.13.3 this week and will proceed with 4.14.x right afterwards, - needs decent user feedback with valuable backtraces which is why a non-dysfunctional DrKonqi is required on all OSX versions, hence this RR. Thomas Lübking wrote: Screw OSX ;-) Afaiu DrKonqui is dysfunctional due to bugzilla server changes atm. (bug #337742) what means either this or review https://git.reviewboard.kde.org/r/120376/ more or less *has* to go into KDE SC4 - regardless on whether it's required for exotic OS' or not. @Ian, please attach Jekyll Wu, George Kiagiadakis, Matthias Fuchs and Dario Andres Rodriguez, they've been the main submitters to bugzillalib. Ian Wadham wrote: Feel free to cancel anything I may commit, Albert, but you could be doing both yourself and KDE software (4 and 5) a disservice. Bug https://bugs.kde.org/show_bug.cgi?id=337742 will remain... I only said perhaps in my statement about the codebase, hoping someone more knowledgeable would step up. If you will read the Description of this review (Note 2) and the dialog in https://git.reviewboard.kde.org/r/120376/ you will find that I have already had some input to the other review and given it some thought. Last week I was expecting a core developer who knows Dr Konqi to review both patches, but nobody did. This week I have a course to prepare, for presentation tomorrow, so I have no time, and KDE 4.14.2 closes on Thursday. I do not even
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 7, 2014, 6:49 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, and Jekyll Wu. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated oct. 7, 2014, 7:42 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Changes --- Added George Kiagiadakis and Matthias Fuchs as suggested Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On oct. 5, 2014, 7:43 a.m., Ben Cooksley wrote: As this is needed to restore the functionality of Dr Konqi, can someone familiar with the codebase please review it so we can get this in? Ian Wadham wrote: Perhaps I am the person most familiar with the codebase of Dr Konqi, having worked on it for a few months now. So, if nobody else steps forward within the next 24 hours, I will do my own Ship It and commit to KDE 4 kde-runtime master in time for Thursday's close of the KDE 4.14.2 bugfix release. If KDE core developers want the Dr Konqi bugs fixed in KF5, they will have to forward-port this change and my earlier changes themselves. I cannot do that. I work on Apple OS X and we do not have KF5 and Qt 5 building there yet. I do not propose to address Thomas's comments. I have more important things to do. Albert Astals Cid wrote: With my release team hat: - Sure commit to master if you can't find someone else to review and you *know* your code is right and you're going to fix it when/if it break - No, don't commit to KDE/4.14 this is a huge change and I don't feel like it is guaranteed to go in, you can be a good guy and review https://git.reviewboard.kde.org/r/120376/ since it seems that one fixes the immediate problems people are having, no? (you say you're the one that knows the code more yet have not reviewed it, why?) - Your obsession to not contribute to KF5 based versions will byte you again when you decide to move to KF5, you should really rethink it. Because we do have KF5 and Qt5 building on MacOsX according to at least one of the members of the MacOSX team, no? Marko Käning wrote: I wouldn not phrase it an obsession to not to contribute to KF5. ;) In fact, it has been pointed out multiple times on KDE-MAC, in pm as well as in various RRs, that the MacOSX team at the moment mainly tries to get KDE4 into a working state, which is why Ian wants to push this forward. And yes, we have KF5 on Qt5 in a state where my OSX/CI(/Jenkins) systems are able to build and install many projects successfully. BUT, unfortunately, this does NOT mean that I am able to RUN every of these apps successfully, as the OSX/CI system's specifics (being that all frameworks, libs and apps live in their own install roots) in conjunction with a (still missing) working QStandardPaths patch lead to the problem that most of the apps can't find their config and data files at runtime at this point in time. :( As I am *alone* on KF5, I haven't managed to proceed with the QStandardPaths issue, since many other things kept me far too busy (like the inclusion of new projects on OSX/CI, dealing with Jenkins master [also for Linux], tending project dependencies, creating a KDE4.13 branch on our macports-kde git repo, testing KDE4 applications, etc...). Eventually I conclude herewith that the MacOSX team: - does contribute directly to Qt5/KF5 big time - althought it is only me ATM, - does indirectly contribute to Qt5/KF5, as many RRs can be modified easily for inclusion into KF5, as it has happened already for e.g. https://git.reviewboard.kde.org/r/119847/ and https://git.reviewboard.kde.org/r/119848/ - believes that 1st priority should be to get KDE4 in good shape on OSX, which is why Nicolas wants to release KDE 4.13.3 this week and will proceed with 4.14.x right afterwards, - needs decent user feedback with valuable backtraces which is why a non-dysfunctional DrKonqi is required on all OSX versions, hence this RR. Thomas Lübking wrote: Screw OSX ;-) Afaiu DrKonqui is dysfunctional due to bugzilla server changes atm. (bug #337742) what means either this or review https://git.reviewboard.kde.org/r/120376/ more or less *has* to go into KDE SC4 - regardless on whether it's required for exotic OS' or not. @Ian, please attach Jekyll Wu, George Kiagiadakis, Matthias Fuchs and Dario Andres Rodriguez, they've been the main submitters to bugzillalib. Ian Wadham wrote: Feel free to cancel anything I may commit, Albert, but you could be doing both yourself and KDE software (4 and 5) a disservice. Bug https://bugs.kde.org/show_bug.cgi?id=337742 will remain... I only said perhaps in my statement about the codebase, hoping someone more knowledgeable would step up. If you will read the Description of this review (Note 2) and the dialog in https://git.reviewboard.kde.org/r/120376/ you will find that I have already had some input to the other review and given it some thought. Last week I was expecting a core developer who knows Dr Konqi to review both patches, but nobody did. This week I have a course to prepare, for presentation tomorrow, so I have no time, and KDE 4.14.2 closes on Thursday. I do not even
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 5, 2014, 7:43 a.m., Ben Cooksley wrote: As this is needed to restore the functionality of Dr Konqi, can someone familiar with the codebase please review it so we can get this in? Ian Wadham wrote: Perhaps I am the person most familiar with the codebase of Dr Konqi, having worked on it for a few months now. So, if nobody else steps forward within the next 24 hours, I will do my own Ship It and commit to KDE 4 kde-runtime master in time for Thursday's close of the KDE 4.14.2 bugfix release. If KDE core developers want the Dr Konqi bugs fixed in KF5, they will have to forward-port this change and my earlier changes themselves. I cannot do that. I work on Apple OS X and we do not have KF5 and Qt 5 building there yet. I do not propose to address Thomas's comments. I have more important things to do. Albert Astals Cid wrote: With my release team hat: - Sure commit to master if you can't find someone else to review and you *know* your code is right and you're going to fix it when/if it break - No, don't commit to KDE/4.14 this is a huge change and I don't feel like it is guaranteed to go in, you can be a good guy and review https://git.reviewboard.kde.org/r/120376/ since it seems that one fixes the immediate problems people are having, no? (you say you're the one that knows the code more yet have not reviewed it, why?) - Your obsession to not contribute to KF5 based versions will byte you again when you decide to move to KF5, you should really rethink it. Because we do have KF5 and Qt5 building on MacOsX according to at least one of the members of the MacOSX team, no? Marko Käning wrote: I wouldn not phrase it an obsession to not to contribute to KF5. ;) In fact, it has been pointed out multiple times on KDE-MAC, in pm as well as in various RRs, that the MacOSX team at the moment mainly tries to get KDE4 into a working state, which is why Ian wants to push this forward. And yes, we have KF5 on Qt5 in a state where my OSX/CI(/Jenkins) systems are able to build and install many projects successfully. BUT, unfortunately, this does NOT mean that I am able to RUN every of these apps successfully, as the OSX/CI system's specifics (being that all frameworks, libs and apps live in their own install roots) in conjunction with a (still missing) working QStandardPaths patch lead to the problem that most of the apps can't find their config and data files at runtime at this point in time. :( As I am *alone* on KF5, I haven't managed to proceed with the QStandardPaths issue, since many other things kept me far too busy (like the inclusion of new projects on OSX/CI, dealing with Jenkins master [also for Linux], tending project dependencies, creating a KDE4.13 branch on our macports-kde git repo, testing KDE4 applications, etc...). Eventually I conclude herewith that the MacOSX team: - does contribute directly to Qt5/KF5 big time - althought it is only me ATM, - does indirectly contribute to Qt5/KF5, as many RRs can be modified easily for inclusion into KF5, as it has happened already for e.g. https://git.reviewboard.kde.org/r/119847/ and https://git.reviewboard.kde.org/r/119848/ - believes that 1st priority should be to get KDE4 in good shape on OSX, which is why Nicolas wants to release KDE 4.13.3 this week and will proceed with 4.14.x right afterwards, - needs decent user feedback with valuable backtraces which is why a non-dysfunctional DrKonqi is required on all OSX versions, hence this RR. Thomas Lübking wrote: Screw OSX ;-) Afaiu DrKonqui is dysfunctional due to bugzilla server changes atm. (bug #337742) what means either this or review https://git.reviewboard.kde.org/r/120376/ more or less *has* to go into KDE SC4 - regardless on whether it's required for exotic OS' or not. @Ian, please attach Jekyll Wu, George Kiagiadakis, Matthias Fuchs and Dario Andres Rodriguez, they've been the main submitters to bugzillalib. Ian Wadham wrote: Feel free to cancel anything I may commit, Albert, but you could be doing both yourself and KDE software (4 and 5) a disservice. Bug https://bugs.kde.org/show_bug.cgi?id=337742 will remain... I only said perhaps in my statement about the codebase, hoping someone more knowledgeable would step up. If you will read the Description of this review (Note 2) and the dialog in https://git.reviewboard.kde.org/r/120376/ you will find that I have already had some input to the other review and given it some thought. Last week I was expecting a core developer who knows Dr Konqi to review both patches, but nobody did. This week I have a course to prepare, for presentation tomorrow, so I have no time, and KDE 4.14.2 closes on Thursday. I do not even
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On oct. 5, 2014, 7:43 a.m., Ben Cooksley wrote: As this is needed to restore the functionality of Dr Konqi, can someone familiar with the codebase please review it so we can get this in? Ian Wadham wrote: Perhaps I am the person most familiar with the codebase of Dr Konqi, having worked on it for a few months now. So, if nobody else steps forward within the next 24 hours, I will do my own Ship It and commit to KDE 4 kde-runtime master in time for Thursday's close of the KDE 4.14.2 bugfix release. If KDE core developers want the Dr Konqi bugs fixed in KF5, they will have to forward-port this change and my earlier changes themselves. I cannot do that. I work on Apple OS X and we do not have KF5 and Qt 5 building there yet. I do not propose to address Thomas's comments. I have more important things to do. Albert Astals Cid wrote: With my release team hat: - Sure commit to master if you can't find someone else to review and you *know* your code is right and you're going to fix it when/if it break - No, don't commit to KDE/4.14 this is a huge change and I don't feel like it is guaranteed to go in, you can be a good guy and review https://git.reviewboard.kde.org/r/120376/ since it seems that one fixes the immediate problems people are having, no? (you say you're the one that knows the code more yet have not reviewed it, why?) - Your obsession to not contribute to KF5 based versions will byte you again when you decide to move to KF5, you should really rethink it. Because we do have KF5 and Qt5 building on MacOsX according to at least one of the members of the MacOSX team, no? Marko Käning wrote: I wouldn not phrase it an obsession to not to contribute to KF5. ;) In fact, it has been pointed out multiple times on KDE-MAC, in pm as well as in various RRs, that the MacOSX team at the moment mainly tries to get KDE4 into a working state, which is why Ian wants to push this forward. And yes, we have KF5 on Qt5 in a state where my OSX/CI(/Jenkins) systems are able to build and install many projects successfully. BUT, unfortunately, this does NOT mean that I am able to RUN every of these apps successfully, as the OSX/CI system's specifics (being that all frameworks, libs and apps live in their own install roots) in conjunction with a (still missing) working QStandardPaths patch lead to the problem that most of the apps can't find their config and data files at runtime at this point in time. :( As I am *alone* on KF5, I haven't managed to proceed with the QStandardPaths issue, since many other things kept me far too busy (like the inclusion of new projects on OSX/CI, dealing with Jenkins master [also for Linux], tending project dependencies, creating a KDE4.13 branch on our macports-kde git repo, testing KDE4 applications, etc...). Eventually I conclude herewith that the MacOSX team: - does contribute directly to Qt5/KF5 big time - althought it is only me ATM, - does indirectly contribute to Qt5/KF5, as many RRs can be modified easily for inclusion into KF5, as it has happened already for e.g. https://git.reviewboard.kde.org/r/119847/ and https://git.reviewboard.kde.org/r/119848/ - believes that 1st priority should be to get KDE4 in good shape on OSX, which is why Nicolas wants to release KDE 4.13.3 this week and will proceed with 4.14.x right afterwards, - needs decent user feedback with valuable backtraces which is why a non-dysfunctional DrKonqi is required on all OSX versions, hence this RR. Thomas Lübking wrote: Screw OSX ;-) Afaiu DrKonqui is dysfunctional due to bugzilla server changes atm. (bug #337742) what means either this or review https://git.reviewboard.kde.org/r/120376/ more or less *has* to go into KDE SC4 - regardless on whether it's required for exotic OS' or not. @Ian, please attach Jekyll Wu, George Kiagiadakis, Matthias Fuchs and Dario Andres Rodriguez, they've been the main submitters to bugzillalib. Ian Wadham wrote: Feel free to cancel anything I may commit, Albert, but you could be doing both yourself and KDE software (4 and 5) a disservice. Bug https://bugs.kde.org/show_bug.cgi?id=337742 will remain... I only said perhaps in my statement about the codebase, hoping someone more knowledgeable would step up. If you will read the Description of this review (Note 2) and the dialog in https://git.reviewboard.kde.org/r/120376/ you will find that I have already had some input to the other review and given it some thought. Last week I was expecting a core developer who knows Dr Konqi to review both patches, but nobody did. This week I have a course to prepare, for presentation tomorrow, so I have no time, and KDE 4.14.2 closes on Thursday. I do not even
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 5, 2014, 9:43 a.m., Ben Cooksley wrote: As this is needed to restore the functionality of Dr Konqi, can someone familiar with the codebase please review it so we can get this in? Ian Wadham wrote: Perhaps I am the person most familiar with the codebase of Dr Konqi, having worked on it for a few months now. So, if nobody else steps forward within the next 24 hours, I will do my own Ship It and commit to KDE 4 kde-runtime master in time for Thursday's close of the KDE 4.14.2 bugfix release. If KDE core developers want the Dr Konqi bugs fixed in KF5, they will have to forward-port this change and my earlier changes themselves. I cannot do that. I work on Apple OS X and we do not have KF5 and Qt 5 building there yet. I do not propose to address Thomas's comments. I have more important things to do. Albert Astals Cid wrote: With my release team hat: - Sure commit to master if you can't find someone else to review and you *know* your code is right and you're going to fix it when/if it break - No, don't commit to KDE/4.14 this is a huge change and I don't feel like it is guaranteed to go in, you can be a good guy and review https://git.reviewboard.kde.org/r/120376/ since it seems that one fixes the immediate problems people are having, no? (you say you're the one that knows the code more yet have not reviewed it, why?) - Your obsession to not contribute to KF5 based versions will byte you again when you decide to move to KF5, you should really rethink it. Because we do have KF5 and Qt5 building on MacOsX according to at least one of the members of the MacOSX team, no? Marko Käning wrote: I wouldn not phrase it an obsession to not to contribute to KF5. ;) In fact, it has been pointed out multiple times on KDE-MAC, in pm as well as in various RRs, that the MacOSX team at the moment mainly tries to get KDE4 into a working state, which is why Ian wants to push this forward. And yes, we have KF5 on Qt5 in a state where my OSX/CI(/Jenkins) systems are able to build and install many projects successfully. BUT, unfortunately, this does NOT mean that I am able to RUN every of these apps successfully, as the OSX/CI system's specifics (being that all frameworks, libs and apps live in their own install roots) in conjunction with a (still missing) working QStandardPaths patch lead to the problem that most of the apps can't find their config and data files at runtime at this point in time. :( As I am *alone* on KF5, I haven't managed to proceed with the QStandardPaths issue, since many other things kept me far too busy (like the inclusion of new projects on OSX/CI, dealing with Jenkins master [also for Linux], tending project dependencies, creating a KDE4.13 branch on our macports-kde git repo, testing KDE4 applications, etc...). Eventually I conclude herewith that the MacOSX team: - does contribute directly to Qt5/KF5 big time - althought it is only me ATM, - does indirectly contribute to Qt5/KF5, as many RRs can be modified easily for inclusion into KF5, as it has happened already for e.g. https://git.reviewboard.kde.org/r/119847/ and https://git.reviewboard.kde.org/r/119848/ - believes that 1st priority should be to get KDE4 in good shape on OSX, which is why Nicolas wants to release KDE 4.13.3 this week and will proceed with 4.14.x right afterwards, - needs decent user feedback with valuable backtraces which is why a non-dysfunctional DrKonqi is required on all OSX versions, hence this RR. Thomas Lübking wrote: Screw OSX ;-) Afaiu DrKonqui is dysfunctional due to bugzilla server changes atm. (bug #337742) what means either this or review https://git.reviewboard.kde.org/r/120376/ more or less *has* to go into KDE SC4 - regardless on whether it's required for exotic OS' or not. @Ian, please attach Jekyll Wu, George Kiagiadakis, Matthias Fuchs and Dario Andres Rodriguez, they've been the main submitters to bugzillalib. Ian Wadham wrote: Feel free to cancel anything I may commit, Albert, but you could be doing both yourself and KDE software (4 and 5) a disservice. Bug https://bugs.kde.org/show_bug.cgi?id=337742 will remain... I only said perhaps in my statement about the codebase, hoping someone more knowledgeable would step up. If you will read the Description of this review (Note 2) and the dialog in https://git.reviewboard.kde.org/r/120376/ you will find that I have already had some input to the other review and given it some thought. Last week I was expecting a core developer who knows Dr Konqi to review both patches, but nobody did. This week I have a course to prepare, for presentation tomorrow, so I have no time, and KDE 4.14.2 closes on Thursday. I do not even
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120431/#review68051
---
My 2¢
Bugzilla will require an update anyway and that means at some point it'll be
(then silently) broken in KDE SC4 again and somebody has to step up and fix
it with another patch.
In the meantime we've diverging codebases for KDE 4 5 - meh.
I agree with Albert that this patch looks a bit scaringly complex (at least
compared to Frédéric's patch), but believe that the complexity can be vastly
reduced and like a forward compatible and 4+5 common patch better.
drkonqi/bugzillalib.h
https://git.reviewboard.kde.org/r/120431/#comment47435
This is only used locally, no need for a member.
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47432
The patch largely consists of hand-crafted version handling.
replacing this by int version = KDE_MAKE_VERSION(major, minor, release);
and simple integer metricts for comparism should considerably lower complexity
(thus make the patch easier to be accepted ;-)
Yes, I know it's crap to write a lot of code and remove it afterwards.
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47434
I'll frankly call this over-engineered (though likely just as result of the
handcrafted version stuff):
m_security = UseCookies;
m_otherStuff = OldDefault;
if (version KDE_MAKE_VERSION(4,4,3)) {
m_security = UseTokens;
}
if (version KDE_MAKE_VERSION(4,4,4)) {
m_otherStuff = NewStuff;
}
if (version KDE_MAKE_VERSION(5,0,0)) {
m_security = UsePasswords;
}
...
This adds better connection between version requirement and impact.
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47431
Afaiu the bugzilla roadmap, they become mandatory with 5.0.0, not 4.5.0?
(only deprecated w/ 4.5 but this would add more time to invoke kwallet etc.)
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47433
this apprently either enfores a strict numeric order in the changes array
or verify your comparism algo.
It should not abort outside debug builds (and I don't see why it should
abort for changes order at all)
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47436
This does not belong into the implementation (esp. not commented) but the
test class.
Frédéric's patch includes required (?) updates for the test anyway, so he
could add it there.
- Thomas Lübking
On Okt. 7, 2014, 7:42 vorm., Ian Wadham wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120431/
---
(Updated Okt. 7, 2014, 7:42 vorm.)
Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío
Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs.
Bugs: 337742
http://bugs.kde.org/show_bug.cgi?id=337742
Repository: kde-runtime
Description
---
When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security
method used by Bugzilla changed from cookies to tokens that had to be
supplied as parameters with every secure remote-procedure call. Further
changes to security methods have been announced by Bugzilla and are
documented for unstable 4.5.x versions of Bugzilla software. Tokens will be
deprecated and then discontinued. When this happens, Dr Konqi will need to
supply a user-login name and a password with every secure remote-procedure
call. Furthermore, the traditional User.login call presently used by Dr
Konqi will be deprecated and discontinued.
This patch fixes the tokens problem, which has given rise to several bug
reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also
provides for automatic switching to passwords-only security as and when the
Bugzilla version changes again. This uses
a general data-driven approach which can be easily updated, ahead of time,
next time Bugzilla announces a change that affects Dr Konqi, whether it be in
security methods or some other feature.
NOTES:
1. This patch is intended to be forward-portable to Frameworks/KF5, but I
work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do
the porting and testing. So could someone else please do it?
2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses
the tokens issue only, but it should be reviewed and shipped as a matter of
urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE
4.14 being due for tagging on Thursday, 9 October. That will leave more time
for this review (120431) of my more
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On oct. 5, 2014, 7:43 a.m., Ben Cooksley wrote: As this is needed to restore the functionality of Dr Konqi, can someone familiar with the codebase please review it so we can get this in? Ian Wadham wrote: Perhaps I am the person most familiar with the codebase of Dr Konqi, having worked on it for a few months now. So, if nobody else steps forward within the next 24 hours, I will do my own Ship It and commit to KDE 4 kde-runtime master in time for Thursday's close of the KDE 4.14.2 bugfix release. If KDE core developers want the Dr Konqi bugs fixed in KF5, they will have to forward-port this change and my earlier changes themselves. I cannot do that. I work on Apple OS X and we do not have KF5 and Qt 5 building there yet. I do not propose to address Thomas's comments. I have more important things to do. Albert Astals Cid wrote: With my release team hat: - Sure commit to master if you can't find someone else to review and you *know* your code is right and you're going to fix it when/if it break - No, don't commit to KDE/4.14 this is a huge change and I don't feel like it is guaranteed to go in, you can be a good guy and review https://git.reviewboard.kde.org/r/120376/ since it seems that one fixes the immediate problems people are having, no? (you say you're the one that knows the code more yet have not reviewed it, why?) - Your obsession to not contribute to KF5 based versions will byte you again when you decide to move to KF5, you should really rethink it. Because we do have KF5 and Qt5 building on MacOsX according to at least one of the members of the MacOSX team, no? Marko Käning wrote: I wouldn not phrase it an obsession to not to contribute to KF5. ;) In fact, it has been pointed out multiple times on KDE-MAC, in pm as well as in various RRs, that the MacOSX team at the moment mainly tries to get KDE4 into a working state, which is why Ian wants to push this forward. And yes, we have KF5 on Qt5 in a state where my OSX/CI(/Jenkins) systems are able to build and install many projects successfully. BUT, unfortunately, this does NOT mean that I am able to RUN every of these apps successfully, as the OSX/CI system's specifics (being that all frameworks, libs and apps live in their own install roots) in conjunction with a (still missing) working QStandardPaths patch lead to the problem that most of the apps can't find their config and data files at runtime at this point in time. :( As I am *alone* on KF5, I haven't managed to proceed with the QStandardPaths issue, since many other things kept me far too busy (like the inclusion of new projects on OSX/CI, dealing with Jenkins master [also for Linux], tending project dependencies, creating a KDE4.13 branch on our macports-kde git repo, testing KDE4 applications, etc...). Eventually I conclude herewith that the MacOSX team: - does contribute directly to Qt5/KF5 big time - althought it is only me ATM, - does indirectly contribute to Qt5/KF5, as many RRs can be modified easily for inclusion into KF5, as it has happened already for e.g. https://git.reviewboard.kde.org/r/119847/ and https://git.reviewboard.kde.org/r/119848/ - believes that 1st priority should be to get KDE4 in good shape on OSX, which is why Nicolas wants to release KDE 4.13.3 this week and will proceed with 4.14.x right afterwards, - needs decent user feedback with valuable backtraces which is why a non-dysfunctional DrKonqi is required on all OSX versions, hence this RR. Thomas Lübking wrote: Screw OSX ;-) Afaiu DrKonqui is dysfunctional due to bugzilla server changes atm. (bug #337742) what means either this or review https://git.reviewboard.kde.org/r/120376/ more or less *has* to go into KDE SC4 - regardless on whether it's required for exotic OS' or not. @Ian, please attach Jekyll Wu, George Kiagiadakis, Matthias Fuchs and Dario Andres Rodriguez, they've been the main submitters to bugzillalib. Ian Wadham wrote: Feel free to cancel anything I may commit, Albert, but you could be doing both yourself and KDE software (4 and 5) a disservice. Bug https://bugs.kde.org/show_bug.cgi?id=337742 will remain... I only said perhaps in my statement about the codebase, hoping someone more knowledgeable would step up. If you will read the Description of this review (Note 2) and the dialog in https://git.reviewboard.kde.org/r/120376/ you will find that I have already had some input to the other review and given it some thought. Last week I was expecting a core developer who knows Dr Konqi to review both patches, but nobody did. This week I have a course to prepare, for presentation tomorrow, so I have no time, and KDE 4.14.2 closes on Thursday. I do not even
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On oct. 7, 2014, 1:13 p.m., Thomas Lübking wrote: My 2¢ Bugzilla will require an update anyway and that means at some point it'll be (then silently) broken in KDE SC4 again and somebody has to step up and fix it with another patch. In the meantime we've diverging codebases for KDE 4 5 - meh. I agree with Albert that this patch looks a bit scaringly complex (at least compared to Frédéric's patch), but believe that the complexity can be vastly reduced and like a forward compatible and 4+5 common patch better. You have a point here, if it's possible that Frédéric's patch gets broken in the timeframe we still have users around using kde-runtime4 then that would be a good reason to use this patch. I'd appreciate an assesment on how much more future-proof this patch is versus Frédéric's one. - Albert --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68051 --- On oct. 7, 2014, 7:42 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated oct. 7, 2014, 7:42 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Okt. 7, 2014, 1:13 nachm., Thomas Lübking wrote: My 2¢ Bugzilla will require an update anyway and that means at some point it'll be (then silently) broken in KDE SC4 again and somebody has to step up and fix it with another patch. In the meantime we've diverging codebases for KDE 4 5 - meh. I agree with Albert that this patch looks a bit scaringly complex (at least compared to Frédéric's patch), but believe that the complexity can be vastly reduced and like a forward compatible and 4+5 common patch better. Albert Astals Cid wrote: You have a point here, if it's possible that Frédéric's patch gets broken in the timeframe we still have users around using kde-runtime4 then that would be a good reason to use this patch. I'd appreciate an assesment on how much more future-proof this patch is versus Frédéric's one. Afaiu it will break when the bugzilla server upgrades to 5.0 (the token security model will be dropped) but I could not find a schedule for future bugzilla releases (nor know about bugs.kde.org update policy) - Ben? If users around using kde-runtime4 is the critical condition, this seems a likely threat, though (given eg. RHEL lifetimes - RHEL7 extended support ends 2027 ;-) - Thomas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68051 --- On Okt. 7, 2014, 7:42 vorm., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Okt. 7, 2014, 7:42 vorm.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 7, 2014, 1:13 p.m., Thomas Lübking wrote: drkonqi/bugzillalib.cpp, line 81 https://git.reviewboard.kde.org/r/120431/diff/4/?file=316623#file316623line81 The patch largely consists of hand-crafted version handling. replacing this by int version = KDE_MAKE_VERSION(major, minor, release); and simple integer metricts for comparism should considerably lower complexity (thus make the patch easier to be accepted ;-) Yes, I know it's crap to write a lot of code and remove it afterwards. This is exactly the kind of advice I hope for in a review. I did look to see, several weeks ago, if there were some version-compare methods in KDE or Qt or on Google, but did not find any. I also considered something like version = (major * 100 + minor * 1000 + release), but thought it would be rather a 1960s style kludge... ;-) I have lived and worked through all the trouble those YYMMDD strings caused (Y2K and all that). Still, I suppose the parts of Bugzilla versions are unlikely to go past 255... I have no objection to deleting code and replacing it with something shorter and simpler. I guess I would still need the code to convert a QString version (from Bugzilla and the bugs.kde.org database) to a single KDE_MAKE_VERSION integer, or is there something else in KDE/Qt to do that? What *does* worry me is 5-minutes-to-midnight re-programming, i.e. I would have to make and test all changes on Thursday, just hours before Albert starts tagging KDE 4.14.2. In my experience, that could be riskier than committing my patch as it stands. But either way, we at least have a month to fix any problems before KDE 4.14.3, the last release of KDE 4. Then there is always KDE 14.12 and Dr Konqi is an app, not a library... @Albert and Thomas: Please let me know what you would like me to commit on Thursday: my patch as it stands, my patch simplified as Thomas suggests or Frédéric's patch. On Oct. 7, 2014, 1:13 p.m., Thomas Lübking wrote: drkonqi/bugzillalib.cpp, line 131 https://git.reviewboard.kde.org/r/120431/diff/4/?file=316623#file316623line131 Afaiu the bugzilla roadmap, they become mandatory with 5.0.0, not 4.5.0? (only deprecated w/ 4.5 but this would add more time to invoke kwallet etc.) AFAICS Bugzilla doco writers sometimes use 5.0 as a shorthand for 4.5.0. See REFERENCES: in the Description section of this review, starting at api in the top Bugzilla doco webpage. Confusing, isn't it? The Bugzilla version after 4.5.x might be 4.6.x or 5.0.x. Who can tell? - Ian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68051 --- On Oct. 7, 2014, 7:42 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 7, 2014, 7:42 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Okt. 7, 2014, 1:13 nachm., Thomas Lübking wrote:
drkonqi/bugzillalib.cpp, line 81
https://git.reviewboard.kde.org/r/120431/diff/4/?file=316623#file316623line81
The patch largely consists of hand-crafted version handling.
replacing this by int version = KDE_MAKE_VERSION(major, minor,
release); and simple integer metricts for comparism should considerably
lower complexity (thus make the patch easier to be accepted ;-)
Yes, I know it's crap to write a lot of code and remove it afterwards.
Ian Wadham wrote:
This is exactly the kind of advice I hope for in a review. I did look to
see, several weeks ago, if there were some version-compare methods in KDE or
Qt or on Google, but did not find any. I also considered something like
version = (major * 100 + minor * 1000 + release), but thought it would be
rather a 1960s style kludge... ;-) I have lived and worked through all the
trouble those YYMMDD strings caused (Y2K and all that). Still, I suppose the
parts of Bugzilla versions are unlikely to go past 255...
I have no objection to deleting code and replacing it with something
shorter and simpler. I guess I would still need the code to convert a QString
version (from Bugzilla and the bugs.kde.org database) to a single
KDE_MAKE_VERSION integer, or is there something else in KDE/Qt to do that?
What *does* worry me is 5-minutes-to-midnight re-programming, i.e. I
would have to make and test all changes on Thursday, just hours before Albert
starts tagging KDE 4.14.2. In my experience, that could be riskier than
committing my patch as it stands.
But either way, we at least have a month to fix any problems before KDE
4.14.3, the last release of KDE 4. Then there is always KDE 14.12 and Dr
Konqi is an app, not a library...
@Albert and Thomas: Please let me know what you would like me to commit
on Thursday: my patch as it stands, my patch simplified as Thomas suggests or
Frédéric's patch.
Albert Astals Cid wrote:
I would prefer the simplified version. If you really feel you're going to
break the code, i'll take a commitment to do the simplified version for 4.14.3
I guess I would still need the code to convert a QString version (from
Bugzilla and the bugs.kde.org database) to a single KDE_MAKE_VERSION integer,
or is there something else in KDE/Qt to do that?
You'll have to split the received string into 3 integers (major, minor,
release) for the KDE_MAKE_VERSION macro.
I found no spec for it. Is it really more than 1.2.3?
In case: the present number splitting just collects the first 3 numbers - you'd
rather have to isolate [0-9]*.[0-9]*.[0-9]* first.
If not:
```cpp
QStringList l = string.split(QLatin1Char('1'));
if (l.count() == 3)
version = KDE_MAKE_VERSION(l.at(0).toUInt(), l.at(1).toUInt(),
l.at(2).toUInt());
else
kDebug() sth's severely fucked up here;
fi
- Thomas
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120431/#review68051
---
On Okt. 7, 2014, 7:42 vorm., Ian Wadham wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120431/
---
(Updated Okt. 7, 2014, 7:42 vorm.)
Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío
Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs.
Bugs: 337742
http://bugs.kde.org/show_bug.cgi?id=337742
Repository: kde-runtime
Description
---
When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security
method used by Bugzilla changed from cookies to tokens that had to be
supplied as parameters with every secure remote-procedure call. Further
changes to security methods have been announced by Bugzilla and are
documented for unstable 4.5.x versions of Bugzilla software. Tokens will be
deprecated and then discontinued. When this happens, Dr Konqi will need to
supply a user-login name and a password with every secure remote-procedure
call. Furthermore, the traditional User.login call presently used by Dr
Konqi will be deprecated and discontinued.
This patch fixes the tokens problem, which has given rise to several bug
reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also
provides for automatic switching to passwords-only security as and when the
Bugzilla version changes again. This uses
a general data-driven approach which can be easily updated, ahead of time,
next time Bugzilla announces a change that affects Dr Konqi, whether it be in
security methods or some other feature.
NOTES:
1. This patch is intended to be forward-portable to Frameworks/KF5, but I
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 7, 2014, 1:13 p.m., Thomas Lübking wrote: My 2¢ Bugzilla will require an update anyway and that means at some point it'll be (then silently) broken in KDE SC4 again and somebody has to step up and fix it with another patch. In the meantime we've diverging codebases for KDE 4 5 - meh. I agree with Albert that this patch looks a bit scaringly complex (at least compared to Frédéric's patch), but believe that the complexity can be vastly reduced and like a forward compatible and 4+5 common patch better. Albert Astals Cid wrote: You have a point here, if it's possible that Frédéric's patch gets broken in the timeframe we still have users around using kde-runtime4 then that would be a good reason to use this patch. I'd appreciate an assesment on how much more future-proof this patch is versus Frédéric's one. Thomas Lübking wrote: Afaiu it will break when the bugzilla server upgrades to 5.0 (the token security model will be dropped) but I could not find a schedule for future bugzilla releases (nor know about bugs.kde.org update policy) - Ben? If users around using kde-runtime4 is the critical condition, this seems a likely threat, though (given eg. RHEL lifetimes - RHEL7 extended support ends 2027 ;-) bugs.kde.org is updated when it becomes necessary (security issues) or when someone gets around to deploying the latest release. There isn't really a schedule as such. Based on the above comment, i'd suggest making Dr Konqi as capable as possible - although do remember that we probably don't want to receive bug reports from extremely old versions of our software, even if RHEL is supporting it. - Ben --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review68051 --- On Oct. 7, 2014, 7:42 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 7, 2014, 7:42 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime, Ben Cooksley, Darío Andrés Rodríguez, George Kiagiadakis, Jekyll Wu, and Matthias Fuchs. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 5, 2014, 7:43 a.m., Ben Cooksley wrote: As this is needed to restore the functionality of Dr Konqi, can someone familiar with the codebase please review it so we can get this in? Perhaps I am the person most familiar with the codebase of Dr Konqi, having worked on it for a few months now. So, if nobody else steps forward within the next 24 hours, I will do my own Ship It and commit to KDE 4 kde-runtime master in time for Thursday's close of the KDE 4.14.2 bugfix release. If KDE core developers want the Dr Konqi bugs fixed in KF5, they will have to forward-port this change and my earlier changes themselves. I cannot do that. I work on Apple OS X and we do not have KF5 and Qt 5 building there yet. I do not propose to address Thomas's comments. I have more important things to do. - Ian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review67946 --- On Oct. 5, 2014, 4:27 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 5, 2014, 4:27 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm.
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On oct. 5, 2014, 7:43 a.m., Ben Cooksley wrote: As this is needed to restore the functionality of Dr Konqi, can someone familiar with the codebase please review it so we can get this in? Ian Wadham wrote: Perhaps I am the person most familiar with the codebase of Dr Konqi, having worked on it for a few months now. So, if nobody else steps forward within the next 24 hours, I will do my own Ship It and commit to KDE 4 kde-runtime master in time for Thursday's close of the KDE 4.14.2 bugfix release. If KDE core developers want the Dr Konqi bugs fixed in KF5, they will have to forward-port this change and my earlier changes themselves. I cannot do that. I work on Apple OS X and we do not have KF5 and Qt 5 building there yet. I do not propose to address Thomas's comments. I have more important things to do. With my release team hat: - Sure commit to master if you can't find someone else to review and you *know* your code is right and you're going to fix it when/if it break - No, don't commit to KDE/4.14 this is a huge change and I don't feel like it is guaranteed to go in, you can be a good guy and review https://git.reviewboard.kde.org/r/120376/ since it seems that one fixes the immediate problems people are having, no? (you say you're the one that knows the code more yet have not reviewed it, why?) - Your obsession to not contribute to KF5 based versions will byte you again when you decide to move to KF5, you should really rethink it. Because we do have KF5 and Qt5 building on MacOsX according to at least one of the members of the MacOSX team, no? - Albert --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review67946 --- On oct. 5, 2014, 4:27 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated oct. 5, 2014, 4:27 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 5, 2014, 9:43 a.m., Ben Cooksley wrote: As this is needed to restore the functionality of Dr Konqi, can someone familiar with the codebase please review it so we can get this in? Ian Wadham wrote: Perhaps I am the person most familiar with the codebase of Dr Konqi, having worked on it for a few months now. So, if nobody else steps forward within the next 24 hours, I will do my own Ship It and commit to KDE 4 kde-runtime master in time for Thursday's close of the KDE 4.14.2 bugfix release. If KDE core developers want the Dr Konqi bugs fixed in KF5, they will have to forward-port this change and my earlier changes themselves. I cannot do that. I work on Apple OS X and we do not have KF5 and Qt 5 building there yet. I do not propose to address Thomas's comments. I have more important things to do. Albert Astals Cid wrote: With my release team hat: - Sure commit to master if you can't find someone else to review and you *know* your code is right and you're going to fix it when/if it break - No, don't commit to KDE/4.14 this is a huge change and I don't feel like it is guaranteed to go in, you can be a good guy and review https://git.reviewboard.kde.org/r/120376/ since it seems that one fixes the immediate problems people are having, no? (you say you're the one that knows the code more yet have not reviewed it, why?) - Your obsession to not contribute to KF5 based versions will byte you again when you decide to move to KF5, you should really rethink it. Because we do have KF5 and Qt5 building on MacOsX according to at least one of the members of the MacOSX team, no? I wouldn not phrase it an obsession to not to contribute to KF5. ;) In fact, it has been pointed out multiple times on KDE-MAC, in pm as well as in various RRs, that the MacOSX team at the moment mainly tries to get KDE4 into a working state, which is why Ian wants to push this forward. And yes, we have KF5 on Qt5 in a state where my OSX/CI(/Jenkins) systems are able to build and install many projects successfully. BUT, unfortunately, this does NOT mean that I am able to RUN every of these apps successfully, as the OSX/CI system's specifics (being that all frameworks, libs and apps live in their own install roots) in conjunction with a (still missing) working QStandardPaths patch lead to the problem that most of the apps can't find their config and data files at runtime at this point in time. :( As I am *alone* on KF5, I haven't managed to proceed with the QStandardPaths issue, since many other things kept me far too busy (like the inclusion of new projects on OSX/CI, dealing with Jenkins master [also for Linux], tending project dependencies, creating a KDE4.13 branch on our macports-kde git repo, testing KDE4 applications, etc...). Eventually I conclude herewith that the MacOSX team: - does contribute directly to Qt5/KF5 big time - althought it is only me ATM, - does indirectly contribute to Qt5/KF5, as many RRs can be modified easily for inclusion into KF5, as it has happened already for e.g. https://git.reviewboard.kde.org/r/119847/ and https://git.reviewboard.kde.org/r/119848/ - believes that 1st priority should be to get KDE4 in good shape on OSX, which is why Nicolas wants to release KDE 4.13.3 this week and will proceed with 4.14.x right afterwards, - needs decent user feedback with valuable backtraces which is why a non-dysfunctional DrKonqi is required on all OSX versions, hence this RR. - Marko --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review67946 --- On Oct. 5, 2014, 6:27 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 5, 2014, 6:27 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Okt. 5, 2014, 7:43 vorm., Ben Cooksley wrote: As this is needed to restore the functionality of Dr Konqi, can someone familiar with the codebase please review it so we can get this in? Ian Wadham wrote: Perhaps I am the person most familiar with the codebase of Dr Konqi, having worked on it for a few months now. So, if nobody else steps forward within the next 24 hours, I will do my own Ship It and commit to KDE 4 kde-runtime master in time for Thursday's close of the KDE 4.14.2 bugfix release. If KDE core developers want the Dr Konqi bugs fixed in KF5, they will have to forward-port this change and my earlier changes themselves. I cannot do that. I work on Apple OS X and we do not have KF5 and Qt 5 building there yet. I do not propose to address Thomas's comments. I have more important things to do. Albert Astals Cid wrote: With my release team hat: - Sure commit to master if you can't find someone else to review and you *know* your code is right and you're going to fix it when/if it break - No, don't commit to KDE/4.14 this is a huge change and I don't feel like it is guaranteed to go in, you can be a good guy and review https://git.reviewboard.kde.org/r/120376/ since it seems that one fixes the immediate problems people are having, no? (you say you're the one that knows the code more yet have not reviewed it, why?) - Your obsession to not contribute to KF5 based versions will byte you again when you decide to move to KF5, you should really rethink it. Because we do have KF5 and Qt5 building on MacOsX according to at least one of the members of the MacOSX team, no? Marko Käning wrote: I wouldn not phrase it an obsession to not to contribute to KF5. ;) In fact, it has been pointed out multiple times on KDE-MAC, in pm as well as in various RRs, that the MacOSX team at the moment mainly tries to get KDE4 into a working state, which is why Ian wants to push this forward. And yes, we have KF5 on Qt5 in a state where my OSX/CI(/Jenkins) systems are able to build and install many projects successfully. BUT, unfortunately, this does NOT mean that I am able to RUN every of these apps successfully, as the OSX/CI system's specifics (being that all frameworks, libs and apps live in their own install roots) in conjunction with a (still missing) working QStandardPaths patch lead to the problem that most of the apps can't find their config and data files at runtime at this point in time. :( As I am *alone* on KF5, I haven't managed to proceed with the QStandardPaths issue, since many other things kept me far too busy (like the inclusion of new projects on OSX/CI, dealing with Jenkins master [also for Linux], tending project dependencies, creating a KDE4.13 branch on our macports-kde git repo, testing KDE4 applications, etc...). Eventually I conclude herewith that the MacOSX team: - does contribute directly to Qt5/KF5 big time - althought it is only me ATM, - does indirectly contribute to Qt5/KF5, as many RRs can be modified easily for inclusion into KF5, as it has happened already for e.g. https://git.reviewboard.kde.org/r/119847/ and https://git.reviewboard.kde.org/r/119848/ - believes that 1st priority should be to get KDE4 in good shape on OSX, which is why Nicolas wants to release KDE 4.13.3 this week and will proceed with 4.14.x right afterwards, - needs decent user feedback with valuable backtraces which is why a non-dysfunctional DrKonqi is required on all OSX versions, hence this RR. Screw OSX ;-) Afaiu DrKonqui is dysfunctional due to bugzilla server changes atm. (bug #337742) what means either this or review https://git.reviewboard.kde.org/r/120376/ more or less *has* to go into KDE SC4 - regardless on whether it's required for exotic OS' or not. @Ian, please attach Jekyll Wu, George Kiagiadakis, Matthias Fuchs and Dario Andres Rodriguez, they've been the main submitters to bugzillalib. - Thomas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review67946 --- On Okt. 5, 2014, 4:27 vorm., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Okt. 5, 2014, 4:27 vorm.) Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review67946 --- As this is needed to restore the functionality of Dr Konqi, can someone familiar with the codebase please review it so we can get this in? - Ben Cooksley On Oct. 5, 2014, 4:27 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 5, 2014, 4:27 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120431/#review67955
---
Not familiar with the codebase, sorry - just some codingstyle nazi remarks (in
case you feel bored ;-)
drkonqi/bugzillalib.h
https://git.reviewboard.kde.org/r/120431/#comment47362
Nitpick:
Coding style - blanks between braces and content.
As a name suggestion:
CookieSecurity etc. to indicate relation?
drkonqi/bugzillalib.h
https://git.reviewboard.kde.org/r/120431/#comment47363
inline ... () const { ...
drkonqi/bugzillalib.h
https://git.reviewboard.kde.org/r/120431/#comment47364
lookupServerVersion ?
drkonqi/bugzillalib.h
https://git.reviewboard.kde.org/r/120431/#comment47365
serverVersionFound ?
It should probably align the slots name and bugzilla is this library
drkonqi/bugzillalib.h
https://git.reviewboard.kde.org/r/120431/#comment47366
blanks
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47367
superfluous whitespace between function and brace.
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47370
static int compareVersions(.)
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47368
++part (yes, i know: gdb will optimize that for you...)
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47369
same line as closing brace
if () {
} else {
}
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47371
blanks
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47372
const QString
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47373
lowercase ok - capitals are usually used for macros
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47374
++part
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47375
or
if (++part = nVersionParts)
=)
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47377
const int
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47376
++n
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47378
kFatal()
drkonqi/bugzillalib.cpp
https://git.reviewboard.kde.org/r/120431/#comment47379
I DO NO KNOW, but maybe kDebug(1410) for DrKonqui?
drkonqi/reportassistantpages_bugzilla.cpp
https://git.reviewboard.kde.org/r/120431/#comment47380
personally i'd not embrace this because it looks as if some || was missing.
- Thomas Lübking
On Okt. 5, 2014, 4:27 vorm., Ian Wadham wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120431/
---
(Updated Okt. 5, 2014, 4:27 vorm.)
Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley.
Bugs: 337742
http://bugs.kde.org/show_bug.cgi?id=337742
Repository: kde-runtime
Description
---
When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security
method used by Bugzilla changed from cookies to tokens that had to be
supplied as parameters with every secure remote-procedure call. Further
changes to security methods have been announced by Bugzilla and are
documented for unstable 4.5.x versions of Bugzilla software. Tokens will be
deprecated and then discontinued. When this happens, Dr Konqi will need to
supply a user-login name and a password with every secure remote-procedure
call. Furthermore, the traditional User.login call presently used by Dr
Konqi will be deprecated and discontinued.
This patch fixes the tokens problem, which has given rise to several bug
reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also
provides for automatic switching to passwords-only security as and when the
Bugzilla version changes again. This uses
a general data-driven approach which can be easily updated, ahead of time,
next time Bugzilla announces a change that affects Dr Konqi, whether it be in
security methods or some other feature.
NOTES:
1. This patch is intended to be forward-portable to Frameworks/KF5, but I
work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do
the porting and testing. So could someone else please do it?
2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses
the tokens issue only, but it should be reviewed and shipped as a matter of
urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE
4.14 being due for tagging on Thursday, 9 October. That will leave more time
for this review (120431) of my
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review67943 --- drkonqi/bugzillalib.cpp https://git.reviewboard.kde.org/r/120431/#comment47358 Perhaps use kWarning() or kDebug() instead? drkonqi/bugzillalib.cpp https://git.reviewboard.kde.org/r/120431/#comment47359 Same issue wrt fprintf vs. kDebug/kWarning. - Ben Cooksley On Oct. 3, 2014, 7:52 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 3, 2014, 7:52 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 5, 2014, 4:27 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley. Changes --- Changed fprintf() to kWarning() in two places. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs (updated) - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Oct. 5, 2014, 2:06 a.m., Ben Cooksley wrote: drkonqi/bugzillalib.cpp, line 161 https://git.reviewboard.kde.org/r/120431/diff/3/?file=316392#file316392line161 Perhaps use kWarning() or kDebug() instead? Done. On Oct. 5, 2014, 2:06 a.m., Ben Cooksley wrote: drkonqi/bugzillalib.cpp, line 182 https://git.reviewboard.kde.org/r/120431/diff/3/?file=316392#file316392line182 Same issue wrt fprintf vs. kDebug/kWarning. Done. - Ian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review67943 --- On Oct. 5, 2014, 4:27 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 5, 2014, 4:27 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 3, 2014, 7:03 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley. Changes --- The code for setFeaturesForVersion() has been re-written completely, to accommodate any type of feature change, in any number of versions, in a simpler and more rugged way. This should also take care of the issues Jan Kundrát raised. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs (updated) - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
On Sept. 30, 2014, 9:22 a.m., Jan Kundrát wrote:
drkonqi/bugzillalib.h, line 431
https://git.reviewboard.kde.org/r/120431/diff/1/?file=315732#file315732line431
These are never saved on disk, right?
I don't think that this makes much sense given that the rest of the
stack will happily use regular QString/QByteArray/QIODevice for actual
network IO.
These are never saved on disk, right?
No. And if nobody else is worried about this, neither am I. We are not playing
for sheep-stations here.
On Sept. 30, 2014, 9:22 a.m., Jan Kundrát wrote:
drkonqi/bugzillalib.h, line 442
https://git.reviewboard.kde.org/r/120431/diff/1/?file=315732#file315732line442
`const QString `
Fixed.
On Sept. 30, 2014, 9:22 a.m., Jan Kundrát wrote:
drkonqi/bugzillalib.cpp, line 88
https://git.reviewboard.kde.org/r/120431/diff/1/?file=315733#file315733line88
It's customary to use smallCaps for these identifiers
Fixed. The new array of structs is called changes.
On Sept. 30, 2014, 9:22 a.m., Jan Kundrát wrote:
drkonqi/bugzillalib.cpp, line 107
https://git.reviewboard.kde.org/r/120431/diff/1/?file=315733#file315733line107
I would suggest a QLatin1String here, but that's me
Done.
On Sept. 30, 2014, 9:22 a.m., Jan Kundrát wrote:
drkonqi/bugzillalib.cpp, line 115
https://git.reviewboard.kde.org/r/120431/diff/1/?file=315733#file315733line115
10 is a default, so I'm not convinced it's worth stating it manually
Fixed.
On Sept. 30, 2014, 9:22 a.m., Jan Kundrát wrote:
drkonqi/bugzillalib.cpp, line 119
https://git.reviewboard.kde.org/r/120431/diff/1/?file=315733#file315733line119
While this code is correct, I dislike the fact that `currentVersion[3]`
and `part 2` are related together.
Yes, this one is longer, but also safer:
if (part = sizeof(currentVersion)/sizeof(*currentVersion)) {
// ...
The 3 is a const int now and the BugzillaVersion triplet is a typedef. I have
used sizeof() to calculate the number of struct items in BugzillaChange
changes[].
On Sept. 30, 2014, 9:22 a.m., Jan Kundrát wrote:
drkonqi/bugzillalib.cpp, line 154
https://git.reviewboard.kde.org/r/120431/diff/1/?file=315733#file315733line154
It would be much cleaner IMHO if there was a single
map/associative_array/... mapping the versions to feature set. The proposed
way is IMHO quite fragile as the code relies on the fact that the
`security` and `Versions` share the same length, yet this is not checked
anywhere.
The changes[] array is now an array of structs, each containing a version at
which the change takes place and an enum value to label the type of change.
The labels are used later in a switch() where the actual feature codes (e.g.
m_security) get set.
On Sept. 30, 2014, 9:22 a.m., Jan Kundrát wrote:
drkonqi/bugzillalib.cpp, lines 164-173
https://git.reviewboard.kde.org/r/120431/diff/1/?file=315733#file315733line164
switch statement to make it obvious that we're checking an enum and
want to react to each and every option?
Done.
On Sept. 30, 2014, 9:22 a.m., Jan Kundrát wrote:
drkonqi/bugzillalib.cpp, line 459
https://git.reviewboard.kde.org/r/120431/diff/1/?file=315733#file315733line459
What guarantees that there's always result[0]?
Bugzilla? Or are their announcements in question? Anyway, I have added a check
that the result list has count() 0.
On Sept. 30, 2014, 9:22 a.m., Jan Kundrát wrote:
drkonqi/bugzillalib.cpp, line 461
https://git.reviewboard.kde.org/r/120431/diff/1/?file=315733#file315733line461
This leaks authentication data to an on-disk log.
Deleted. Also found another instance and fixed it. BTW, tokens are
intentionally short-lived IIUC. You get a new one on every login.
- Ian
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120431/#review67658
---
On Oct. 3, 2014, 7:03 a.m., Ian Wadham wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120431/
---
(Updated Oct. 3, 2014, 7:03 a.m.)
Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley.
Bugs: 337742
http://bugs.kde.org/show_bug.cgi?id=337742
Repository: kde-runtime
Description
---
When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security
method used by Bugzilla changed from cookies to tokens that had to be
supplied as parameters with every secure remote-procedure call. Further
changes to security methods have been announced by Bugzilla and are
documented for unstable 4.5.x versions of Bugzilla software. Tokens will be
deprecated and then discontinued. When
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Oct. 3, 2014, 7:52 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley. Changes --- Fix white space and a slip of the pen. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService/User.html#login User.login will be DEPRECATED in 4.5.x Diffs (updated) - drkonqi/bugzillalib.h 570169b drkonqi/bugzillalib.cpp f74753c drkonqi/reportassistantpages_bugzilla.h b7af5b8 drkonqi/reportassistantpages_bugzilla.cpp 22183f0 Diff: https://git.reviewboard.kde.org/r/120431/diff/ Testing --- Used the bugstest.kde.org database and KDE 4 master on KDE/kde-runtime repository. Tested a range of version numbers (see commented-out test data) against a range of 5 or 6 hypothetical and real Bugzilla versions at which things could or will change. This was to test the basic version-checking and feature-choosing algorithm. Tested submitting both full reports and attached reports, using both the token method and the passwords-only method. Also tested with KWalletD supplying the username and password on Dr Konqi's login dialog. Thanks, Ian Wadham
Re: Review Request 120431: Fix and future-proof Dr Konqi security methods on Bugzilla
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/#review67656 --- Great, it's been ... bugging me that the bug reporter hasn't been able to do its final step since quite some time already. Not to diminish the importance of your work, but someone really ought to provide a means (or document how) to upload a finished bug report without copy/pasting it in bits and pieces. An import button and a shortcut to the last page when launching the reporter manually would be all that's needed once the login step is moved to that page. After all there are other reasons why the final upload in the reported failed. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). Is that really a concern? I suppose no one uses his or her bank account password on a bug reporting site and it's not like open source software will contain industrial secrets either. Make sure the sensitive information is not stored in static memory, make sure regular builds will not contain debug info, and it ought to be hard enough to obtain a password from a memory image. The reporter would normally not be running for a long time anyway. Also, while not an expert on the matter I'm guessing that whatever argument you can forward why caching creds in unencrypted RAM is a bad idea can be extended to every kind of purely in-memory encryption. (Including kwallet if the user doesn't set it to lock after each transaction.) - René J.V. Bertin On Sept. 30, 2014, 9:30 a.m., Ian Wadham wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120431/ --- (Updated Sept. 30, 2014, 9:30 a.m.) Review request for KDE Software on Mac OS X, KDE Runtime and Ben Cooksley. Bugs: 337742 http://bugs.kde.org/show_bug.cgi?id=337742 Repository: kde-runtime Description --- When bugs.kde.org changed over to Bugzilla 4.4.5 in July 2014, the security method used by Bugzilla changed from cookies to tokens that had to be supplied as parameters with every secure remote-procedure call. Further changes to security methods have been announced by Bugzilla and are documented for unstable 4.5.x versions of Bugzilla software. Tokens will be deprecated and then discontinued. When this happens, Dr Konqi will need to supply a user-login name and a password with every secure remote-procedure call. Furthermore, the traditional User.login call presently used by Dr Konqi will be deprecated and discontinued. This patch fixes the tokens problem, which has given rise to several bug reports https://bugs.kde.org/show_bug.cgi?id=337742 and duplicates. It also provides for automatic switching to passwords-only security as and when the Bugzilla version changes again. This uses a general data-driven approach which can be easily updated, ahead of time, next time Bugzilla announces a change that affects Dr Konqi, whether it be in security methods or some other feature. NOTES: 1. This patch is intended to be forward-portable to Frameworks/KF5, but I work on Apple OS X, where it is not yet possible to run Frameworks/KF5 and do the porting and testing. So could someone else please do it? 2. Another Review Request https://git.reviewboard.kde.org/r/120376/ addresses the tokens issue only, but it should be reviewed and shipped as a matter of urgency, both in KDE 4 and Frameworks, the next bug-fixing release for KDE 4.14 being due for tagging on Thursday, 9 October. That will leave more time for this review (120431) of my more long-term and more general patch. 3. The passwords-only part of my patch is currently storing the password in clear. Suggestions re encryption are welcomed --- or the code could be changed to make use of KWalletD mandatory (but that might not be fully portable to all platforms). 4. When the Bugzilla call User.login is discontinued, some re-sequencing of the flow of KAssistantDialog pages will be needed. I have not attempted to do that at this stage. Probably the entry of the user name and password should be delayed until the report has been accepted by the Dr Konqi logic and it is just about to be sent to bugs.kde.org or attached to an existing bug report. REFERENCES: http://www.bugzilla.org/docs/ http://www.bugzilla.org/docs/tip/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.5.x (future) API doco re security http://www.bugzilla.org/docs/4.4/en/html/api/Bugzilla/WebService.html#LOGGING_IN Bugzilla 4.4.5 (current) API doco re security
