Adds flask admin as a feature. Bumps flask admin to 1.6.0 #551
Conversation
oafernandes
force-pushed
the
addsFlaskAdmin
branch
from
9231c04
to
98c2b7a
Compare
| SECRET_KEY=sammy | ||
| SECURITY_PASSWORD_SALT=saltedpop | ||
| ADMIN_EMAIL=test@me.com | ||
| ADMIN_PASSWORD=1234 |
There was a problem hiding this comment.
Can we please add a newline to the end of this file?
| @@ -19,6 +19,8 @@ RUN apt-get update \ | |||
| && pip install poetry \ | |||
| && poetry config virtualenvs.create false | |||
|
|
|||
| RUN poetry lock | |||
There was a problem hiding this comment.
I manually added the dependency versions to the pyproject.toml file. Below link to solution for resolving the "....which doesn't match any versions, version solving failed." error
[Known issue]python-poetry/poetry#1281 (comment)
| @@ -37,6 +37,13 @@ def get_sys_exec_root_or_drive(): | |||
| if not all([algolia_app_id, algolia_api_key]): | |||
| print("Application requires 'ALGOLIA_APP_ID' and 'ALGOLIA_API_KEY' for search") | |||
|
|
|||
| secret_key = os.environ.get('SECRET_KEY', None) | |||
| security_password_hash = 'pbkdf2_sha512' | |||
There was a problem hiding this comment.
Can we make this an environment variable?
There was a problem hiding this comment.
SECURITY_PASSWORD_HASH added to .env
| @@ -37,6 +37,13 @@ def get_sys_exec_root_or_drive(): | |||
| if not all([algolia_app_id, algolia_api_key]): | |||
| print("Application requires 'ALGOLIA_APP_ID' and 'ALGOLIA_API_KEY' for search") | |||
|
|
|||
| secret_key = os.environ.get('SECRET_KEY', None) | |||
There was a problem hiding this comment.
| secret_key = os.environ.get('SECRET_KEY', None) | |
| secret_key = os.environ['SECRET_KEY'] |
There was a problem hiding this comment.
Uses bracket notation instead of .get()
| if not all([secret_key, security_password_salt]): | ||
| print('Application requires "SECRET_KEY" and "SECURITY_HASH"') |
There was a problem hiding this comment.
We can remove this if we use the bracket syntax instead of .get()
There was a problem hiding this comment.
Uses bracket notation instead of .get()
| # @event.listens_for(User.password, 'set', retval=True) | ||
| # def hash_user_password(target, value, oldvalue, initiator): | ||
| # """Encrypts password when new admin created in User View""" | ||
| # if value != oldvalue: | ||
| # return utils.encrypt_password(value) | ||
| # return value |
There was a problem hiding this comment.
This listens for when a new admin password is added in the admin view and encrypts with same method as before_first_request(). Removed.
|
|
||
|
|
||
| @app.before_first_request | ||
| def before_first_request(): |
There was a problem hiding this comment.
Please rename this so we don't accidentally shadow or get confused. Choose a name that is descriptive of what the function is doing
There was a problem hiding this comment.
changed to add_admin_role
| @app.before_first_request | ||
| def before_first_request(): | ||
| """ Adds admin/user roles and default admin account and password if none exists""" | ||
| db.create_all() |
There was a problem hiding this comment.
The only way to check would be to delete the table from the database I think? May need help with this.
| admin_email = os.environ.get('ADMIN_EMAIL', "admin@example.com") | ||
| admin_password = os.environ.get('ADMIN_PASSWORD', 'password') |
There was a problem hiding this comment.
We want this to blow up. Please use bracket syntax here
There was a problem hiding this comment.
Changed to bracket from .get()
| @app.before_first_request | ||
| def before_first_request(): |
There was a problem hiding this comment.
If the admin user already exists when this function runs, what happens? Are there side effects?
-Adds flask admin feature
-Bumps flask admin to 1.6.0 and all dependencies
-Issue with password hashing when trying to add a new admin from admin view, password is not hashed and stored as plaintext.