Skip to content

feat(BA-4906): add REST and GraphQL schema for login session management#9719

Open
HyeockJinKim wants to merge 8 commits intomainfrom
BA-4906
Open

feat(BA-4906): add REST and GraphQL schema for login session management#9719
HyeockJinKim wants to merge 8 commits intomainfrom
BA-4906

Conversation

@HyeockJinKim
Copy link
Collaborator

Summary

  • Add REST endpoints (GET /login-sessions, DELETE /login-sessions/{session_id}) with auth_required middleware, returning NotImplementedError
  • Add login_security_policy field to PATCH /admin/users/{id} request schema with UserUpdaterSpec update
  • Add GraphQL types (LoginSessionGQL, LoginSecurityPolicyGQL, UpdateLoginSecurityPolicyInputGQL), queries (my_login_sessions), and mutations (update_user_login_security_policy, revoke_login_session) — all returning NotImplementedError pending integration story BA-4905

Test plan

  • GET /login-sessions returns NotImplementedError with auth_required middleware
  • DELETE /login-sessions/{session_id} returns NotImplementedError with auth_required middleware
  • PATCH /admin/users/{id} accepts login_security_policy field
  • GraphQL query my_login_sessions raises NotImplementedError
  • GraphQL mutations update_user_login_security_policy and revoke_login_session raise NotImplementedError
  • max_concurrent_logins validated as positive int or None (REST + GraphQL)

Resolves BA-4906

Copilot AI review requested due to automatic review settings March 5, 2026 16:36
HyeockJinKim added a commit that referenced this pull request Mar 5, 2026
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Contributor

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 REST and GraphQL schema definitions for login session management, including stub endpoints that raise NotImplementedError / NotImplementedAPI pending a future integration story (BA-4905). It also extends the user update schema to accept a login_security_policy field.

Changes:

  • New REST endpoints (GET /login-sessions, DELETE /login-sessions/{session_id}) with auth_required middleware, returning NotImplementedAPI
  • New GraphQL types, queries (my_login_sessions), and mutations (update_user_login_security_policy, revoke_login_session) all raising NotImplementedError
  • Extended UpdateUserRequest and UserUpdaterSpec with a login_security_policy field, wired through the REST adapter

Reviewed changes

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

Show a summary per file
File Description
changes/9719.feature.md Changelog entry for the new feature
src/ai/backend/common/dto/manager/login_session/__init__.py Package init with docstring for login session DTOs
src/ai/backend/common/dto/manager/login_session/request.py Request DTOs for revoke and update security policy
src/ai/backend/common/dto/manager/login_session/response.py Response DTOs for listing login sessions
src/ai/backend/common/dto/manager/user/request.py Added login_security_policy field to UpdateUserRequest
src/ai/backend/manager/dto/login_session_request.py Path parameter model for revoking a login session
src/ai/backend/manager/repositories/user/updaters.py Added login_security_policy to UserUpdaterSpec
src/ai/backend/manager/api/rest/user/adapter.py Wired login_security_policy through build_updater
src/ai/backend/manager/api/rest/login_session/__init__.py Package init for login session REST module
src/ai/backend/manager/api/rest/login_session/handler.py Stub handler raising NotImplementedAPI
src/ai/backend/manager/api/rest/login_session/registry.py Route registration for login session endpoints
src/ai/backend/manager/api/rest/tree.py Registered login session routes in the API tree
src/ai/backend/manager/api/gql/login_session/__init__.py Package init for login session GraphQL module
src/ai/backend/manager/api/gql/login_session/types/__init__.py Exports for GraphQL types
src/ai/backend/manager/api/gql/login_session/types/node.py LoginSessionGQL and LoginSecurityPolicyGQL types
src/ai/backend/manager/api/gql/login_session/types/inputs.py UpdateLoginSecurityPolicyInputGQL input with validation
src/ai/backend/manager/api/gql/login_session/types/payloads.py Mutation payload types
src/ai/backend/manager/api/gql/login_session/resolver/__init__.py Exports for GraphQL resolvers
src/ai/backend/manager/api/gql/login_session/resolver/query.py my_login_sessions query stub
src/ai/backend/manager/api/gql/login_session/resolver/mutation.py Mutation stubs for security policy and session revocation
src/ai/backend/manager/api/gql/schema.py Wired login session query/mutations into the root schema

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

container_main_gid: TriState[int] = field(default_factory=TriState.nop)
container_gids: TriState[list[int]] = field(default_factory=TriState.nop)
group_ids: OptionalState[list[str]] = field(default_factory=OptionalState.nop)
login_security_policy: OptionalState[dict[str, Any]] = field(default_factory=OptionalState.nop)
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

The new login_security_policy field is added to UserUpdaterSpec, but build_values() (visible in the context excerpt) never calls self.login_security_policy.update_dict(to_update, ...). This means even when a caller sets a login security policy, it will be silently ignored and never persisted to the database. You need to add self.login_security_policy.update_dict(to_update, "login_security_policy") inside build_values().

Copilot uses AI. Check for mistakes.
Comment on lines 85 to 89
login_security_policy: dict[str, Any] | None = Field(
default=None, description="Login security policy settings (e.g. max_concurrent_logins)"
)


Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

The REST UpdateUserRequest accepts login_security_policy as a free-form dict[str, Any] with no validation, while the GraphQL input (UpdateLoginSecurityPolicyInputGQL) validates max_concurrent_logins > 0. This inconsistency means the REST path silently accepts invalid values (e.g., {"max_concurrent_logins": -1} or arbitrary keys). Consider defining a dedicated Pydantic model (similar to UpdateLoginSecurityPolicyRequest which already has gt=0) and reusing it here to ensure consistent validation across both interfaces.

Suggested change
login_security_policy: dict[str, Any] | None = Field(
default=None, description="Login security policy settings (e.g. max_concurrent_logins)"
)
login_security_policy: "LoginSecurityPolicyUpdateRequest" | None = Field(
default=None, description="Login security policy settings (e.g. max_concurrent_logins)"
)
class LoginSecurityPolicyUpdateRequest(BaseRequestModel):
"""Login security policy settings for updating a user via REST."""
max_concurrent_logins: int | None = Field(
default=None,
gt=0,
description="Maximum number of concurrent logins allowed for the user",
)

Copilot uses AI. Check for mistakes.

id: UUID = strawberry.field(description="Unique identifier of the login session.")
session_token: str = strawberry.field(description="Opaque session token.")
client_ip: str = strawberry.field(
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

In the GraphQL type LoginSessionGQL, client_ip is declared as a non-nullable str, but in the corresponding REST response DTO LoginSessionItemResponse, client_ip is str | None (with default=None). This inconsistency will cause issues when the actual implementation populates these types — sessions without a client IP will fail to serialize in GraphQL. Consider making client_ip optional (str | None) here to match the REST DTO.

Suggested change
client_ip: str = strawberry.field(
client_ip: str | None = strawberry.field(

Copilot uses AI. Check for mistakes.
HyeockJinKim and others added 7 commits March 6, 2026 11:58
Add Pydantic request/response DTOs under common/dto/manager/login_session/:
- RevokeLoginSessionRequest: session_id field
- UpdateLoginSecurityPolicyRequest: max_concurrent_logins (positive int or None)
- LoginSessionItemResponse: id, session_token, client_ip, created_at, expired_at, reason
- ListLoginSessionsResponse: items list

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ment

- Add LoginSessionHandler with list_sessions() and revoke_session() stubs
- Add register_login_session_routes() mounting at /login-sessions prefix
- Add RevokeLoginSessionPathParam for DELETE /{session_id} path param
- Wire LoginSessionHandler into build_api_routes() in tree.py
- Both endpoints raise NotImplementedAPI with auth_required middleware

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…chema

- Add login_security_policy: dict[str, Any] | None to UpdateUserRequest DTO
- Add login_security_policy field to UserUpdaterSpec (OptionalState[dict[str, Any]])
- Pass login_security_policy through in UserAdapter.build_updater()

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add LoginSessionGQL, LoginSecurityPolicyGQL node types,
UpdateLoginSecurityPolicyInputGQL input type, and mutation
payload types (UpdateUserLoginSecurityPolicyPayloadGQL,
RevokeLoginSessionPayloadGQL).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n session management

- Add my_login_sessions query resolver (raises NotImplementedError)
- Add update_user_login_security_policy mutation resolver (admin only, raises NotImplementedError)
- Add revoke_login_session mutation resolver (raises NotImplementedError)
- Register all resolvers in api/gql/schema.py Query and Mutation classes
- Add __post_init__ validator to UpdateLoginSecurityPolicyInputGQL for positive int validation
- Update login_session package __init__.py with resolver re-exports

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ity, login_security_policy validation

- Add login_security_policy to UserUpdaterSpec.build_values() so the field is
  persisted to the DB on update
- Change LoginSessionGQL.client_ip from str to str | None to match REST DTO
- Replace dict[str, Any] in UpdateUserRequest.login_security_policy with a
  dedicated LoginSecurityPolicyRequest Pydantic model that validates
  max_concurrent_logins > 0
- Fix check_admin_only() call in GQL mutation resolver (no argument)
- Rebase onto origin/main and resolve tree.py conflict

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@github-actions github-actions bot added size:L 100~500 LoC comp:manager Related to Manager component comp:common Related to Common component labels Mar 6, 2026
Co-authored-by: octodog <mu001@lablup.com>
@github-actions github-actions bot added size:XL 500~ LoC area:docs Documentations and removed size:L 100~500 LoC labels Mar 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:docs Documentations comp:common Related to Common component comp:manager Related to Manager component size:XL 500~ LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants