-
Notifications
You must be signed in to change notification settings - Fork 43
Integrated OpenID Connect #287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
|
It looks like I messed up my version control and implemented this on top of the Slack integration branch (#286) instead of from my main fork. Sorry about that! I did make certain design decisions based upon anticipated usage for my use case (CtrlH/PDX Hackerspace). For example, enabling OIDC_RP will prevent registration through Django (we don't presently plan on using MemberMatters as part of our member application process); in retrospect I probably should have implemented that behavior as a separate flag. Additionally, the changes I made to the frontend are pretty barebones to just enable use of OIDC. I'm sure that someone with proper design background could refactor this to make it more sensible. |
|
Hey @sibios thanks for your patience. Sorry for the delay in getting back to this one. I wanted to get a new GitHub Actions workflow happening that allows us to automatically build and push a docker image for external PRs to make testing much easier. This was a bit of a pain to do "securely" as any untrusted contributor can open a PR with potentially malicious code to extract repo secrets etc.
Scratch that, it seems to have already worked it out. I'll do some testing and see how it goes. :) |
|
Created image with name |
|
No worries! I did do some testing with our space's standing Authentik IDP instance, but have not exhaustively tested with other providers. As mentioned, I expect that I made some design decisions that would reasonably require refactoring. |
|
@bhartshorn I'll defer to @jabelone on whether rebasing is appropriate, but I think it should merge into dev cleanly. Please let me know if you run into any bugs with your testing as I only did a limited set of tests. |
|
Hi @sibios, @jabelone - another member of my group did some testing and found an issue or two. He posted on the feature request (I think by accident): #285 (comment)
Cc @dterkanian |
|
Oop, thanks for that @bhartshorn! I had totally missed @dterkanian 's comment on the feature request thread! I will work on repro-ing, but can you provide your I will get started on tweaks. Thank you for taking the time to test and report issues!! :) |
|
Created image with name |
jabelone
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh I'm so sorry, I thought I had already hit "submit" but my comments were sitting here pending and invisible! Looks mostly good, just have a few changes requested for making config easier. I've also setup our own authentik instance so I can test properly.
No worries! All variables related to this feature are now available via environment variables, documentation updated accordingly, and reworded phrasing on the registration lock-out message. |
|
Created image with name |
|
Created image with name |
|
Created image with name |
Implements #285 enabling MemberMatters to be an OpenID client