upd: improve user api (#878)

powerdnsadmin/lib/errors.py

@@ -93,6 +93,15 @@ class AccountCreateFail(StructuredException):
         self.name = name
 
 
+class AccountCreateDuplicate(StructuredException):
+    status_code = 409
+
+    def __init__(self, name=None, message="Creation of account failed"):
+        StructuredException.__init__(self)
+        self.message = message
+        self.name = name
+
+
 class AccountUpdateFail(StructuredException):
     status_code = 500
 
@@ -120,6 +129,14 @@ class UserCreateFail(StructuredException):
         self.name = name
 
 
+class UserCreateDuplicate(StructuredException):
+    status_code = 409
+
+    def __init__(self, name=None, message="Creation of user failed"):
+        StructuredException.__init__(self)
+        self.message = message
+        self.name = name
+
 class UserUpdateFail(StructuredException):
     status_code = 500
 
@@ -128,6 +145,14 @@ class UserUpdateFail(StructuredException):
         self.message = message
         self.name = name
 
+class UserUpdateFailEmail(StructuredException):
+    status_code = 409
+
+    def __init__(self, name=None, message="Update of user failed"):
+        StructuredException.__init__(self)
+        self.message = message
+        self.name = name
+
 
 class UserDeleteFail(StructuredException):
     status_code = 500

powerdnsadmin/lib/schema.py

@@ -27,6 +27,11 @@ class ApiPlainKeySchema(Schema):
     plain_key = fields.String()
 
 
+class AccountSummarySchema(Schema):
+    id = fields.Integer()
+    name = fields.String()
+
+
 class UserSchema(Schema):
     id = fields.Integer()
     username = fields.String()
@@ -35,6 +40,14 @@ class UserSchema(Schema):
     email = fields.String()
     role = fields.Embed(schema=RoleSchema)
 
+class UserDetailedSchema(Schema):
+    id = fields.Integer()
+    username = fields.String()
+    firstname = fields.String()
+    lastname = fields.String()
+    email = fields.String()
+    role = fields.Embed(schema=RoleSchema)
+    accounts = fields.Embed(schema=AccountSummarySchema)
 
 class AccountSchema(Schema):
     id = fields.Integer()

powerdnsadmin/models/user.py

@@ -7,6 +7,7 @@ import ldap
 import ldap.filter
 from flask import current_app
 from flask_login import AnonymousUserMixin
+from sqlalchemy import orm
 
 from .base import db
 from .role import Role
@@ -29,6 +30,7 @@ class User(db.Model):
     otp_secret = db.Column(db.String(16))
     confirmed = db.Column(db.SmallInteger, nullable=False, default=0)
     role_id = db.Column(db.Integer, db.ForeignKey('role.id'))
+    accounts = None
 
     def __init__(self,
                  id=None,
@@ -591,6 +593,10 @@ class User(db.Model):
         else:
             return {'status': False, 'msg': 'Role does not exist'}
 
+    @orm.reconstructor
+    def set_account(self):
+        self.accounts = self.get_accounts()
+
     def get_accounts(self):
         """
         Get accounts associated with this user
@@ -602,7 +608,7 @@ class User(db.Model):
             .query(
                 AccountUser,
                 Account)\
-            .filter(User.id == AccountUser.user_id)\
+            .filter(self.id == AccountUser.user_id)\
             .filter(Account.id == AccountUser.account_id)\
             .all()
         for q in query:

powerdnsadmin/routes/api.py

@@ -14,13 +14,16 @@ from ..models import (
 from ..lib import utils, helper
 from ..lib.schema import (
     ApiKeySchema, DomainSchema, ApiPlainKeySchema, UserSchema, AccountSchema,
+    UserDetailedSchema,
 )
 from ..lib.errors import (
     StructuredException,
     DomainNotExists, DomainAlreadyExists, DomainAccessForbidden,
     RequestIsNotJSON, ApiKeyCreateFail, ApiKeyNotUsable, NotEnoughPrivileges,
     AccountCreateFail, AccountUpdateFail, AccountDeleteFail,
-    UserCreateFail, UserUpdateFail, UserDeleteFail,
+    AccountCreateDuplicate,
+    UserCreateFail, UserCreateDuplicate, UserUpdateFail, UserDeleteFail,
+    UserUpdateFailEmail,
 )
 from ..decorators import (
     api_basic_auth, api_can_create_domain, is_json, apikey_auth,
@@ -33,11 +36,14 @@ import string
 api_bp = Blueprint('api', __name__, url_prefix='/api/v1')
 
 apikey_schema = ApiKeySchema(many=True)
+apikey_single_schema = ApiKeySchema()
 domain_schema = DomainSchema(many=True)
-apikey_plain_schema = ApiPlainKeySchema(many=True)
+apikey_plain_schema = ApiPlainKeySchema()
 user_schema = UserSchema(many=True)
+user_single_schema = UserSchema()
+user_detailed_schema = UserDetailedSchema()
 account_schema = AccountSchema(many=True)
-
+account_single_schema = AccountSchema()
 
 def get_user_domains():
     domains = db.session.query(Domain) \
@@ -360,7 +366,7 @@ def api_generate_apikey():
         raise ApiKeyCreateFail(message='Api key create failed')
 
     apikey.plain_key = b64encode(apikey.plain_key.encode('utf-8')).decode('utf-8')
-    return jsonify(apikey_plain_schema.dump([apikey])[0]), 201
+    return jsonify(apikey_plain_schema.dump(apikey)), 201
 
 
 @api_bp.route('/pdnsadmin/apikeys', defaults={'domain_name': None})
@@ -418,7 +424,7 @@ def api_get_apikey(apikey_id):
         if apikey_id not in [a.id for a in get_user_apikeys()]:
             raise DomainAccessForbidden()
 
-    return jsonify(apikey_schema.dump([apikey])[0]), 200
+    return jsonify(apikey_single_schema.dump(apikey)), 200
 
 
 @api_bp.route('/pdnsadmin/apikeys/<int:apikey_id>', methods=['DELETE'])
@@ -566,12 +572,12 @@ def api_update_apikey(apikey_id):
 def api_list_users(username=None):
     if username is None:
         user_list = [] or User.query.all()
+        return jsonify(user_schema.dump(user_list)), 200
     else:
-        user_list = [] or User.query.filter(User.username == username).all()
-        if not user_list:
+        user = User.query.filter(User.username == username).first()
+        if user is None:
             abort(404)
-
-    return jsonify(user_schema.dump(user_list)), 200
+        return jsonify(user_detailed_schema.dump(user)), 200
 
 
 @api_bp.route('/pdnsadmin/users', methods=['POST'])
@@ -639,12 +645,12 @@ def api_create_user():
     if not result['status']:
         current_app.logger.warning('Create user ({}, {}) error: {}'.format(
             username, email, result['msg']))
-        raise UserCreateFail(message=result['msg'])
+        raise UserCreateDuplicate(message=result['msg'])
 
     history = History(msg='Created user {0}'.format(user.username),
                       created_by=current_user.username)
     history.add()
-    return jsonify(user_schema.dump([user])), 201
+    return jsonify(user_single_schema.dump(user)), 201
 
 
 @api_bp.route('/pdnsadmin/users/<int:user_id>', methods=['PUT'])
@@ -708,7 +714,10 @@ def api_update_user(user_id):
     if not result['status']:
         current_app.logger.warning('Update user ({}, {}) error: {}'.format(
             username, email, result['msg']))
-        raise UserCreateFail(message=result['msg'])
+        if result['msg'].startswith('New email'):
+            raise UserUpdateFailEmail(message=result['msg'])
+        else:
+            raise UserCreateFail(message=result['msg'])
 
     history = History(msg='Updated user {0}'.format(user.username),
                       created_by=current_user.username)
@@ -759,25 +768,18 @@ def api_list_accounts(account_name):
     else:
         if account_name is None:
             account_list = [] or Account.query.all()
+            return jsonify(account_schema.dump(account_list)), 200
         else:
-            account_list = [] or Account.query.filter(
-                Account.name == account_name).all()
-            if not account_list:
+            account = Account.query.filter(
+                Account.name == account_name).first()
+            if account is None:
                 abort(404)
-    if account_name is None:
-        return jsonify(account_schema.dump(account_list)), 200
-    else:
-        return jsonify(account_schema.dump(account_list)[0]), 200
+            return jsonify(account_single_schema.dump(account)), 200
 
 
 @api_bp.route('/pdnsadmin/accounts', methods=['POST'])
 @api_basic_auth
 def api_create_account():
-    account_exists = [] or Account.query.filter(Account.name == account_name).all()
-    if len(account_exists) > 0:
-        msg = "Account name already exists"
-        current_app.logger.debug(msg)
-        raise AccountCreateFail(message=msg)
     if current_user.role.name not in ['Administrator', 'Operator']:
         msg = "{} role cannot create accounts".format(current_user.role.name)
         raise NotEnoughPrivileges(message=msg)
@@ -790,6 +792,12 @@ def api_create_account():
         current_app.logger.debug("Account name missing")
         abort(400)
 
+    account_exists = [] or Account.query.filter(Account.name == name).all()
+    if len(account_exists) > 0:
+        msg = "Account {} already exists".format(name)
+        current_app.logger.debug(msg)
+        raise AccountCreateDuplicate(message=msg)
+
     account = Account(name=name,
                       description=description,
                       contact=contact,
@@ -806,7 +814,7 @@ def api_create_account():
     history = History(msg='Create account {0}'.format(account.name),
                       created_by=current_user.username)
     history.add()
-    return jsonify(account_schema.dump([account])[0]), 201
+    return jsonify(account_single_schema.dump(account)), 201
 
 
 @api_bp.route('/pdnsadmin/accounts/<int:account_id>', methods=['PUT'])
@@ -871,6 +879,7 @@ def api_delete_account(account_id):
 
 
 @api_bp.route('/pdnsadmin/accounts/users/<int:account_id>', methods=['GET'])
+@api_bp.route('/pdnsadmin/accounts/<int:account_id>/users', methods=['GET'])
 @api_basic_auth
 @api_role_can('list account users')
 def api_list_account_users(account_id):
@@ -885,6 +894,9 @@ def api_list_account_users(account_id):
 @api_bp.route(
     '/pdnsadmin/accounts/users/<int:account_id>/<int:user_id>',
     methods=['PUT'])
+@api_bp.route(
+    '/pdnsadmin/accounts/<int:account_id>/users/<int:user_id>',
+    methods=['PUT'])
 @api_basic_auth
 @api_role_can('add user to account')
 def api_add_account_user(account_id, user_id):
@@ -909,6 +921,9 @@ def api_add_account_user(account_id, user_id):
 @api_bp.route(
     '/pdnsadmin/accounts/users/<int:account_id>/<int:user_id>',
     methods=['DELETE'])
+@api_bp.route(
+    '/pdnsadmin/accounts/<int:account_id>/users/<int:user_id>',
+    methods=['DELETE'])
 @api_basic_auth
 @api_role_can('remove user from account')
 def api_remove_account_user(account_id, user_id):

powerdnsadmin/swagger-spec.yaml

@@ -1,6 +1,6 @@
 swagger: '2.0'
 info:
-  version: "0.0.13"
+  version: "0.0.14"
   title: PowerDNS Admin Authoritative HTTP API
   license:
     name: MIT
@@ -1041,6 +1041,10 @@ paths:
           description: Unprocessable Entry, the User data provided has issues
           schema:
             $ref: '#/definitions/Error'
+        '409':
+          description: Duplicate Entry, either the Name or the Email is already in use
+          schema:
+            $ref: '#/definitions/Error'
         '500':
           description: Internal Server Error. There was a problem creating the user
           schema:
@@ -1063,7 +1067,7 @@ paths:
         '200':
           description: Retrieve a specific User
           schema:
-            $ref: '#/definitions/User'
+            $ref: '#/definitions/UserDetailed'
         '404':
           description: Not found. The User with the specified username does not exist
           schema:
@@ -1206,6 +1210,10 @@ paths:
           description: Unprocessable Entry, the Account data provided has issues.
           schema:
             $ref: '#/definitions/Error'
+        '409':
+          description: Duplicate Entry, the Name is already in use
+          schema:
+            $ref: '#/definitions/Error'
         '500':
           description: Internal Server Error. There was a problem creating the account
           schema:
@@ -1299,7 +1307,7 @@ paths:
           description: Internal Server Error. Contains error message
           schema:
             $ref: '#/definitions/Error'
-  '/pdnsadmin/accounts/users/{account_id}':
+  '/pdnsadmin/accounts/{account_id}/users':
     parameters:
       - name: account_id
         type: integer
@@ -1316,7 +1324,7 @@ paths:
         - user
       responses:
         '200':
-          description: List of User objects
+          description: List of Summarized User objects
           schema:
             type: array
             items:
@@ -1329,7 +1337,7 @@ paths:
           description: Internal Server Error, accounts could not be retrieved. Contains error message
           schema:
             $ref: '#/definitions/Error'
-  '/pdnsadmin/accounts/users/{account_id}/{user_id}':
+  '/pdnsadmin/accounts/{account_id}/users/{user_id}':
     parameters:
       - name: account_id
         type: integer
@@ -1380,7 +1388,6 @@ paths:
           schema:
             $ref: '#/definitions/Error'
 
-
 definitions:
   Server:
     title: Server
@@ -1603,9 +1610,9 @@ definitions:
           type: string
           description: 'Name of the zone'
 
-  PDNSAdminApiKeyRole:
-    title: PDNSAdminApiKeyRole
-    description: Role of ApiKey, defines privileges on domains
+  PDNSAdminRole:
+    title: PDNSAdminRole
+    description: Roles of PowerDNS Admin
     properties:
       id:
         type: integer
@@ -1613,7 +1620,7 @@ definitions:
         readOnly: true
       name:
         type: string
-        description: 'Name of role'
+        description: 'The Name of PDNSAdmin role'
 
   ApiKey:
     title: ApiKey
@@ -1630,12 +1637,10 @@ definitions:
         type: string
         description: 'not used on POST, POSTing to server generates the key material'
       domains:
-        type: array
-        items:
-          $ref: '#/definitions/PDNSAdminZones'
+        $ref: '#/definitions/PDNSAdminZones'
         description: 'domains to which this apikey has access'
       role:
-        $ref: '#/definitions/PDNSAdminApiKeyRole'
+        $ref: '#/definitions/PDNSAdminRole'
       description:
         type: string
         description: 'Some user defined description'
@@ -1676,10 +1681,51 @@ definitions:
         type: boolean
         description: The confirmed status
         readOnly: false
-      role_id:
+      role:
+        $ref: '#/definitions/PDNSAdminRole'
+
+  UserDetailed:
+    title: User
+    description: User that can access the gui/api
+    properties:
+      id:
         type: integer
-        description: The ID of the role
+        description: The ID for this user (unique)
+        readOnly: true
+      username:
+        type: string
+        description: The username for this user (unique, immutable)
         readOnly: false
+      password:
+        type: string
+        description: The hashed password for this user
+        readOnly: false
+      firstname:
+        type: string
+        description: The firstname of this user
+        readOnly: false
+      lastname:
+        type: string
+        description: The lastname of this user
+        readOnly: false
+      email:
+        type: string
+        description: Email addres for this user
+        readOnly: false
+      otp_secret:
+        type: string
+        description: OTP secret
+        readOnly: false
+      confirmed:
+        type: boolean
+        description: The confirmed status
+        readOnly: false
+      role:
+        $ref: '#/definitions/PDNSAdminRole'
+      accounts:
+        type: array
+        items:
+          $ref: '#/definitions/AccountSummary'
 
   Account:
     title: Account
@@ -1706,6 +1752,19 @@ definitions:
         description: The email address of the contact for this account
         readOnly: false
 
+  AccountSummary:
+    title: AccountSummry
+    description: Summary of an Account that 'owns' zones
+    properties:
+      id:
+        type: integer
+        description: The ID for this account (unique)
+        readOnly: true
+      name:
+        type: string
+        description: The name for this account (unique, immutable)
+        readOnly: false
+
   ConfigSetting:
     title: ConfigSetting
     properties: