mail-in-a-box / mailinabox

Mail-in-a-Box helps individuals take back control of their email by defining a one-click, easy-to-deploy SMTP+everything else server: a mail server in a box.
https://mailinabox.email/
Creative Commons Zero v1.0 Universal
13.98k stars 1.44k forks source link

Suggestion: move to API keys #1217

Open nomandera opened 7 years ago

nomandera commented 7 years ago

The current method of interacting with the Custom DNS API uses the user name and password

e.g.

curl -X VERB [-d "value"] --user {email}:{password} https://domain/admin/dns/custom[/qname[/rtype]]

this encourages leaving credentials, sometimes that of the main user, unencrypted within scripts or implicitly trusting 3rd party tools.

An API key that can revoked and does not afford an attacker the means to login and gain complete control seems more sensible.

JoshData commented 7 years ago

The control panel uses an API to generate an API key for itself when you log in. It just needs to be documented. It'll work for all of the APIs.

nomandera commented 7 years ago

Can you point me in the direction of something to get me started understanding this?

yodax commented 7 years ago

This line prints the api key when you locally start the deamon: https://github.com/mail-in-a-box/mailinabox/blob/master/management/daemon.py#L615

neanere commented 3 years ago

As of v0.51 with TOTP enabled there is a need to use a non-volatile apiKey for remote authentication. Currently if we want to use the curl API I was only able to do it remotely if I pass the master apiKey as the user (no password) or if I include the TOTP token as part of the normal call in an additional 'X-Auth-Token' header.

The need for a non-volatile apiKey comes from the fact that the system generates a master apiKey on restart so any remotely configured automatic service (such as one used for dynamic DNS purposes) couldn't rely on a given apiKey that will expire on the MiaB's system restart.

fspoettel commented 3 years ago

@JoshData would you accept a PR that implements basic support for API keys? We identified the problem that @neanere describes as well when reviewing #1814 . I'm willing to do the legwork and have some time to work on this in the coming weeks.

A minimal implementation would consist of adding an admin page to manage API keys, a storage mechanism and an extension of the authentication check. We could think about building in basic support for scopes and expires_in fields that we can later extend to implement the following improvements:

fspoettel commented 3 years ago

I thought a bit about the implementation today. I strongly concur with @anoma's assessment that an api key [should] not afford an attacker the means to login and gain complete control.

We'll have to make sure that user-generated api keys do not have permission to do the following things:

Let me know if I missed anything here.

Implementation-wise, this could be done with a @logged_in_admin_only decorator that configures the auth service to only allow internal user api keys (and in the case of /me user/password).

If it turns out that some use cases require these permissions, we could allow to relax these restrictions (besides login) with individual scopes after showing a fat warning .

fspoettel commented 3 years ago

played a bit with the user interface as well. what do you think about something like this:

image

JoshData commented 3 years ago

Hey. This looks like a good path forward. A first version shouldn't worry about scopes --- just get the mechanics of authentication working. Leave a TEXT field in a database column for scopes for a later debate.

fspoettel commented 3 years ago

I won't have time to continue with this anytime soon. If someone wants to pick up and finish it, code can be found here.

Status of the branch:

PeetMcK commented 1 year ago

Ooof I hat to poke a stick at something from Jan 2021 ... but If there's any one out there that does have the time work on this I'm buying beer. I'd be happy to handle any testing and documentation, but would likely be less useful for anything else.

So here's a belated +1