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

Allows configuration of default form fields and values #243

Open
wants to merge 14 commits into
base: development
Choose a base branch
from

Conversation

helrond
Copy link
Member

@helrond helrond commented Sep 9, 2022

Adds models and UI (via Django Admin) for customization of default form fields and values for Aeon requests.

Since some of these values are derived from data passed to the Request Broker, I've added a flag to the config to indicate when the value provided is actually a key for the request data dictionary.

Would welcome feedback on this approach/improvements to documentation.

Fixes #240

Copy link
Contributor

@ctgraham ctgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems straightforward to me with a few questions:

  • How will we recommend folks create the initial superuser account for Django? I am assuming that asking them to shell in to the docker container to run manage.py createsuperuser might be too technical an ask. Adding some config variables for DJANGO_SUPERUSER_PASSWORD, DJANGO_SUPERUSER_USERNAME, and DJANGO_SUPERUSER_EMAIL could be simple enough, but then if we run createsuperuser on their behalf in the entrypoint, it will require checking to see if the user has already been created.
  • The default Django UI affords the ability to create permissioned groups, but not users. If we start using the UI for tasks like this, should we enable user creation as well? (or am I missing it?)
  • Trivial, but the default Django UI also exposes a "Home" link, which does not have a router in request_broker currently.

@helrond
Copy link
Member Author

helrond commented Sep 26, 2022

Thanks @ctgraham - good point on creating a base superuser. I've added configs and a short script to do this (create_users.py), and improved the documentation in the README.

I've also added Users to the Django admin and remove the "return to site" link, since there's really no UI for this application other than Django admin.

Copy link
Contributor

@ctgraham ctgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To pedantic observations:

The creation of a user via the admin UI offers no way to set the new user's password. The password field as exposed is the password hash representation.

If someone builds without first configuring config.py, and the default gets copied, this will create an insecure user. They may then perform the config.py customization which will create a new superuser per their specifications. We might want to leave the DJANGO_SUPERUSER_USERNAME blank in the example file to prevent that possible unnoticed creation of an insecure user.

Copy link
Contributor

@ctgraham ctgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also need a:
python manage.py collectstatic
in entrypoint.sh, true?

@helrond
Copy link
Member Author

helrond commented Sep 30, 2022

Pardon my excessive pushes while I tried to get my rusty bash/Docker environment variable skills to play nice.

A couple of updates

  • Replaced the existing ModelAdmin for users with a UserAdmin, which provides appropriate UI for managing users in Django Admin
  • Tweaked the example config to remove the username and the create_users.py scripts so the container will blow up on startup if an attempt is made to create the default superuser without configuring values first.
  • In terms of collectstatic - since we're running the Django dev server in this Dockerfile, we actually don't need to run that command in order for things to work properly. https://docs.djangoproject.com/en/4.1/ref/contrib/staticfiles/#static-file-development-view

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

Successfully merging this pull request may close these issues.

2 participants