You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "Mukezh (via GitHub)" <gi...@apache.org> on 2026/09/23 15:44:32 UTC
[PR] feat(python): return identity details from password and PAT login [iggy]
Mukezh opened a new pull request, #4276:
URL: https://github.com/apache/iggy/pull/4276
## Which issue does this PR address?
<!--
We generally require a GitHub issue for all bug fixes and enhancements. Link it with GitHub syntax, keep the line that applies and delete the other:
- `Closes #123` auto-closes the issue when this PR merges (full fix).
- `Relates to #123` links without closing (partial or related work).
-->
Closes #
Relates to #
## Rationale
<!--
Why is this change needed? If the issue explains it well, a one-liner is fine.
-->
## What changed?
<!--
2-4 sentences. Problem first (before), then solution (after).
GOOD:
"Messages were unavailable when background message_saver committed the
journal and started async disk I/O before completion. Polling during
this window found neither journal nor disk data.
The fix freezes journal batches in the in-flight buffer before async persist."
GOOD:
"When many small messages accumulate in the journal, the flush passes
thousands of IO vectors to writev(), exceeding IOV_MAX (1024 on Linux)."
BAD:
- Walls of text
- "This PR adds..." (we can see the diff)
-->
## Local Execution
- Passed / not passed
- Pre-commit hooks ran / not ran
<!--
You must run your code locally before submitting.
"Relying on CI" is not acceptable - PRs from authors who haven't run the code will be closed.
Did you have `prek` installed? It runs automatically on commit and covers all project languages. See [CONTRIBUTING.md](https://github.com/apache/iggy/blob/master/CONTRIBUTING.md).
-->
## AI Usage
<!--
If AI tools were used, please answer:
1. Which tools? (e.g., GitHub Copilot, Claude, ChatGPT)
2. Scope of usage? (e.g., autocomplete, generated functions, entire implementation)
3. How did you verify the generated code works correctly?
4. Can you explain every line of the code if asked?
If no AI tools were used, write "None" or delete this section.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "Mukezh (via GitHub)" <gi...@apache.org>.
Mukezh commented on code in PR #4276:
URL: https://github.com/apache/iggy/pull/4276#discussion_r4174007052
##########
foreign/python/tests/test_user.py:
##########
@@ -208,6 +256,43 @@ async def test_create_user_accepts_multibyte_credentials(
await iggy_client.delete_user(created.id)
+class TestLoginIdentity:
+ """Test login returns IdentityInfo with transport-specific access_token."""
+
+ @pytest.mark.asyncio
+ async def test_login_user_returns_identity_without_access_token(
+ self, iggy_client: IggyClient
+ ):
+ """Test TCP login returns user_id and no HTTP access token."""
+ root = await iggy_client.get_user("iggy")
+ assert root is not None
+
+ host, port = get_server_config()
+ client = IggyClient(f"{host}:{port}")
+ await client.connect()
+ identity = await client.login_user("iggy", "iggy")
+
+ assert isinstance(identity, IdentityInfo)
+ assert identity.user_id == root.id
+ assert identity.access_token is None
+
+ @pytest.mark.asyncio
+ async def test_login_with_personal_access_token(self, personal_access_token):
+ """Test PAT login with a token minted out of band."""
Review Comment:
By “out of band,” I meant that the PAT was created directly through the HTTP API because PAT creation is not exposed by the Python SDK in this issue. I replaced the wording with “created directly through HTTP” in the test and “created separately” in the API documentation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on PR #4276:
URL: https://github.com/apache/iggy/pull/4276#issuecomment-5798237284
@Mukezh approved, but please next time make sure that your PR is rebased. currently it's 41 commits behind HEAD of master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on code in PR #4276:
URL: https://github.com/apache/iggy/pull/4276#discussion_r4237603602
##########
foreign/python/tests/test_user.py:
##########
@@ -208,6 +266,221 @@ async def test_create_user_accepts_multibyte_credentials(
await iggy_client.delete_user(created.id)
+class TestLoginIdentity:
+ """Test login returns IdentityInfo with transport-specific access_token."""
+
+ @pytest.mark.parametrize(
+ "create_client",
+ [
+ lambda: IggyClient(f"{get_server_config()[0]}:{get_server_config()[1]}"),
+ lambda: IggyClient(
+ QuicConfig(
+ server_address=(
+ f"{get_quic_server_config()[0]}:{get_quic_server_config()[1]}"
+ ),
+ reconnection=QuicReconnectionConfig(enabled=False),
+ )
+ ),
+ lambda: IggyClient(
+ WebSocketConfig(
+ server_address=(
+ f"{get_websocket_server_config()[0]}:"
+ f"{get_websocket_server_config()[1]}"
+ ),
+ reconnection=WebSocketReconnectionConfig(enabled=False),
+ )
+ ),
+ ],
+ ids=["tcp", "quic", "websocket"],
+ )
+ @pytest.mark.asyncio
+ async def test_binary_login_returns_identity_without_access_token(
+ self, iggy_client: IggyClient, create_client: Callable[[], IggyClient]
+ ):
+ """Test binary transports return user_id and no HTTP access token."""
+ root = await iggy_client.get_user("iggy")
+ assert root is not None
+
+ client = create_client()
+ await client.connect()
+ await wait_for_ping(client)
+ identity = await client.login_user("iggy", "iggy")
+
+ assert isinstance(identity, IdentityInfo)
+ assert identity.user_id == root.id
+ assert identity.access_token is None
+
+ @pytest.mark.asyncio
+ async def test_http_login_returns_access_token(self, iggy_client: IggyClient):
+ """Test HTTP login returns user_id and a session TokenInfo."""
+ host, port = get_http_server_config()
+ client = IggyClient(HttpConfig(api_url=f"http://{host}:{port}"))
+ await client.connect()
+ await wait_for_ping(client)
+
+ identity = await client.login_user("iggy", "iggy")
+ root = await iggy_client.get_user("iggy")
+ assert root is not None
+
+ assert isinstance(identity, IdentityInfo)
+ assert identity.user_id == root.id
+ assert identity.access_token is not None
+ assert identity.access_token.token
+ assert identity.access_token.expiry > 0
+ assert identity.access_token.token not in repr(identity)
+ assert identity.access_token.token not in repr(identity.access_token)
+
+ @pytest.mark.asyncio
+ async def test_login_with_personal_access_token(self, unique_name):
+ """Test PAT login using a token created directly through HTTP."""
+ name = unique_name(min_bytes=16, max_bytes=30)
+ http_host, http_port = get_http_server_config()
+ api = f"http://{http_host}:{http_port}"
+ login_req = urllib.request.Request( # noqa: S310
Review Comment:
simplification: this test duplicates the token setup and cleanup helpers. reuse `_create_personal_access_token` and `_delete_personal_access_token` with the same arguments.
##########
foreign/python/tests/test_user.py:
##########
@@ -208,6 +266,221 @@ async def test_create_user_accepts_multibyte_credentials(
await iggy_client.delete_user(created.id)
+class TestLoginIdentity:
+ """Test login returns IdentityInfo with transport-specific access_token."""
+
+ @pytest.mark.parametrize(
+ "create_client",
+ [
+ lambda: IggyClient(f"{get_server_config()[0]}:{get_server_config()[1]}"),
+ lambda: IggyClient(
+ QuicConfig(
+ server_address=(
+ f"{get_quic_server_config()[0]}:{get_quic_server_config()[1]}"
+ ),
+ reconnection=QuicReconnectionConfig(enabled=False),
+ )
+ ),
+ lambda: IggyClient(
+ WebSocketConfig(
+ server_address=(
+ f"{get_websocket_server_config()[0]}:"
+ f"{get_websocket_server_config()[1]}"
+ ),
+ reconnection=WebSocketReconnectionConfig(enabled=False),
+ )
+ ),
+ ],
+ ids=["tcp", "quic", "websocket"],
+ )
+ @pytest.mark.asyncio
+ async def test_binary_login_returns_identity_without_access_token(
+ self, iggy_client: IggyClient, create_client: Callable[[], IggyClient]
+ ):
+ """Test binary transports return user_id and no HTTP access token."""
+ root = await iggy_client.get_user("iggy")
+ assert root is not None
+
+ client = create_client()
+ await client.connect()
+ await wait_for_ping(client)
+ identity = await client.login_user("iggy", "iggy")
+
+ assert isinstance(identity, IdentityInfo)
+ assert identity.user_id == root.id
+ assert identity.access_token is None
+
+ @pytest.mark.asyncio
+ async def test_http_login_returns_access_token(self, iggy_client: IggyClient):
+ """Test HTTP login returns user_id and a session TokenInfo."""
+ host, port = get_http_server_config()
+ client = IggyClient(HttpConfig(api_url=f"http://{host}:{port}"))
+ await client.connect()
+ await wait_for_ping(client)
+
+ identity = await client.login_user("iggy", "iggy")
+ root = await iggy_client.get_user("iggy")
+ assert root is not None
+
+ assert isinstance(identity, IdentityInfo)
+ assert identity.user_id == root.id
+ assert identity.access_token is not None
+ assert identity.access_token.token
+ assert identity.access_token.expiry > 0
+ assert identity.access_token.token not in repr(identity)
+ assert identity.access_token.token not in repr(identity.access_token)
+
+ @pytest.mark.asyncio
+ async def test_login_with_personal_access_token(self, unique_name):
+ """Test PAT login using a token created directly through HTTP."""
+ name = unique_name(min_bytes=16, max_bytes=30)
+ http_host, http_port = get_http_server_config()
+ api = f"http://{http_host}:{http_port}"
+ login_req = urllib.request.Request( # noqa: S310
+ f"{api}/users/login",
+ data=json.dumps({"username": "iggy", "password": "iggy"}).encode(),
+ headers={"Content-Type": "application/json"},
+ method="POST",
+ )
+ with urllib.request.urlopen(login_req, timeout=10) as response: # noqa: S310
+ jwt = json.loads(response.read())["access_token"]["token"]
+ create_req = urllib.request.Request( # noqa: S310
+ f"{api}/personal-access-tokens",
+ data=json.dumps({"name": name, "expiry": NEVER_EXPIRE}).encode(),
+ headers={
+ "Content-Type": "application/json",
+ "Authorization": f"Bearer {jwt}",
+ },
+ method="POST",
+ )
+ with urllib.request.urlopen(create_req, timeout=10) as response: # noqa: S310
+ personal_access_token = json.loads(response.read())["token"]
+
+ try:
+ host, port = get_server_config()
+ client = IggyClient(f"{host}:{port}")
+ await client.connect()
+ identity = await client.login_with_personal_access_token(
+ personal_access_token
+ )
+
+ assert isinstance(identity, IdentityInfo)
+ assert identity.access_token is None
+ user = await client.get_user("iggy")
+ assert user is not None
+ assert identity.user_id == user.id
+ assert personal_access_token not in repr(identity)
+ finally:
+ delete_req = urllib.request.Request( # noqa: S310
+ f"{api}/personal-access-tokens/{name}",
+ headers={"Authorization": f"Bearer {jwt}"},
+ method="DELETE",
+ )
+ with urllib.request.urlopen(delete_req, timeout=10): # noqa: S310
+ pass
+
+ @pytest.mark.parametrize(
+ "token",
+ ["unknown-personal-access-token", "%%%garbled-token%%%"],
+ ids=["unknown", "garbled"],
+ )
+ @pytest.mark.asyncio
+ async def test_invalid_personal_access_token_fails(self, token):
+ """Test unknown and garbled PAT values are rejected."""
+ host, port = get_server_config()
+ client = IggyClient(f"{host}:{port}")
+ await client.connect()
+
+ with pytest.raises(RuntimeError):
+ await client.login_with_personal_access_token(token)
+
+ @pytest.mark.asyncio
+ async def test_expired_personal_access_token_fails(self, unique_name):
+ """Test an expired PAT cannot authenticate."""
+ name = unique_name(min_bytes=16, max_bytes=30)
+ http_host, http_port = get_http_server_config()
+ api = f"http://{http_host}:{http_port}"
+ jwt, personal_access_token = _create_personal_access_token(
+ api, "iggy", "iggy", name, 1
+ )
+
+ host, port = get_server_config()
+ client = IggyClient(f"{host}:{port}")
+ await client.connect()
+
+ try:
+ with pytest.raises(RuntimeError):
+ await client.login_with_personal_access_token(personal_access_token)
+ finally:
+ _delete_personal_access_token(api, jwt, name)
Review Comment:
warning: cleanup fails with HTTP 404 if the server cleaner deletes the expired token first. accept 404 for this cleanup and propagate other errors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "Mukezh (via GitHub)" <gi...@apache.org>.
Mukezh commented on code in PR #4276:
URL: https://github.com/apache/iggy/pull/4276#discussion_r4174004356
##########
foreign/python/tests/test_user.py:
##########
@@ -30,12 +34,53 @@
MAX_USERNAME_BYTES,
MIN_PASSWORD_BYTES,
MIN_USERNAME_BYTES,
+ get_http_server_config,
get_server_config,
unique_credentials,
wait_for_ping,
wait_for_server,
)
+# IggyExpiry::NeverExpire serializes as u64::MAX.
+NEVER_EXPIRE = 18446744073709551615
+
+
+@pytest.fixture
+def personal_access_token(unique_name):
+ """Mint a PAT via HTTP so login tests do not need the Python create API."""
+ name = unique_name(min_bytes=16, max_bytes=30)
Review Comment:
Done. removed the one-use fixture and moved the successful PAT test’s setup and cleanup directly into the test.
##########
foreign/python/tests/test_http_config.py:
##########
@@ -232,6 +232,26 @@ async def test_client_connects_and_pings(self):
await client.connect()
await wait_for_ping(client)
+ @pytest.mark.asyncio
+ async def test_login_user_returns_http_access_token(self):
+ """Test HTTP login returns user_id and a session TokenInfo."""
Review Comment:
addressed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "Mukezh (via GitHub)" <gi...@apache.org>.
Mukezh commented on PR #4276:
URL: https://github.com/apache/iggy/pull/4276#issuecomment-5971286479
@hubcio Thanks for the review. I added the requested PAT failure cases and verified that TCP, QUIC, and WebSocket do not return an HTTP access token.
Current `master` explicitly tests that login before `connect()` succeeds through automatic reconnection, so I left that behavior unchanged. Should it be reconsidered separately?
All affected tests and checks pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on code in PR #4276:
URL: https://github.com/apache/iggy/pull/4276#discussion_r4109168107
##########
foreign/python/tests/test_user.py:
##########
@@ -208,6 +256,43 @@ async def test_create_user_accepts_multibyte_credentials(
await iggy_client.delete_user(created.id)
+class TestLoginIdentity:
+ """Test login returns IdentityInfo with transport-specific access_token."""
+
+ @pytest.mark.asyncio
+ async def test_login_user_returns_identity_without_access_token(
+ self, iggy_client: IggyClient
+ ):
+ """Test TCP login returns user_id and no HTTP access token."""
+ root = await iggy_client.get_user("iggy")
+ assert root is not None
+
+ host, port = get_server_config()
+ client = IggyClient(f"{host}:{port}")
+ await client.connect()
+ identity = await client.login_user("iggy", "iggy")
+
+ assert isinstance(identity, IdentityInfo)
+ assert identity.user_id == root.id
+ assert identity.access_token is None
+
+ @pytest.mark.asyncio
+ async def test_login_with_personal_access_token(self, personal_access_token):
+ """Test PAT login with a token minted out of band."""
Review Comment:
What is out of band?
##########
foreign/python/tests/test_user.py:
##########
@@ -30,12 +34,53 @@
MAX_USERNAME_BYTES,
MIN_PASSWORD_BYTES,
MIN_USERNAME_BYTES,
+ get_http_server_config,
get_server_config,
unique_credentials,
wait_for_ping,
wait_for_server,
)
+# IggyExpiry::NeverExpire serializes as u64::MAX.
+NEVER_EXPIRE = 18446744073709551615
+
+
+@pytest.fixture
+def personal_access_token(unique_name):
+ """Mint a PAT via HTTP so login tests do not need the Python create API."""
+ name = unique_name(min_bytes=16, max_bytes=30)
Review Comment:
There is no need to create a fixture just for one test. Inline this code into the test that uses it .
##########
foreign/python/tests/test_http_config.py:
##########
@@ -232,6 +232,26 @@ async def test_client_connects_and_pings(self):
await client.connect()
await wait_for_ping(client)
+ @pytest.mark.asyncio
+ async def test_login_user_returns_http_access_token(self):
+ """Test HTTP login returns user_id and a session TokenInfo."""
Review Comment:
Move this into TestLoginIdentity
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "Mukezh (via GitHub)" <gi...@apache.org>.
Mukezh commented on PR #4276:
URL: https://github.com/apache/iggy/pull/4276#issuecomment-5806997162
@hubcio Rebased onto the latest master, and the PR is now ready for review. The updated checks are pending. When you have time, could you please review it? Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "Mukezh (via GitHub)" <gi...@apache.org>.
Mukezh commented on PR #4276:
URL: https://github.com/apache/iggy/pull/4276#issuecomment-5798198507
Hi @jiengup , could you please approve the workflow run for this draft PR? I’ve run the repository hooks and the affected login tests locally. Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4276:
URL: https://github.com/apache/iggy/pull/4276#issuecomment-6244001756
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs.
If you need a review, please ensure CI is green and the PR is rebased on the latest master. Don't hesitate to ping the maintainers - either `@core` on Discord or by mentioning them directly here on the PR.
Thank you for your contribution!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "Mukezh (via GitHub)" <gi...@apache.org>.
Mukezh commented on code in PR #4276:
URL: https://github.com/apache/iggy/pull/4276#discussion_r4321795946
##########
foreign/python/tests/test_user.py:
##########
@@ -208,6 +266,221 @@ async def test_create_user_accepts_multibyte_credentials(
await iggy_client.delete_user(created.id)
+class TestLoginIdentity:
+ """Test login returns IdentityInfo with transport-specific access_token."""
+
+ @pytest.mark.parametrize(
+ "create_client",
+ [
+ lambda: IggyClient(f"{get_server_config()[0]}:{get_server_config()[1]}"),
+ lambda: IggyClient(
+ QuicConfig(
+ server_address=(
+ f"{get_quic_server_config()[0]}:{get_quic_server_config()[1]}"
+ ),
+ reconnection=QuicReconnectionConfig(enabled=False),
+ )
+ ),
+ lambda: IggyClient(
+ WebSocketConfig(
+ server_address=(
+ f"{get_websocket_server_config()[0]}:"
+ f"{get_websocket_server_config()[1]}"
+ ),
+ reconnection=WebSocketReconnectionConfig(enabled=False),
+ )
+ ),
+ ],
+ ids=["tcp", "quic", "websocket"],
+ )
+ @pytest.mark.asyncio
+ async def test_binary_login_returns_identity_without_access_token(
+ self, iggy_client: IggyClient, create_client: Callable[[], IggyClient]
+ ):
+ """Test binary transports return user_id and no HTTP access token."""
+ root = await iggy_client.get_user("iggy")
+ assert root is not None
+
+ client = create_client()
+ await client.connect()
+ await wait_for_ping(client)
+ identity = await client.login_user("iggy", "iggy")
+
+ assert isinstance(identity, IdentityInfo)
+ assert identity.user_id == root.id
+ assert identity.access_token is None
+
+ @pytest.mark.asyncio
+ async def test_http_login_returns_access_token(self, iggy_client: IggyClient):
+ """Test HTTP login returns user_id and a session TokenInfo."""
+ host, port = get_http_server_config()
+ client = IggyClient(HttpConfig(api_url=f"http://{host}:{port}"))
+ await client.connect()
+ await wait_for_ping(client)
+
+ identity = await client.login_user("iggy", "iggy")
+ root = await iggy_client.get_user("iggy")
+ assert root is not None
+
+ assert isinstance(identity, IdentityInfo)
+ assert identity.user_id == root.id
+ assert identity.access_token is not None
+ assert identity.access_token.token
+ assert identity.access_token.expiry > 0
+ assert identity.access_token.token not in repr(identity)
+ assert identity.access_token.token not in repr(identity.access_token)
+
+ @pytest.mark.asyncio
+ async def test_login_with_personal_access_token(self, unique_name):
+ """Test PAT login using a token created directly through HTTP."""
+ name = unique_name(min_bytes=16, max_bytes=30)
+ http_host, http_port = get_http_server_config()
+ api = f"http://{http_host}:{http_port}"
+ login_req = urllib.request.Request( # noqa: S310
Review Comment:
Updated the successful PAT login test to use _create_personal_access_token and _delete_personal_access_token with the same arguments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #4276:
URL: https://github.com/apache/iggy/pull/4276#issuecomment-5799923237
## [Codecov](https://app.codecov.io/gh/apache/iggy/pull/4276?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `95.45455%` with `2 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 87.24%. Comparing base ([`b68a506`](https://app.codecov.io/gh/apache/iggy/commit/b68a506be65b0a559db522106f9f87875f63686a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`522e0fd`](https://app.codecov.io/gh/apache/iggy/commit/522e0fd04b1ea24e4d691303f67a24366ca0e83c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 41 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/4276?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [foreign/python/src/user.rs](https://app.codecov.io/gh/apache/iggy/pull/4276?src=pr&el=tree&filepath=foreign%2Fpython%2Fsrc%2Fuser.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Zm9yZWlnbi9weXRob24vc3JjL3VzZXIucnM=) | 92.30% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/iggy/pull/4276?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #4276 +/- ##
=========================================
Coverage 87.23% 87.24%
Complexity 1455 1455
=========================================
Files 1273 1273
Lines 217093 217135 +42
Branches 181708 181708
=========================================
+ Hits 189390 189430 +40
- Misses 23149 23151 +2
Partials 4554 4554
```
| [Components](https://app.codecov.io/gh/apache/iggy/pull/4276/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [Rust Core](https://app.codecov.io/gh/apache/iggy/pull/4276/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `88.28% <ø> (ø)` | |
| [Java SDK](https://app.codecov.io/gh/apache/iggy/pull/4276/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `67.58% <ø> (ø)` | |
| [C# SDK](https://app.codecov.io/gh/apache/iggy/pull/4276/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.11% <ø> (ø)` | |
| [Python SDK](https://app.codecov.io/gh/apache/iggy/pull/4276/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `91.02% <95.45%> (+0.04%)` | :arrow_up: |
| [PHP SDK](https://app.codecov.io/gh/apache/iggy/pull/4276/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.67% <ø> (ø)` | |
| [Node SDK](https://app.codecov.io/gh/apache/iggy/pull/4276/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `96.26% <ø> (ø)` | |
| [Go SDK](https://app.codecov.io/gh/apache/iggy/pull/4276/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.45% <ø> (ø)` | |
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/4276?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [foreign/python/src/client.rs](https://app.codecov.io/gh/apache/iggy/pull/4276?src=pr&el=tree&filepath=foreign%2Fpython%2Fsrc%2Fclient.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Zm9yZWlnbi9weXRob24vc3JjL2NsaWVudC5ycw==) | `99.77% <100.00%> (+<0.01%)` | :arrow_up: |
| [foreign/python/src/lib.rs](https://app.codecov.io/gh/apache/iggy/pull/4276?src=pr&el=tree&filepath=foreign%2Fpython%2Fsrc%2Flib.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Zm9yZWlnbi9weXRob24vc3JjL2xpYi5ycw==) | `100.00% <100.00%> (ø)` | |
| [foreign/python/src/user.rs](https://app.codecov.io/gh/apache/iggy/pull/4276?src=pr&el=tree&filepath=foreign%2Fpython%2Fsrc%2Fuser.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Zm9yZWlnbi9weXRob24vc3JjL3VzZXIucnM=) | `84.72% <92.30%> (+4.28%)` | :arrow_up: |
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "Mukezh (via GitHub)" <gi...@apache.org>.
Mukezh commented on code in PR #4276:
URL: https://github.com/apache/iggy/pull/4276#discussion_r4321794765
##########
foreign/python/tests/test_user.py:
##########
@@ -208,6 +266,221 @@ async def test_create_user_accepts_multibyte_credentials(
await iggy_client.delete_user(created.id)
+class TestLoginIdentity:
+ """Test login returns IdentityInfo with transport-specific access_token."""
+
+ @pytest.mark.parametrize(
+ "create_client",
+ [
+ lambda: IggyClient(f"{get_server_config()[0]}:{get_server_config()[1]}"),
+ lambda: IggyClient(
+ QuicConfig(
+ server_address=(
+ f"{get_quic_server_config()[0]}:{get_quic_server_config()[1]}"
+ ),
+ reconnection=QuicReconnectionConfig(enabled=False),
+ )
+ ),
+ lambda: IggyClient(
+ WebSocketConfig(
+ server_address=(
+ f"{get_websocket_server_config()[0]}:"
+ f"{get_websocket_server_config()[1]}"
+ ),
+ reconnection=WebSocketReconnectionConfig(enabled=False),
+ )
+ ),
+ ],
+ ids=["tcp", "quic", "websocket"],
+ )
+ @pytest.mark.asyncio
+ async def test_binary_login_returns_identity_without_access_token(
+ self, iggy_client: IggyClient, create_client: Callable[[], IggyClient]
+ ):
+ """Test binary transports return user_id and no HTTP access token."""
+ root = await iggy_client.get_user("iggy")
+ assert root is not None
+
+ client = create_client()
+ await client.connect()
+ await wait_for_ping(client)
+ identity = await client.login_user("iggy", "iggy")
+
+ assert isinstance(identity, IdentityInfo)
+ assert identity.user_id == root.id
+ assert identity.access_token is None
+
+ @pytest.mark.asyncio
+ async def test_http_login_returns_access_token(self, iggy_client: IggyClient):
+ """Test HTTP login returns user_id and a session TokenInfo."""
+ host, port = get_http_server_config()
+ client = IggyClient(HttpConfig(api_url=f"http://{host}:{port}"))
+ await client.connect()
+ await wait_for_ping(client)
+
+ identity = await client.login_user("iggy", "iggy")
+ root = await iggy_client.get_user("iggy")
+ assert root is not None
+
+ assert isinstance(identity, IdentityInfo)
+ assert identity.user_id == root.id
+ assert identity.access_token is not None
+ assert identity.access_token.token
+ assert identity.access_token.expiry > 0
+ assert identity.access_token.token not in repr(identity)
+ assert identity.access_token.token not in repr(identity.access_token)
+
+ @pytest.mark.asyncio
+ async def test_login_with_personal_access_token(self, unique_name):
+ """Test PAT login using a token created directly through HTTP."""
+ name = unique_name(min_bytes=16, max_bytes=30)
+ http_host, http_port = get_http_server_config()
+ api = f"http://{http_host}:{http_port}"
+ login_req = urllib.request.Request( # noqa: S310
+ f"{api}/users/login",
+ data=json.dumps({"username": "iggy", "password": "iggy"}).encode(),
+ headers={"Content-Type": "application/json"},
+ method="POST",
+ )
+ with urllib.request.urlopen(login_req, timeout=10) as response: # noqa: S310
+ jwt = json.loads(response.read())["access_token"]["token"]
+ create_req = urllib.request.Request( # noqa: S310
+ f"{api}/personal-access-tokens",
+ data=json.dumps({"name": name, "expiry": NEVER_EXPIRE}).encode(),
+ headers={
+ "Content-Type": "application/json",
+ "Authorization": f"Bearer {jwt}",
+ },
+ method="POST",
+ )
+ with urllib.request.urlopen(create_req, timeout=10) as response: # noqa: S310
+ personal_access_token = json.loads(response.read())["token"]
+
+ try:
+ host, port = get_server_config()
+ client = IggyClient(f"{host}:{port}")
+ await client.connect()
+ identity = await client.login_with_personal_access_token(
+ personal_access_token
+ )
+
+ assert isinstance(identity, IdentityInfo)
+ assert identity.access_token is None
+ user = await client.get_user("iggy")
+ assert user is not None
+ assert identity.user_id == user.id
+ assert personal_access_token not in repr(identity)
+ finally:
+ delete_req = urllib.request.Request( # noqa: S310
+ f"{api}/personal-access-tokens/{name}",
+ headers={"Authorization": f"Bearer {jwt}"},
+ method="DELETE",
+ )
+ with urllib.request.urlopen(delete_req, timeout=10): # noqa: S310
+ pass
+
+ @pytest.mark.parametrize(
+ "token",
+ ["unknown-personal-access-token", "%%%garbled-token%%%"],
+ ids=["unknown", "garbled"],
+ )
+ @pytest.mark.asyncio
+ async def test_invalid_personal_access_token_fails(self, token):
+ """Test unknown and garbled PAT values are rejected."""
+ host, port = get_server_config()
+ client = IggyClient(f"{host}:{port}")
+ await client.connect()
+
+ with pytest.raises(RuntimeError):
+ await client.login_with_personal_access_token(token)
+
+ @pytest.mark.asyncio
+ async def test_expired_personal_access_token_fails(self, unique_name):
+ """Test an expired PAT cannot authenticate."""
+ name = unique_name(min_bytes=16, max_bytes=30)
+ http_host, http_port = get_http_server_config()
+ api = f"http://{http_host}:{http_port}"
+ jwt, personal_access_token = _create_personal_access_token(
+ api, "iggy", "iggy", name, 1
+ )
+
+ host, port = get_server_config()
+ client = IggyClient(f"{host}:{port}")
+ await client.connect()
+
+ try:
+ with pytest.raises(RuntimeError):
+ await client.login_with_personal_access_token(personal_access_token)
+ finally:
+ _delete_personal_access_token(api, jwt, name)
Review Comment:
Updated the expired-PAT cleanup to accept HTTP 404 when the server has already removed the token. Other HTTP errors are still raised. The full test_user.py suite passed locally (64 tests).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(python): return identity details from password and PAT login [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio merged PR #4276:
URL: https://github.com/apache/iggy/pull/4276
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org