Admin: Add Google SSO for Compiler users #1855

machikoyasuda · 2024-01-17T00:56:39Z

After merging this PR and ensuring the proper secrets are set up in Terraform, anyone with a compiler.la email address will be able to log into the Benefits Admin application. When the user is authenticated, the application will create a user with their Google first name, last name and e-mail address in Benefits.

What this PR does

Uses https://megalus.github.io/django-google-sso/ library for Google single-sign on. Also uses https://github.com/encode/httpx/ for simple HTTP requests, to fetch Google API data.
Set up Google SSO log in to allow for compiler.la domains, if the users email address is in the correct secret (see below)
Set up Google SSO secrets with Terraform
Fixes issue where Django JS files were not being allowed on Admin site
Set user's username as their Google email address, and also save their profile icon URL, first and last name from Google.
Outside of this PR, the necessary secrets have been set up. These secrets allow us to control exactly which compiler.la addresses can log in and get staff or superuser status:

GOOGLE_SSO_STAFF_LIST - [email protected]
GOOGLE_SSO_SUPERUSER_LIST - [email protected],[email protected],[email protected]

Done

About `django-google-sso`

Supports Django 5.0 https://github.com/megalus/django-google-sso/releases
Supports Python 3.11 https://megalus.github.io/django-google-sso/#install
Has a sibling Microsoft and GitHub library: https://megalus.github.io/django-google-sso/multiple/

How to test

Engineers: Try to log in via your Compiler email address. Should be able to log in successfully.
Engineers: Try to log in via a personal Gmail address. Should not be able to log in.

benefits/settings.py

machikoyasuda · 2024-01-17T01:20:17Z

To test this locally so far:

I used Google SSO test credentials from a personal app I created long, long ago. I can DM you the credentials if you want. And I replaced those lines here https://github.com/cal-itp/benefits/pull/1855/files#diff-d71405aa44f1e899a9cd920e5187aa1d02affdbd0da472eca95f814b0a8c1210R40-R42

    # GOOGLE_SSO_CLIENT_ID = "XXXX"
    # GOOGLE_SSO_PROJECT_ID = "XXXX"
    # GOOGLE_SSO_CLIENT_SECRET = "XXXX"

Recreate devcontainer. Make sure it downloaded the Django-Google-SSO library. Running a ./bin/init.sh runs migrate - and that should work successfully.
Go to localhost:8000/admin
Try logging in with a Compiler and a non-Compiler Google account. Should be able to log in and not log in, respectively.

machikoyasuda · 2024-01-17T01:21:00Z

Not sure why GitHub Actions pytest/Cypress are failing here: https://github.com/cal-itp/benefits/actions/runs/7549568812/job/20553782925?pr=1855#step:7:22

I am able to run migrate locally and run all the tests locally, too.

cc @angela-tran @thekaveman

angela-tran · 2024-01-17T15:51:36Z

Not sure why GitHub Actions pytest/Cypress are failing here: https://github.com/cal-itp/benefits/actions/runs/7549568812/job/20553782925?pr=1855#step:7:22

I am able to run migrate locally and run all the tests locally, too.

cc @angela-tran @thekaveman

It's failing because when the tests run, DJANGO_ADMIN is false, so django.contrib.auth is not installed. This is the problem I was running into while trying to write tests for #1851.

From the workflow logs, it looks like django-google-sso depends on django.contrib.auth and tries to call get_user_model.

  File "/opt/hostedtoolcache/Python/3.11.7/x64/lib/python3.11/site-packages/django_google_sso/models.py", line 6, in <module>
    User = get_user_model()
           ^^^^^^^^^^^^^^^^
  File "/opt/hostedtoolcache/Python/3.11.7/x64/lib/python3.11/site-packages/django/contrib/auth/__init__.py", line 194, in get_user_model
    raise ImproperlyConfigured(
django.core.exceptions.ImproperlyConfigured: AUTH_USER_MODEL refers to model 'auth.User' that has not been installed

angela-tran · 2024-01-17T15:54:15Z

@machikoyasuda I think if you were to run locally with DJANGO_ADMIN set to false, you would see the same issue when running migrations.

This can probably be resolved by moving line 32 to go after line 49.

github-actions · 2024-01-17T20:07:38Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
benefits
settings.py					40-49
urls.py					42
benefits/core
admin.py					5-7, 26-44
Project Total

_{This report was generated by python-coverage-comment-action}

machikoyasuda · 2024-01-17T23:18:53Z

@thekaveman @angela-tran Google SSO credential information for Localhost, Dev, Test and Prod are now saved in LastPass, Shared-Compiler Engineering.

machikoyasuda · 2024-01-18T22:35:50Z

Terraform updated for 4 Google SSO env vars for Dev, Test and Prod.

thekaveman · 2024-01-30T17:51:57Z

@machikoyasuda I'm taking a look at this now. One quick question, you mentioned:

Also uses https://github.com/encode/httpx/ for simple HTTP requests

Is it possible to use requests here, which was already listed as a requirement? Rather than adding a new package that essentially does the same thing?

thekaveman

This is looking fantastic so far!! 👏 👏

Steps I took for local setup

Created the following environment variables in my local `.env` file:

DJANGO_ADMIN=true
DJANGO_SUPERUSER_EMAIL=[email protected]
DJANGO_SUPERUSER_PASSWORD=password1234!
DJANGO_SUPERUSER_USERNAME=benefits-admin

DJANGO_LOCAL_PORT=11369

GOOGLE_SSO_CLIENT_ID=<value from https://console.cloud.google.com/apis/credentials/oauthclient/415252694921-b03mul9832970e54gbkj4ae8l4f6fu4q.apps.googleusercontent.com?project=benefits-admin-411518>
GOOGLE_SSO_PROJECT_ID=benefits-admin-411518
GOOGLE_SSO_CLIENT_SECRET=<https://console.cloud.google.com/apis/credentials/oauthclient/415252694921-b03mul9832970e54gbkj4ae8l4f6fu4q.apps.googleusercontent.com?project=benefits-admin-411518>
GOOGLE_SSO_ALLOWABLE_DOMAINS=compiler.la
GOOGLE_SSO_STAFF_LIST=[email protected]
GOOGLE_SSO_SUPERUSER_LIST=[email protected]

Rebuild and Reopen devcontainer

Confirm in log output that Admin is enabled and setup:

Operations to perform:
  Apply all migrations: admin, auth, contenttypes, core, django_google_sso, sessions
Running migrations:
  Applying contenttypes.0001_initial... OK
  Applying auth.0001_initial... OK
  Applying admin.0001_initial... OK
  Applying admin.0002_logentry_remove_auto_add... OK
  Applying admin.0003_logentry_add_action_flag_choices... OK
...

Login with the standard superuser

☑️ Using the username/password as shown above

Login with my personal compiler.la Google account

☑️ A user was created for my Google account
☑️ That user is marked as a superuser

Login with the [email protected] Google account

☑️ A user was created for this Google account
☑️ That user is marked as a normal staff user

Attempt login with a non-compiler.la Google account

☑️ Access is blocked due to non-matching domain

A couple of questions

Asked in another comment: do we need to use httpx here when we already have requests?
Do we need the user's Google profile pic? I don't see that showing up anywhere in the Admin interface. Should I?
Slightly off-topic for this PR, but related: I think we can remove 127.0.0.1 from the default ALLOWED_HOSTS list -- since the Google integration is setup for localhost redirects only, we can simplify that setting a bit.
~~Can we get some quick unit tests written for benefits/admin.py, the pre_login_user function?~~ Skip these for now, it's complicated to test models/database type stuff.

machikoyasuda · 2024-01-30T20:19:13Z

@thekaveman Re: The icon - shows up here: http://localhost:11369/admin/django_google_sso/googlessouser/1/change/

and the URL is used here: http://localhost:11369/admin/auth/user/1/change/

The GoogleSSOUser model comes with the picture_url attribute baked in (https://megalus.github.io/django-google-sso/model/#the-googlessouser-model), so I recommend just keeping it - even if we don't use it in the admin app.

benefits/admin.py

thekaveman · 2024-02-01T21:08:21Z

@machikoyasuda any updates on adding some basic unit tests here? Any blockers / questions?

tests/pytest/conftest.py

tests/pytest/test_admin.py

angela-tran

I was able to test out setting all the environment variables up and logging in with my Compiler Google account today. I got through successfully too 👍

benefits/admin.py

thekaveman · 2024-02-06T19:17:00Z

I'm looking at this again now 👀

EDIT (12pm Pacific): OK I am actually looking at this RIGHT NOW 😅

machikoyasuda · 2024-02-06T20:24:38Z

Terraform dev has been updated to include:

GOOGLE_SSO_STAFF_LIST - x
GOOGLE_SSO_SUPERUSER_LIST - x,x,x

Thank you @angela-tran

thekaveman

I'm a little confused as to if this is totally done?

It seems like ADMIN checks were removed but the setting is still around.
Locally I'm seeing this test failing, should probably be deleted if we aren't using this setting anymore.
The code in benefits/admin.py is now duplicated in benefits/core/admin.py.
There is still code that checks for the DJANGO_ADMIN environment variable, e.g. in bin/init.sh -- if we are now saying in this PR that admin is always on, that code should probably be cleaned up as well.

This PR doesn't feel quite ready / cleaned up.

benefits/settings.py

benefits/admin.py

Duplicate code needs to be removed, and need to agree on what to do about unit test

machikoyasuda · 2024-02-07T04:13:17Z

@thekaveman @angela-tran Sorry for the delay --- I removed the test, un-did the settings.py changes, removed extra admin file, updated the function call setting ( "benefits.core.admin.pre_login_user" - has to add core in there ). And re-tested it all locally.

angela-tran

This looks good to me

thekaveman

Yep, me too! I went through all 4 scenarios again:

I can login with the DJANGO_SUPERUSER_USERNAME and password
I can login with my own compiler.la email configured in the superuser list
I can login with a different compiler.la email configured in the user list
I cannot login with a personal Gmail email not in the compiler.la domain

quick follow-up to #1855 that was missed in review

machikoyasuda commented Jan 17, 2024

View reviewed changes

benefits/settings.py Outdated Show resolved Hide resolved

machikoyasuda changed the title ~~Feat/1817 admin google sso~~ Admin: Add Google SSO Jan 17, 2024

machikoyasuda changed the title ~~Admin: Add Google SSO~~ Admin: Add Google SSO for Compiler users Jan 17, 2024

machikoyasuda commented Jan 17, 2024

View reviewed changes

benefits/settings.py Outdated Show resolved Hide resolved

machikoyasuda self-assigned this Jan 17, 2024

machikoyasuda added this to the Admin tool: foundation milestone Jan 17, 2024

machikoyasuda marked this pull request as ready for review January 29, 2024 20:42

machikoyasuda requested a review from a team as a code owner January 29, 2024 20:42

thekaveman reviewed Jan 30, 2024

View reviewed changes

machikoyasuda commented Jan 30, 2024

View reviewed changes

benefits/admin.py Outdated Show resolved Hide resolved

machikoyasuda added 5 commits January 30, 2024 14:45

feat(admin): first pass @ django_google_sso; allow compiler.la domains

1ce210d

fix(settings): allow wikimedia link

60d3d95

fix(pyproject): require django-google-sso

33edf7d

fix(csp): add admin.js files, add google sso user icons to allowlist

e1a61c9

fix(tests): only install django-google-sso, only add sso url if admin

f603890

feat(test): test in progress

660f269

machikoyasuda commented Feb 2, 2024

View reviewed changes

tests/pytest/conftest.py Outdated Show resolved Hide resolved

machikoyasuda commented Feb 2, 2024

View reviewed changes

tests/pytest/test_admin.py Outdated Show resolved Hide resolved

angela-tran reviewed Feb 2, 2024

View reviewed changes

machikoyasuda added 2 commits February 6, 2024 01:53

fix(tests): use mocker; fixed tests yay

0d5984e

chore: undo test changes

2e28ec7

machikoyasuda commented Feb 6, 2024

View reviewed changes

benefits/admin.py Outdated Show resolved Hide resolved

machikoyasuda added 2 commits February 6, 2024 18:19

refactor(admin): move code to Admin.py

a0c4b2e

fix(settings): remove if ADMIN checks

4f98e63

machikoyasuda requested review from thekaveman and angela-tran February 6, 2024 18:44

angela-tran previously approved these changes Feb 6, 2024

View reviewed changes

machikoyasuda mentioned this pull request Feb 6, 2024

Initialize Django with admin, superuser, and existing config data #1856

Closed

22 tasks

thekaveman requested changes Feb 6, 2024

View reviewed changes

benefits/settings.py Show resolved Hide resolved

benefits/admin.py Outdated Show resolved Hide resolved

machikoyasuda added 4 commits February 7, 2024 03:38

fix(test): fix test

0c28146

fix: rename to core, remove unused file

5c00e19

fix(tests): remove tests, un-remove admin check

49c78a3

fix: undo admin fixture

147cd51

machikoyasuda mentioned this pull request Feb 7, 2024

Write product spec for Admin interface MVP (Use by Internal Eng) #1313

Open

angela-tran approved these changes Feb 7, 2024

View reviewed changes

thekaveman approved these changes Feb 7, 2024

View reviewed changes

machikoyasuda merged commit 8faf1a7 into dev Feb 8, 2024
13 checks passed

machikoyasuda deleted the feat/1817-admin-google-sso branch February 8, 2024 00:18

thekaveman added a commit that referenced this pull request Feb 23, 2024

fix(terraform): correct Google SSO secret names

e07665e

quick follow-up to #1855 that was missed in review

thekaveman mentioned this pull request Feb 23, 2024

Fix: correct Google SSO secret names #1904

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Admin: Add Google SSO for Compiler users #1855

Admin: Add Google SSO for Compiler users #1855

machikoyasuda commented Jan 17, 2024 •

edited

Loading

machikoyasuda commented Jan 17, 2024

machikoyasuda commented Jan 17, 2024

angela-tran commented Jan 17, 2024 •

edited

Loading

angela-tran commented Jan 17, 2024

github-actions bot commented Jan 17, 2024 •

edited

Loading

machikoyasuda commented Jan 17, 2024

machikoyasuda commented Jan 18, 2024

thekaveman commented Jan 30, 2024

thekaveman left a comment •

edited

Loading

machikoyasuda commented Jan 30, 2024 •

edited

Loading

thekaveman commented Feb 1, 2024 •

edited

Loading

angela-tran left a comment

thekaveman commented Feb 6, 2024 •

edited

Loading

machikoyasuda commented Feb 6, 2024

thekaveman left a comment •

edited

Loading

machikoyasuda commented Feb 7, 2024

angela-tran left a comment

thekaveman left a comment

Admin: Add Google SSO for Compiler users #1855

Admin: Add Google SSO for Compiler users #1855

Conversation

machikoyasuda commented Jan 17, 2024 • edited Loading

What this PR does

Done

About django-google-sso

How to test

machikoyasuda commented Jan 17, 2024

machikoyasuda commented Jan 17, 2024

angela-tran commented Jan 17, 2024 • edited Loading

angela-tran commented Jan 17, 2024

github-actions bot commented Jan 17, 2024 • edited Loading

Coverage report

machikoyasuda commented Jan 17, 2024

machikoyasuda commented Jan 18, 2024

thekaveman commented Jan 30, 2024

thekaveman left a comment • edited Loading

Choose a reason for hiding this comment

Steps I took for local setup

Created the following environment variables in my local .env file:

Rebuild and Reopen devcontainer

Login with the standard superuser

Login with my personal compiler.la Google account

Login with the [email protected] Google account

Attempt login with a non-compiler.la Google account

A couple of questions

machikoyasuda commented Jan 30, 2024 • edited Loading

thekaveman commented Feb 1, 2024 • edited Loading

angela-tran left a comment

Choose a reason for hiding this comment

thekaveman commented Feb 6, 2024 • edited Loading

machikoyasuda commented Feb 6, 2024

thekaveman left a comment • edited Loading

Choose a reason for hiding this comment

machikoyasuda commented Feb 7, 2024

angela-tran left a comment

Choose a reason for hiding this comment

thekaveman left a comment

Choose a reason for hiding this comment

machikoyasuda commented Jan 17, 2024 •

edited

Loading

About `django-google-sso`

angela-tran commented Jan 17, 2024 •

edited

Loading

github-actions bot commented Jan 17, 2024 •

edited

Loading

thekaveman left a comment •

edited

Loading

Created the following environment variables in my local `.env` file:

machikoyasuda commented Jan 30, 2024 •

edited

Loading

thekaveman commented Feb 1, 2024 •

edited

Loading

thekaveman commented Feb 6, 2024 •

edited

Loading

thekaveman left a comment •

edited

Loading