feat(bitwarden): added field for server url during setup for self-hosted servers and others #2246
Conversation
UnKnoWn-Consortium
force-pushed
the
bitwarden_alt_instance
branch
from
6cf9e16
to
2790375
Compare
|
This seems reasonable from reading over it; can you explain "fixed confusing cache logic that does not cache credential request responses properly"? |
|
@PalmerAL In the original code, the cache list min/js/passwordManager/bitwarden.js Line 87 in 5bd1544 |
|
@PalmerAL Thanks for bringing us multi-window support. |
…ted servers fix(bitwarden): fixed confusing cache logic that does not cache credential request responses properly refactor(prompt): retrofitted the `createPrompt` function with promise refactor(prompt): updated `createPrompt` usage for the promise retrofit refactor: unwrapped AutofillSetup as individual functions and exported the `initialize` function that is used externally refactor: unwrapped PasswordManagers as individual functions and exported functions that are used externally refactor: replaced some usages of `var` with `let` and `const`
…e now thrown directly fix(passwordCapture): renamed typo `handleRecieveCredentials` to `handleReceivedCredentials` and updated usage refactor(passwordCapture): updated error handling for `getSuggestions` in `handleReceivedCredentials`
UnKnoWn-Consortium
force-pushed
the
bitwarden_alt_instance
branch
from
105c09d
to
317ab4d
Compare
…in the event handler for `show-prompt`
|
@PalmerAL Sorry to ping. Conflict resolved now. |
|
Sorry, I haven't had time to test this. Will do in the next few days. |
|
@PalmerAL Sorry my fault. Please take your time. |
| var signInFields = [ | ||
| // show ask-for-credential dialog | ||
| const signInFields = [ | ||
| { placeholder: 'Server URL (Leave blank for the default Bitwarden server)', id: 'url', type: 'text' }, |
There was a problem hiding this comment.
This should probably be a translated string - add it to en-US.json, then do l(stringName) like the examples below.
Actually, maybe "client ID" and "Client Secret" should be translated also, although I think the reason not to translate them was so that it would be clear in Bitwarden what we're referring to.
| ok: l('dialogConfirmButton'), | ||
| cancel: l('dialogSkipButton'), | ||
| width: 500, | ||
| height: 260 |
There was a problem hiding this comment.
The dialog is a little bit too short now, at least on macOS:
| var formattedHostname = hostname | ||
| if (formattedHostname.startsWith('www.')) { | ||
| formattedHostname = formattedHostname.slice(4) |
There was a problem hiding this comment.
This breaks autofill on sites starting with www., because we pass hostname in the autofill response, and it then makes this check fail:
min/js/preload/passwordFill.js
Lines 273 to 275 in 323e5df
(We should keep the check, it's guarding against us accidentally sending IPC messages to the wrong tab)
| BW_CLIENTID: credentials.clientID.trim(), | ||
| BW_CLIENTSECRET: credentials.clientSecret.trim() | ||
| }) | ||
| credentials.url = credentials.url || 'bitwarden.com' |
There was a problem hiding this comment.
It wasn't clear to me whether I was supposed to type "bitwarden.com" or "vault.bitwarden.com" - do they both work? If not, maybe we should special-case this so if the user enters one we convert to the other.
feat(bitwarden): added field for server url during setup for self-hosted servers
fix(bitwarden): fixed confusing cache logic that does not cache credential request responses properly
refactor(prompt): retrofitted the
createPromptfunction with promiserefactor(prompt): updated
createPromptusage for the promise retrofitrefactor: unwrapped AutofillSetup as individual functions and exported the
initializefunction that is used externallyrefactor: unwrapped PasswordManagers as individual functions and exported functions that are used externally
refactor: replaced some usages of
varwithletandconst