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

Problem with PyJWT 2.10: {'msg': 'Subject must be a string'} #30995

Closed
3 tasks done
amotl opened this issue Nov 20, 2024 · 15 comments
Closed
3 tasks done

Problem with PyJWT 2.10: {'msg': 'Subject must be a string'} #30995

amotl opened this issue Nov 20, 2024 · 15 comments

Comments

@amotl
Copy link
Contributor

amotl commented Nov 20, 2024

Bug description

When setting up a fresh sandbox environment, PyJWT 2.10 gets installed, released on Nov 17, i.e. three days ago. That breaks a little integration test suite we are running 1. This is the exception being raised:

AssertionError: {'msg': 'Subject must be a string'}

When downgrading to use pyjwt<2.10, the test suite succeeds again.

You may want to accompany this by potentially adjusting dependencies or code in Apache Superset?

Superset version

3.x and 4.x

Additional context

The software test suite maintained here can be used to reproduce the problem.

We added relevant details to this ticket, where we started to investigate this issue.

Checklist

  • I have searched Superset docs and Slack and didn't find a solution to my problem.
  • I have searched the GitHub issue tracker and didn't find a similar bug report.
  • I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.

Footnotes

  1. ... which orchestrates CLI invocations of the superset program and HTTP calls to the Superset API, in order to validate it works well together with CrateDB.

@amotl
Copy link
Contributor Author

amotl commented Nov 20, 2024

We just found this is most likely the root cause.

The canonical recommendation is to downgrade to PyJWT in the meanwhile.

pip install 'pyjwt<2.10'

@amotl amotl changed the title Problem with PyJWT 2.10 Problem with PyJWT 2.10: Subject must be a string Nov 20, 2024
@amotl
Copy link
Contributor Author

amotl commented Nov 21, 2024

That patch submitted by Dependabot also demonstrates the problem.

@amotl
Copy link
Contributor Author

amotl commented Nov 21, 2024

@jkogut: Do you have any idea why only we might be affected, but Superset's test suite seems to still succeed, and nobody else seems to be tripped? Is it related to the value of the SECRET_KEY maybe?

@jkogut
Copy link

jkogut commented Nov 22, 2024

@amotl yes indeed that was caused by the incorrect SECRET_KEY. Happened during test migration of superset instance. So indeed please double check SECRET_KEY. 🙏

@jkogut
Copy link

jkogut commented Nov 22, 2024

@amotl actually I was wrong, correct SECRET_KEY only allowed me to get access token but cannot proceed further with getting chart list for instance:

def get_chart_list():
    headers = {
        'Authorization': f"Bearer {get_bearer_token()}"
    }
    response = requests.get(f"{base_url}/chart/", headers=headers, verify=False)
    return response.json()

problem observed as reported on Superset 3.1.3, and fixed as recommended with installing PyJWT==2.9.0.

So still looks like new PyJWT 2.10.0 release can cause some problems with Superset API access.

@amotl amotl changed the title Problem with PyJWT 2.10: Subject must be a string Problem with PyJWT 2.10: {'msg': 'Subject must be a string'} Nov 22, 2024
@amotl
Copy link
Contributor Author

amotl commented Nov 22, 2024

We re-generated the value for SECRET_KEY using openssl rand -base64 42, as advertised in Superset's documentation, but it did not improve the situation.

@amotl
Copy link
Contributor Author

amotl commented Nov 22, 2024

We can still observe the problem, even after upgrading to Flask-JWT-Extended 4.7.1.

$ uv pip install --upgrade "PyJWT>=2.10" "Flask-JWT-Extended>=4.7.1" "Flask<3"
 - flask-jwt-extended==4.7.0
 + flask-jwt-extended==4.7.1
pytest -k api
            response = requests.delete(resource_to_delete, headers=get_auth_headers())
>           assert response.status_code in [200, 404], response.json()
E           AssertionError: {'msg': 'Subject must be a string'}
E           assert 422 in [200, 404]
E            +  where 422 = <Response [422]>.status_code

conftest.py:74: AssertionError

/cc @jlucier, @vimalloc

@jlucier
Copy link

jlucier commented Nov 22, 2024

Have you configured flask-jwt-extended to not verify the sub claim? 'JWT_VERIFY_SUB=False'

@amotl
Copy link
Contributor Author

amotl commented Nov 22, 2024

Thank you so much for your quick response, @jlucier. Actually, we are just approaching this problem as users of Apache Superset, not having any knowledge about its code base and its dependencies at all, other than knowing a bit about Flask.

We found that Flask-JWT-Extended is being used by Flask-AppBuilder, so this is probably the right spot to address this configuration update needed for the most recent PyJWT library? In this spirit, we figure creating a corresponding issue over there makes sense?

Please correct me if I am wrong with my evaluations.

/cc @dpgaspar

@jlucier
Copy link

jlucier commented Nov 22, 2024

Since my patch, this isn't a problem with flask-jwt-extended anymore. If you don't want enforce that the sub claim is a string, you just need to configure 'JWT_VERIFY_SUB' to be False. That's your choice to make, not the library's.

@amotl
Copy link
Contributor Author

amotl commented Nov 22, 2024

As a user of Apache Superset, I am was not touching JWT concerns at all, so [I thought] I can't make any choices about how JWT is used by its ingredients.

In this case, I guess it will be the concern of Flask-AppBuilder to configure Flask-JWT-Extended correctly, that's why I created a ticket there, but I may be wrong. Please educate me where you see a flaw in my thinking, as I don't have any strong knowledge of the subsystems we are talking about.

@amotl
Copy link
Contributor Author

amotl commented Nov 22, 2024

All right, I wasn't aware that JWT_VERIFY_SUB = False could easily be configured within the Flask application configuration file, which is effectively the superset_config.py file in our case.

After getting it right, this update perfectly resolves the problem we have been observing.

Thank you so much! 💯

@rusackas
Copy link
Member

@amotl are we good to close this then, if your problem is solved? If you think there's a good way/place to capture this in the docs, let us know or open a PR, it'd be appreciated!

@amotl
Copy link
Contributor Author

amotl commented Nov 26, 2024

Hi Evan. First things first, thanks for and keep up your excellent work on Superset and Preset.

Thank you for your reminder. I guess it will be good to close. As it turned out it might have only been a niche thing tripping us, I think having it documented on behalf of this very ticket will be good enough to support others being tripped by the same thing.

Please re-open if you think it deserves being captured on the docs explicitly, so we can check if we can come up with a corresponding patch.

@amotl amotl closed this as completed Nov 26, 2024
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

No branches or pull requests

4 participants