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

file modified
+3

@@ -27,3 +27,6 @@ 

  

  #Adding fas-admin-details.json so that the username and password does not get pushed

  fas-admin-details.json

+ 

+ # YAML Configuration file for django 

+ config.yml 

\ No newline at end of file

file added
+13

@@ -0,0 +1,13 @@ 

+ base:

+   admins: 

+   # following will populate ADMIN variable in settings/base.py 

+     - !!python/tuple ['Anna Philips', 'algogator@fedoraproject.org']

+     - !!python/tuple ['Jona Azizaj', 'jonatoni@fedoraproject.org']

+     - !!python/tuple ['Bhagyashree Uday', 'bee2502@fedoraproject.org']

+ auth:

+   admins:

+   # following will be given superuser privileges

+     - jflory7

+     - jonatoni

+     - bt0dotninja

+     - anxh3l0

@@ -34,6 +34,8 @@ 

  

  #. Create a fas-admin-details.json file and add a json object with your FAS-Username and FAS-Password. See fas-admin-details.json.example.

  

+ #. Create a config.yml file and populate it with the user's details and usernames for `ADMINS <https://docs.djangoproject.com/en/2.1/ref/settings/#admins>`_ and superuser privileges respectively.

+ 

  Although the Dockerfile runs the script to check if a client_secrets.json file is present, please generate it before starting the Docker container, so that client secrets are not being constantly generated every time the image is rebuilt.

  

  In order to run the web server, alongside the Redis queue and celery worker instance, simply run ``docker-compose up``.

@@ -1,6 +1,23 @@ 

  from mozilla_django_oidc.auth import OIDCAuthenticationBackend

  

+ import yaml

+ 

+ with open("config.yml", 'r') as ymlfile:

+     cfg = yaml.full_load(ymlfile)

+ 

  class OIDC(OIDCAuthenticationBackend):

+     def update_user(self, user, claims):

+         if user.username in cfg['auth']['admins']:

+             if not user.is_superuser: 

+                 user.is_superuser = True

+                 user.is_staff = True

+         else:

+             if user.is_superuser:

+                 user.is_staff = False

+                 user.is_superuser = False

+         user.save()

+         return user

+     

      def create_user(self, claims):

          user = super(OIDC, self).create_user(claims)

          user.username = claims.get('nickname', '')

@@ -10,4 +27,5 @@ 

          except:

              user.first_name = user.username

          user.save()

-         return user

+         return self.update_user(user,claims)

+          

\ No newline at end of file

@@ -98,7 +98,6 @@ 

          return HttpResponseRedirect(self.success_url)

  

  class MessageSendView(LoginRequiredMixin, FormView):

-     login_url = '/oidc/authenticate/'

      template_name = 'messaging/message_send_form.html'

      form_class = MessageSendForm

  

@@ -200,7 +199,6 @@ 

  

  

  class UserMessageView(LoginRequiredMixin, ListView):

-     login_url = '/oidc/authenticate/'

      model = Message

      paginate_by = 5

  

@@ -4,6 +4,11 @@ 

  from django.core.exceptions import ImproperlyConfigured

  from unipath import Path

  

+ import yaml

+ 

+ with open("config.yml", 'r') as ymlfile:

+     cfg = yaml.full_load(ymlfile)

+ 

  PROJECT_DIR = Path(__file__).ancestor(3)

  

  # For clean_pyc to work without complaining

@@ -11,11 +16,7 @@ 

  

  DEBUG = False

  

- ADMINS = (

-     ('Anna Philips', 'algogator@fedoraproject.org'),

-     ('Jona Azizaj', 'jonatoni@fedoraproject.org'),

-     ('Bhagyashree Uday', 'bee2502@fedoraproject.org'),

- )

+ ADMINS = cfg['base']['admins']

  SERVER_EMAIL = ADMINS[0][1]

  

  DEFAULT_FROM_EMAIL = "Happiness Packets <fedora.happinesspackets@gmail.com>"

@@ -140,6 +141,7 @@ 

  LOGIN_REDIRECT_URL = '/'

  LOGOUT_REDIRECT_URL = '/'

  LOGIN_REDIRECT_URL_FAILURE = '/error'

+ LOGIN_URL = '/oidc/authenticate/'

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

  OIDC_RP_SCOPES = 'openid profile email'

  

  LOGGING = {

@@ -216,4 +218,4 @@ 

          return os.environ[var_name]

      except KeyError:

          error_msg = "Set the %s environment variable" % var_name

-         raise ImproperlyConfigured(error_msg)

+         raise ImproperlyConfigured(error_msg) 

\ No newline at end of file

file modified
+6 -3

@@ -5,6 +5,7 @@ 

  from django.urls import include, re_path, path

  from django.conf.urls.static import static

  from django.contrib import admin

+ from django.views.generic.base import RedirectView

  

  urlpatterns = [

      re_path(r'^oidc/', include('mozilla_django_oidc.urls')),

@@ -12,11 +13,13 @@ 

  ]

  

  if settings.ADMIN_ENABLED or settings.DEBUG:

-     urlpatterns.append(re_path(r'^drunken-octo-lama/', admin.site.urls))

+     urlpatterns += [

+         re_path(r'^drunken-octo-lama/login/', RedirectView.as_view(url=settings.LOGIN_URL, permanent=True, query_string=True)),

+         re_path(r'^drunken-octo-lama/', admin.site.urls)

+     ]

  

  if settings.DEBUG:

      import debug_toolbar

      urlpatterns += [

          path('__debug__/', include(debug_toolbar.urls))

-         ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)

- 

+     ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) 

\ No newline at end of file

file modified
+3

@@ -44,3 +44,6 @@ 

  

  #python-fedora for f-a-s API

  python-fedora==0.10.0

+ 

+ # Dependency for YAML file

+ pyyaml==5.1

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

8 months ago

rebased onto 95d6a87ff41b6cb4db029c1532b13896e19eb0c3

8 months ago

Metadata Update from @jflory7:
- Request assigned

8 months ago

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

8 months ago

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

8 months ago

1 new commit added

  • new
8 months ago

rebased onto 06f3dbb8a9ee40898232eb91c691401c1199261f

8 months ago

rebased onto d1691745df6282e5717a85b94ee5fa9241431c8c

8 months ago

rebased onto 4ef5ef7b9cff8b59921bdd751a4483768ce94fa6

8 months ago

rebased onto 18a52b9

8 months ago

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

8 months ago

Pull-Request has been merged by jflory7

8 months ago