Use the idiomatic None for no required grant instead of empty string

Author: JBorrow

soauth/api/app_manager.py

@@ -5,6 +5,7 @@
 from typing import Annotated
 
 from fastapi import APIRouter, Depends, HTTPException, Request, status
+from pydantic import BaseModel
 
 from soauth.core.app import AppData
 from soauth.core.models import AppDetailResponse, AppRefreshResponse
@@ -36,6 +37,14 @@ async def handle_app_manager_user(request: Request) -> SOUserWithGrants:
 app_management_routes = APIRouter(tags=["App Management"])
 
 
+class AppCreationRequest(BaseModel):
+    name: str
+    domain: str
+    redirect_url: str
+    visibility_grant: str | None = None
+    api_access: bool = False
+
+
 @app_management_routes.put(
     "/app",
     summary="Create a new application",
@@ -49,26 +58,22 @@ async def handle_app_manager_user(request: Request) -> SOUserWithGrants:
     },
 )
 async def create_app(
-    name: str,
-    domain: str,
-    redirect_url: str,
-    visibility_grant: str,
-    api_access: bool,
+    model: AppCreationRequest,
     user: AppManagerUser,
     conn: DatabaseDependency,
     settings: SettingsDependency,
     log: LoggerDependency,
 ) -> AppRefreshResponse:
-    log = log.bind(user=user, domain=domain)
+    log = log.bind(user=user, creation_request=model)
     # Note: the 'user' as given by the auth system is not the same as our database
     # user, it's re-created from the webtoken.
     database_user = await user_service.read_by_id(user_id=user.user_id, conn=conn)
     app = await app_service.create(
-        name=name,
-        domain=domain,
-        redirect_url=redirect_url,
-        visibility_grant=visibility_grant,
-        api_access=api_access,
+        name=model.name,
+        domain=model.domain,
+        redirect_url=model.redirect_url,
+        visibility_grant=model.visibility_grant,
+        api_access=model.api_access,
         user=database_user,
         settings=settings,
         conn=conn,
@@ -102,7 +107,10 @@ async def apps(
 ) -> list[AppData]:
     log = log.bind(user=user)
     created_by = None if "admin" in user.grants else user.user_id
-    result = await app_service.get_app_list(created_by_user_id = created_by, user_name = user.display_name, conn=conn)
+    database_user = await user_service.read_by_id(user_id=user.user_id, conn=conn)
+    result = await app_service.get_app_list(
+        created_by_user_id=created_by, user=database_user, conn=conn
+    )
     log.bind(number_of_apps=len(result))
     await log.ainfo("api.appmanager.apps")
     return result

soauth/api/keys.py

@@ -84,14 +84,30 @@ async def list(
     user: AuthenticatedUserDependency,
 ) -> list[AppDetailResponse]:
     # In theory we can chain these futures, but not worth it.
+    database_user = await user_service.read_by_id(user_id=user.user_id, conn=conn)
+
+    log = log.bind(user_id=database_user.user_id, user_name=database_user.user_name)
+
     app_list = await app_service.get_app_list(
-        created_by_user_id=None, user_name=user.display_name , conn=conn, require_api_access=False
+        created_by_user_id=None, user=database_user, conn=conn, require_api_access=False
+    )
+
+    log = log.bind(
+        number_of_apps=len(app_list),
+        app_list=[x.app_name for x in app_list],
     )
 
     login_list = await refresh_service.get_all_logins_for_user(
         user_id=user.user_id, conn=conn, log=log
     )
 
+    log = log.bind(
+        number_of_logins=len(login_list),
+        login_list=[f"{y.app_id} ({y.app_name})" for y in login_list],
+    )
+
+    await log.ainfo("api.key_management.list")
+
     return [
         AppDetailResponse(app=x, users=[y for y in login_list if y.app_id == x.app_id])
         for x in app_list

soauth/app/apps.py

@@ -57,26 +57,31 @@ def app_create_post(
     log: LoggerDependency,
     name: Annotated[str, Form()],
     domain: Annotated[str, Form()],
-    visibility_grant: Annotated[str, Form()],
     redirect: Annotated[str, Form()],
+    visibility_grant: Annotated[str | None, Form()] = None,
     api: Annotated[bool | None, Form()] = None,
 ):
-    log = log.bind(domain=domain)
-
     if api is None:
         api = False
 
+    if not visibility_grant:
+        visibility_grant = None
+
+    content = {
+        "name": name,
+        "domain": domain,
+        "redirect_url": redirect,
+        "visibility_grant": visibility_grant,
+        "api_access": api,
+    }
+
+    log = log.bind(**content)
+
     response = handle_request(
         url=f"{request.app.app_detail_url}",
         request=request,
         method="put",
-        params={
-            "name": name,
-            "domain": domain,
-            "redirect_url": redirect,
-            "visibility_grant": visibility_grant,
-            "api_access": api,
-        },
+        json=content,
     )
 
     log.bind(response=response.json())

soauth/app/templates/app_detail.html

@@ -36,6 +36,7 @@ <h2>{{app.app_name}}</h2>
       <li><span class="nes-text is-primary">Created at</span> {{app.created_at}}</li>
       <li><span class="nes-text is-primary">Domain</span> {{app.domain}}</li>
       <li><span class="nes-text is-primary">API Access?</span> {{app.api_access}}</li>
+      <li><span class="nes-text is-primary">Required grant</span> {{app.visibility_grant}}</li>
     </ul>
   </div>
 </section>

soauth/core/app.py

@@ -17,7 +17,7 @@ class AppData(BaseModel):
     created_by_user_name: str | None
     created_at: datetime
     domain: str
-    visibility_grant: str
+    visibility_grant: str | None = None
 
 
 class LoggedInUserData(BaseModel):

soauth/database/app.py

@@ -30,7 +30,7 @@ class App(SQLModel, table=True):
     created_at: datetime = Field(sa_column=Column(DateTime(timezone=True)))
 
     domain: str
-    visibility_grant: str = Field(default="")
+    visibility_grant: str | None = None
 
     key_pair_type: str
     # Note that the 'public key' is not really public - it should
@@ -53,7 +53,6 @@ def has_visibility_grant(self, user_grant: set[str]) -> bool:
             return True
         return self.visibility_grant in user_grant
 
-
     def to_core(self) -> AppData:
         return AppData(
             app_name=self.app_name,

soauth/service/app.py

@@ -15,7 +15,6 @@
 from soauth.core.uuid import UUID
 from soauth.database.app import App
 from soauth.database.user import User
-from soauth.service import user as user_service
 
 
 class AppNotFound(Exception):
@@ -70,7 +69,7 @@ async def create(
 
 async def get_app_list(
     created_by_user_id: UUID | None,
-    user_name: str,
+    user: User,
     conn: AsyncSession,
     require_api_access: bool = False,
 ) -> list[AppData]:
@@ -87,9 +86,31 @@ async def get_app_list(
     if require_api_access:
         query = query.filter_by(api_access=True)
 
+    grants = user.get_effective_grants()
+
+    if "admin" not in grants:
+        query = query.filter(
+            App.visibility_grant.in_(grants) | App.visibility_grant.is_(None)
+        )
+
     res = await conn.execute(query)
-    user_details = await user_service.read_by_name(user_name=user_name, conn=conn)
-    return [x.to_core() for x in res.scalars().unique().all() if x.has_visibility_grant(user_details.get_effective_grants())]
+
+    # Check again, make sure our results do really have the right visibility
+    # grant.
+
+    apps = res.unique().scalars().all()
+
+    for app in apps:
+        if "admin" not in grants and app.visibility_grant not in grants:
+            if app.visibility_grant is None:
+                continue
+
+            raise AppNotFound(
+                f"User {user.user_name} does not have visibility grant "
+                f"{app.visibility_grant} for app {app.app_name}."
+            )
+
+    return [app.to_core() for app in apps]
 
 
 async def read_by_id(app_id: UUID, conn: AsyncSession) -> App: