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.

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

[KYUUBI #6709] Fix seq and set equals check bug while check auth type in AuthenticationFilter.initAuthHandlers #6711

Closed
wants to merge 1 commit into from
Closed

[KYUUBI #6709] Fix seq and set equals check bug while check auth type in AuthenticationFilter.initAuthHandlers #6711

wants to merge 1 commit into from

Conversation

lifulong
Copy link
Contributor

@lifulong lifulong commented Sep 25, 2024 •
edited by turboFei
Loading

🔍 Description

Issue References 🔗

This pull request fixes #6709

Describe Your Solution 🔧

transfer the type of authTypes from seq to set before do equals check

Types of changes 🔖

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

test in our produce kyuubi env

Behavior Without This Pull Request ⚰️

image image

always hint to login in all sub tabs of management tab in kyuubi web ui, but can not login in actually, while use confkyuubi.authentication NOSASL, and our java version is 1.8 scala version is 2.12.18

Behavior With This Pull Request 🎉

image

Related Unit Tests

none, no need i think

Checklist 📝

Be nice. Be informative.

@codecov-commenter
Copy link

codecov-commenter commented Sep 25, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (ebe7e92) to head (850bda4).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...ver/http/authentication/AuthenticationFilter.scala 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@          Coverage Diff           @@
##           master   #6711   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files         684     684           
  Lines       42281   42281           
  Branches     5766    5766           
======================================
  Misses      42281   42281
Flag Coverage Δ
0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@lifulong
Copy link
Contributor Author

@pan3793 @ulysses-you @turboFei anyone have time review this change ?

@lifulong
Copy link
Contributor Author

thx for review @pan3793 @turboFei , but how to merge, looks like i have no permission to mege

@turboFei turboFei changed the title fix seq and set equals check bug while check auth type in Authenticat… Fix seq and set equals check bug while check auth type in Authenticat… Sep 26, 2024
@turboFei turboFei changed the title Fix seq and set equals check bug while check auth type in Authenticat… [KYUUBI #6709] Fix seq and set equals check bug while check auth type in AuthenticationFilter.initAuthHandlers Sep 26, 2024
@turboFei turboFei added this to the v1.8.3 milestone Sep 26, 2024
@turboFei turboFei closed this in 14bf56f Sep 26, 2024
turboFei pushed a commit that referenced this pull request Sep 26, 2024
@lifulong @turboFei
[KYUUBI #6709] Fix seq and set equals check bug while check auth type…
8d30f42 
… in AuthenticationFilter.initAuthHandlers

# 🔍 Description
## Issue References 🔗

This pull request fixes #6709

## Describe Your Solution 🔧

transfer the type of authTypes from seq to set before do equals check

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪
test in our produce kyuubi env
#### Behavior Without This Pull Request ⚰️
<img width="1431" alt="image" src="https://github.com/user-attachments/assets/6fd40d49-8d6b-446a-8feb-70df8c92604d">
<img width="603" alt="image" src="https://github.com/user-attachments/assets/637e5788-bfe0-4bab-a2a5-f6a79fb93fa6">

always hint to login  in all sub tabs of management tab in kyuubi web ui, but can not login in actually, while use confkyuubi.authentication NOSASL, and our java version is 1.8 scala version is 2.12.18
#### Behavior With This Pull Request 🎉
<img width="1422" alt="image" src="https://github.com/user-attachments/assets/211b76ee-d937-456b-bed4-f94dd41896f0">

#### Related Unit Tests

none, no need i think
---

# Checklist 📝

- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes #6711 from lifulong/authentication_filter_auth_type_bug_fix.

Closes #6709

850bda4 [lifulong] fix seq and set equals check bug while check auth type in AuthenticationFilter.initAuthHandlers

Authored-by: lifulong <[email protected]>
Signed-off-by: Wang, Fei <[email protected]>
(cherry picked from commit 14bf56f)
Signed-off-by: Wang, Fei <[email protected]>
turboFei pushed a commit that referenced this pull request Sep 26, 2024
@lifulong @turboFei
[KYUUBI #6709] Fix seq and set equals check bug while check auth type…
dc71e38 
… in AuthenticationFilter.initAuthHandlers

# 🔍 Description
## Issue References 🔗

This pull request fixes #6709

## Describe Your Solution 🔧

transfer the type of authTypes from seq to set before do equals check

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪
test in our produce kyuubi env
#### Behavior Without This Pull Request ⚰️
<img width="1431" alt="image" src="https://github.com/user-attachments/assets/6fd40d49-8d6b-446a-8feb-70df8c92604d">
<img width="603" alt="image" src="https://github.com/user-attachments/assets/637e5788-bfe0-4bab-a2a5-f6a79fb93fa6">

always hint to login  in all sub tabs of management tab in kyuubi web ui, but can not login in actually, while use confkyuubi.authentication NOSASL, and our java version is 1.8 scala version is 2.12.18
#### Behavior With This Pull Request 🎉
<img width="1422" alt="image" src="https://github.com/user-attachments/assets/211b76ee-d937-456b-bed4-f94dd41896f0">

#### Related Unit Tests

none, no need i think
---

# Checklist 📝

- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes #6711 from lifulong/authentication_filter_auth_type_bug_fix.

Closes #6709

850bda4 [lifulong] fix seq and set equals check bug while check auth type in AuthenticationFilter.initAuthHandlers

Authored-by: lifulong <[email protected]>
Signed-off-by: Wang, Fei <[email protected]>
(cherry picked from commit 14bf56f)
Signed-off-by: Wang, Fei <[email protected]>
@turboFei
Copy link
Member

thanks @lifulong merged to master/branch-1.9/branch-1.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pan3793 pan3793 pan3793 approved these changes

@turboFei turboFei turboFei approved these changes

Assignees

@lifulong lifulong

Labels
module:server
Projects
None yet
Milestone
v1.8.3
Development

Successfully merging this pull request may close these issues.

[Bug] AuthenticationFilter auth type check has bug while check equals of set and seq type
4 participants
@lifulong @codecov-commenter @turboFei @pan3793