|Summary:||[ews-build] Cancel build and similar operations should have authentication|
|Product:||WebKit||Reporter:||Aakash Jain <aakash_jain>|
|Component:||Tools / Tests||Assignee:||Aakash Jain <aakash_jain>|
|Severity:||Normal||CC:||aakash_jain, ap, commit-queue, dewei_zhu, kocsen_chung, lforschler, webkit-bug-importer|
Description Aakash Jain 2019-04-02 17:44:36 PDT
Currently there is no authentication on ews-build webpages. Anyone can press 'cancel' button and cancel all the running builds and all the pending buildrequests. Similarly anyone can pause/shutdown the workers. These operations should not be allowed without authentication.
Comment 2 dewei_zhu 2019-04-02 18:38:23 PDT
Comment on attachment 366562 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366562&action=review > Tools/BuildSlaveSupport/ews-build/master.cfg:4 > +from buildbot.plugins import * I would prefer to import exactly the models those will be used in this file so that it would be much easier to locate certain module / function. > Tools/BuildSlaveSupport/ews-build/master.cfg:22 > +admin_username = os.getenv('EWS_ADMIN_USERNAME') > +admin_password = os.getenv('EWS_ADMIN_PASSWORD') Does it work when neither environment is set? What happens if only either of them is set?
Comment 4 Aakash Jain 2019-04-03 07:00:42 PDT
> I would prefer to import exactly the models those will be used in this file so that it would be much easier to locate certain module / function. Done. > Does it work when neither environment is set? Yes, in that case it will work without authentication. 'if admin_username and admin_password:' condition wouldn't be met and we won't set any authentication, this might be useful for local testing. > What happens if only either of them is set? Same as above, we won't configure the authentication since the 'if' condition won't be met.
Comment 5 Lucas Forschler 2019-04-03 13:01:28 PDT
Comment on attachment 366598 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366598&action=review > Tools/BuildSlaveSupport/ews-build/master.cfg:32 > + I'm concerned that if we have no environment variable set, the default state of the server is to not require auth... I think if we fail to find an environment variable, we should probably not continue.
Comment 6 Kocsen Chung 2019-04-03 13:22:17 PDT
Agree with Lucas, we probably want to check if there is no username and password and fail.
Comment 8 Aakash Jain 2019-04-03 17:35:55 PDT
> I think if we fail to find an environment variable, we should probably not continue. Done.
Comment 9 WebKit Commit Bot 2019-04-03 20:57:16 PDT
Comment on attachment 366677 [details] Patch Clearing flags on attachment: 366677 Committed r243854: <https://trac.webkit.org/changeset/243854>
Comment 10 WebKit Commit Bot 2019-04-03 20:57:18 PDT
All reviewed patches have been landed. Closing bug.