Skip to content

Conversation

mcasimir
Copy link
Collaborator

@mcasimir mcasimir commented Jan 25, 2022

Main change:

  • add the Kerberos authentication tab

Side changes:

  • fix type for query param values (only string should be allowed)
  • refactor all connection string manipulation helpers in one place
  • move encode/decode password and username to its own helpers as is now reused
  • automatically add authSource=$external when selecting authMechanisms that needs it

Screenshot 2022-01-25 at 20 44 14

Copy link
Member

@Anemy Anemy left a comment

Choose a reason for hiding this comment

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

lgtm! one question on documentation links

"packages-version": "lerna version --allow-branch main --no-push --no-private -m \"chore(release): Bump package versions\"",
"release": "npm run release --workspace mongodb-compass --",
"reformat": "lerna run reformat --stream --no-bail",
"reformat-changed": "lerna run reformat --stream --no-bail --since origin/HEAD --exclude-dependents",
Copy link
Member

Choose a reason for hiding this comment

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

🙌

});

expect(screen.getByLabelText('Username').getAttribute('value')).to.equal(
expect(screen.getByLabelText('Password').getAttribute('value')).to.equal(
Copy link
Member

Choose a reason for hiding this comment

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

Ah thanks for catching.

label="Principal"
errorMessage={kerberosPrincipalError}
state={kerberosPrincipalError ? 'error' : undefined}
value={principal || ''}
Copy link
Member

Choose a reason for hiding this comment

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

Previously we had an info link to https://docs.mongodb.com/manual/core/kerberos/#principals
on this input:
Screen Shot 2022-01-25 at 3 57 22 PM
Do we want to keep it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah that's a good one, yeah, i'll merge this and add it right away

@mcasimir mcasimir merged commit e81d5e8 into main Jan 26, 2022
@mcasimir mcasimir deleted the new-connect-form-kerberos branch January 26, 2022 10:20
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.

2 participants