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: Regex extracting claims (addressing #420) #421

Merged
merged 3 commits into from
Dec 5, 2024

Conversation

Mrfence97
Copy link
Contributor

Overview

Updates the regex extracting the base-64 encoded claims in the WWW-Authenticate header. More details provided in issue #420.

Related Issue

Fixes #420

Updates the regex extracting the base-64 encoded claims in the
WWW-Authenticate header. More details provided in issue microsoft#420.
@Mrfence97 Mrfence97 requested a review from a team as a code owner December 3, 2024 11:28
baywet
baywet previously approved these changes Dec 3, 2024
Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

@baywet baywet enabled auto-merge (squash) December 3, 2024 13:05
musale
musale previously approved these changes Dec 4, 2024
@musale
Copy link
Contributor

musale commented Dec 4, 2024

@Mrfence97 could you please check some tests are failing.

Copy link
Contributor

@Ndiritu Ndiritu left a comment

Choose a reason for hiding this comment

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

Adding a unit test for this would be great.

auto-merge was automatically disabled December 4, 2024 13:10

Head branch was pushed to by a user without write access

@Mrfence97 Mrfence97 dismissed stale reviews from musale and baywet via 608c549 December 4, 2024 13:10
Copy link

sonarqubecloud bot commented Dec 4, 2024

@Mrfence97
Copy link
Contributor Author

@Mrfence97 could you please check some tests are failing.

The failing test was due to a bug in the test. In particular, there is a bug in the assertion of the test_retries_on_cae_failure test in http/httpx/tests/test_httpx_request_adapter.py. The mock claim entered in mock_cae_failure_response in http/httpx/tests/conftest.py ends with a =. It is unclear how this test ever passed previously!

I've added a commit to fix the test.

@Mrfence97
Copy link
Contributor Author

Adding a unit test for this would be great.

Unfortunately I don't know your testing framework well enough to implement something myself, but hopefully somebody at Microsoft can do it!

There is a bug in the assertion of the test_retries_on_cae_failure test
in http/httpx/tests/test_httpx_request_adapter.py. The mock claim
entered in mock_cae_failure_response in http/httpx/tests/conftest.py
ends with a "=". It is unclear how this test ever passed previously!
@Mrfence97
Copy link
Contributor Author

Mrfence97 commented Dec 4, 2024 via email

Copy link

sonarqubecloud bot commented Dec 5, 2024

@andrueastman andrueastman merged commit b48a9fb into microsoft:main Dec 5, 2024
53 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done ✔️
Development

Successfully merging this pull request may close these issues.

http - incorrect regex causing claim parsing to fail
5 participants