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

fix: Provide useful error message for empty or missing passwords for SCRAM auth #2290

Merged
merged 1 commit into from
Oct 12, 2021

Conversation

sehrope
Copy link
Member

@sehrope sehrope commented Oct 8, 2021

Fixes #2288 to provide better error messages if the password is missing or empty during SCRAM auth.

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests?
  2. Does ./gradlew autostyleCheck checkstyleAll pass ?

Changes to Existing Features:

  • Does this break existing behaviour? If so please explain.
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully run tests with your changes locally?

…SCRAM auth

Previously the driver would throw an internal error if the server requested SCRAM authentication
and no or an empty password was specified. Fixes things so that the driver provides a helpful
error message to the user that a non-empty password is required.

Also adds tests to verify the behavior and expected error messages.
@sehrope
Copy link
Member Author

sehrope commented Oct 8, 2021

Not sure why, but one of the two AppVeyor builds failed for the ScramTest, but it's for the test that I didn't touch: https://ci.appveyor.com/project/davecramer/pgjdbc/builds/41075794/job/nrrwrcsa5osywna1#L242

And the complaint is that the SQL state error code does not match the expected value:

org.opentest4j.AssertionFailedError: expected: <28P01> but was: <08001>
        at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
        at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)
        at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
        at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
        at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1124)
        at org.postgresql.jdbc.ScramTest.testPasswordWithoutSpace(ScramTest.java:99)

Not sure the cause but I'm guessing it's either some transient error on the CI server. I'm inclined to just ignore it for now and track down the cause for it later.

If anybody has any suggestions for improving the new error messages please let me know. Otherwise, I'll merge this in a few days.

@davecramer
Copy link
Member

+1 I'd push 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

Successfully merging this pull request may close these issues.

Missing password causes SCRAM auth to give confusing and unhelpful error message
2 participants