From bec8ca107a447affb8277f0e2b6c8e5339f00281 Mon Sep 17 00:00:00 2001 From: Krishnakanth Alagiri Date: Sun, 17 Apr 2022 04:28:55 +0530 Subject: [PATCH] Bump version to 1.2.0 (#8) * Added basic 404 handling * Bug fixes & added support for password policy --- .github/ISSUE_TEMPLATE/feature_request.md | 15 +++++ .github/workflows/ci.yml | 6 +- .github/workflows/codeql-analysis.yml | 16 +---- Access/userpass.py | 68 ++++++++++++++++--- Api/__init__.py | 1 + Api/api.py | 4 +- Api/errors/__init__.py | 1 + Api/errors/errors.py | 14 ++++ Api/resources/auth/userpass_resource.py | 25 +++++-- Api/resources/secrets/kv_resource.py | 33 ++++++--- Dockerfile | 2 +- Engines/kv.py | 81 +++++++++++------------ build.sh | 2 +- connection.py | 22 ++++-- docs/README_dockerhub.md | 3 +- 15 files changed, 193 insertions(+), 100 deletions(-) create mode 100644 .github/ISSUE_TEMPLATE/feature_request.md create mode 100644 Api/__init__.py create mode 100644 Api/errors/__init__.py create mode 100644 Api/errors/errors.py diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 0000000..0c8c515 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,15 @@ +--- +name: Feature Request +about: "For feature requests. Please search for existing issues first. Also see CONTRIBUTING." +--- + +#### Feature description +> Please present a concise description of the problem to be addressed by this feature request. Please be clear what parts of the problem are considered to be in-scope and out-of-scope. + +#### Suggest a solution (Optional) +> Replace This Text: A concise description of your preferred solution. Things to address include: +> * Details of the technical implementation +> * Tradeoffs made in design decisions +> * Caveats and considerations for the future +> +> If there are multiple solutions, please present each one separately. Save comparisons for the very end. \ No newline at end of file diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7bfa863..6d51e80 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,8 +2,6 @@ name: Build and deploy multiarch image on: push: -# branches: -# - 'releases/v*' tags: - 'v*' paths-ignore: @@ -37,9 +35,9 @@ jobs: env: IMG_NAME: ${{ 'krishnaalagiri/ssm' }} # Versioning: MAJOR.MINOR.PATCH (eg., 1.2.3) - VERSION_FULL: ${{ '1.1.2' }} + VERSION_FULL: ${{ '1.2.0' }} # For v1.2.3, VERSION_SHORT is '1.2' - VERSION_SHORT: ${{ '1.1' }} + VERSION_SHORT: ${{ '1.2' }} # For v1.2.3, VERSION_MAJOR is '1' VERSION_MAJOR: ${{ '1' }} with: diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 63d06ba..46f29e6 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -1,15 +1,4 @@ -# For most projects, this workflow file will not need changing; you simply need -# to commit it to your repository. -# -# You may wish to alter this file to override the set of languages analyzed, -# or to provide custom queries or build logic. -# -# ******** NOTE ******** -# We have attempted to detect the languages in your repository. Please check -# the `language` matrix defined below to confirm you have the correct set of -# supported CodeQL languages. -# -name: "CodeQL" +name: "Code Quality Analysis" on: push: @@ -33,9 +22,6 @@ jobs: fail-fast: false matrix: language: [ 'python' ] - # CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python', 'ruby' ] - # Learn more about CodeQL language support at https://git.io/codeql-language-support - steps: - name: Checkout repository uses: actions/checkout@v3 diff --git a/Access/userpass.py b/Access/userpass.py index a6f194e..c703a0c 100644 --- a/Access/userpass.py +++ b/Access/userpass.py @@ -1,9 +1,54 @@ #!/usr/bin/env python3 """ User-Pass authentication for Secrets Manager """ +from werkzeug.security import generate_password_hash, check_password_hash from bson.timestamp import Timestamp +from textwrap import dedent import datetime as dt -from werkzeug.security import generate_password_hash, check_password_hash +import re +import os + + +class _password_policy: + def __init__(self): + # Username policy: regex pattern + self.uname_pat = "[a-zA-Z0-9_]+" + # min length (default: 6) + self.length = os.environ.get("PASSWORD_POLICY_LENGTH", 6) + # need min. (default: 1) uppercase letters + self.uppercase = os.environ.get("PASSWORD_POLICY_UPPERCASE", 1) + # need min. (default: 1) uppercase letters + self.lowercase = os.environ.get("PASSWORD_POLICY_LOWERCASE", 1) + # need min. (default: 1) digits + self.numbers = os.environ.get("PASSWORD_POLICY_NUMBERS", 1) + # need min. (default: 1) special characters + self.special = os.environ.get("PASSWORD_POLICY_SPECIAL", 1) + + def __repr__(self): + policy_str = f""" + (1) Minimum of { self.length } characters in length. + (2) Must have at least { self.lowercase } lowercase characters. + (3) Must have at least { self.uppercase }uppercase characters. + (4) Must have at least { self.numbers } numbers. + (5) Must have at least { self.special } special characters. + """ + policy_str = dedent(policy_str).replace('\n', ' ') + return policy_str + + def check(self, password): + """ Password policy/rules to encourage users to employ strong passwords. + Args: + password (str): Password string + Returns: + bool: True if password policy is satisfied + """ + if len(password) >= self.length and \ + len(re.findall("[a-z]", password)) >= self.lowercase and \ + len(re.findall("[A-Z]", password)) >= self.uppercase and \ + len(re.findall("[0-9]", password)) >= self.numbers and \ + len(re.findall("[^a-z^A-Z^0-9]", password)) >= self.special: + return True + return False class User_Pass: @@ -15,6 +60,7 @@ def __init__(self, userpass_auth_col): # * Create unique index on 'username' for secrets_manager_auth.userpass # * db.userpass.createIndex( { "username": 1 }, { unique: true } ) self._userpass = userpass_auth_col + self.p_pol = _password_policy() def register(self, username, password): """ Register a new user @@ -24,6 +70,12 @@ def register(self, username, password): Returns: dict : Dictionary with operation status """ + # Username policy check + if not re.fullmatch(self.p_pol.uname_pat, username): + return f"Username does not match { self.p_pol.uname_pat }", 400 + # Password policy check + if not self.p_pol.check(password): + return f"Password policy not met. { self.p_pol }", 400 finder = self._userpass.find_one({"username": username}) if not finder: password = generate_password_hash(password, method='sha256') @@ -34,9 +86,8 @@ def register(self, username, password): } _ = self._userpass.insert_one(data) status = {"status": "OK"} - else: - status = {"status": "User already exist"} - return status + return status, 200 + return "User already exist", 400 def remove(self, username): """ Deletes an existing user @@ -47,11 +98,10 @@ def remove(self, username): """ finder = self._userpass.find_one({"username": username}) if not finder: - result = {"status": "Username does not exist"} - else: - _ = self._userpass.delete_one({"username": username}) - result = {"status": "OK"} - return result + return "User does not exist", 400 + _ = self._userpass.delete_one({"username": username}) + result = {"status": "OK"} + return result, 200 def is_authorized(self, username, password): """ Check if a userpass is valid diff --git a/Api/__init__.py b/Api/__init__.py new file mode 100644 index 0000000..e5a0d9b --- /dev/null +++ b/Api/__init__.py @@ -0,0 +1 @@ +#!/usr/bin/env python3 diff --git a/Api/api.py b/Api/api.py index 767ed59..e8b1b47 100644 --- a/Api/api.py +++ b/Api/api.py @@ -17,7 +17,7 @@ conn = Connection() api_v1 = Blueprint("api", __name__, url_prefix="/api") -api = Api(api_v1, version="1.1.2", title="Simple Secrets Manager", +api = Api(api_v1, version="1.2.0", title="Simple Secrets Manager", description="Secrets management simplified", authorizations=authorizations) app = Flask(__name__) @@ -34,3 +34,5 @@ # Authentication methods from Api.resources.auth.userpass_resource \ import Auth_Userpass_delete, Auth_Userpass_register # noqa: F401 + # Handling HTTP Errors + from Api.errors import errors # noqa: F401 diff --git a/Api/errors/__init__.py b/Api/errors/__init__.py new file mode 100644 index 0000000..e5a0d9b --- /dev/null +++ b/Api/errors/__init__.py @@ -0,0 +1 @@ +#!/usr/bin/env python3 diff --git a/Api/errors/errors.py b/Api/errors/errors.py new file mode 100644 index 0000000..747418b --- /dev/null +++ b/Api/errors/errors.py @@ -0,0 +1,14 @@ +#!/usr/bin/env python3 +from Api.api import app +from flask import jsonify + + +@app.errorhandler(404) +def not_found(e): + return jsonify(error="Resource not found"), 404 + + +@app.errorhandler(Exception) +def server_error(e): + app.logger.exception(e) + return jsonify(error="Server error. Contact administrator"), 500 diff --git a/Api/resources/auth/userpass_resource.py b/Api/resources/auth/userpass_resource.py index 96d6e0d..499e079 100644 --- a/Api/resources/auth/userpass_resource.py +++ b/Api/resources/auth/userpass_resource.py @@ -30,10 +30,10 @@ post_userpass_parser = api.parser() post_userpass_parser.add_argument( "username", type=str, required=True, location="form", - help="Username must atleast be 2 characters long") + help="Username must atleast be 1 characters long") post_userpass_parser.add_argument( "password", type=str, required=True, location="form", - help="Password should atleast be 6 characters long") + help="Password should satisfy policy") @userpass_ns.route("/delete") @@ -42,16 +42,21 @@ params={}) class Auth_Userpass_delete(Resource): """Userpass operations""" - @api.doc( description="Revoke a given user", - responses={200: "User account removed"}, + responses={ + 200: "User account removed", + 400: "User does not exist", + }, parser=delete_userpass_parser) @api.marshal_with(userpass_model) def delete(self): """Revoke a given user""" args = delete_userpass_parser.parse_args() - return conn.userpass.remove(username=args['username']) + status, code = conn.userpass.remove(username=args['username']) + if code != 200: + api.abort(code, status) + return status @userpass_ns.route("/register") @@ -60,9 +65,12 @@ def delete(self): params={}) class Auth_Userpass_register(Resource): """Userpass operations""" - @api.doc( description="Register new user.", + responses={ + 200: "User account created", + 400: "Invalid username or password", + }, parser=post_userpass_parser) @api.marshal_with(userpass_model) def post(self): @@ -70,4 +78,7 @@ def post(self): # TODO: Support for root key to create new users args = post_userpass_parser.parse_args() _usr, _pass = args['username'], args['password'] - return conn.userpass.register(username=_usr, password=_pass) + status, code = conn.userpass.register(username=_usr, password=_pass) + if code != 200: + api.abort(code, status) + return status diff --git a/Api/resources/secrets/kv_resource.py b/Api/resources/secrets/kv_resource.py index ca1d00e..5bfc9f2 100644 --- a/Api/resources/secrets/kv_resource.py +++ b/Api/resources/secrets/kv_resource.py @@ -35,8 +35,8 @@ @kv_ns.route("//") @api.doc( responses={ - 404: "Path or KV not found", - 403: 'Not Authorized'}, + 401: "Unauthorized", + 404: "Path or KV not found"}, params={ "path": "Path to a KV store", "key": "Key (index) in path where a secret (value) is stored", @@ -53,17 +53,24 @@ def put(self, path, key): args = kv_parser.parse_args() API_KEY = request.headers.get('X-API-KEY', type=str, default=None) abort_if_authorization_fail(API_KEY) - return conn.kv.update(path, key, args['value']) + status, code = conn.kv.update(path, key, args['value']) + if code != 200: + api.abort(code, status) + return None + return status @api.doc( description="Delete a KV from a path", security='Token', - responses={204: "Secrets deleted"}) + responses={200: "Secrets deleted"}) def delete(self, path, key): """Delete a given kv""" - # ! Appropriate HTTP response codes need to be returned API_KEY = request.headers.get('X-API-KEY', type=str, default=None) abort_if_authorization_fail(API_KEY) - return conn.kv.delete(path, key) + status, code = conn.kv.delete(path, key) + if code != 200: + api.abort(code, status) + return None + return status @api.doc( description="Add a KV to a path", security='Token', @@ -71,18 +78,22 @@ def delete(self, path, key): @api.marshal_with(kv_model) def post(self, path, key): """Add a new kv to a path""" - # ! Appropriate HTTP response codes need to be returned args = kv_parser.parse_args() API_KEY = request.headers.get('X-API-KEY', type=str, default=None) abort_if_authorization_fail(API_KEY) - - return conn.kv.add(path, key, args['value']) + status, code = conn.kv.add(path, key, args['value']) + if code != 200: + api.abort(code, status) + return None + return status @api.doc(description="Return a KV from a path", security='Token') @api.marshal_with(kv_model) def get(self, path, key): """Fetch a given KV from a path""" - # ! Appropriate HTTP response codes need to be returned API_KEY = request.headers.get('X-API-KEY', type=str, default=None) abort_if_authorization_fail(API_KEY) - return conn.kv.get(path, key) + status, code = conn.kv.get(path, key) + if code != 200: + api.abort(code, str(status)) + return status diff --git a/Dockerfile b/Dockerfile index 424998b..691396d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ FROM python:3.8-slim-buster LABEL com.ssm.title="Simple Secrets Manager" -LABEL com.ssm.version="1.1.2" +LABEL com.ssm.version="1.2.0" LABEL com.ssm.author.name="Krishnakanth Alagiri" LABEL com.ssm.author.github="https://github.com/bearlike" LABEL com.ssm.repo="https://github.com/bearlike/simple-secrets-manager" diff --git a/Engines/kv.py b/Engines/kv.py index f79f343..6cb84fe 100644 --- a/Engines/kv.py +++ b/Engines/kv.py @@ -17,24 +17,21 @@ def __init__(self, kv_col): def get(self, path, key): finder = self._kv.find_one({"path": path}) if not finder: - result = {"status": "Path not found"} - elif key not in finder["data"].keys(): - result = {"status": f"Key not found in '{path}'"} - else: - result = { - "value": finder["data"][key], - "status": "OK" - } - result.update({ + return "'Path' not found in KV engine", 404 + if key not in finder["data"].keys(): + return f"Key not found in '{path}'", 404 + result = { + "value": finder["data"][key], + "status": "OK", "path": path, - "key": key, - }) - return result + "key": key + } + return result, 200 def add(self, path, key, value): pattern = "[a-zA-Z0-9_]+" if not (re.fullmatch(pattern, key) and re.fullmatch(pattern, path)): - return {"status": f"Key and/or Path does not match {pattern}"} + return f"Key and/or Path does not match {pattern}", 400 finder = self._kv.find_one({"path": path}) if finder is None: # Create a Path where kv(s) goes into @@ -47,44 +44,42 @@ def add(self, path, key, value): # After Creating path, add kv self._kv.update_one( {"path": path}, {"$set": {f"data.{key}": value}}) - result = {"status": "success"} + result = { + "status": "success", + "path": path, + "key": key, + } else: - result = {"status": f"Key already exist in '{path}'"} - result.update({ - "path": path, - "key": key, - }) - return result + return {"status": f"Key already exist in '{path}'"}, 400 + return result, 200 def delete(self, path, key): finder = self._kv.find_one({"path": path}) if not finder: - result = {"status": "Path not found"} - elif key not in finder["data"].keys(): - result = {"status": f"Key not found in '{path}'"} - else: - self._kv.update_one( - {"path": path}, {"$unset": {f"data.{key}": 1}}) - # TODO: Delete document if path has no kv(s) - result = {"status": "OK"} - result.update({ + return "Path not found", 404 + if key not in finder["data"].keys(): + return f"Key not found in '{path}'", 404 + self._kv.update_one( + {"path": path}, {"$unset": {f"data.{key}": 1}}) + # TODO: Delete document if path has no kv(s) + result = { + "status": "OK", "path": path, - "key": key, - }) - return result + "key": key + } + return result, 200 def update(self, path, key, value): finder = self._kv.find_one({"path": path}) if not finder: - result = {"status": "Path not found"} - elif key not in finder["data"].keys(): - result = {"status": f"Key not found in '{path}'"} - else: - self._kv.update_one( - {"path": path}, {"$set": {f"data.{key}": value}}) - result = {"status": "OK"} - result.update({ + return "Path not found", 404 + if key not in finder["data"].keys(): + return f"Key not found in '{path}'", 404 + self._kv.update_one( + {"path": path}, {"$set": {f"data.{key}": value}}) + result = { + "status": "OK", "path": path, - "key": key, - }) - return result + "key": key + } + return result, 200 diff --git a/build.sh b/build.sh index 41059ac..6c6e627 100644 --- a/build.sh +++ b/build.sh @@ -4,7 +4,7 @@ # $ docker login -u # # We try to follow [SemVer v2.0.0](https://semver.org/) -VERSION="1.1.2" +VERSION="1.2.0" # If $VERSION = "1.2.3" # ${VERSION::3} will be "1.2" # ${VERSION::1} will be "1" diff --git a/connection.py b/connection.py index ebe5af1..c066d9c 100644 --- a/connection.py +++ b/connection.py @@ -12,17 +12,25 @@ from Access.userpass import User_Pass as _User_Pass -class Connection: +class __connection: def __init__(self): if os.environ.get("CONNECTION_STRING") is None: logging.error("CONNECTION_STRING variable not found") sys.exit(-1) # Create a connection using MongoClient. - self._client = pymongo.MongoClient(os.environ["CONNECTION_STRING"]) - self._data = self._client["secrets_manager_data"] - self._auth = self._client["secrets_manager_auth"] + self.__client = pymongo.MongoClient(os.environ["CONNECTION_STRING"]) + self.__data = self.__client["secrets_manager_data"] + self.__auth = self.__client["secrets_manager_auth"] # Secret Engines - self.kv = _KV(self._data['kv']) + self.kv = _KV(self.__data['kv']) # Auth Methods - self.tokens = _Tokens(self._auth['tokens']) - self.userpass = _User_Pass(self._auth['userpass']) + self.tokens = _Tokens(self.__auth['tokens']) + self.userpass = _User_Pass(self.__auth['userpass']) + + +class Connection(__connection): + # Singleton class to prevent multiple connections + def __new__(cls): + if not hasattr(cls, 'instance'): + cls.instance = super(Connection, cls).__new__(cls) + return cls.instance diff --git a/docs/README_dockerhub.md b/docs/README_dockerhub.md index df51e37..c8641cf 100644 --- a/docs/README_dockerhub.md +++ b/docs/README_dockerhub.md @@ -13,7 +13,8 @@ Secure storage, and delivery for tokens, passwords, API keys, and other secrets ## Supported tags and respective [Dockerfile](https://github.com/bearlike/simple-secrets-manager/blob/main/Dockerfile) links -- [`1.1.2`, `1.1`, `1`, `latest`](https://github.com/bearlike/simple-secrets-manager/blob/releases/v1.1.2/Dockerfile) +- [`1.2.0`, `1.2`, `1`, `latest`](https://github.com/bearlike/simple-secrets-manager/blob/releases/v1.2.0/Dockerfile) +- [`1.1.2`, `1.1`](https://github.com/bearlike/simple-secrets-manager/blob/releases/v1.1.2/Dockerfile) - [`1.1.1`](https://github.com/bearlike/simple-secrets-manager/blob/releases/v1.1.1/Dockerfile) - [`1.1.0`](https://github.com/bearlike/simple-secrets-manager/blob/releases/v1.1.0/Dockerfile) - [`1.0.0`, `1.0`](https://github.com/bearlike/simple-secrets-manager/blob/releases/v1.0.0/Dockerfile)