fix(user auth context): do not overwrite provided client options Authorization header #766
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Garee - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 3 issues found
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| _: Client = create_client(url, key) | ||
|
|
||
|
|
||
| def test_uses_key_as_authorization_header_by_default() -> None: |
There was a problem hiding this comment.
suggestion (testing): Consider adding a test case for when the Authorization header is explicitly set to None.
This would help ensure that the default behavior is correctly handled even when the header is explicitly set to None, rather than just not provided.
| def test_uses_key_as_authorization_header_by_default() -> None: | |
| def test_uses_key_as_authorization_header_when_explicitly_set_to_none() -> None: | |
| url = os.environ.get("SUPABASE_TEST_URL") | |
| key = os.environ.get("SUPABASE_TEST_KEY") | |
| options = ClientOptions(headers={"Authorization": None}) | |
| client = create_client(url, key, options) | |
| assert client.options.headers.get("apiKey") == key | |
| assert client.options.headers.get("Authorization") is None |
| assert client.storage.session.headers.get("Authorization") == f"Bearer {key}" | ||
|
|
||
|
|
||
| def test_supports_setting_a_global_authorization_header() -> None: |
There was a problem hiding this comment.
suggestion (testing): Please add a test case to verify the behavior when an invalid JWT is provided in the Authorization header.
This test would help ensure that the system behaves as expected when an invalid JWT is used, potentially rejecting the request or handling it gracefully.
| def test_supports_setting_a_global_authorization_header() -> None: | |
| def test_rejects_invalid_jwt() -> None: | |
| url = os.environ.get("SUPABASE_TEST_URL") | |
| key = os.environ.get("SUPABASE_TEST_KEY") | |
| invalid_jwt = "invalid_jwt" | |
| client = create_client(url, key, ClientOptions(headers={"Authorization": f"Bearer {invalid_jwt}"})) | |
| response = client.postgrest.get("some_endpoint") | |
| assert response.status_code == 401 |
| assert client.storage.session.headers.get("Authorization") == authorization | ||
|
|
||
|
|
||
| def test_updates_the_authorization_header_on_auth_events() -> None: |
There was a problem hiding this comment.
suggestion (testing): Add a test to verify header updates when the auth event is 'SIGNED_OUT'.
This would ensure that headers are correctly reset or handled when a user signs out.
| def test_updates_the_authorization_header_on_auth_events() -> None: | |
| def test_authorization_header_reset_on_sign_out() -> None: | |
| client._listen_to_auth_events("SIGNED_OUT", MagicMock(access_token=None)) | |
| assert client.options.headers.get("Authorization") is None | |
| assert client.postgrest.session.headers.get("Authorization") is None | |
| assert client.auth._headers.get("Authorization") is None | |
| assert client.storage.session.headers.get("Authorization") is None |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #766 +/- ##
==========================================
+ Coverage 43.97% 52.98% +9.00%
==========================================
Files 13 13
Lines 332 302 -30
==========================================
+ Hits 146 160 +14
+ Misses 186 142 -44 ☔ View full report in Codecov by Sentry. |
|
I'm currently reviewing this PR. I will need to test it out for a bit as some code has been removed which were to fix a previous issue. |
What kind of change does this PR introduce?
Bug fix for #420
Supports setting the client auth context for a particular user and therefore RLS.
What is the current behavior?
If a client is created with an
Authorizationheader provided inClientOptions, it is overwritten with the supabase key.For example:
What is the new behavior?
The
Authorizationheader provided via client options is used for requests.If no header is provided the supabase key is used.
Additional context
This allows us to set the auth context in the same manner described in the official Edge Function documentation.