resources_api icon indicating copy to clipboard operation
resources_api copied to clipboard

Feature/implement flask admin

Open fumblehool opened this issue 5 years ago • 12 comments

fixes #134

fumblehool avatar Oct 19 '20 18:10 fumblehool

@aaron-suarez this PR contains the admin routes and the access control but some UI related things like - login page UI, logout page routing is remaining.

fumblehool avatar Oct 19 '20 18:10 fumblehool

@fumblehool Thanks so much for your contribution! I haven't pulled it down to run it just yet, but I'd like to give it a really thorough review because it changes the models.

I think that this is also going to require a migration file to update the database, right? Can we get that added and the conflicts resolved and then I'll give it a really good review? Thanks!

aaron-junot avatar Oct 19 '20 23:10 aaron-junot

@aaron-suarez I've pushed the migrations + fixed the lint issues. Please have a look.

fumblehool avatar Oct 20 '20 05:10 fumblehool

@aaron-suarez have you tried it locally?

fumblehool avatar Oct 27 '20 04:10 fumblehool

So locally, I got it running in the flask shell, but I'm a bit unclear as to the interface for creating a new user and assigning roles. Would you mind commenting with an example of how to add an admin user so I can do that locally and ensure that everything works as expected?

I'm sure it's in the Flask-Login docs somewhere, feel free to just link to the relevant page in the docs if that's easier.

Thanks again for this contribution, it's looking good so far, just gotta go through the flow to make sure everything works 😄

aaron-junot avatar Oct 27 '20 23:10 aaron-junot

Also, please rebase to resolve conflicts

aaron-junot avatar Oct 28 '20 02:10 aaron-junot

I ran it and navigated to /login and I got this error

172.26.0.1 - - [29/Oct/2020 19:03:30] "GET /login HTTP/1.1" 500 -
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 2464, in __call__
    return self.wsgi_app(environ, start_response)
  File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 2450, in wsgi_app
    response = self.handle_exception(e)
  File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 1867, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/usr/local/lib/python3.7/site-packages/flask/_compat.py", line 39, in reraise
    raise value
  File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 2447, in wsgi_app
    response = self.full_dispatch_request()
  File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 1945, in full_dispatch_request
    self.try_trigger_before_first_request_functions()
  File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 1993, in try_trigger_before_first_request_functions
    func()
  File "/src/run.py", line 47, in before_first_request
    user_datastore.create_user(admin_email, password=encrypted_password)
TypeError: create_user() takes 1 positional argument but 2 were given

Let me know when you're ready for another review (because I assume you're still trying to work this out). If you need help, feel free to hit me up on slack.

aaron-junot avatar Oct 29 '20 19:10 aaron-junot

@aaron-suarez I've fixed the issues -

  • I missed the positional params for user_creation
  • Fixed a bug in password hashing, on every run a different secret_key and `hash was used.

fumblehool avatar Oct 30 '20 18:10 fumblehool

In order for the tests to pass, we'll probably need a default 'SECRET_KEY' and 'SECURITY_PASSWORD_SALT'

aaron-junot avatar Oct 30 '20 22:10 aaron-junot

Other than that, it looks like it works at a first pass. I'll do some more extensive testing later, but I really appreciate the effort here!

aaron-junot avatar Oct 30 '20 22:10 aaron-junot

@aaron-suarez I've added default values for secret_key and security_hash. It should work now. Once the tests are updated, we can change/remove the default values for security reasons.

fumblehool avatar Oct 31 '20 12:10 fumblehool

@aaron-suarez I've resolved the comment. Please review, Thanks :smile:

fumblehool avatar Nov 07 '20 09:11 fumblehool