vaultwarden
vaultwarden copied to clipboard
OIDC SSO
Continuing the PR by @Sheap #1955. Work in progress.
@pinpox is this PR ready for review?
@pinpox is this PR ready for review?
I'm sure there is still stuff to do, but it would be great if you could could give me a first review, so I know where to continue on working.
Does this implementation include support for key-connector or would that be a separate feature request / PR? Are users still required to have a master password?
Does this implementation include support for key-connector or would that be a separate feature request / PR?
The key-connector is not part of this PR.
Are users still required to have a master password?
Yes, the password is used to encrypt the vault and still required.
First, sorry for not checking this earlier.
I did some quick checking. And i probably missed some important stuff to nag about 😉
Overall i think it looks nice. There are some comments here and there which need to be removed etc...
All the commits should be squashed into one i think, and it needs a re-base and probably a new clippy
run.
The patch which is currently in this PR is probably also out-of-date.
If it is only needed to remove some CSS hide options, maybe a simple sed
would be enough as a comment so that people can try?
What I would also like, is some small howto on how i can test this my self? Maybe a simple docker-compose example to start and run a LDAP server with some users seeded to it, or something like that. That would save a lot of work for me and @dani-garcia if we need to test this.
About LDAP servers, I am building a LDAP server with a test suite in full docker. Feel free to steal anything you may need: https://github.com/sudo-bot/docker-openldap Else I recommend this docker image that is unmaintained: https://github.com/osixia/docker-openldap Here you will find an example LDAP server for emails: https://github.com/datacenters-network/mails/blob/250432abe68c5b56f8c916f5dbffbfd68498db54/docker-compose.yml#L121-L183 Feel free to ping me for LDAP docker things, I will try to help if I can
Hey, thanks for looking into it!
Overall i think it looks nice. There are some comments here and there which need to be removed etc... All the commits should be squashed into one i think, and it needs a re-base and probably a new
clippy
run.
The PR is still marked as Draft
for this reason, it needs still some cleanup before it can be merged but I'm trying to get it working first.
What I would also like, is some small howto on how i can test this my self? I'm currently using a gitea instance to test this. This doesn't have anything to do with the git functinality, but gitea can act as OIDC provider and you can start an instance easily without much configuration. I tried keycloak aswell, but it's way more complicated as it supportes a ton of features we don't need here.
Here are the basic steps to test this, keep in mind it's not all working yet.
Start a gitea (e.g. with the docker instructions from their site) and add an application under `https://mygitea.tld/user/settings/applications/oauth2/1` like this
Login to vaultwarden (the normal way) and create a organization, set the identifier to the organization name you set before
Then, in the web-vault go to `Enterprise single sign-on" and login with the identifier you created
In case someone can help out or knows how to fix it, this is the current blocker: ~https://github.com/dani-garcia/vaultwarden/pull/1955#issuecomment-1196590595~
EDIT: https://github.com/dani-garcia/vaultwarden/pull/1955#issuecomment-1200950120
Might be easier to use something made for SSO authentication - https://keycloak.org. Bit clunky but it works
Might be easier to use something made for SSO authentication - https://keycloak.org. Bit clunky but it works
I have tried keycloak but it is quite complex for the very few options we need to test this. It is for sure "the right tool" but I recommend testing with gitea just because it's simpler to set up in a minute.
In case someone can help out or knows how to fix it, this is the current blocker: ~#1955 (comment)~
EDIT: #1955 (comment)
Is this https://your.domain.com/sso/oidc-signin. I got it from https://bitwarden.com/help/oidc-azure/ https://bitwarden.com/help/oidc-okta/
I have got OIDC login working now! :partying_face:
I still need to look in to an issue with the front-end (Currently configuring it by setting the values in the DB manually) and some other minor issues.
In case someone can help out or knows how to fix it, this is the current blocker: ~#1955 (comment)~ EDIT: #1955 (comment)
Is this https://your.domain.com/sso/oidc-signin. I got it from https://bitwarden.com/help/oidc-azure/ https://bitwarden.com/help/oidc-okta/
The current implementation indeed uses /#/sso
for both paths, but I'm not sure we should leave it like that, because e.g. ADFS does not support anchors.
Ah this the only thing left to make sure my mom can use it without her forgetting her password. Just curious how far are you?
Ah this the only thing left to make sure my mom can use it without her forgetting her password. Just curious how far are you?
This will not prevent forgetting a password, since you still need a master password which can be the same as the linked sso of course, but that will not be secure of course.
Wait it will be an extra layer. So you will need oidc then login with master password then you will access your vault
That's what key-connector is for.
Just curious how far are you?
This branch is already functional in the backend, you can login with SSO already. What is missing is fixing the front-end part, currently you have to configure it on the DB itself by just setting the values in sso_config
. For some reason I have not found out yet, the frontend does not show the configured values, you can set new ones though, even though they are not shown. The actual OIDC authentication is working. If someone wants to help out with the front-end part, let me know.
Wait it will be an extra layer. So you will need oidc then login with master password then you will access your vault
That is correct. The master password is used to derive the encryption key for your vault, so it needs to be provided either by you or by the key-connector.
That's what key-connector is for.
The connector isn't part of this PR and I have not looked into it yet. Is that something we can even support? Is that a separate piece of software or is it part of the backend?
The connector isn't part of this PR and I have not looked into it yet. Is that something we can even support? Is that a separate piece of software or is it part of the backend?
The key connector is described here: https://bitwarden.com/help/about-key-connector/ Repository for the key connector can be found here: https://github.com/bitwarden/key-connector
Just curious how far are you?
This branch is already functional in the backend, you can login with SSO already. What is missing is fixing the front-end part, currently you have to configure it on the DB itself by just setting the values in
sso_config
. For some reason I have not found out yet, the frontend does not show the configured values, you can set new ones though, even though they are not shown. The actual OIDC authentication is working. If someone wants to help out with the front-end part, let me know.
A quick look at the code (without having it compiled etc..) i think that the response for the get_organization_sso
function is incorrect.
It should look like this, not sure if that is what you currently get, or that it is wrapped into another layer.
{
"enabled": true,
"data":
{
"configType": 2,
"keyConnectorEnabled": false,
"keyConnectorUrl": "",
"...CUT...": "...CUT..."
},
"urls":
{
"callbackPath": "https://vw.domain.tld/sso/oidc-signin",
"signedOutCallbackPath": "https://vw.domain.tld/sso/oidc-signedout",
"spEntityId": "https://vw.domain.tld/sso/saml2",
"spMetadataUrl": "https://vw.domain.tld/sso/saml2/{org_uuid}",
"spAcsUrl": "https://vw.domain.tld/sso/saml2/{org_uuid}Acs"
},
"object": "organizationSso"
}
Can this be merged? Would be just awesome as we already use vaultwarden.
Same for me.
Can this be merged? Would be just awesome as we already use vaultwarden.
The authentication process is functionally working (you can login via OIDC) but I need some guidance on what is missing for merging. With the help of someone responsible for merging, this shouldn't take much work.
Can this be merged? Would be just awesome as we already use vaultwarden.
The authentication process is functionally working (you can login via OIDC) but I need some guidance on what is missing for merging. With the help of someone responsible for merging, this shouldn't take much work.
Well, first I would say to rebase on the current main for one. And second, squash all these commits into one, 35+ is in my opinion way to much, and net verify beneficial when checking the changes done.
This also needs some changes to the web-vault, so a PR for the bw_web_builds
linked to this is probably also needed.
@pinpox Thanks for your hard work on this! OIDC is a must for us and it’s the only reason we haven’t moved to Vaultwarden yet.
I''m running al my services behind Authentik and would love to run Vaultwarden behind it. Is it possible to test it with a docker test container?
Hi @pinpox I tried to rebase your code in a separate pull request -> https://github.com/dani-garcia/vaultwarden/pull/2787
Of course, all the credits goes to you @pinpox, I just made a new pull request because your PR wasn't compiling and it did generate about one hundred conflicts
I sincerely hope it helps
Hi @pinpox I tried to rebase your code in a separate pull request -> #2787
Hope it helps
That's nice but you are losing history and authorship and I find that not cool
@m4w0lf
I sincerely hope it helps
Thanks a lot!
@williamdes
That's nice but you are losing history and authorship and I find that not cool
Will look at this changes now and integrate them into this PR if possible, I think that is the cleanest way.
I included the rebase made by @m4w0lf, that should resolve most of the conflicts. Hope that is a clean way to go about this
I included the rebase made by @m4w0lf, that should resolve most of the conflicts. Hope that is a clean way to go about this
It doesn't look very clean and also still has conflicts that must be resolved. There are many changes done on completely unrelated files which should still not be part of this PR (like the files in src/static and docker/, .github/workflows/build.yml, .pre-commit-config.yaml and probably more).
How to fix this mess? @BlackDex already told you when he asked if you could rebase on main and squash the entire pull request into a single commit (like what @m4w0lf did in PR #2787).
Also the changes needed to the bw_web_builds repository refers mostly to the web-vault-sso.patch
which should not be part of this repository once this PR is done.
Thank you to everyone working on this.
Currently, src/db/models/organization.rs
has some code in it that probably does not belong there. @pinpox 's other branch squash-rebase-oidc
looks better.
Is there something we can do to help with this? Can we test something? Thanks and cheers!