From ed5f89913ba6945fff41d4e11a1773bb2ffd8a1b Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Mon, 4 Nov 2024 15:53:04 -0800 Subject: [PATCH 01/19] Add BaseModel for JWT and `HanaToken` (to use for auth and refresh) Update TokenConfig model to reflect expected database schema Refactor AccessRoles values to allow for more in-between values Add methods to read/dump PermissionsConfig to JWT-compatible role strings --- neon_data_models/models/user/database.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/neon_data_models/models/user/database.py b/neon_data_models/models/user/database.py index af6f5fc..6399543 100644 --- a/neon_data_models/models/user/database.py +++ b/neon_data_models/models/user/database.py @@ -150,15 +150,21 @@ def to_roles(self): @deprecated(f"Use `neon_data_models.models.api.jwt.HanaToken`") class TokenConfig(BaseModel): - username: str - client_id: str - permissions: Dict[str, bool] - refresh_token: str - expiration: int = Field( - description="Unix timestamp of auth token expiration") + """ + Data model for storing token data in the user database. Note that the actual + tokens are not included here, only metadata used to validate or invalidate a + token and present a list of issued tokens to the user. + """ + token_name: str = Field(description="Human-readable token identifier") + token_id: str = Field(description="Unique token identifier", alias="jti") + user_id: str = Field(description="User ID the token is associated with", + alias="sub") + client_id: str = Field(description="Client ID the token is associated with") + permissions: PermissionsConfig = Field( + description="Permissions for this token " + "(overrides user-level permissions)") refresh_expiration: int = Field( description="Unix timestamp of refresh token expiration") - token_name: str creation_timestamp: int = Field( description="Unix timestamp of token creation (auth+refresh)") last_refresh_timestamp: int = Field( From d3ad7436f0fc66d3f5b69b8a86e5a70c6c00f679 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Mon, 4 Nov 2024 16:38:05 -0800 Subject: [PATCH 02/19] Update TokenConfig to better accept alternate named parameters with comment Update unit test to account for token config change --- neon_data_models/models/user/database.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/neon_data_models/models/user/database.py b/neon_data_models/models/user/database.py index 6399543..36ed37f 100644 --- a/neon_data_models/models/user/database.py +++ b/neon_data_models/models/user/database.py @@ -155,15 +155,23 @@ class TokenConfig(BaseModel): tokens are not included here, only metadata used to validate or invalidate a token and present a list of issued tokens to the user. """ + def __init__(self, **kwargs): + # The JWT standard uses "jti" and "sub" for encoded values. Outside of + # that context, those keys are not very descriptive, so we use our own + if jti := kwargs.get("jti"): + kwargs.setdefault("token_id", jti) + if sub := kwargs.get("sub"): + kwargs.setdefault("user_id", sub) + BaseModel.__init__(self, **kwargs) + token_name: str = Field(description="Human-readable token identifier") - token_id: str = Field(description="Unique token identifier", alias="jti") - user_id: str = Field(description="User ID the token is associated with", - alias="sub") + token_id: str = Field(description="Unique token identifier") + user_id: str = Field(description="User ID the token is associated with") client_id: str = Field(description="Client ID the token is associated with") permissions: PermissionsConfig = Field( description="Permissions for this token " "(overrides user-level permissions)") - refresh_expiration: int = Field( + refresh_expiration_timestamp: int = Field( description="Unix timestamp of refresh token expiration") creation_timestamp: int = Field( description="Unix timestamp of token creation (auth+refresh)") From 7c8fa476f7983dec762fc502eed641cbcb083cb9 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Wed, 6 Nov 2024 15:58:47 -0800 Subject: [PATCH 03/19] Update TokenConfig to handle JWT standard field names Add test coverage for TokenConfig --- neon_data_models/models/user/database.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/neon_data_models/models/user/database.py b/neon_data_models/models/user/database.py index 36ed37f..0089e83 100644 --- a/neon_data_models/models/user/database.py +++ b/neon_data_models/models/user/database.py @@ -162,6 +162,10 @@ def __init__(self, **kwargs): kwargs.setdefault("token_id", jti) if sub := kwargs.get("sub"): kwargs.setdefault("user_id", sub) + if iat := kwargs.get("iat"): + kwargs.setdefault("creation_timestamp", iat) + if exp := kwargs.get("exp"): + kwargs.setdefault("refresh_expiration_timestamp", exp) BaseModel.__init__(self, **kwargs) token_name: str = Field(description="Human-readable token identifier") From f7eb6c989a5a79507fc44388a0566439fb85bc00 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Wed, 6 Nov 2024 16:05:17 -0800 Subject: [PATCH 04/19] Update comment in TokenConfig --- neon_data_models/models/user/database.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/neon_data_models/models/user/database.py b/neon_data_models/models/user/database.py index 0089e83..8fcb754 100644 --- a/neon_data_models/models/user/database.py +++ b/neon_data_models/models/user/database.py @@ -156,8 +156,8 @@ class TokenConfig(BaseModel): token and present a list of issued tokens to the user. """ def __init__(self, **kwargs): - # The JWT standard uses "jti" and "sub" for encoded values. Outside of - # that context, those keys are not very descriptive, so we use our own + # The JWT standard uses these standard keys; outside of that context, + # they are not very descriptive, so the database uses its own schema if jti := kwargs.get("jti"): kwargs.setdefault("token_id", jti) if sub := kwargs.get("sub"): From 5026c470a5996ee59fd9a721adc420761d003114 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Mon, 11 Nov 2024 17:30:56 -0800 Subject: [PATCH 05/19] Mark `TokenConfig` as deprecated Refactor `HanaToken` to include params previously used in `TokenConfig` --- neon_data_models/models/user/database.py | 37 +++++++----------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/neon_data_models/models/user/database.py b/neon_data_models/models/user/database.py index 8fcb754..3301267 100644 --- a/neon_data_models/models/user/database.py +++ b/neon_data_models/models/user/database.py @@ -150,33 +150,18 @@ def to_roles(self): @deprecated(f"Use `neon_data_models.models.api.jwt.HanaToken`") class TokenConfig(BaseModel): - """ - Data model for storing token data in the user database. Note that the actual - tokens are not included here, only metadata used to validate or invalidate a - token and present a list of issued tokens to the user. - """ - def __init__(self, **kwargs): - # The JWT standard uses these standard keys; outside of that context, - # they are not very descriptive, so the database uses its own schema - if jti := kwargs.get("jti"): - kwargs.setdefault("token_id", jti) - if sub := kwargs.get("sub"): - kwargs.setdefault("user_id", sub) - if iat := kwargs.get("iat"): - kwargs.setdefault("creation_timestamp", iat) - if exp := kwargs.get("exp"): - kwargs.setdefault("refresh_expiration_timestamp", exp) - BaseModel.__init__(self, **kwargs) - - token_name: str = Field(description="Human-readable token identifier") - token_id: str = Field(description="Unique token identifier") - user_id: str = Field(description="User ID the token is associated with") - client_id: str = Field(description="Client ID the token is associated with") - permissions: PermissionsConfig = Field( - description="Permissions for this token " - "(overrides user-level permissions)") - refresh_expiration_timestamp: int = Field( + from ovos_utils.log import log_deprecation + log_deprecation("Use `neon_data_models.models.api.jwt.HanaToken`", + "0.0.1") + username: str + client_id: str + permissions: Dict[str, bool] + refresh_token: str + expiration: int = Field( + description="Unix timestamp of auth token expiration") + refresh_expiration: int = Field( description="Unix timestamp of refresh token expiration") + token_name: str creation_timestamp: int = Field( description="Unix timestamp of token creation (auth+refresh)") last_refresh_timestamp: int = Field( From 4eac171b81b0c06e11dee7c38fdf7eba3afd1281 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Mon, 11 Nov 2024 17:57:48 -0800 Subject: [PATCH 06/19] Update deprecation warning to use builtin decorator --- neon_data_models/models/user/database.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/neon_data_models/models/user/database.py b/neon_data_models/models/user/database.py index 3301267..af6f5fc 100644 --- a/neon_data_models/models/user/database.py +++ b/neon_data_models/models/user/database.py @@ -150,9 +150,6 @@ def to_roles(self): @deprecated(f"Use `neon_data_models.models.api.jwt.HanaToken`") class TokenConfig(BaseModel): - from ovos_utils.log import log_deprecation - log_deprecation("Use `neon_data_models.models.api.jwt.HanaToken`", - "0.0.1") username: str client_id: str permissions: Dict[str, bool] From e5c0d348c97e94b4ae7a8de4fbaf3c1f325224d3 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Thu, 7 Nov 2024 18:25:04 -0800 Subject: [PATCH 07/19] Refactors `UserDbRequest` into seprate schemas for CRUD operations Maintains compat. with users service by determining model based on the requested `operation` --- neon_data_models/models/api/mq.py | 56 ++++++++++++++++++++++++++----- tests/models/api/test_mq.py | 42 +++++++++++++++++++++++ 2 files changed, 89 insertions(+), 9 deletions(-) diff --git a/neon_data_models/models/api/mq.py b/neon_data_models/models/api/mq.py index 37602b3..fe1144b 100644 --- a/neon_data_models/models/api/mq.py +++ b/neon_data_models/models/api/mq.py @@ -24,17 +24,55 @@ # NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -from typing import Literal, Optional +from typing import Literal, Optional, Annotated, Union + +from pydantic import Field, TypeAdapter + from neon_data_models.models.base.contexts import MQContext -from neon_data_models.models.user.database import User +from neon_data_models.models.user.database import User, TokenConfig + + +class CreateUserRequest(MQContext): + operation: Literal["create"] = "create" + user: User = Field(description="User object to create") + # TODO: Support optional auth + + +class ReadUserRequest(MQContext): + operation: Literal["read"] = "read" + user_spec: str = Field(description="Username or User ID to read") + access_token: Optional[TokenConfig] = Field( + None, description="Token associated with `user_spec`") + password: Optional[str] = Field(None, + description="Password associated with " + "`user_spec`") + + +class UpdateUserRequest(MQContext): + operation: Literal["update"] = "update" + user: User = Field(description="Updated User object to write to database") + password: str = Field(description="Password associated with `user_spec`") + + +class DeleteUserRequest(MQContext): + operation: Literal["delete"] = "delete" + user: User = Field(description="Exact User object to remove from the " + "database") + +class UserDbRequest: + """ + Generic class to dynamically build a UserDB CRUD request object based on the + requested `operation` + """ + ta = TypeAdapter(Annotated[Union[CreateUserRequest, ReadUserRequest, + UpdateUserRequest, DeleteUserRequest], + Field(discriminator='operation')]) -class UserDbRequest(MQContext): - operation: Literal["create", "read", "update", "delete"] - username: str - password: Optional[str] = None - access_token: Optional[str] = None - user: Optional[User] = None + def __new__(cls, *args, **kwargs): + return cls.ta.validate_python(kwargs) -__all__ = [UserDbRequest.__name__] +__all__ = [CreateUserRequest.__name__, ReadUserRequest.__name__, + UpdateUserRequest.__name__, DeleteUserRequest.__name__, + UserDbRequest.__name__] diff --git a/tests/models/api/test_mq.py b/tests/models/api/test_mq.py index 610dc27..627f802 100644 --- a/tests/models/api/test_mq.py +++ b/tests/models/api/test_mq.py @@ -42,3 +42,45 @@ def test_user_db_request(self): user="test_user", message_id="test") with self.assertRaises(ValidationError): UserDbRequest(operation="create", username="test_user") + + def test_create_user_db_request(self): + from neon_data_models.models.api.mq import CreateUserRequest + + # Test create user valid + valid_kwargs = {"message_id": "test_id", "operation": "create", + "user": {"username": "test_user"}} + create_request = CreateUserRequest(**valid_kwargs) + self.assertIsInstance(create_request, CreateUserRequest) + generic_request = UserDbRequest(**valid_kwargs) + self.assertIsInstance(generic_request, CreateUserRequest) + self.assertEqual(generic_request.user.username, + create_request.user.username) + + # Test invalid + with self.assertRaises(ValidationError): + UserDbRequest(operation="create", message_id="test0") + + def test_read_user_db_request(self): + from neon_data_models.models.api.mq import ReadUserRequest + + # Test read user valid + valid_kwargs = {"message_id": "test_id", "operation": "read", + "user_spec": "test_user"} + read_request = ReadUserRequest(**valid_kwargs) + self.assertIsInstance(read_request, ReadUserRequest) + generic_request = UserDbRequest(**valid_kwargs) + self.assertIsInstance(generic_request, ReadUserRequest) + self.assertEqual(generic_request.user_spec, + read_request.user_spec) + + # Test invalid + with self.assertRaises(ValidationError): + UserDbRequest(operation="create", message_id="test0") + + def test_update_user_db_request(self): + from neon_data_models.models.api.mq import UpdateUserRequest + # TODO + + def test_delete_user_db_request(self): + from neon_data_models.models.api.mq import DeleteUserRequest + # TODO From f64aea358fb69099337806e66e779afbeb007fe1 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Fri, 8 Nov 2024 10:22:37 -0800 Subject: [PATCH 08/19] Add optional authentication to `UpdateUserRequest` model to allow for admin-authenticated changes Finish test coverage for MQ user database CRUD models --- neon_data_models/models/api/mq.py | 24 +++++++++- tests/models/api/test_mq.py | 74 ++++++++++++++++++++++++------- 2 files changed, 80 insertions(+), 18 deletions(-) diff --git a/neon_data_models/models/api/mq.py b/neon_data_models/models/api/mq.py index fe1144b..9f22017 100644 --- a/neon_data_models/models/api/mq.py +++ b/neon_data_models/models/api/mq.py @@ -26,7 +26,7 @@ from typing import Literal, Optional, Annotated, Union -from pydantic import Field, TypeAdapter +from pydantic import Field, TypeAdapter, model_validator from neon_data_models.models.base.contexts import MQContext from neon_data_models.models.user.database import User, TokenConfig @@ -51,7 +51,27 @@ class ReadUserRequest(MQContext): class UpdateUserRequest(MQContext): operation: Literal["update"] = "update" user: User = Field(description="Updated User object to write to database") - password: str = Field(description="Password associated with `user_spec`") + auth_username: str = Field(default="", + description="Username to authorize database " + "change. If unset, this will use " + "`user.username`") + auth_password: str = Field(default="", + description="Password (clear or hashed) associated " + "with `auth_username`. If unset, this " + "will use `user.password_hash`. If " + "changing the password, this must " + "contain the existing password, with " + "the new password specified in `user`") + + @model_validator(mode="after") + def get_auth_username(self) -> 'UpdateUserRequest': + if not self.auth_username: + self.auth_username = self.user.username + if not self.auth_password: + self.auth_password = self.user.password_hash + if not all((self.auth_username, self.auth_password)): + raise ValueError("Missing username or password") + return self class DeleteUserRequest(MQContext): diff --git a/tests/models/api/test_mq.py b/tests/models/api/test_mq.py index 627f802..caea6b2 100644 --- a/tests/models/api/test_mq.py +++ b/tests/models/api/test_mq.py @@ -31,18 +31,6 @@ class TestMQ(TestCase): - def test_user_db_request(self): - valid_model = UserDbRequest(operation="create", username="test_user", - message_id="test") - self.assertIsInstance(valid_model, UserDbRequest) - with self.assertRaises(ValidationError): - UserDbRequest(operation="get", username="test", message_id="test") - with self.assertRaises(ValidationError): - UserDbRequest(operation="delete", username="test_user", - user="test_user", message_id="test") - with self.assertRaises(ValidationError): - UserDbRequest(operation="create", username="test_user") - def test_create_user_db_request(self): from neon_data_models.models.api.mq import CreateUserRequest @@ -58,7 +46,8 @@ def test_create_user_db_request(self): # Test invalid with self.assertRaises(ValidationError): - UserDbRequest(operation="create", message_id="test0") + UserDbRequest(operation="create", username="test", + message_id="test0") def test_read_user_db_request(self): from neon_data_models.models.api.mq import ReadUserRequest @@ -75,12 +64,65 @@ def test_read_user_db_request(self): # Test invalid with self.assertRaises(ValidationError): - UserDbRequest(operation="create", message_id="test0") + UserDbRequest(operation="read", user={"username": "test"}, + message_id="test0") def test_update_user_db_request(self): from neon_data_models.models.api.mq import UpdateUserRequest - # TODO + + # Test update user valid + valid_kwargs = {"message_id": "test_id", "operation": "update", + "password": "test_password", + "user": {"username": "test_user", + "skills": {"skill_id": {"test": True}}}} + update_request = UpdateUserRequest(**valid_kwargs) + self.assertIsInstance(update_request, UpdateUserRequest) + self.assertEqual(update_request.auth_username, + update_request.user.username) + generic_request = UserDbRequest(**valid_kwargs) + self.assertIsInstance(generic_request, UpdateUserRequest) + self.assertEqual(generic_request.user.username, + update_request.user.username) + + # Test update read username/password from User object + update = UpdateUserRequest(message_id="test_id", operation="update", + user={"username": "user", + "password_hash": "password"}) + self.assertEqual(update.auth_username, "user") + self.assertEqual(update.auth_password, "password") + + # Test update with separate authentication user + update = UpdateUserRequest(message_id="test_id", operation="update", + user={"username": "user", + "password_hash": "password"}, + auth_username="admin", auth_password="admin_pass") + self.assertEqual(update.user.username, "user") + self.assertEqual(update.user.password_hash, "password") + + self.assertEqual(update.auth_username, "admin") + self.assertEqual(update.auth_password, "admin_pass") + + # Test invalid + with self.assertRaises(ValidationError): + UserDbRequest(operation="update", user={"username": "test_user", + "skills": {"skill_id": { + "test": True}}}, + message_id="test0") def test_delete_user_db_request(self): from neon_data_models.models.api.mq import DeleteUserRequest - # TODO + + # Test delete user valid + valid_kwargs = {"message_id": "test_id", "operation": "delete", + "user": {"username": "test_user"}} + delete_request = DeleteUserRequest(**valid_kwargs) + self.assertIsInstance(delete_request, DeleteUserRequest) + generic_request = UserDbRequest(**valid_kwargs) + self.assertIsInstance(generic_request, DeleteUserRequest) + self.assertEqual(generic_request.user.username, + delete_request.user.username) + + # Test invalid + with self.assertRaises(ValidationError): + UserDbRequest(operation="delete", username="test_user", + message_id="test0") From 1dca865a7629880ec3b51f5338d66824d40cf319 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Fri, 8 Nov 2024 10:31:11 -0800 Subject: [PATCH 09/19] Update tests to handle refactored model params Update raised exception to use exact keys --- neon_data_models/models/api/mq.py | 23 +++++++++++------------ tests/models/api/test_mq.py | 2 +- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/neon_data_models/models/api/mq.py b/neon_data_models/models/api/mq.py index 9f22017..50a23a4 100644 --- a/neon_data_models/models/api/mq.py +++ b/neon_data_models/models/api/mq.py @@ -51,17 +51,16 @@ class ReadUserRequest(MQContext): class UpdateUserRequest(MQContext): operation: Literal["update"] = "update" user: User = Field(description="Updated User object to write to database") - auth_username: str = Field(default="", - description="Username to authorize database " - "change. If unset, this will use " - "`user.username`") - auth_password: str = Field(default="", - description="Password (clear or hashed) associated " - "with `auth_username`. If unset, this " - "will use `user.password_hash`. If " - "changing the password, this must " - "contain the existing password, with " - "the new password specified in `user`") + auth_username: str = Field( + default="", description="Username to authorize database change. If " + "unset, this will use `user.username`") + auth_password: str = Field( + default="", description="Password (clear or hashed) associated with " + "`auth_username`. If unset, this will use " + "`user.password_hash`. If changing the " + "password, this must contain the existing " + "password, with the new password specified in " + "`user`") @model_validator(mode="after") def get_auth_username(self) -> 'UpdateUserRequest': @@ -70,7 +69,7 @@ def get_auth_username(self) -> 'UpdateUserRequest': if not self.auth_password: self.auth_password = self.user.password_hash if not all((self.auth_username, self.auth_password)): - raise ValueError("Missing username or password") + raise ValueError("Missing auth_username or auth_password") return self diff --git a/tests/models/api/test_mq.py b/tests/models/api/test_mq.py index caea6b2..4768ad9 100644 --- a/tests/models/api/test_mq.py +++ b/tests/models/api/test_mq.py @@ -72,7 +72,7 @@ def test_update_user_db_request(self): # Test update user valid valid_kwargs = {"message_id": "test_id", "operation": "update", - "password": "test_password", + "auth_password": "test_password", "user": {"username": "test_user", "skills": {"skill_id": {"test": True}}}} update_request = UpdateUserRequest(**valid_kwargs) From 798e9370f300afbab1af8266c4ac3af59ce258c5 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Fri, 8 Nov 2024 12:50:36 -0800 Subject: [PATCH 10/19] Refactor `ReadUserRequest` to require some form of authentication for any read operation Define "READ_USERS" role and annotate special roles --- neon_data_models/enum.py | 1 + neon_data_models/models/api/mq.py | 14 +++++++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/neon_data_models/enum.py b/neon_data_models/enum.py index 345be3f..47fccae 100644 --- a/neon_data_models/enum.py +++ b/neon_data_models/enum.py @@ -49,6 +49,7 @@ class AccessRoles(IntEnum): # 50 Reserved for "unlimited access" NODE = -1 + READ_USERS = -2, "Used by service accounts to read users from the database" class UserData(IntEnum): diff --git a/neon_data_models/models/api/mq.py b/neon_data_models/models/api/mq.py index 50a23a4..9056224 100644 --- a/neon_data_models/models/api/mq.py +++ b/neon_data_models/models/api/mq.py @@ -35,17 +35,25 @@ class CreateUserRequest(MQContext): operation: Literal["create"] = "create" user: User = Field(description="User object to create") - # TODO: Support optional auth class ReadUserRequest(MQContext): operation: Literal["read"] = "read" user_spec: str = Field(description="Username or User ID to read") + auth_user_spec: str = Field( + default="", description="Username or ID to authorize database read. " + "If unset, this will use `user_spec`") access_token: Optional[TokenConfig] = Field( - None, description="Token associated with `user_spec`") + None, description="Token associated with `auth_username`") password: Optional[str] = Field(None, description="Password associated with " - "`user_spec`") + "`auth_username`") + + @model_validator(mode="after") + def get_auth_username(self) -> 'ReadUserRequest': + if not self.auth_user_spec: + self.auth_user_spec = self.user_spec + return self class UpdateUserRequest(MQContext): From 3e72f885dbc080f91d93324219b524339e2d990b Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Fri, 8 Nov 2024 12:54:46 -0800 Subject: [PATCH 11/19] Fix syntax error in commented AccessRoles and update docstring to include comments --- neon_data_models/enum.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/neon_data_models/enum.py b/neon_data_models/enum.py index 47fccae..98c8bf9 100644 --- a/neon_data_models/enum.py +++ b/neon_data_models/enum.py @@ -34,6 +34,10 @@ class AccessRoles(IntEnum): non-user roles. In this way, an activity can require, for example, `permission > AccessRoles.GUEST` to grant access to all registered users, admins, and owners. + + Special Roles: + NODE: Reserved for use by a Node service account + READ_USERS: Used by service accounts to read users from the database """ NONE = 0 # 1-9 reserved for unauthenticated connections From af9baf2ea71b643fc6719d765cb9199b7ac00967 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Mon, 11 Nov 2024 17:29:48 -0800 Subject: [PATCH 12/19] Refactor `read` requests to accept a token for auth Validate passed token as an access token, rather than refresh --- neon_data_models/models/api/mq.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/neon_data_models/models/api/mq.py b/neon_data_models/models/api/mq.py index 9056224..dc1dd53 100644 --- a/neon_data_models/models/api/mq.py +++ b/neon_data_models/models/api/mq.py @@ -28,8 +28,9 @@ from pydantic import Field, TypeAdapter, model_validator +from neon_data_models.models.api.jwt import HanaToken from neon_data_models.models.base.contexts import MQContext -from neon_data_models.models.user.database import User, TokenConfig +from neon_data_models.models.user.database import User class CreateUserRequest(MQContext): @@ -43,16 +44,19 @@ class ReadUserRequest(MQContext): auth_user_spec: str = Field( default="", description="Username or ID to authorize database read. " "If unset, this will use `user_spec`") - access_token: Optional[TokenConfig] = Field( + access_token: Optional[HanaToken] = Field( None, description="Token associated with `auth_username`") password: Optional[str] = Field(None, description="Password associated with " "`auth_username`") @model_validator(mode="after") - def get_auth_username(self) -> 'ReadUserRequest': + def validate_params(self) -> 'ReadUserRequest': if not self.auth_user_spec: self.auth_user_spec = self.user_spec + if self.access_token and self.access_token.purpose != "access": + raise ValueError(f"Expected an access token but got: " + f"{self.access_token.purpose}") return self From 131c5fa43dfc90c8ac9bf3d1b371a4139dd72cf6 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Tue, 19 Nov 2024 15:21:24 -0800 Subject: [PATCH 13/19] Update docstring for AccessRoles special roles Annotate config values in PermissionsConfig to describe application of permissions Add a `users` permission to define access to the users service independent of the rest of the backend (matches `node` and `llm` behavior) --- neon_data_models/enum.py | 8 ++++--- neon_data_models/models/user/database.py | 28 ++++++++++++++++++------ 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/neon_data_models/enum.py b/neon_data_models/enum.py index 98c8bf9..ec4ac72 100644 --- a/neon_data_models/enum.py +++ b/neon_data_models/enum.py @@ -36,8 +36,10 @@ class AccessRoles(IntEnum): admins, and owners. Special Roles: - NODE: Reserved for use by a Node service account - READ_USERS: Used by service accounts to read users from the database + NODE: Reserved for use by a Node device service account to access + various services + RW_USERS: Reserved for use by service accounts to access and modify the + users database """ NONE = 0 # 1-9 reserved for unauthenticated connections @@ -53,7 +55,7 @@ class AccessRoles(IntEnum): # 50 Reserved for "unlimited access" NODE = -1 - READ_USERS = -2, "Used by service accounts to read users from the database" + RW_USERS = -2 class UserData(IntEnum): diff --git a/neon_data_models/models/user/database.py b/neon_data_models/models/user/database.py index af6f5fc..e2054e9 100644 --- a/neon_data_models/models/user/database.py +++ b/neon_data_models/models/user/database.py @@ -115,14 +115,28 @@ class BrainForgeConfig(BaseModel): class PermissionsConfig(BaseModel): """ - Defines roles for supported projects/service families. + Defines roles for supported projects/services. """ - klat: AccessRoles = AccessRoles.NONE - core: AccessRoles = AccessRoles.NONE - diana: AccessRoles = AccessRoles.NONE - node: AccessRoles = AccessRoles.NONE - hub: AccessRoles = AccessRoles.NONE - llm: AccessRoles = AccessRoles.NONE + klat: AccessRoles = Field( + AccessRoles.NONE, description="Defines access to Klat chat services.") + core: AccessRoles = Field( + AccessRoles.NONE, description="Defines access to Neon core services.") + diana: AccessRoles = Field( + AccessRoles.NONE, + description="Defines access to DIANA backend services. " + "(i.e. API proxy, email proxy).") + users: AccessRoles = Field( + AccessRoles.NONE, description="Defines access to the users service.") + node: AccessRoles = Field( + AccessRoles.NONE, + description="Defines access to the node websocket in HANA.") + hub: AccessRoles = Field( + AccessRoles.NONE, description="Defines access to a hub device.") + llm: AccessRoles = Field( + AccessRoles.NONE, + description="Defines access to the BrainForge LLM backend. Note that " + "per-model permissions may also apply and further restrict " + "a user's access to some models for inference.") class Config: use_enum_values = True From a278b192ad6936ffd1b60089da8d6def0309c37d Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Tue, 19 Nov 2024 15:25:47 -0800 Subject: [PATCH 14/19] Refactor to remove `RW_USERS` role since the `USER` and `ADMIN` roles already define read and write access, respectively --- neon_data_models/enum.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/neon_data_models/enum.py b/neon_data_models/enum.py index ec4ac72..f506307 100644 --- a/neon_data_models/enum.py +++ b/neon_data_models/enum.py @@ -38,8 +38,6 @@ class AccessRoles(IntEnum): Special Roles: NODE: Reserved for use by a Node device service account to access various services - RW_USERS: Reserved for use by service accounts to access and modify the - users database """ NONE = 0 # 1-9 reserved for unauthenticated connections @@ -55,7 +53,6 @@ class AccessRoles(IntEnum): # 50 Reserved for "unlimited access" NODE = -1 - RW_USERS = -2 class UserData(IntEnum): From d4bd68a614948d9d6fa16c79d7e3ba4656f181e8 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Tue, 19 Nov 2024 16:39:59 -0800 Subject: [PATCH 15/19] Refactor imports to resolve circular import errors Add import tests to ensure all imports resolve --- neon_data_models/models/base/messagebus.py | 4 +- neon_data_models/models/user/database.py | 4 +- tests/test_imports.py | 54 ++++++++++++++++++++++ 3 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 tests/test_imports.py diff --git a/neon_data_models/models/base/messagebus.py b/neon_data_models/models/base/messagebus.py index 08721bf..8773b8b 100644 --- a/neon_data_models/models/base/messagebus.py +++ b/neon_data_models/models/base/messagebus.py @@ -30,8 +30,8 @@ from neon_data_models.models.base import BaseModel from neon_data_models.models.base.contexts import (SessionContext, KlatContext, TimingContext, MQContext) -from neon_data_models.models.client import NodeData -from neon_data_models.models.user import NeonUserConfig +from neon_data_models.models.client.node import NodeData +from neon_data_models.models.user.database import NeonUserConfig class MessageContext(BaseModel): diff --git a/neon_data_models/models/user/database.py b/neon_data_models/models/user/database.py index e2054e9..05b2951 100644 --- a/neon_data_models/models/user/database.py +++ b/neon_data_models/models/user/database.py @@ -29,7 +29,7 @@ from typing_extensions import deprecated from uuid import uuid4 -from neon_data_models.models.api.jwt import HanaToken +# from neon_data_models.models.api import HanaToken from neon_data_models.models.base import BaseModel from pydantic import Field from datetime import date @@ -189,7 +189,7 @@ class User(BaseModel): klat: KlatConfig = KlatConfig() llm: BrainForgeConfig = BrainForgeConfig() permissions: PermissionsConfig = PermissionsConfig() - tokens: Optional[List[HanaToken]] = [] + tokens: Optional[List['HanaToken']] = [] def __eq__(self, other): return self.model_dump() == other.model_dump() diff --git a/tests/test_imports.py b/tests/test_imports.py new file mode 100644 index 0000000..626a926 --- /dev/null +++ b/tests/test_imports.py @@ -0,0 +1,54 @@ +# NEON AI (TM) SOFTWARE, Software Development Kit & Application Development System +# All trademark and other rights reserved by their respective owners +# Copyright 2008-2024 Neongecko.com Inc. +# BSD-3 +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are met: +# 1. Redistributions of source code must retain the above copyright notice, +# this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright notice, +# this list of conditions and the following disclaimer in the documentation +# and/or other materials provided with the distribution. +# 3. Neither the name of the copyright holder nor the names of its +# contributors may be used to endorse or promote products derived from this +# software without specific prior written permission. +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR +# PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR +# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +# EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +# PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, +# OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +# LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +# NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +from unittest import TestCase + + +class TestImports(TestCase): + def test_import_api(self): + from neon_data_models.models.api import CreateUserRequest + from neon_data_models.models.api.mq import CreateUserRequest as _CUR + self.assertEqual(CreateUserRequest, _CUR) + + def test_import_client(self): + from neon_data_models.models.client import NodeData + from neon_data_models.models.client.node import NodeData as _ND + self.assertEqual(NodeData, _ND) + + def test_import_user(self): + from neon_data_models.models.user import User + from neon_data_models.models.user.database import User as _User + self.assertEqual(User, _User) + + def test_import_subclasses(self): + # Addressing circular import noted in users service unit tests + from neon_data_models.models.user.database import User + from neon_data_models.models.client.node import NodeData + from neon_data_models.models.api.mq import CreateUserRequest + from neon_data_models.models.base import BaseModel + self.assertTrue(issubclass(CreateUserRequest, BaseModel)) + self.assertTrue(issubclass(NodeData, BaseModel)) + self.assertTrue(issubclass(User, BaseModel)) From f24342602c8b8bab9824f9368bd454695b44513e Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Tue, 19 Nov 2024 16:55:56 -0800 Subject: [PATCH 16/19] Remove unused import Refactor tests to resolve issue with reloading base classes --- neon_data_models/models/base/contexts.py | 3 +-- tests/models/test_base.py | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/neon_data_models/models/base/contexts.py b/neon_data_models/models/base/contexts.py index 46db3d7..f715169 100644 --- a/neon_data_models/models/base/contexts.py +++ b/neon_data_models/models/base/contexts.py @@ -23,11 +23,10 @@ # LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING # NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + from datetime import datetime, timedelta from typing import Literal, List, Optional -from pydantic import Field - from neon_data_models.models.base import BaseModel diff --git a/tests/models/test_base.py b/tests/models/test_base.py index 47ae5ba..e2f3b72 100644 --- a/tests/models/test_base.py +++ b/tests/models/test_base.py @@ -32,9 +32,6 @@ from time import time from pydantic import ValidationError -from neon_data_models.models.client import NodeData -from neon_data_models.models.user import NeonUserConfig - class TestBaseModel(TestCase): def test_base_model(self): @@ -60,6 +57,15 @@ def test_base_model(self): self.assertEqual(model.model_config["extra"], "ignore") self.assertEqual(allowed.model_config["extra"], "allow") + importlib.reload(neon_data_models.models.base) + + def test_base_model_inheritance(self): + from neon_data_models.models.base import BaseModel + from neon_data_models.models.user.database import PermissionsConfig + config = PermissionsConfig() + self.assertTrue(isinstance(config, BaseModel)) + self.assertIsInstance(config.model_config["extra"], str) + class TestContexts(TestCase): def test_session_context(self): @@ -135,6 +141,8 @@ def test_mq_context(self): class TestMessagebus(TestCase): def test_base_model(self): from neon_data_models.models.base.messagebus import BaseMessage + from neon_data_models.models.client import NodeData + from neon_data_models.models.user import NeonUserConfig with self.assertRaises(ValidationError): BaseMessage() @@ -162,6 +170,8 @@ def test_base_model(self): def test_message_context(self): from neon_data_models.models.base.messagebus import MessageContext + from neon_data_models.models.client import NodeData + from neon_data_models.models.user import NeonUserConfig # Default Behavior default_context = MessageContext() From 7c0da8febacb69142e7bcf61d076c72190957208 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Tue, 19 Nov 2024 17:50:10 -0800 Subject: [PATCH 17/19] Troubleshoot module reloading in unit tests --- tests/models/test_base.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/models/test_base.py b/tests/models/test_base.py index e2f3b72..0665af8 100644 --- a/tests/models/test_base.py +++ b/tests/models/test_base.py @@ -27,7 +27,6 @@ import importlib import os from datetime import datetime, timedelta - from unittest import TestCase from time import time from pydantic import ValidationError @@ -57,7 +56,11 @@ def test_base_model(self): self.assertEqual(model.model_config["extra"], "ignore") self.assertEqual(allowed.model_config["extra"], "allow") - importlib.reload(neon_data_models.models.base) + # Ensure modules are unloaded for future inheritance tests + import sys + for module in list(sys.modules.keys()): + if module.startswith("neon_data_models"): + del sys.modules[module] def test_base_model_inheritance(self): from neon_data_models.models.base import BaseModel From bf941cab885215742c7faaf6842c10212cf00755 Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Tue, 19 Nov 2024 18:00:44 -0800 Subject: [PATCH 18/19] Add check to ensure `HanaToken` is defined for `User` object with unit test Issue noted in https://github.com/NeonGeckoCom/neon-users-service/actions/runs/11923783196/job/33236966442 --- neon_data_models/models/user/database.py | 5 +++++ tests/test_imports.py | 2 ++ 2 files changed, 7 insertions(+) diff --git a/neon_data_models/models/user/database.py b/neon_data_models/models/user/database.py index 05b2951..0fba1d2 100644 --- a/neon_data_models/models/user/database.py +++ b/neon_data_models/models/user/database.py @@ -181,6 +181,11 @@ class TokenConfig(BaseModel): class User(BaseModel): + def __init__(self, **kwargs): + # Ensure `HanaToken` is populated from the import space + self.model_rebuild() + BaseModel.__init__(self, **kwargs) + username: str password_hash: Optional[str] = None user_id: str = Field(default_factory=lambda: str(uuid4())) diff --git a/tests/test_imports.py b/tests/test_imports.py index 626a926..fcf3c0d 100644 --- a/tests/test_imports.py +++ b/tests/test_imports.py @@ -42,6 +42,8 @@ def test_import_user(self): from neon_data_models.models.user import User from neon_data_models.models.user.database import User as _User self.assertEqual(User, _User) + user = User(username="test_user", password_hash="test_pass") + self.assertIsInstance(user, User) def test_import_subclasses(self): # Addressing circular import noted in users service unit tests From c37eca5c729dc87e822ebc9bf80ee221e9a01c1b Mon Sep 17 00:00:00 2001 From: Daniel McKnight Date: Tue, 19 Nov 2024 18:03:27 -0800 Subject: [PATCH 19/19] Add explicit import of `HanaToken` https://github.com/NeonGeckoCom/neon-users-service/actions/runs/11923783196/job/33237158775 --- neon_data_models/models/user/database.py | 1 + 1 file changed, 1 insertion(+) diff --git a/neon_data_models/models/user/database.py b/neon_data_models/models/user/database.py index 0fba1d2..4e7e885 100644 --- a/neon_data_models/models/user/database.py +++ b/neon_data_models/models/user/database.py @@ -183,6 +183,7 @@ class TokenConfig(BaseModel): class User(BaseModel): def __init__(self, **kwargs): # Ensure `HanaToken` is populated from the import space + from neon_data_models.models.api.jwt import HanaToken self.model_rebuild() BaseModel.__init__(self, **kwargs)