Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update web-vault to v2023.2.0 #113

Merged
merged 1 commit into from Feb 21, 2023

Conversation

stefan0xC
Copy link
Contributor

There has been a new web-vault release web-v2023.2.0

From a first test run with vaultwarden's current main branch and a quick glance at the changes (v2023.1.0..v2023.2.0 and web-v2023.1.1...web-v2023.2.0) there seems to be at least a new GET /organizations/<org_uuid>/collections/details endpoint we need to implement in vaultwarden.

Copy link
Collaborator

@BlackDex BlackDex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing a change to libs/components/src/menu/menu.component.html b/libs/components/src/menu/menu.component.html which adds tw-overflow-x-auto

diff --git a/libs/components/src/menu/menu.component.html b/libs/components/src/menu/menu.component.html
index 15f78351a..6d3421ffc 100644
--- a/libs/components/src/menu/menu.component.html
+++ b/libs/components/src/menu/menu.component.html
@@ -1,7 +1,7 @@
 <ng-template>
   <div
     (click)="closed.emit()"
-    class="tw-flex tw-shrink-0 tw-flex-col tw-rounded tw-border tw-border-solid tw-border-secondary-500 tw-bg-background tw-bg-clip-padding tw-py-2"
+    class="tw-flex tw-shrink-0 tw-flex-col tw-rounded tw-border tw-border-solid tw-border-secondary-500 tw-bg-background tw-bg-clip-padding tw-py-2 tw-overflow-x-auto"
     [attr.role]="ariaRole"
     [attr.aria-label]="ariaLabel"
     cdkTrapFocus

patches/v2023.2.0.patch Show resolved Hide resolved
@tessus
Copy link
Contributor

tessus commented Feb 15, 2023

Hi stefan0xC. I also noticed a few commits in the bw server that have to do with something called SecretsManager. Unfortunately their PRs do not explain what that is, nor did I find anything in the bw documentation.

These changes are huge. I this something that must be implemented in vw as well?

@BlackDex
Copy link
Collaborator

Hi stefan0xC. I also noticed a few commits in the bw server that have to do with something called SecretsManager. Unfortunately their PRs do not explain what that is, nor did I find anything in the bw documentation.

These changes are huge. I this something that must be implemented in vw as well?

That is a Bitwarden licensed feature, so not something we can implement. It's kinda a hashicorp vault feature.

+ return false;
}

static isSelfHost(): boolean {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make sure this also returns false?
Not sure where this is used and if we need to do that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like isSelfHost() now calls the (recently-added) static version of itself, so the static version (or both versions) should be modified to return false. IIRC, if isSelfHost() returns true, then the web vault performs licensing checks in various places.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking. I've set both to false for now.

@tessus
Copy link
Contributor

tessus commented Feb 15, 2023

Ok, so is this missing endpoint in the server for a new feature or will it break current functionality when installing the v2023.2.0 client?

What I am wondering is what will happen, if I install the v2023.2.0 clients and use them with the current version of vw server (or the main branch).

@stefan0xC
Copy link
Contributor Author

stefan0xC commented Feb 15, 2023

You can't change the collections of an entry and the (new) organization view just looks like this:
org_notloading

@tessus
Copy link
Contributor

tessus commented Feb 15, 2023

Thanks, thus I won't update the clients. Hmm, but the other clients don't have that page anyway, so it's probably just the web client. I'll test on a VM to update the desktop client

@BlackDex
Copy link
Collaborator

BlackDex commented Feb 15, 2023

You can't change the collections of an entry and the (new) organization view just looks like this:

It can be partially fixed by creating that endpoint and just redirect it to get_org_collections(org_id, headers, conn).await.

But there are a lot of other items changed, like the groups show which collections, the collections show which groups they belong to etc.. So, there are some more changes we need to make for it to work fully i think.

@tessus
Copy link
Contributor

tessus commented Feb 15, 2023

But how is bw handling this? people who use the self-hosted system must also upgrade the server when they upgrade to 2023.2.0? Somehow this strategy is a bit flawed, because it is rather fragile. I'm also a bit fuzzy as to why they change the API this often. I've been using bw (clients) for less than a year and there were already 2 or 3 breaking changes.

@BlackDex
Copy link
Collaborator

But how is bw handling this? people who use the self-hosted system must also upgrade the server when they upgrade to 2023.2.0? Somehow this strategy is a bit flawed, because it is rather fragile. I'm also a bit fuzzy as to why they change the API this often. I've been using bw (clients) for less than a year and there were already 2 or 3 breaking changes.

They handle both front and backend. And they release/deploy the server/backend first and frontend later.

For self-hosted they have a script which Installs the correct versions together. That is actually the same as we do, only we need to build the vault first and check the PR's what has changed and what we need to add or update. Not only the clients repo, but also the server of course.

@dani-garcia dani-garcia merged commit df9da39 into dani-garcia:master Feb 21, 2023
@stefan0xC stefan0xC deleted the update-to-v2023.2.0 branch February 21, 2023 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants