Changed user logic to display name and refined requirements page design
This commit is contained in:
@@ -32,6 +32,8 @@ class User(Base):
|
||||
|
||||
id: Mapped[int] = mapped_column(Integer, primary_key=True, autoincrement=True)
|
||||
sub: Mapped[str] = mapped_column(Text, nullable=False, unique=True) # Keycloak subject ID
|
||||
username: Mapped[str] = mapped_column(Text, nullable=False) # Keycloak preferred_username
|
||||
full_name: Mapped[Optional[str]] = mapped_column(Text, nullable=True) # Keycloak name claim
|
||||
role_id: Mapped[int] = mapped_column(Integer, ForeignKey("roles.id"), nullable=False)
|
||||
created_at: Mapped[datetime] = mapped_column(
|
||||
DateTime(timezone=True),
|
||||
|
||||
@@ -314,6 +314,20 @@ def _require_role(user, allowed_role_ids: List[int], action: str = "perform this
|
||||
)
|
||||
|
||||
|
||||
def _get_display_name(user) -> str:
|
||||
"""
|
||||
Get the best display name for a user.
|
||||
Falls back in order: full_name -> username -> sub.
|
||||
|
||||
Args:
|
||||
user: The database user object
|
||||
|
||||
Returns:
|
||||
The best available display name for the user
|
||||
"""
|
||||
return user.full_name or user.username or user.sub
|
||||
|
||||
|
||||
async def _verify_project_membership(project_id: int, user_id: int, db: AsyncSession):
|
||||
"""Helper to verify user is a member of a project."""
|
||||
project_repo = ProjectRepository(db)
|
||||
@@ -570,7 +584,7 @@ async def get_project_members(
|
||||
return [
|
||||
ProjectMemberResponse(
|
||||
id=member.id,
|
||||
sub=member.sub,
|
||||
sub=_get_display_name(member),
|
||||
role_id=member.role_id,
|
||||
role_name=member.role.role_name if member.role else "unknown",
|
||||
role_display_name=ROLE_DISPLAY_NAMES.get(member.role.role_name, member.role.role_name.title()) if member.role else "Unknown",
|
||||
@@ -647,7 +661,7 @@ async def update_member_role(
|
||||
|
||||
return ProjectMemberResponse(
|
||||
id=updated_user.id,
|
||||
sub=updated_user.sub,
|
||||
sub=_get_display_name(updated_user),
|
||||
role_id=updated_user.role_id,
|
||||
role_name=role.role_name,
|
||||
role_display_name=ROLE_DISPLAY_NAMES.get(role.role_name, role.role_name.title()),
|
||||
@@ -772,9 +786,9 @@ def _build_requirement_response(req) -> RequirementResponse:
|
||||
# Get the latest validation
|
||||
latest_validation = max(req.validations, key=lambda v: v.created_at or req.created_at)
|
||||
validation_status = latest_validation.status.status_name if latest_validation.status else "Not Validated"
|
||||
# Try to get username from user relationship
|
||||
# Try to get display name from user relationship
|
||||
if latest_validation.user:
|
||||
validated_by = latest_validation.user.sub
|
||||
validated_by = _get_display_name(latest_validation.user)
|
||||
validated_at = latest_validation.created_at
|
||||
validation_version = latest_validation.req_version_snapshot
|
||||
|
||||
@@ -1112,7 +1126,7 @@ async def create_validation(
|
||||
req_version_snapshot=validation.req_version_snapshot,
|
||||
comment=validation.comment,
|
||||
created_at=validation.created_at,
|
||||
validator_username=user.sub,
|
||||
validator_username=_get_display_name(user),
|
||||
validator_id=user.id
|
||||
)
|
||||
|
||||
@@ -1159,7 +1173,7 @@ async def get_validation_history(
|
||||
req_version_snapshot=v.req_version_snapshot,
|
||||
comment=v.comment,
|
||||
created_at=v.created_at,
|
||||
validator_username=v.user.sub,
|
||||
validator_username=_get_display_name(v.user),
|
||||
validator_id=v.user_id
|
||||
)
|
||||
for v in validations
|
||||
@@ -1416,7 +1430,7 @@ async def create_requirement_link(
|
||||
"req_name": target_req.req_name,
|
||||
"tag_code": target_req.tag.tag_code
|
||||
},
|
||||
created_by_username=user.sub,
|
||||
created_by_username=_get_display_name(user),
|
||||
created_by_id=user.id,
|
||||
created_at=link.created_at
|
||||
)
|
||||
|
||||
@@ -8,6 +8,13 @@ from sqlalchemy.ext.asyncio import AsyncSession
|
||||
from src.db_models import RequirementLink, Requirement, RelationshipType, User
|
||||
|
||||
|
||||
def _get_creator_display_name(user: User | None) -> str | None:
|
||||
"""Get the best display name for a creator user."""
|
||||
if not user:
|
||||
return None
|
||||
return user.full_name or user.username or user.sub
|
||||
|
||||
|
||||
class RequirementLinkRepository:
|
||||
"""Repository for requirement link CRUD operations."""
|
||||
|
||||
@@ -66,7 +73,7 @@ class RequirementLinkRepository:
|
||||
"req_name": link.target_requirement.req_name,
|
||||
"tag_code": link.target_requirement.tag.tag_code
|
||||
},
|
||||
"created_by_username": link.creator.sub if link.creator else None,
|
||||
"created_by_username": _get_creator_display_name(link.creator),
|
||||
"created_by_id": link.created_by,
|
||||
"created_at": link.created_at
|
||||
})
|
||||
@@ -83,7 +90,7 @@ class RequirementLinkRepository:
|
||||
"req_name": link.source_requirement.req_name,
|
||||
"tag_code": link.source_requirement.tag.tag_code
|
||||
},
|
||||
"created_by_username": link.creator.sub if link.creator else None,
|
||||
"created_by_username": _get_creator_display_name(link.creator),
|
||||
"created_by_id": link.created_by,
|
||||
"created_at": link.created_at
|
||||
})
|
||||
|
||||
@@ -111,18 +111,20 @@ class UserRepository:
|
||||
)
|
||||
return result.scalar_one_or_none()
|
||||
|
||||
async def create(self, sub: str, role_id: int) -> User:
|
||||
async def create(self, sub: str, role_id: int, username: str, full_name: str | None = None) -> User:
|
||||
"""
|
||||
Create a new user.
|
||||
|
||||
Args:
|
||||
sub: The Keycloak subject ID
|
||||
role_id: The role ID to assign
|
||||
username: The Keycloak preferred_username
|
||||
full_name: The Keycloak name claim (optional)
|
||||
|
||||
Returns:
|
||||
The created User
|
||||
"""
|
||||
user = User(sub=sub, role_id=role_id)
|
||||
user = User(sub=sub, role_id=role_id, username=username, full_name=full_name)
|
||||
self.session.add(user)
|
||||
await self.session.flush()
|
||||
await self.session.refresh(user)
|
||||
@@ -148,6 +150,25 @@ class UserRepository:
|
||||
await self.session.refresh(user)
|
||||
return user
|
||||
|
||||
async def update_profile(self, user: User, username: str, full_name: str | None = None) -> User:
|
||||
"""
|
||||
Update a user's profile info (username and full_name) from Keycloak.
|
||||
Called on subsequent logins to sync changes from Keycloak.
|
||||
|
||||
Args:
|
||||
user: The user to update
|
||||
username: The Keycloak preferred_username
|
||||
full_name: The Keycloak name claim (optional)
|
||||
|
||||
Returns:
|
||||
The updated User
|
||||
"""
|
||||
user.username = username
|
||||
user.full_name = full_name
|
||||
await self.session.flush()
|
||||
await self.session.refresh(user)
|
||||
return user
|
||||
|
||||
async def get_or_create_default_role(self) -> Role:
|
||||
"""
|
||||
Get the default user role, creating it if it doesn't exist.
|
||||
@@ -164,13 +185,16 @@ class UserRepository:
|
||||
|
||||
return role
|
||||
|
||||
async def get_or_create_user(self, sub: str) -> tuple[User, bool]:
|
||||
async def get_or_create_user(self, sub: str, username: str, full_name: str | None = None) -> tuple[User, bool]:
|
||||
"""
|
||||
Get an existing user or create a new one (Just-in-Time Provisioning).
|
||||
This is the main method called during login.
|
||||
Also updates username/full_name on subsequent logins to sync with Keycloak.
|
||||
|
||||
Args:
|
||||
sub: The Keycloak subject ID
|
||||
username: The Keycloak preferred_username
|
||||
full_name: The Keycloak name claim (optional)
|
||||
|
||||
Returns:
|
||||
Tuple of (User, created) where created is True if a new user was created
|
||||
@@ -180,6 +204,8 @@ class UserRepository:
|
||||
|
||||
if user is not None:
|
||||
logger.debug(f"Found existing user with sub: {sub}")
|
||||
# Update profile info on subsequent logins to sync with Keycloak
|
||||
user = await self.update_profile(user, username, full_name)
|
||||
return user, False
|
||||
|
||||
# User doesn't exist, create them with default role
|
||||
@@ -189,7 +215,7 @@ class UserRepository:
|
||||
default_role = await self.get_or_create_default_role()
|
||||
|
||||
# Create the user
|
||||
user = await self.create(sub=sub, role_id=default_role.id)
|
||||
user = await self.create(sub=sub, role_id=default_role.id, username=username, full_name=full_name)
|
||||
|
||||
logger.info(f"Created new user with id: {user.id}, sub: {sub}")
|
||||
logger.info(f"Created new user with id: {user.id}, sub: {sub}, username: {username}")
|
||||
return user, True
|
||||
|
||||
@@ -126,6 +126,7 @@ class UserService:
|
||||
) -> tuple[int, bool]:
|
||||
"""
|
||||
Provision a user in the database on first login (JIT provisioning).
|
||||
Also updates username/full_name on subsequent logins to sync with Keycloak.
|
||||
|
||||
Args:
|
||||
token: The access token from Keycloak
|
||||
@@ -134,9 +135,11 @@ class UserService:
|
||||
Returns:
|
||||
Tuple of (user_id, is_new_user)
|
||||
"""
|
||||
# Decode the token to get the 'sub' claim
|
||||
# Decode the token to get user claims
|
||||
token_info = AuthService.decode_token(token)
|
||||
sub = token_info.get("sub")
|
||||
username = token_info.get("preferred_username", "unknown")
|
||||
full_name = token_info.get("name") # Keycloak uses 'name' claim for full name
|
||||
|
||||
if not sub:
|
||||
raise HTTPException(
|
||||
@@ -144,13 +147,13 @@ class UserService:
|
||||
detail="Token does not contain 'sub' claim"
|
||||
)
|
||||
|
||||
# Get or create the user
|
||||
# Get or create the user (will also update profile on subsequent logins)
|
||||
user_repo = UserRepository(db)
|
||||
user, created = await user_repo.get_or_create_user(sub)
|
||||
user, created = await user_repo.get_or_create_user(sub, username, full_name)
|
||||
|
||||
if created:
|
||||
logger.info(f"New user provisioned: {sub} -> user_id: {user.id}")
|
||||
logger.info(f"New user provisioned: {sub} -> user_id: {user.id}, username: {username}")
|
||||
else:
|
||||
logger.debug(f"Existing user logged in: {sub} -> user_id: {user.id}")
|
||||
logger.debug(f"Existing user logged in: {sub} -> user_id: {user.id}, username: {username}")
|
||||
|
||||
return user.id, created
|
||||
|
||||
Reference in New Issue
Block a user