vaultwarden icon indicating copy to clipboard operation
vaultwarden copied to clipboard

OIDC SSO

Open pinpox opened this issue 2 years ago • 48 comments

Continuing the PR by @Sheap #1955. Work in progress.

pinpox avatar May 02 '22 09:05 pinpox

@pinpox is this PR ready for review?

technowhizz avatar May 31 '22 02:05 technowhizz

@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.

pinpox avatar May 31 '22 07:05 pinpox

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?

Kab1r avatar Jun 27 '22 03:06 Kab1r

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.

pinpox avatar Jun 29 '22 06:06 pinpox

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.

BlackDex avatar Jul 20 '22 19:07 BlackDex

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

williamdes avatar Jul 20 '22 20:07 williamdes

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

image

Login to vaultwarden (the normal way) and create a organization, set the identifier to the organization name you set before

image

Then, in the web-vault go to `Enterprise single sign-on" and login with the identifier you created

image

pinpox avatar Jul 27 '22 10:07 pinpox

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

pinpox avatar Jul 28 '22 08:07 pinpox

Might be easier to use something made for SSO authentication - https://keycloak.org. Bit clunky but it works

gal avatar Aug 02 '22 11:08 gal

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.

pinpox avatar Aug 02 '22 12:08 pinpox

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/

shreyasajj avatar Aug 07 '22 19:08 shreyasajj

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.

pinpox avatar Aug 08 '22 07:08 pinpox

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?

shreyasajj avatar Aug 17 '22 18:08 shreyasajj

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.

BlackDex avatar Aug 17 '22 18:08 BlackDex

Wait it will be an extra layer. So you will need oidc then login with master password then you will access your vault

shreyasajj avatar Aug 17 '22 18:08 shreyasajj

That's what key-connector is for.

Kab1r avatar Aug 17 '22 18:08 Kab1r

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?

pinpox avatar Aug 18 '22 10:08 pinpox

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

pReya avatar Aug 18 '22 11:08 pReya

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"
}

BlackDex avatar Aug 18 '22 13:08 BlackDex

Can this be merged? Would be just awesome as we already use vaultwarden.

maaft avatar Sep 25 '22 15:09 maaft

Same for me.

Akruidenberg avatar Sep 25 '22 20:09 Akruidenberg

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.

pinpox avatar Sep 26 '22 11:09 pinpox

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.

BlackDex avatar Sep 26 '22 12:09 BlackDex

@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.

binaryfire avatar Oct 02 '22 09:10 binaryfire

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?

Akruidenberg avatar Oct 02 '22 10:10 Akruidenberg

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

m4w0lf avatar Oct 02 '22 22:10 m4w0lf

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

williamdes avatar Oct 03 '22 07:10 williamdes

@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.

pinpox avatar Oct 04 '22 07:10 pinpox

I included the rebase made by @m4w0lf, that should resolve most of the conflicts. Hope that is a clean way to go about this

pinpox avatar Oct 04 '22 09:10 pinpox

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.

stefan0xC avatar Oct 04 '22 11:10 stefan0xC

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!

hellerbarde avatar Oct 10 '22 13:10 hellerbarde