jakeswenson / BitBetter

Modify bit warden to provide my own licensing for self hosting
500 stars 112 forks source link

Full Unified support including Linux and Windows #155

Closed GieltjE closed 1 year ago

GieltjE commented 1 year ago

Did some initial work, rewrote some parts, but most importantly implemented a new flow.

It now stops all applicable docker containers, pulls the latest version, extracts the required files, modifies them, writes them to a new base image and starts the requested (possibly multiple) containers.

For now only the powershell scripts are (mostly) finished giving it full Windows compat, but for now no Linux support (will do this later).

The most important note, for now no generated licenses are accepted, this is just a new framework.

Please make a new branch and I'll resubmit the pull request.

GieltjE commented 1 year ago

Fully functional on Windows now, only circleci and Linux too go.

GieltjE commented 1 year ago

All scripts should function properly now, both on Linux and Windows

GieltjE commented 1 year ago

@h44z Any thoughts?

h44z commented 1 year ago

can you please keep the folder structure with /src/bitBetter and /src/licenseGen

and is the whole functionality of update-bitwarden.sh covered in the new build script?

GieltjE commented 1 year ago

What is the purpouse of retaining the src directory?

Also this is only compatible with the new unified container (forgot to mention that), there doesn't appear to be a "update-bitwarden.sh" script for that (nor do I think there will be one, it's just a simple single container with the normal docker interactions).

sniper7kills commented 1 year ago

@GieltjE, instead of running the unified container to modify it, I think it might be better to create a new docker file that uses the unified container as a source.

PATCHINSTANCE=$(docker run -d --name bitwarden-patch bitwarden/self-host:beta)

I had an issue where when I launched the modified container, it ignored my BW_ENABLE_SSO environmental variable and I had to modify the build script to get it to be enabled.

GieltjE commented 1 year ago

@sniper7kills could you please elaborate what you mean with new docker file?

Currently it follows the following scheme: 1: clear all potential previous cruft 2: generate keys if they don't exist 3: build our patching instance (e.g. bitBetter) 4: stop all currently running bitbetter/bitwarden instances 5: update/get the bitwarden self-host image 6: start a temporary bitwarden instance so we can extract the files we need to patch (seems to be no easy way to retrieve/store them directly from an image) 7: patch all the extracted files 8: push the files back to the temporary instance 9: commit the temporary instance to a new image 10: ensure the keygen can be build with up to date libraries 11: start all servers specified in the serverlist 12: remove our patching image 13: build the license generator 14: remove some more cruft

This way we end up with a new image that can be deployed multiple times while the original image remains untouched, so both patched and non-patched instances can be created.

During step 11 all containers are started, all variables should be passed there, the same way one would start regular docker instances.

sniper7kills commented 1 year ago

Instead of

8: push the files back to the temporary instance
9: commit the temporary instance to a new image

Have a docker file such as:

FROM bitwarden/self-host:beta

COPY Api/Core.dll /app/Api/Core.dll
COPY Identity/Core.dll /app/Identity/Core.dll

so steps 8 & 9 would become something like: docker build . --tag bitwarden-patch

I don't believe its a good idea to tag the temporary instance as the new instance as the entrypoint.sh script will be run when you start the temporary instance causing the Identity certificate to be the same for everyone who shares a published image (https://github.com/bitwarden/server/blob/c6e2db1ff519353e1a676d8764176e70276e8a47/docker-unified/entrypoint.sh#L35-L55), and the services to be disabled based on the the environment settings when starting the temporary instances. https://github.com/bitwarden/server/blob/c6e2db1ff519353e1a676d8764176e70276e8a47/docker-unified/entrypoint.sh#L82-L89

GieltjE commented 1 year ago

Would that not just create te exact same eventual image?

sniper7kills commented 1 year ago

No, they will be different.

When you tag the temporary instance, you are also including all of the changes the entrypoint.sh made when you started that instance. When you use the docker file, you'll only be including the modified files without any of the changes from the entrypoint.sh script.


If you view the logs on a "bitwarden-patch" image when it starts, the user and group that is created from the "entrypoint.sh" script already exist.

image

Also try:

TEMP=$(docker run --env BW_ENABLE_SSO=true -d bitwarden-patch)
docker exec $TEMP cat /etc/supervisor.d/sso.ini

you'll see the following

[program:sso]
autostart=false
autorestart=true
command=/usr/bin/dotnet "Sso.dll"
directory=/app/Sso
environment=ASPNETCORE_URLS="http://+:5007"
redirect_stderr=true
startsecs=15
stdout_logfile=/var/log/bitwarden/sso.log

but we should expect to see autostart=true

h44z commented 1 year ago

What is the purpouse of retaining the src directory?

Also this is only compatible with the new unified container (forgot to mention that), there doesn't appear to be a "update-bitwarden.sh" script for that (nor do I think there will be one, it's just a simple single container with the normal docker interactions).

The src directory keeps all C# code in a unique location. We should keep the original structure unless there is a good reason to change it. This project should still support the "old", non unified installation method of Bitwarden. So please update your PR to be backward compatible.

GieltjE commented 1 year ago

The latest commit should address both the src directory and image creation issue.

As for backwards compatibility I'm not sure as of this moment, for now the unified container is in a testing phase and will probably supersede the old method after the stable release. Could we push it to a new branch for now and see how the situation develops?

GieltjE commented 1 year ago

Does anyone know some proper tests for docker image/container existance? Would really like to fix the last warnings.

h44z commented 1 year ago

I have created a unified branch. We can merge it there.

GieltjE commented 1 year ago

The docker build doesn't allow me to build the image to something like "bitbetter/self-host", real shame was hoping to build some more consistency.

Feel free to merge it into the self-host branch at your leisure.

h44z commented 1 year ago

code resides in https://github.com/jakeswenson/BitBetter/tree/unified for now. Lets see how the unified Bitwarden stuff evolves =)