mirror of
https://github.com/BookStackApp/BookStack.git
synced 2026-02-08 19:06:06 +03:00
Implement a new AUTH_PRE_REGISTER logical theme event, intended to allow custom logic for registration events
#4453
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @AMDHome on GitHub (Feb 6, 2024).
Describe the feature you'd like
Allow users to manually specify whether or not they want to auto register OIDC authorized users.
This is similar to #2174 but for OIDC instead of SAML
Describe the benefits this would bring to existing BookStack users
Currently if you set up OIDC, anyone who attempts to log in with your provider will have a profile created for them if it doesn't exist. This feature would disable auto user creation and allow you to specify users in the user table
OIDC_AUTO_REGISTER=true- would have the same behavior as it does nowOIDC_AUTO_REGISTER=false- would require a user to be in the user table before granting accessCan the goal of this request already be achieved via other means?
This can be manually modded into bookstacks by editing the source code as shown in my PR #4831.
Although having it bundled into the releases would be nice.
Have you searched for an existing open/closed issue?
How long have you been using BookStack?
Under 3 months
Additional context
No response
@ssddanbrown commented on GitHub (Feb 8, 2024):
Thanks for the request @AMDHome.
From my usage of OIDC/SAML2 auth systems, they typically provide decent (and often role-based) per-app access control on the auth side of things. In my view it's always made sense that we retain delegation for access control management to the auth systems themselves.
Is there a particular reason why this can't be managed at the auth-system-level in your scenario?
@AMDHome commented on GitHub (Feb 8, 2024):
So the short version is I don't have access to those access controls so I have to do them on the client side.
The long version is:
I work for a public university, and would like to hook into their SSO services for login. For various reasons, our SSO department has not released the access control panel for me (and other people) to manage user access. They give out various reasons, but I feel like it has more to do with workplace bureaucracy. Either way, there's nothing I can do to get access.
In theory I can ask someone from the SSO department to make the changes on my behalf, but they are typically reluctant and/or slow. On top of that my department employs a number of student assistants who would need access. Student assistants have a relatively high turnover rate, so I would need to bother them every few weeks or so.
In the end, its easier to just do it client side because I have the ability to do it myself.
@ssddanbrown commented on GitHub (Feb 8, 2024):
Thanks @AMDHome for the added context, that makes sense.
As reflected within your PR #4831, I'm hesitant to add options, especially where they're to suit less common business process like this. Especially as common functionality would then be expected across other main auth methods, and I'm worried we'd just get future requests for additional filter control to suit other logic that can't be controlled auth-side (I vaguely remember a request in the past for filtering on specific user data).
I'm thinking, as a way to allow all of this, across all auth methods, we could add a logical theme event of
AUTH_PRE_REGISTER.We already emit an
AUTH_REGISTERevent, which is fired just after a new user is registered.Instead we can fire the
AUTH_PRE_REGISTERjust before the user is saved, but after all other existing logic.The event could pass in the intended user details, and allow a boolean return type to indicate if the registration should be allowed or not.
Using this, you could just return false from such an event for your intended functionality.
Otherwise, it's open to be flexibly used. For example, you could lookup user details against a file/database/api to decide if registration is allowed.
@AMDHome commented on GitHub (Feb 8, 2024):
No worries, I completely understand your feelings about the feature request for businesses.
Realistically the PR is just for me to be lazy so I don't have to manually add it back in myself after every update. Also since I already figured it out, might as well share the solution to anyone else who might want it.
The AUTH_PRE_REGISTER event sounds like it would work.
@ssddanbrown commented on GitHub (Feb 9, 2024):
@AMDHome Thanks for the feedback.
In that case, I'll update this issue to focus on adding the new theme system event.
Should be something easy to add for next feature release, so I'll add this to that milestone.
I'll also close your existing PR, thanks again for offering that.
Coincidently, I had another unique request on Reddit yesterday which could be solved by such a mechanism being in place.
@ssddanbrown commented on GitHub (Feb 21, 2024):
The discussed theme event has now been added in
055bbf17de, to be part of the next feature release.An example of a logical theme system
functions.phpfile, to prevent login, would look like this: