feat(connect-form): add username/password auth options COMPASS-5435 #2707
Conversation
.../connect-form/src/components/advanced-options-tabs/authentication-tab/authentication-tab.tsx
Outdated
Show resolved
Hide resolved
| (event: React.ChangeEvent<HTMLInputElement>) => { | ||
| event.preventDefault(); | ||
| updateConnectionFormField({ | ||
| type: 'update-search-param', |
There was a problem hiding this comment.
Currently this action also sets empty values, ie. it would set &authMechanism=& for a null value, could we also add updatedSearchParams.delete(action.currentKey); in case the value is empty here https://github.com/mongodb-js/compass/blob/main/packages/connect-form/src/hooks/use-connect-form.ts#L386-L393?
There was a problem hiding this comment.
I see how that helper could be useful here, but I think we want to allow empty values to be set since we use this in the advanced tab to help populate the advanced options. In that case setting something to empty is useful as part of the process of entering those options:
There was a problem hiding this comment.
I'm not sure that is so useful though, there is no search param that makes sense as empty, and I don't know why someone would prefer to populate the value in the connection string rather than in the form after they selected the key there. I believe is a good feature if we produce a cleaner connection string without random empty search params.
Do you have an alternative to at least delete authMechanism and the other params if they are empty?
There was a problem hiding this comment.
On this page the authMechanism will always be set since it's a radio with 3 options and unsetting isn't one.
Yup we can use the delete-search-param action.
I see what you mean. I'm cool with either in that case, could be nice to have that logic in the update search param handler, but we'll have to update the uri options component to keep a bit of state. Not sure this is the pr for that change.
| errors: ConnectionFormError[]; | ||
| updateConnectionFormField: UpdateConnectionFormField; | ||
| }): React.ReactElement { | ||
| const password = connectionStringUrl.password; |
There was a problem hiding this comment.
Is this relevant here? https://jira.mongodb.org/browse/COMPASS-5097
can we at least add a test or 2 with the weird password there?
There was a problem hiding this comment.
Could we do that work when we do that ticket? This work is definitely related, but since we have a ticket for it, that might be a better place to make a pr, test all of the cases and make those changes.
Moved it into this sprint, can pickup after this is merged.
2 participants
COMPASS-5435
This adds auth inputs for the Username/Password authentication tab.
Drive by cleanup - removed the
directConnectionsetting handler and made the toggle use the general search param one we've added.How do y'all feel on the behavior around setting a username empty or setting a password when there's no username?
Here's how it looks:
authentication.inputs.mp4