Skip to content

Added token cache map to fix use of unintended auth token for subsequent connections #2341

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Mar 20, 2024
Merged

Added token cache map to fix use of unintended auth token for subsequent connections #2341

merged 10 commits into from
Mar 20, 2024

Conversation

@tkyc
Copy link
Contributor

@tkyc tkyc commented Mar 1, 2024 •
edited
Loading

@codecov
Copy link

codecov bot commented Mar 1, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 23.48993% with 114 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (main@ba88da8). Click here to learn what that means.

❗ Current head 7fbc9e1 differs from pull request most recent head 5eb8e84. Consider uploading reports for the commit 5eb8e84 to get more accurate results

Files Patch % Lines
...soft/sqlserver/jdbc/SQLServerDatabaseMetaData.java 8.62% 53 Missing ⚠️
...microsoft/sqlserver/jdbc/SQLServerMSAL4JUtils.java 0.00% 41 Missing ⚠️
.../microsoft/sqlserver/jdbc/SQLServerConnection.java 59.37% 5 Missing and 8 partials ⚠️
...lserver/jdbc/PersistentTokenCacheAccessAspect.java 20.00% 3 Missing and 1 partial ⚠️
...m/microsoft/sqlserver/jdbc/SQLServerException.java 50.00% 0 Missing and 3 partials ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #2341   +/-   ##
=======================================
  Coverage        ?   50.00%           
  Complexity      ?     3789           
=======================================
  Files           ?      143           
  Lines           ?    33223           
  Branches        ?     5638           
=======================================
  Hits            ?    16612           
  Misses          ?    14227           
  Partials        ?     2384

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Jeffery-Wasty Jeffery-Wasty requested review from Jeffery-Wasty, barryw-mssql and lilgreenbird and removed request for lilgreenbird March 5, 2024 21:38
@Jeffery-Wasty Jeffery-Wasty linked an issue Mar 8, 2024 that may be closed by this pull request
Client credentials authentication - token cache returns unintended auth token #2334
Closed
Jeffery-Wasty
Jeffery-Wasty reviewed Mar 8, 2024
View reviewed changes
@Jeffery-Wasty Jeffery-Wasty added this to the 12.7.0 milestone Mar 8, 2024
@tkyc tkyc merged commit 1d4b7d6 into main Mar 20, 2024
@tkyc tkyc deleted the token-cache-caching branch March 20, 2024 18:22
@Eng-Fouad Eng-Fouad mentioned this pull request Aug 1, 2024
Merged
@wayneguow wayneguow mentioned this pull request Aug 1, 2024
Closed
dongjoon-hyun pushed a commit to apache/spark that referenced this pull request Aug 3, 2024
@wayneguow @dongjoon-hyun
[SPARK-49080][SQL][TEST] Upgrade mssql-jdbc to 12.8.0.jre11 and MsS…
94b572a 
…QLServer docker image tag to `2022-CU14-ubuntu-22.04`

### What changes were proposed in this pull request?

This PR aims to upgrade `mssql-jdbc` to 12.8.0.jre11 and MySQLServer docker image to `mcr.microsoft.com/mssql/server:2022-CU14-ubuntu-22.04`.

### Why are the changes needed?

This is the latest stable version of `mssql-jdbc`,  related release notes:

https://github.com/microsoft/mssql-jdbc/releases/tag/v12.7.0
https://github.com/microsoft/mssql-jdbc/releases/tag/v12.7.1
https://github.com/microsoft/mssql-jdbc/releases/tag/v12.8.0

Some fixed issues:

- Fix to ensure metadata returned follows JDBC data type specs microsoft/mssql-jdbc#2326
- Added token cache map to fix use of unintended auth token for subsequent connections microsoft/mssql-jdbc#2341
- Clear prepared statement handle before reconnect microsoft/mssql-jdbc#2364
- Reset socketTimeout to original value after a successful connection open microsoft/mssql-jdbc#2355
- Clear prepared statement cache when resetting statement pool connection microsoft/mssql-jdbc#2361
- Fixed ClassLoader leak of ActivityCorrelator ThreadLocal microsoft/mssql-jdbc#2366

### Does this PR introduce _any_ user-facing change?

No.
### How was this patch tested?

Pass GA.

### Was this patch authored or co-authored using generative AI tooling?

No.

Closes #47569 from wayneguow/ms_12_8.

Authored-by: Wei Guo <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lilgreenbird lilgreenbird Awaiting requested review from lilgreenbird

2 more reviewers

@Jeffery-Wasty Jeffery-Wasty Jeffery-Wasty approved these changes

@barryw-mssql barryw-mssql barryw-mssql approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

MSSQL JDBC
Status: Closed/Merged PRs

Milestone

12.7.0

Development

Successfully merging this pull request may close these issues.

Client credentials authentication - token cache returns unintended auth token

4 participants

@tkyc @Jeffery-Wasty @barryw-mssql