mdn / kuma

The project that powers MDN.
https://developer.mozilla.org
Mozilla Public License 2.0
1.93k stars 679 forks source link

Auth with mozilla-django-oidc #7966

Closed peterbe closed 3 years ago

peterbe commented 3 years ago

This adds full authentication flow with OIDC. In particular, Firefox Accounts OIDC through the mozilla_django_oidc library.

To use this you need to set up the following:

OIDC_RP_CLIENT_ID=ed...
OIDC_RP_CLIENT_SECRET=c49....

...in your .env file.

By default, it uses the Prod account. To point it to the Stage account (which your client ID/secret might be for), you have to set all of these things:

OIDC_CONFIGURATION_URL=https://accounts.stage.mozaws.net
OIDC_OP_AUTHORIZATION_ENDPOINT=https://accounts.stage.mozaws.net/authorization
OIDC_OP_TOKEN_ENDPOINT=https://oauth.stage.mozaws.net/v1/token
OIDC_OP_USER_ENDPOINT=https://profile.stage.mozaws.net/v1/profile
OIDC_OP_JWKS_ENDPOINT=https://oauth.stage.mozaws.net/v1/jwks

OIDC_REDIRECT_ALLOWED_HOSTS=localhost:3000

OIDC_CONFIGURATION_CHECK=true

The first URL there is used to download https://accounts.stage.mozaws.net/.well-known/openid-configuration and compare that JSON with all the other settings. It's quite obsolete to have to have both. The /.well-known/openid-configuration could instead automatically set all the relevant OIDC_OP_... Django settings. But right now, it's just used as a validation that makes sure your OIDC_OP_* settings are sane.

By default, the decision to compare and check all the settings is based on settings.DEBUG. I.e. if you're not in DEBUG mode, it will check your configuration. But if you use DEBUG=true in your .env, the OIDC_CONFIGURATION_CHECK=true can be really helpful at this stage.

The OIDC_REDIRECT_ALLOWED_HOSTS one is for when you use Yari in localhost:3000 and from there, when you click to sign in, it'll attach a ?next=http://localhost:3000/en-US/docs/Where/You/Were to and since it's not a relative URL, it'll be ignored if you don't set OIDC_REDIRECT_ALLOWED_HOSTS=localhost:3000.

This PR also takes care of creating and updating a user profile for each signed in user. If you're a "Paid subscriber" on the subscription platform, it will set .is_subscriber in your user profile. And this is directly fed to the /api/v1/whoami which is what, for example, the Plus Bookmarks is depending on.

peterbe commented 3 years ago

@peterbe Starter review comment. I had to add libffi-dev back into the kuma_base Dockerfile in order to get docker-compose up --build to work.

I find that really strange. Docker is suppose to be self-contained. Why is docker-compose build --no-cache working on my MacBook but not on yours? And why is it working merrily fine on the Ubuntu that runs in GitHub Actions?!?!

codecov-commenter commented 3 years ago

Codecov Report

Merging #7966 (7724712) into master (e7374a6) will increase coverage by 2.35%. The diff coverage is 99.73%.

:exclamation: Current head 7724712 differs from pull request most recent head 7f3445e. Consider uploading reports for the commit 7f3445e to get more accurate results Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7966      +/-   ##
==========================================
+ Coverage   88.23%   90.59%   +2.35%     
==========================================
  Files          61       67       +6     
  Lines        1989     2317     +328     
  Branches      140      159      +19     
==========================================
+ Hits         1755     2099     +344     
+ Misses        206      192      -14     
+ Partials       28       26       -2     
Impacted Files Coverage Δ
kuma/users/models.py 93.33% <93.33%> (ø)
kuma/api/v1/plus/bookmarks.py 98.03% <100.00%> (+0.08%) :arrow_up:
kuma/api/v1/tests/test_bookmarks.py 100.00% <100.00%> (ø)
kuma/api/v1/tests/test_views.py 100.00% <100.00%> (ø)
kuma/api/v1/views.py 100.00% <100.00%> (+34.69%) :arrow_up:
kuma/conftest.py 85.71% <100.00%> (+2.38%) :arrow_up:
kuma/core/apps.py 93.75% <100.00%> (+0.89%) :arrow_up:
kuma/settings/common.py 95.51% <100.00%> (+0.37%) :arrow_up:
kuma/settings/pytest.py 100.00% <100.00%> (ø)
kuma/urls.py 88.88% <100.00%> (+0.65%) :arrow_up:
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e7374a6...7f3445e. Read the comment docs.

escattone commented 3 years ago

@peterbe and I discussed this on Slack. My strong suspicion is that docker-compose build --no-cache will not use cached images for any of the services defined in the docker-compose .yml file, but it will use cached images from which those services are built, like mdnwebdocs/kuma_base. So, in other words, I think @peterbe wasn't seeing the error when building the cffi wheel because he was using a cached mdnwebdocs/kuma_base that included the ffi.h header file that the build requires.