Skip to content

Made driver throw SQLSTATE 28000 for invalid username or password #7

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

Merged
merged 5 commits into from
Jun 29, 2023

Conversation

GumpacG
Copy link

@GumpacG GumpacG commented Jun 21, 2023

Description

This change throws SQLSTATE 28000 for invalid username or password which is required by Tableau. This only covers 401 responses, responses like 403 from SIGV4 returns a different message.

Before:
Screenshot 2023-06-15 at 11 20 50 AM

After:
Screenshot 2023-06-21 at 3 30 51 PM

Issues Resolved

opensearch-project#95

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Copy link

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

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

Do we need to test it with SAML or SIGv4 auth?

@GumpacG GumpacG force-pushed the dev-auth-sqlstate branch from 35d3205 to 1282e7e Compare June 27, 2023 23:22
if (ex.getStatusCode() == 401) {
logAndThrowSQLException(log, new SQLException("Connection error " + ex.getMessage(),
INCORRECT_CREDENTIALS_SQLSTATE, ex));
} else {

Choose a reason for hiding this comment

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

I thought we agreed this was supposed to be only in ex.getStatusCode() == 403. Alternatively, only respond with this if there's a 4XX-level error. A 5XX means something more serious happened (and the connection crashed).

Copy link
Author

Choose a reason for hiding this comment

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

I think this should be for any attempt at connecting. https://tableau.github.io/connector-plugin-sdk/docs/drivers mentions that "Client unable to connect to server".

Copy link
Author

Choose a reason for hiding this comment

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

Removed state "08001" as it's scope is unclear

Signed-off-by: Guian Gumpac <[email protected]>
@GumpacG GumpacG merged commit 1369d12 into integ-auth-sqlstate Jun 29, 2023
GumpacG added a commit that referenced this pull request Jun 29, 2023
* Made driver throw SQLSTATE 28000 for invalid username or password

Signed-off-by: Guian Gumpac <[email protected]>

* Made SQLSTATE string a constant and added a separate catch block for HTTPException

Signed-off-by: Guian Gumpac <[email protected]>

* Changed constant name to follow naming convention

Signed-off-by: Guian Gumpac <[email protected]>

* Added 08001 SQLSTATE to catch connection errors

Signed-off-by: Guian Gumpac <[email protected]>

* Reverted 08001 change

Signed-off-by: Guian Gumpac <[email protected]>

---------

Signed-off-by: Guian Gumpac <[email protected]>
GumpacG added a commit that referenced this pull request Jul 10, 2023
…ensearch-project#102)

* Made driver throw SQLSTATE 28000 for invalid username or password (#7)

* Made driver throw SQLSTATE 28000 for invalid username or password

Signed-off-by: Guian Gumpac <[email protected]>

* Made SQLSTATE string a constant and added a separate catch block for HTTPException

Signed-off-by: Guian Gumpac <[email protected]>

* Changed constant name to follow naming convention

Signed-off-by: Guian Gumpac <[email protected]>

* Added 08001 SQLSTATE to catch connection errors

Signed-off-by: Guian Gumpac <[email protected]>

* Reverted 08001 change

Signed-off-by: Guian Gumpac <[email protected]>

---------

Signed-off-by: Guian Gumpac <[email protected]>

* Added to release notes

Signed-off-by: Guian Gumpac <[email protected]>

---------

Signed-off-by: Guian Gumpac <[email protected]>
@GumpacG GumpacG deleted the dev-auth-sqlstate branch July 10, 2023 22:54
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.

6 participants