mirror of
https://github.com/BookStackApp/BookStack.git
synced 2026-02-15 19:06:40 +03:00
Question about vendor packages for custom Socialite providers #3439
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 @neosonic2 on GitHub (Jan 11, 2023).
Attempted Debugging
Searched GitHub Issues
Describe the Scenario
I am new to Bookstack and set up a fresh instance earlier today. I've found the app to be great so far and am currently trying to inject the Laravel Passport Socialite provider into my instance by way of the logical theme system, but am running into troubles due to vague documentation.
The logical theme system documentation doesn't give specifics about how custom Composer packages are to be installed. In the example Reddit provider configuration, it mentions that PHP require() statements can be used to reference the Socialite provider files contained within a vendor folder inside the custom theme folder, but when installing "socialiteproviders/laravelpassport" into that folder via Composer, I notice that base packages (like "laravel/socialite", "socialiteproviders/manager", etc.) are installed as well, duplicating packages already provided within the main Bookstack vendor directory.
Is there a way to add custom Socialite providers into Bookstack without having to install all of the base Socialite dependencies since they are already installed?
Exact BookStack Version
22.11.1
Log Content
No response
PHP Version
8.0.27
Hosting Environment
Laravel Valet on a local testing environment (for now)
@ssddanbrown commented on GitHub (Jan 12, 2023):
Hi @neosonic2,
I have not yet found a good way to support additional composer packages via the theme system.
For my example, I simply copied the additional provider's source files into a relative local directory then
require'd them in.@neosonic2 commented on GitHub (Jan 12, 2023):
Hi @ssddanbrown and thanks for your reply.
After some research, it appears there may be a way that Bookstack could support custom composer.json files located in theme folders, using a "merge plugin" package. The first answer in this StackOverflow post has details: https://stackoverflow.com/questions/46580522/composer-install-from-two-composer-json-file
Is this something that could be implemented? From what I read on StackOverflow and the example composer.json file shown in the answer, it appears one could create a themes/[my-theme]/composer.json file and that could then be merged into the main Bookstack composer.json file using the merge plugin, without any additional work required on anyone's part as long as the merge plugin package is installed with Bookstack.
@ssddanbrown commented on GitHub (Jan 12, 2023):
Yeah, I'm aware of that approach but from testing a while back, I wasn't too keen on sacrificing the
composer.lockusage we currently enjoy and the addition of a fundamental dependency, for little benefit (This is the first actual request I've had for composer package support) that can be worked around via simple manual means for current use-cases.It would also complicate theme system usage, which I want to keep unobstructed as we find it's use-cases and as it develops into something further.
@neosonic2 commented on GitHub (Jan 12, 2023):
Ah, I understand. Those are all very good points; I wouldn’t want to sacrifice composer.lock either or make the logical theme system (which seems very promising) any more complicated.
So I guess my next question is this - can I create a composer.json file in my custom theme and put my Laravel Passport Socialite provider package (socialiteproviders/laravelpassport) in it, and run “composer install” in that folder to install what I need, but somehow exclude all of the base packages (like laravel/socialite) that Bookstack already has in its vendor directory?
I don’t want to duplicate things here (by installing packages in my custom theme which Bookstack already has), but at the same time manually copying in vendor files and using require() on them means I also have to keep them manually updated, etc. If Composer can do this for me via a custom composer.json file in my theme folder but also not have to pull in base packages Bookstack already has, that would be the ideal setup. I am unfortunately not advanced enough with Composer to know if this is possible though, outside of doing something like a merge as we’ve discussed.
@ssddanbrown commented on GitHub (Jan 12, 2023):
You could probably use composer within your theme folder, then manually require in the files.
You just can't autoload files/packages within that folder since that's when you'll get conflicts.
TBH. you'd still have to manually update that theme composer folder to keep up with changes and possibly manually resolve conflicts, It doesn't really solve much to use composer in this context, apart from easier downloading.
You could instead clone this repo to a subfolder via git, then you can use git to update the files via a
git pullwith that subfolder.@neosonic2 commented on GitHub (Jan 12, 2023):
I think I see what you’re saying; cut out Composer entirely (i.e. no composer.json file in my theme’s folder) and just clone in the Socialite provider package I need and manually require the files that way, updating the folder by hand when new versions are released.
That may work, especially considering right now all I want to do in my custom logical theme is bring in the Laravel Passport Socialite provider (i.e. my theme isn’t going to use any other top level packages), but what about its dependencies? I know the base Bookstack installation already provides Laravel Socialite and many of its requirements, but are there dependencies in the Passport provider that Bookstack doesn’t already supply? If so, I’d have to either clone all of their folders as well and manage them with Git, or actually resort to Composer without autoloading.
My main point of contention here, which I think we’re coming down to a solution for, is that Bookstack already provides Laravel Socialite and the dependencies needed for its operation. I don’t want to duplicate those in my custom theme when I bring in the Laravel Passport Socialite provider package, especially if I can somehow tell that package to look in the base Bookstack vendor folder for its dependencies that Bookstack already satisfies. As an example, here are the many packages Composer tries to install simply because I required Socialite’s Passport provider; I haven’t looked at Bookstack’s base composer.json file but I assume it already has many of these:
@ssddanbrown commented on GitHub (Jan 12, 2023):
Here are the current top-level dependancies for the provider:
de93b2de64/composer.json (L24-L26)They're quite simple and already satisfied by BookStack's composer dependencies, and they will exist in the "live PHP environment" since they'll be accessible via BookStack's own composer autoloading.
You simply need to require in the two files that BookStack won't know about already, as per our socialite example, since they're not part of BookStack's own autoloading.
@neosonic2 commented on GitHub (Jan 12, 2023):
Oh wow, that definitely seems a lot simpler than I surmised. So because Bookstack already includes all of the dependencies of the Passport Socialite provider, I just need to pull the provider package itself into my custom theme and the live PHP environment will find the rest of the requirements through Bookstack’s existing autoloading mechanisms. At least I then only have to manage one package rather than a package and its dependencies, and I can definitely drop using Composer in my theme if everything it would install is already satisfied by Bookstack.
On a related note, are there any plans to support oAuth authentication in Bookstack? I know OpenID Connect/OIDC is already supported, as well as many third party providers, even those who themselves use oAuth, but Bookstack itself doesn’t support oAuth and Laravel Passport doesn’t support OIDC. I have an existing Passport instance that uses Google as its backend, and so rather than creating a new Google project for my Bookstack-powered app I figured I could just use my existing Passport instance and create a client key pair for Bookstack instead. I was honestly surprised to hear Bookstack supported OIDC but not oAuth since oAuth appears to be the more popular authentication framework, but for now it looks like the logical theme system will allow me to do exactly what I want without having to modify core app code directly.
@ssddanbrown commented on GitHub (Jan 12, 2023):
Yeah, you just need to require in those two php files within the provider-specific repo. Works fine from my testing and others have done the same for other providers.
Not really since I like to stick to standards. OAuth is only partially a standard, with the other part varying (Hence having separate providers). OIDC builds it into a much more complete standard.
While we could add a generic provider, and expose a whole bunch of options, it'll be a pain to assess and support all the variation required. I don't mean just from a technical point of view, but that variation would have a burden on my (or the project's) time as people try to configure for their environment and need support. Auth issues can be a significant pain to debug since they're very reliant on environment so can take a lot of time.
We already have SAML2, email+password, OIDC & LDAP. I really don't want to grow this scope further, Not at least without very significant benefit, which I don't see there being looking back over the last 7 years.
@neosonic2 commented on GitHub (Jan 12, 2023):
Your response makes sense, and in fact I’m very impressed at all the authentication providers Bookstack already supports and I can’t imagine adding one more, or a generic provider, would make the burden of project development and maintenance any easier. I also didn’t know oAuth wasn’t a complete standard so I’ve learned something new today. I always thought it was as the majority of sites and services I’ve seen use oAuth in some capacity but Bookstack is the first project I’ve co me across that supports OpenID.
Anyways, at least the logical theme system seems like a way for me to quickly add oAuth support via the provider I want and in lieu of adding additional authentication mechanisms to Bookstack itself this was probably the best solution. I’ll continue working on my implementation for now and come back to this issue if I run into problems with it, but I think you’ve given me enough to go on for now. I’ll close the issue once I get my implementation working which should be within the next 24 hours or so. Thanks for your consistent responses and assistance on this issue.
@neosonic2 commented on GitHub (Jan 13, 2023):
While working on my implementation, I wanted to ask a question I can’t seem to find the answer to in the Bookstack docs (but if it has already ben answered, please feel free to point me in the right direction).
When creating an oAuth client key pair in my Laravel Passport instance, I have to specify a redirect URI to which the end user will be sent after authentication as they make their journey back to Bookstack. How do I determine what this would be in my Bookstack app? Does Bookstack have a standard callback/redirect URI so that my Socialite provider can complete the rest of the authentication process and hopefully interface with Bookstack to log the user in?
@ssddanbrown commented on GitHub (Jan 13, 2023):
@neosonic2 The redirect URIs would be as follows:
Where
https://example.comis the BookStack root URL and<provider>is the first param you provide to theTheme::addSocialDriverfunction call.@neosonic2 commented on GitHub (Jan 13, 2023):
Thank you for the quick response; I feel like I’ve seen those endpoints somewhere in the Bookstack docs when I was reading through them the other day but can’t remember where; can you link to the page where they’re discussed?
One other question I know I definitely haven’t seen in the docs - can I store sensitive credentials for my Socialite provider (i.e. client secret) in the application’s .env file? If so, how can I reference them in my custom theme? I’ve got some development experience with Laravel, and normally when you want to reference an environment variable you first add it to a config array, like the one in config/services.php, and then reference it via Laravel’s config() helper. In this way, you’re not referencing the .env file directly and you can provide a second argument to config() to be used if the variable is not defined in the config array. But since I can’t modify core files like config/services.php, what should I do instead?
@ssddanbrown commented on GitHub (Jan 13, 2023):
They're listed in the per-service details of the third party auth page: https://www.bookstackapp.com/docs/admin/third-party-auth/
Not explicitly detailed for custom providers via the theme system, but just a case of following the pattern.
You could use
env()to pull in the value, which is not recommended outside of config files but will mostly be fine as long as you don't cache the config (which is never part of our guidance/instructions). Otherwise there's no custom config extension in the theme system.@neosonic2 commented on GitHub (Jan 13, 2023):
OK, I think I’ve got my theme’s functions.php file and my Laravel Passport Socialite provider files set up correctly as we’ve been discussing, and I can see a link on the Bookstack login page to log in with the provider I’ve added, but when I click on that link I get a message that says “An unknown error occurred.” The storage/logs/laravel.log file shows the following:
And my functions.php file looks like this:
What would cause a driver not to be supported by Bookstack?
@neosonic2 commented on GitHub (Jan 13, 2023):
Never mind - I think I’ve solved my issue. It turns out the problem was simply that I had to pass “laravelpassport” as the first argument to Theme::addSocialDriver() as opposed to just “passport”. For some reason I thought the first argument was arbitrary and didn’t have to match the actual Socialite provider package name but I guess it does.
However, I do continue to see this error sprinkled throughout laravel.log that does not seem to be related to all my custom theme work, but I can open a new issue for this if you’d like:
[2023-01-13 18:46:41] production.ERROR: Cannot redeclare versioned_asset() (previously declared in /Users/lmerrill/code/valet/bookstack/app/helpers.php:13) {"exception":"[object] (Symfony\Component\ErrorHandler\Error\FatalError(code: 0): Cannot redeclare versioned_asset() (previously declared in /Users/lmerrill/code/valet/bookstack/app/helpers.php:13) at /Users/lmerrill/code/valet/bookstack/app/helpers.php:13)
[stacktrace]
#0 {main}
“}
@ssddanbrown commented on GitHub (Jan 22, 2023):
Sorry, lost track of this thread.
Good to see you got it working! Will therefore close this off.
I'm not sure about that error. It's not something I've see, at least in recent versions.
If you can gain any extra details regarding it's occurrence then yeah, feel free to raise as a new issue, otherwise I feel it may be something environment specific so I'll need more clues to attempt replication. Even if it's just knowing it's frequency (Every request?) and weather or not it's only logged when your custom theme is active.