Code review comment for lp:~jimmy-sigint/mailman/restapi_auth

Revision history for this message
Jimmy Bergman (jimmy-sigint) wrote :

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.

« Back to merge proposal