Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft OpenApi Specification #1

Open
NorseGaud opened this issue Jun 23, 2021 · 16 comments
Open

Draft OpenApi Specification #1

NorseGaud opened this issue Jun 23, 2021 · 16 comments
Assignees

Comments

@NorseGaud
Copy link
Member

NorseGaud commented Jun 23, 2021

Related: #3


Goal: Discuss and Create the v1 API spec to use with in the app

@LukeMarlin
Copy link

LukeMarlin commented Jun 27, 2021

Here we go:

openapi: 3.0.1
info:
  title: Docker-mailserver Administration API
  version: 0.0.1
servers:
  - url: 'https://contoso.com/'
paths:
  /api/admin/v1/users/{emailAddress}/changePassword:
    post:
      summary: Changes a given user's password
      tags:
        - V1 User operations
        - V1 Admin API
      security:
        - ApiKeyAuth: []
      parameters:
        - in: path
          name: emailAddress
          required: true
          schema:
            type: string
      requestBody:
        description: The new password for the target user
        content:
          application/json:
            schema:
              type: string
      responses:
        '204':
          description: The password was successfully modified
        '404':
          $ref: '#/components/responses/UserNotFoundError'
        '401':
          $ref: '#/components/responses/UnauthorizedError'
components:
  schemas:
    ErrorBase:
      type: object
      required:
        - errorCode
        - message
      properties:
        errorCode:
          type: string
          description: Code name of the error
        message:
          type: string
          description: Additional details about the error
  securitySchemes:
    ApiKeyAuth:
      type: apiKey
      in: header
      name: X-API-KEY
  responses:
    UserNotFoundError:
      description: User does not exist
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/ErrorBase'
    UnauthorizedError:
      description: API key is missing or invalid
      content:
        application/json:
          schema:
            $ref: '#/components/schemas/ErrorBase'

You can copy & paste this in https://editor.swagger.io/ to see how it would look like

Key points:

  • URI prefix leaves room for both a user and an admin API (user api = self service)
  • URI prefix allows for separate versioning between user and admin APIs
  • Errors have a code and a message. The code is part of the contract, while message can change at any time. This allows callers to provide their own messages by checking the reliable errorCode
  • As discussed in the issues, it seems that an API key was the preferred method. This is reflected in the specification, and should be in a X-API-KEY header
  • Application/Json will be the only supported medium

Feel free to discuss any point from the spec in that issue.
Next step will be generating the app skeleton, I'll do it during the week. I already started, based on another spec, so there shouldn't be much to be done

@NorseGaud
Copy link
Member Author

Sounds and looks good. Nothing stuck out as problematic to me.

Small question: I'm a novice, so I'm curious if it's typical to have /user/{username} and a separate /users which would list all users for APIs? The plural /users/{email}/changePassword is grammatically odd to me.

@LukeMarlin
Copy link

LukeMarlin commented Jun 28, 2021

While I've seen the debate between plural and singular, I've never seen an API where you have both. Doing this, you somewhat break the clear hierarchy so I don't think it's common at all. I don't claim to know everything though, obviously!

Maybe reading that as "From /users, change the password of {email}" might help grammatically :)

When it comes to all plural vs all singular, I prefer to have plural because "/user" for listing looks weird to me.

Resources that might be of interest: https://jsonapi.org/recommendations/#urls-resource-collections

JsonAPI is a nicely specified way to design an API, so I like to take inspiration from there!

@DerOetzi
Copy link

I'm not sure the API-Key Auth is enough for users changing their password by them self. I think it is neccessary for this use case to authenticate by old password?

@LukeMarlin
Copy link

@DerOetzi I have looked into that possibility, and that's even what I wanted to do in the first place. However, when trying to do a dummy implementation some weeks ago, I noticed it wasn't so simple: we wouldn't be able to use the shell functions from docker-mailserver, we'd need to read the postfix-accounts.cf file, parse it to get the user, get the salt and finally we can check if given old password is good.

While it is doable, it seems that users mostly wanted to have a layer above the API with a simple key, meaning -I assumed-, they'd manage authentication on their UI instead.

With that in mind, I therefore prefixed this api with /admin to make it clear it's not intended to users. If there's a demand for self-service as well, we can always add a route later on under a /self section to the API.

@DerOetzi
Copy link

But how is a single user getting his personal API-Key? Let us asume their a two users [email protected] and [email protected]. How is secured that [email protected] can only change it's own password and not of [email protected]?

@LukeMarlin
Copy link

There is no personal API key for /admin. From what I've gathered from the other thread, the aim was to have an admin API/panel, and some people wanted to have an API that could be used by another component (webmail, another UI). In the latter case, as mentioned above, the user authentication is another layer.

Again, that doesn't prevent having /self routes on the side, which would be based on the user's current password anyway.

@dumblob
Copy link

dumblob commented Nov 2, 2021

Does this also address forgotten passwords?

I mean sending a one-time hash link to a user-defined address (which will be checked for existence and if not existent, no reply - not even about non-existence! - will be issued) with a form to change password. Or something along that line.

This helps enormously in practice as passwords are number one reason to contact the admin/support.

@LukeMarlin
Copy link

Right now this doesn't.
This could become a feature of the admin API, but it is currently out of scope. For now the goal is to provide a simple yet useful API with the first requested use case. Other features can be discussed when that base is available :)

@andrewlow
Copy link

I got here from docker-mailserver/docker-mailserver#2092

This is certainly interesting - and I'd like to suggest that the API expand to at minimum all of the commands you can perform via docker exec -it mailserver setup - is that the intent? Or is this scoped more narrowly?

@LukeMarlin
Copy link

This is certainly interesting - and I'd like to suggest that the API expand to at minimum all of the commands you can perform via docker exec -it mailserver setup - is that the intent?

I don't know all of them, but for there should be no issue with having them all available with the API

@andrewlow
Copy link

I believe the password change capability would be setup email update <email> <password>

Here is a full list of the commands supported by setup

[SUB]COMMANDS
    COMMAND email :=
        /usr/local/bin/setup email add <EMAIL ADDRESS> [<PASSWORD>]
        /usr/local/bin/setup email update <EMAIL ADDRESS> [<PASSWORD>]
        /usr/local/bin/setup email del [ OPTIONS... ] <EMAIL ADDRESS> [ <EMAIL ADDRESS>... ]
        /usr/local/bin/setup email restrict <add|del|list> <send|receive> [<EMAIL ADDRESS>]
        /usr/local/bin/setup email list

    COMMAND alias :=
        /usr/local/bin/setup alias add <EMAIL ADDRESS> <RECIPIENT>
        /usr/local/bin/setup alias del <EMAIL ADDRESS> <RECIPIENT>
        /usr/local/bin/setup alias list
        
    COMMAND quota :=
        /usr/local/bin/setup quota set <EMAIL ADDRESS> [<QUOTA>]
        /usr/local/bin/setup quota del <EMAIL ADDRESS>

    COMMAND config :=
        /usr/local/bin/setup config dkim [ ARGUMENTS... ]
        
    COMMAND relay :=
        /usr/local/bin/setup relay add-domain <DOMAIN> <HOST> [<PORT>]
        /usr/local/bin/setup relay add-auth <DOMAIN> <USERNAME> [<PASSWORD>]
        /usr/local/bin/setup relay exclude-domain <DOMAIN>
        
    COMMAND debug :=
        /usr/local/bin/setup debug fetchmail
        /usr/local/bin/setup debug fail2ban [unban <IP>]
        /usr/local/bin/setup debug show-mail-logs
        /usr/local/bin/setup debug inspect
        /usr/local/bin/setup debug login <COMMANDS>```

@pn2
Copy link

pn2 commented Jan 22, 2022

Isn't doing these changes via setup a security risk because it requires root privileges, or at least the privileges docker is running with? I am not an expert, but somehow feel it would be better to 'export' these function from docker-mailserver (and only these!) through a simple interface (does not have to be http-based), or modify the account files directly.

@andrewlow
Copy link

Yes and no.

The dockermailserver container runs as root. So that as a whole is the main security issue.

setup also runs as root - but part of that is to ensure that it can read/write to all of the files. If the dockermailserver project as a whole decided to be more strict on file ownership/permissions - it would be possible to have the setup script drop permissions and run as the 'user'.

Of course - that is exactly the same restrictions you'd need to create a limited API as you describe.

  1. Ensure that all config files are owned by a non-root user and have appropriate permissions
  2. Run the API (or setup) as that non-root user.

It is unlikely due to the software stack dockermailserver is built on to avoid the container running as root. Possible, but probably more trouble than it is worth.

@pn2
Copy link

pn2 commented Jan 24, 2022

Yes and no.

The dockermailserver container runs as root. So that as a whole is the main security issue.

setup also runs as root

Yes, but isn't the difference that setup (or docker exec -ti ... setup ...) would have to be run as root on the host, not in a container ?

@andrewlow
Copy link

You are correct that either you have configured docker such that users can run docker (which in itself is a security risk) or you have to be root to issue the docker command.

I do think you've interpreted the setup command incorrectly in this thread. I had brought this up because today - the setup command offers a suite of features. The API should probably includes all of these features. How the API decides to implement these features - well that's up to the API.

Today - with dockermailserver if you modify the config files - the container notices and refreshes. You don't have to use setup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants