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

Auto confirm invited users #754

Closed
dcelasun opened this issue Dec 2, 2019 · 19 comments
Closed

Auto confirm invited users #754

dcelasun opened this issue Dec 2, 2019 · 19 comments

Comments

@dcelasun
Copy link

dcelasun commented Dec 2, 2019

Inviting a user and then manually confirming them after they've registered is annoying and becomes a headache for large organizations. Would it be ok if I sent a PR adding a config option to disable it? It would change this line to UserOrgStatus::Confirmed.

@dani-garcia
Copy link
Owner

Confirming a user isn’t just done as a mostly useless extra step, it’s during confirmation when the user gets assigned the encryption keys used to decrypt the organization vault, so changing that wouldn’t work.

@dcelasun
Copy link
Author

dcelasun commented Dec 2, 2019

Sorry I missed that the first time, but I can also copy that bit to the invite accept endpoint. Would that be OK?

@dani-garcia
Copy link
Owner

Well there's not much to do in the accept endpoint because that's called by the user, and the user doesn't have the encryption keys yet, this would need to be done by the organization admins.

You could modify the server to make the invites autoaccepted, so you could immediately confirm the user afterwards, which should work because it's what's being done when email is disabled:

https://github.com/dani-garcia/bitwarden_rs/blob/adc443ea80be98a5aaf260eb8c2c7be6e43f6377/src/api/core/organizations.rs#L475-L479

Another option would be to have a running process with bitwarden_cli autoconfirming all members of the org: https://help.bitwarden.com/article/cli/#confirm

@dcelasun
Copy link
Author

dcelasun commented Dec 2, 2019

and the user doesn't have the encryption keys yet, this would need to be done by the organization admins

Why can't we do that in the invite accept handler?

You could modify the server to make the invites autoaccepted, so you could immediately confirm the user afterwards

That doesn't really help me. My use case is:

  • Invite up to 20 users at a time with access to certain collections
  • Users register via the invite link
  • Users log in and see all passwords in those collections

Basically, I want to be able to invite someone and forget about them. This is especially important in a large organization where Bitwarden's unusal two step signup causes a lot of friction.

@dani-garcia
Copy link
Owner

Because the invite accept handler is executed by the user, and the user doesn't have the encryption keys for the org until an admin gives them during the confirmation process.

Any changes to this process would imply modifying the web vaults code considerably and that's something we've been trying to avoid so far.

@dcelasun
Copy link
Author

dcelasun commented Dec 2, 2019

OK, thanks. I'll try to do this on a private fork and will open a PR if I end up with something decent.

@dcelasun dcelasun closed this as completed Dec 2, 2019
@etfz
Copy link

etfz commented May 2, 2023

You could modify the server to make the invites autoaccepted, so you could immediately confirm the user afterwards, which should work because it's what's being done when email is disabled:

https://github.com/dani-garcia/bitwarden_rs/blob/adc443ea80be98a5aaf260eb8c2c7be6e43f6377/src/api/core/organizations.rs#L475-L479

I don't know whether this will get seen here, but is a modification like this still viable? I noticed that particular block has since been removed.

@BlackDex
Copy link
Collaborator

BlackDex commented May 2, 2023

See: #2773
I'm not really sure if/why this was changed? Maybe an oversight? @stefan0xC ?

@stefan0xC
Copy link
Contributor

The existing code has just been refactored a bit. The logic is still there.

// automatically accept existing users if mail is disabled
if !CONFIG.mail_enabled() && !user.password_hash.is_empty() {
user_org_status = UserOrgStatus::Accepted as i32;
}

@BlackDex
Copy link
Collaborator

BlackDex commented May 2, 2023

The existing code has just been refactored a bit. The logic is still there.

// automatically accept existing users if mail is disabled
if !CONFIG.mail_enabled() && !user.password_hash.is_empty() {
user_org_status = UserOrgStatus::Accepted as i32;
}

That is only for existing users, not for new users. In the previous version that was also the case for newly invited users.

@stefan0xC
Copy link
Contributor

A new user will be automatically accepted on registration if mail is disabled:

} else if Invitation::take(&email, &mut conn).await {
for user_org in UserOrganization::find_invited_by_user(&user.uuid, &mut conn).await.iter_mut() {
user_org.status = UserOrgStatus::Accepted as i32;
user_org.save(&mut conn).await?;
}
user
} else if CONFIG.is_signup_allowed(&email)

@BlackDex
Copy link
Collaborator

BlackDex commented May 2, 2023

Ah, it's moved to an other location :). Yea that seems to be a bit more logical.
Good, thanks for the pointers.

@etfz
Copy link

etfz commented May 2, 2023

I don't fully understand how things work there, but is that not just the logic for the regular registration process? If you are invited to an organisation without already having an account, you accept the invitation by registering. That is the normal behavior, as far as I know. I can't see any checks for mail configuration there, but that might just be beyond my understanding.

The original code, as far as I understand, would mark the invitation as accepted as soon as it was sent by the organization administrator. Although perhaps that requires the account already being registered, in which case it does not really help my cause. Perhaps that is what OP meant by that solution not really helping them, come to think of it. So yeah, I'm referring to cases where an existing user account does not exist. Disabling email did not seem to change anything about the invitation process in that case.

It's just sort of a frustrating process.

  1. User requests an invite (signups disabled)
  2. X hour delay
  3. Admin sends invitation
  4. X hour delay
  5. User accepts invite and registers account
  6. X hour delay
  7. Admin confirms user
  8. X hour delay
  9. User notices confirmation and can get to work

Certainly this describes sort of a worst case communication scenario, but it's just so many trips, where logically two of those delays could be eliminated. I don't suppose we're in a better place to improve any of this today, three years later? I understand some of it is tied to upstreams code.

@BlackDex
Copy link
Collaborator

BlackDex commented May 2, 2023

An Admin always needs to confirm the user. During this moment some certs are generated which allows the users to have access to the shared vault items.

I'm not sure how you would speed this up. A user needs to be invited, a user needs to create an account a user needs to be confirmed. No way to minimize this.

@etfz
Copy link

etfz commented May 2, 2023

I suppose that if a user account was created when the invitation was sent, with some finalizing steps left to the invitee, it could be doable. I understand of course that nothing is really designed around that sort of process. I guess such a suggestion would be better to take upstreams.

@hvanmegen
Copy link

A new user will be automatically accepted on registration if mail is disabled:

} else if Invitation::take(&email, &mut conn).await {
for user_org in UserOrganization::find_invited_by_user(&user.uuid, &mut conn).await.iter_mut() {
user_org.status = UserOrgStatus::Accepted as i32;
user_org.save(&mut conn).await?;
}
user
} else if CONFIG.is_signup_allowed(&email)

I'm not entirely sure how to read that code snippet; What toggle in the admin interface do I have to disable in order to have auto-accepted invites?

@hvanmegen
Copy link

Confirming a user isn’t just done as a mostly useless extra step, it’s during confirmation when the user gets assigned the encryption keys used to decrypt the organization vault, so changing that wouldn’t work.

Why would this step need to be a manual process? Doesn't the system have some kind of cron handler or event handler that could handle this? I'm setting up Vaultwarden as a free service for volunteers within our organization for our local schools, but we have over 250 people that will be joining.. having to assign these all manually will be quite the PITA 😅

@BlackDex
Copy link
Collaborator

@hvanmegen There is no flag for auto accept invites. The only it does that is if the server does not have a SMTP server configured.
The reason for that is that a user can't really accept an invite, since they never received an invite. And acceptance only works via a token which is embedded into the link they need to click.

Vaultwarden does have a cron/job-handler but that can't confirm users. That needs to be done by some who already has access to the organization and have an unlocked vault/decrypted keys within the client. Because those are used to generate keys for the confirmed user. Since Vaultwarden has no knowledge of any password or keys it can't do any of that, nor would that be safe!

@hvanmegen
Copy link

@hvanmegen There is no flag for auto accept invites. The only it does that is if the server does not have a SMTP server configured. The reason for that is that a user can't really accept an invite, since they never received an invite. And acceptance only works via a token which is embedded into the link they need to click.

Vaultwarden does have a cron/job-handler but that can't confirm users. That needs to be done by some who already has access to the organization and have an unlocked vault/decrypted keys within the client. Because those are used to generate keys for the confirmed user. Since Vaultwarden has no knowledge of any password or keys it can't do any of that, nor would that be safe!

Ah I see.. That makes sense. I'll just have to set up a macro or something to babysit that page on the web interface then.. it's a real shame that there isn't a more elegant (read: automated) solution for this.

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

No branches or pull requests

6 participants