oidc: enable mozilla django oidc and hack in login/logout buttons for SSO #2464

osresearch · 2022-12-01T09:39:46Z

WARNING: I don't know how to django. Feel free to suggest better ways to do this!

TODO:

If the OIDC server includes role information in the userinfo, it will be checked for admin and moderator roles, which will be applied to the user on each login. If OIDC_ENABLED=true then the normal login forms will be replaced with the (currently unstyled) "Login with OIDC" link. This also silences some of the debugging printouts.

bookwyrm/views/setup.py

osresearch · 2022-12-01T18:00:22Z

I think I've solved all of the issues that I can other than the site setup hack to not create the admin user and also switch off registrations.

Configuring the SSO to work with Keycloak should only require setting a few parameters. Other OIDC providers have not been tested.

Passwords and email can not be changed through bookwyrm's preferences since they are managed by the SSO system. 2FA is also handled through the SSO, so it can not be enabled. Accounts can be deleted, although this will not delete it from the SSO. What happens if they try to re-login?

osresearch · 2022-12-06T15:21:35Z

Fixed the pylint issues, although I'm not sure how to address the failed pytest run https://github.com/bookwyrm-social/bookwyrm/actions/runs/3612489859

mouse-reeve · 2022-12-06T20:22:41Z

Dang, I'm not sure why that test is failing either! I'll try to take another look through it later this week.

Is it possible to support OIDC and regular authentication simultaneously?

osresearch · 2022-12-07T08:37:48Z

Supporting both normal login and OIDC is probably possible, maybe by overloading the mozilla-django-oidc auth backend. I'm not familiar at all with that part of the code flow to know for sure. It looks like people have reported succesfully doing so, although they didn't post links to their code...

If we do go down that road, this extra logging looks like it will be useful: https://mozilla-django-oidc.readthedocs.io/en/stable/installation.html#troubleshooting

osresearch · 2022-12-11T21:55:28Z

Looks like there is a minor usability issue with django refreshing the auth tickets similar to mozilla/mozilla-django-oidc#435 (comment) I see multiple requests in the logs very quickly, so it seems like the race condition is the problem. Their second suggestion of setting SESSION_ENGINE = "django.contrib.sessions.backends.cache" is what we already have enabled, so I need to try the sleep idea.

osresearch · 2022-12-13T19:22:47Z

now I'm at a loss as to how the tests are working... by moving the import statements for the mozilla oidc modules above class User in models/user.py, it created a circular dependency and my test instance isn't starting up. I thought I had run this patch set in testing, but must have been getting some cached state of some sort.

I'm also trying to track down the session refresh failures. The sleep suggestion avoids the 400 errors, but the tokens are still not renewed and the user is logged out. So this is not ready for prime time yet.

osresearch · 2022-12-13T21:41:52Z

when the token times out, the notification API endpoint gets redirected to the OIDC refresh endpoint, except that the XHR doesn't honor the redirect.

web_1             | id token is still valid (1670967131.5312643 > 1670967120.6054788)
web_1             | "GET /api/updates/notifications HTTP/1.0" 200 35
web_1             | id token has expired
web_1             | "GET /api/updates/notifications HTTP/1.0" 302 0
web_1             | id token has expired
web_1             | "GET /api/updates/notifications HTTP/1.0" 302 0

When a "real" page load is done by the user, the redirect works:

nginx_1           | BYPASS - 172.20.0.1 [13/Dec/2022:21:33:43 +0000] "GET / HTTP/1.0" 302 0 "https://books.dev.v.st/discover" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.60 Safari/537.36" 0.031 0.031
web_1             | "GET / HTTP/1.0" 302 0
web_1             | request is not refreshable
web_1             | ***** delaying to avoid race condition
web_1             | hudson@trmm.net: 'T H' logged in via OIDC roles ['admin']
web_1             | "GET /oidc/callback/?state=YSvbRMNqVvjM9G5TkPAiJtn7yuQqs890&session_state=81331c7c-e554-48b8-818a-202cd5402c6c&code=15b5bdc1-b9a2-4d05-addd-d194a3c466e1.81331c7c-e554-48b8-818a-202cd5402c6c.c0645e36-4cca-4900-9554-f1f636d7fa57 HTTP/1.0" 302 0
nginx_1           | BYPASS - 172.20.0.1 [13/Dec/2022:21:33:45 +0000] "GET /oidc/callback/?state=YSvbRMNqVvjM9G5TkPAiJtn7yuQqs890&session_state=81331c7c-e554-48b8-818a-202cd5402c6c&code=15b5bdc1-b9a2-4d05-addd-d194a3c466e1.81331c7c-e554-48b8-818a-202cd5402c6c.c0645e36-4cca-4900-9554-f1f636d7fa57 HTTP/1.0" 302 0 "-" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.60 Safari/537.36" 1.680 1.679

This means that as long as the user is still active, the refresh should work correctly. If they leave their browser for a while, eventually they are outside of the refresh period and they are unable to get a new token.

osresearch · 2022-12-13T22:33:20Z

Looks like this might be the way to handle it: https://mozilla-django-oidc.readthedocs.io/en/stable/xhr.html

Unfortunately the code in bookwyrm/static/js/bookwyrm.js that fetches /api/updates/notifications gets the 302 and then errors since there is a CORS failure on following the redirect:

> fetch("https://books.dev.v.st/api/updates/notifications").then((r) => console.log(r))
Promise {<pending>}
books.dev.v.st/:1 Access to fetch at 'https://login.dev.v.st/realms/hackerspace/protocol/openid-connect/auth?response_type=code&client_id=bookwyrm&redirect_uri=http%3A%2F%2Fbooks.dev.v.st%2Foidc%2Fcallback%2F&state=53SaDHQx33YDvCsk23xBPIzXwJtgbA2I&scope=openid%20email&prompt=none&nonce=cZTVAl0pZFXjOqztmtEz39MESoshU46A' (redirected from 'https://books.dev.v.st/api/updates/notifications') from origin 'https://books.dev.v.st' has been blocked by CORS policy: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.
VM1682:1          GET https://login.dev.v.st/realms/hackerspace/protocol/openid-connect/auth?response_type=code&client_id=bookwyrm&redirect_uri=http%3A%2F%2Fbooks.dev.v.st%2Foidc%2Fcallback%2F&state=53SaDHQx33YDvCsk23xBPIzXwJtgbA2I&scope=openid%20email&prompt=none&nonce=cZTVAl0pZFXjOqztmtEz39MESoshU46A net::ERR_FAILED 302
(anonymous) @ VM1682:1
VM1682:1                  Uncaught (in promise) TypeError: Failed to fetch
    at <anonymous>:1:1

osresearch · 2022-12-14T16:45:32Z

I thought the client secret was for bookwyrm to authenticate to the SSO system and is never revealed to the users, so I'm not sure where the threat of leaking the secret is here. Plus the idea is that there is a single approved SSO that bookwyrm is trusting for auth, which means that there is no way for users to login or signup without going through the SSO (when OIDC is enabled).

…

------- Original Message -------

On Wednesday, December 14th, 2022 at 5:18 PM, Jascha Urbach ***@***.***> wrote: A little remark: OIDC should be on a single user base - every user should configure their own OICD provider after signup. With this code you need to create one OIDC-provider beforehand and use this one for all users (except you run a single user instant, than this works fine) which is a bad idea as all users would share the client secret. — Reply to this email directly, [view it on GitHub](#2464 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAXNHK5HY5QMSE6VYT3BUS3WNHXMLANCNFSM6AAAAAASQSKS2Y). You are receiving this because you authored the thread.Message ID: ***@***.***>

jaschaurbach · 2022-12-14T16:51:17Z

Sorry, I was wrong and got a little bit confused... it is not the

I was asking myself why not use https://django-allauth.readthedocs.io/en/latest/ as a library - it supports user creation via OIDC, login, etc. and you can use OIDC as well as User/Pass login.

And: It supports not only kwycloak compliant OIDC providers but many more.

jaschaurbach · 2022-12-14T17:27:25Z

I just went through this tutorial to see if allauth would be feasible: http://www.sarahhagstrom.com/2013/09/the-missing-django-allauth-tutorial/ (this tutorial is for having all auth as well as user/password login)

it would be some work but can be done...

osresearch · 2022-12-15T16:49:56Z

allauth looks like it is much more general purpose and maybe more useful for some sites that want more flexibility in allowing external logins. One downside is that it requires a new database table, I think. For our use the ease of configuration of the mozilla oidc interface is really useful.

Since this PR is (hopefully!) nearing a good state, perhaps you could start another one for allauth and we could compare the two approaches?

osresearch · 2022-12-16T22:17:22Z

Looks like the fetch() should get a 403, rather than a 302. the OIDC middelware is expecting the "X-Requested-With": "XMLHttpRequest" headers, which aren't sent anymore? https://github.com/mozilla/mozilla-django-oidc/blob/10a279a1421948b2a8dd95f7ac4910c9a081f4ea/mozilla_django_oidc/middleware.py#L161

osresearch mentioned this pull request Dec 1, 2022

SSO support (OAuth2/OIDC, LDAP) #2379

Open

oidc: enable mozilla django oidc and hack in login/logout buttons

297b355

osresearch force-pushed the oidc branch 2 times, most recently from df1b026 to e57fd9e Compare December 1, 2022 17:53

osresearch force-pushed the oidc branch from e57fd9e to 18aa083 Compare December 1, 2022 17:57

osresearch marked this pull request as ready for review December 1, 2022 17:58

osresearch commented Dec 1, 2022

View reviewed changes

bookwyrm/views/setup.py Outdated Show resolved Hide resolved

osresearch added 3 commits December 1, 2022 19:25

Document OIDC settings in .env.example

3fefe31

Configuring the SSO to work with Keycloak should only require setting a few parameters. Other OIDC providers have not been tested.

oidc: back out email change for now

1453f5a

osresearch force-pushed the oidc branch from 18aa083 to 1453f5a Compare December 1, 2022 19:26

osresearch mentioned this pull request Dec 3, 2022

docker: move initial setup into docker startup to simplify deployment #2467

Open

osresearch added 3 commits December 4, 2022 08:27

oidc: format with black and address pylint

ead0e29

oidc: only skip install mode if oidc is enabled

fb1386c

oidc: add documentation and address pylint concerns

21b6ee4

osresearch force-pushed the oidc branch from 9637450 to 21b6ee4 Compare December 6, 2022 15:19

osresearch marked this pull request as draft December 13, 2022 19:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

oidc: enable mozilla django oidc and hack in login/logout buttons for SSO #2464

oidc: enable mozilla django oidc and hack in login/logout buttons for SSO #2464

osresearch commented Dec 1, 2022 •

edited

osresearch commented Dec 1, 2022

osresearch commented Dec 6, 2022

mouse-reeve commented Dec 6, 2022

osresearch commented Dec 7, 2022

osresearch commented Dec 11, 2022

osresearch commented Dec 13, 2022

osresearch commented Dec 13, 2022

osresearch commented Dec 13, 2022

osresearch commented Dec 14, 2022 via email

jaschaurbach commented Dec 14, 2022

jaschaurbach commented Dec 14, 2022 •

edited

osresearch commented Dec 15, 2022

osresearch commented Dec 16, 2022

oidc: enable mozilla django oidc and hack in login/logout buttons for SSO #2464

Are you sure you want to change the base?

oidc: enable mozilla django oidc and hack in login/logout buttons for SSO #2464

Conversation

osresearch commented Dec 1, 2022 • edited

osresearch commented Dec 1, 2022

osresearch commented Dec 6, 2022

mouse-reeve commented Dec 6, 2022

osresearch commented Dec 7, 2022

osresearch commented Dec 11, 2022

osresearch commented Dec 13, 2022

osresearch commented Dec 13, 2022

osresearch commented Dec 13, 2022

osresearch commented Dec 14, 2022 via email

jaschaurbach commented Dec 14, 2022

jaschaurbach commented Dec 14, 2022 • edited

osresearch commented Dec 15, 2022

osresearch commented Dec 16, 2022

osresearch commented Dec 1, 2022 •

edited

jaschaurbach commented Dec 14, 2022 •

edited