[Merge] lp:~jimmy-sigint/mailman/restapi_auth into lp:mailman
Jimmy Bergman has proposed merging lp:~jimmy-sigint/mailman/restapi_auth into lp:mailman. Requested reviews: Mailman Coders (mailman-coders) In my opinion the REST API needs to be authenticated for the following reasons: 1. Even though it is by default exposed only on localhost, this means that all local users can administer mailing-lists instead of only some specific user like root. 2. It makes sense to use the REST API for integrating with external systems. These external systems will often be on other servers, causing the need for exposing the REST API on different interfaces than the loopback interface. For this authentication is a requirement. The change in my branch solves this by adding a single shared username/password in the webservice section of the config using the parameters admin_user and admin_pass. The API is then changed to require HTTP basic auth using these credentials. -- https://code.launchpad.net/~jimmy-sigint/mailman/restapi_auth/+merge/36833 Your team Mailman Coders is requested to review the proposed merge of lp:~jimmy-sigint/mailman/restapi_auth into lp:mailman.
Thanks for the branch Jimmy. I think you make some good points about enabling some form of authentication in the REST server. I've looked at: http://ish.io/embedded/restish/guard.html which provides examples of hooking up repoze.who. Did you think about that and if so, why did you choose not to use it (adding a dependency and the extra code complexity is a valid answer :). Have you thought about using something like OAuth? Are you concerned at all about cleartext passwords? Finally, while your patch looks basically decent (I'd quibble with some whitespace, but that's unimportant), please consider adding a test and/or some documentation (the latter perhaps as a doctest). And are you willing and able to assign your copyright to the FSF? -- https://code.launchpad.net/~jimmy-sigint/mailman/restapi_auth/+merge/36833 Your team Mailman Coders is requested to review the proposed merge of lp:~jimmy-sigint/mailman/restapi_auth into lp:mailman.
Hi Well, adding a dependency and the extra code complexity was mainly the reason - yes. I think the use case for repoze.who is more "I need good way to use an existing authentication store in an WSGI application". In the mailman REST server case I didn't think that there existed an existing user store to plug into (at least with the same meaning that I was aiming for, i.e. global access to the installation like in the current REST server). So while I could of course use repoze.who, I don't think it has that much merit for this simple case (one credential for the complete app). OAuth for this feels overkill as well to me. If the goal was more granular auth in the REST server (perhaps ability to use authenticate as list owner/subscriber/site admin and become authorized only for the related things), then I would have gone for repoze.who and perhaps OAuth. But I figured that your choice to not have any authentication at all reflected a wish to KISS, and that wish is something which I very much agree with; hence the single user/pass shared secret. Regarding clear text passwords, sure - but I think that a simple solution for this is to use SSL for the transport - which I think is supported with the use_https property. If you're ok with the code (and by all means you can suggest indentation-changes, I like real tabs myself - but tried to use the project style, perhaps unsuccessfully) then I'll go ahead and commit test/doc to the branch. Regarding the copyright, sure - this is no problem, just point me in the right direction. -- https://code.launchpad.net/~jimmy-sigint/mailman/restapi_auth/+merge/36833 Your team Mailman Coders is requested to review the proposed merge of lp:~jimmy-sigint/mailman/restapi_auth into lp:mailman.
I agree with your rationale, and I do think that basic auth is better than nothing. ;) I haven't tried it, but your code looks essentially correct. Please do extend your branch with tests and docs. Contact me at barry at list dot org and I'll get the FSF assignment process started. Thanks for you contribution to Mailman! -- https://code.launchpad.net/~jimmy-sigint/mailman/restapi_auth/+merge/36833 Your team Mailman Coders is requested to review the proposed merge of lp:~jimmy-sigint/mailman/restapi_auth into lp:mailman.
participants (2)
-
Barry Warsaw -
Jimmy Bergman