Skip to content

Conversation

@xogas
Copy link

@xogas xogas commented Nov 26, 2025

No description provided.

@xogas xogas force-pushed the feat/bkpaas_auth-add-time-zone branch from 6ec8a35 to feda41a Compare November 26, 2025 06:23

def _create_authenticated_user(
self, username: str, provider_type: ProviderType, tenant_id: str | None = None
self, username: str, provider_type: ProviderType, time_zone: str | None = None, tenant_id: str | None = None
Copy link
Contributor

Choose a reason for hiding this comment

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

APIGatewayAuthBackend 似乎与用户 time_zone 无关,网关JWT 里不会传递用户 time_zone

下同

Copy link
Author

Choose a reason for hiding this comment

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

已修改

@xogas xogas requested a review from nannan00 November 28, 2025 06:15
"data": {
"bk_username": settings.USER_NAME,
"LoginName": settings.USER_NAME,
"ChineseName": settings.USER_NICKNAME,
Copy link
Collaborator

Choose a reason for hiding this comment

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

从使用者的角度,这个 SDK 有必要提供其他和用户时区相关的 methods/functions,来让功能更易用吗?

Copilot AI review requested due to automatic review settings December 3, 2025 10:34
Copilot finished reviewing on behalf of xogas December 3, 2025 10:38
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a time_zone field to the User model in the bkpaas-auth SDK, enabling user timezone information to be retrieved from authentication APIs and stored in user objects. The implementation follows the existing pattern used for the tenant_id field, treating time_zone as an optional field that defaults to None when not provided by the API.

Key changes:

  • Added time_zone as an optional field throughout the authentication flow (UserAccount, UserInfo, User model)
  • Updated test fixtures and added comprehensive test coverage for both presence and absence of the field
  • Modified function signature defaults in APIGatewayAuthBackend.authenticate_with_signature_v3

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
bkpaas_auth/models.py Added time_zone to USERINFO_FIELDS tuple for dynamic field population
bkpaas_auth/core/user_info.py Added time_zone extraction from kwargs in UserInfo.__init__
bkpaas_auth/core/token.py Added time_zone field to UserAccount NamedTuple with documentation; updated _request_apigw to extract time_zone from API response
bkpaas_auth/backends.py Passed time_zone to UserInfo in authentication flow; added default values to authenticate_with_signature_v3 parameters
tests/conftest.py Updated get_rtx_user_info_response fixture to include time_zone; added new fixture get_rtx_user_info_response_without_time_zone
tests/test_models.py Added assertions for time_zone field; added new test test_user_info_without_time_zone to verify graceful handling of missing field
tests/test_backends.py Added time_zone assertion to tenant mode test; added new test test_authenticate_bk_token_for_tenant_mode_without_time_zone for missing field scenario

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@xogas xogas requested review from jiayuan929 and piglei December 4, 2025 03:41
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

Successfully merging this pull request may close these issues.

4 participants