SwissDataScienceCenter / renku-data-services

Services that handle reading and writing data from a database
Apache License 2.0
3 stars 2 forks source link

Use @dataclass(frozen=True) on APIUser dataclass #409

Open olevski opened 6 days ago

olevski commented 6 days ago

Currently we do a bit of unecessary gymnastics to prevent someone simply flipping the is_admin flag on an api user instance in the code.

But we can eliminate all of this code by simply making the dataclass frozen.

@dataclass(kw_only=True)
class APIUser:
    """The model for a user of the API, used for authentication."""

    id: Optional[str] = None  # the sub claim in the access token - i.e. the Keycloak user ID
    access_token: Optional[str] = field(repr=False, default=None)
    refresh_token: Optional[str] = field(repr=False, default=None)
    full_name: Optional[str] = None
    first_name: Optional[str] = None
    last_name: Optional[str] = None
    email: Optional[str] = None
    access_token_expires_at: datetime | None = None
    is_admin_init: InitVar[bool] = False
    __is_admin: bool = field(init=False, repr=False)

    def __post_init__(self, is_admin_init: bool) -> None:
        self.__is_admin: bool = is_admin_init

    @property
    def is_authenticated(self) -> bool:
        """Indicates whether the user has successfully logged in."""
        return self.id is not None

    @property
    def is_admin(self) -> bool:
        """Indicates whether the user is a Renku platform administrator."""
        return self.__is_admin

Instead of the above we can simply use the stuff below.

@dataclass(kw_only=True, frozen=True)
class APIUser:
    """The model for a user of the API, used for authentication."""

    id: Optional[str] = None  # the sub claim in the access token - i.e. the Keycloak user ID
    access_token: Optional[str] = field(repr=False, default=None)
    refresh_token: Optional[str] = field(repr=False, default=None)
    full_name: Optional[str] = None
    first_name: Optional[str] = None
    last_name: Optional[str] = None
    email: Optional[str] = None
    access_token_expires_at: datetime | None = None
    is_admin: bool = False

    @property
    def is_authenticated(self) -> bool:
        """Indicates whether the user has successfully logged in."""
        return self.id is not None
olevski commented 6 days ago

Thanks to @sgaist for calling this out and also for suggesting how to fix the problem.

sgaist commented 5 days ago

After a quick test and some fixes, frozen dataclass are working but mypy start screaming.