#214 Add config.yml and sets admin permissions
Merged by jflory7. Opened by shraddhaag.
fedora-commops/ shraddhaag/fedora-happiness-packets django-admin  into  master

Download 214.patch

This commit adds the following:

  1. Adds update_user method in auth.py to set admin permissions for
    new accounts and pre-existing accounts of admin users. It also
    clears the permissions when a user is removed from the admin list.

  2. A config.yml is added to get the admin list in auth.py and ADMINS
    in settings/base.py.

The ADMINS can be verified by checking its value in the Django Debug
Toolbar's Settings panel.

Metadata Update from @jflory7:
- Pull-request tagged with: improvement, needs testing, type - backend, type - summer coding

rebased onto 95d6a87ff41b6cb4db029c1532b13896e19eb0c3

Metadata Update from @jflory7:
- Request assigned

Metadata Update from @jflory7:
- Pull-request untagged with: needs testing

Thanks @shraddhaag, nice work :thumbsup: I tested this and everything worked as expected. :tada: I had three comments when I reviewed:

1. Navigating to admin portal URL when unauthenticated shows Django authentication backend

In happinesspackets/messaging/views.py, some classes set the login_url as follows:

login_url = '/oidc/authenticate/'

The admin portal authentication does not happen in views.py. I assume it happens somewhere else, possibly happinesspackets/messaging/admin.py. I'm not familiar enough with Django to be certain. cc: @jonatoni @anxh3l0 @bt0dotninja

For now, if there is an easy fix to add an OIDC auth to the admin portal, that can go into this PR. Either way, we should also file a new issue to refactor authentication code into a single module to reuse across FHP.

2. Change config.yml to config.yml.example, update .gitignore

This way, we can ship an example config file without conflicting with an actual production config file. The .gitignore file should ignore the path to the config file. Even though this is minor, it makes FHP easier to deploy later.

3. Add documentation comments to config file

I suggest adding a few comment lines in the example config file to briefly explain what they mean, what data input is expected, and how they should be formatted.

Otherwise, this PR looks great. :smile:

Metadata Update from @jflory7:
- Pull-request tagged with: needs changes

1 new commit added

  • new

rebased onto 06f3dbb8a9ee40898232eb91c691401c1199261f

rebased onto d1691745df6282e5717a85b94ee5fa9241431c8c

rebased onto 4ef5ef7b9cff8b59921bdd751a4483768ce94fa6

rebased onto 18a52b9a220cea35da63cde5bffb5b05d5cd6939

Hi @jflory7! I have incorporated all the changes as suggested.

  1. Added login to Django Admin SIte using OIDC.
  2. Renamed config.yml and added changes to .gitignore.
  3. Added comments to config.yml and also added documentation in docs/setup/development.rst.

The admin portal authentication does not happen in views.py. I assume it happens somewhere else, possibly happinesspackets/messaging/admin.py. I'm not familiar enough with Django to be certain. cc: @jonatoni @anxh3l0 @bt0dotninja
For now, if there is an easy fix to add an OIDC auth to the admin portal, that can go into this PR. Either way, we should also file a new issue to refactor authentication code into a single module to reuse across FHP.

I have incorporated login to Admin site using OIDC. What's remaining is prompting for credentials using OIDC auth again if a non admin logged in user tries to access the admin site. This is due to the fact that when users are logged out they are not logged out of the OpenID Connect provider. Hence when trying to login again, users are not prompted for credentials. This is detailed here and needs to be handled in a custom logout function as detailed in the link. I'll add this in the issue with refactoring authentication code.

Please let me know if I need to make anymore changes here! :)

Good idea pulling this out as a variable. :thumbsup:

I tested this and everything works as expected. Thanks @shraddhaag, nice work! Merging. :clapper:

Metadata Update from @jflory7:
- Pull-request untagged with: needs changes

Pull-Request has been merged by jflory7