|
1 | 1 | """Unit tests for workspace_service — pure helpers, no DB.""" |
2 | 2 |
|
3 | | -from lif.mdr_services.workspace_service import Workspace, find_workspace, list_workspaces_for_groups |
| 3 | +from lif.mdr_services.workspace_service import ( |
| 4 | + Workspace, |
| 5 | + compute_display_name, |
| 6 | + find_workspace, |
| 7 | + list_workspaces_for_groups, |
| 8 | + to_workspace_item, |
| 9 | +) |
4 | 10 |
|
5 | 11 |
|
6 | 12 | class TestListWorkspacesForGroups: |
@@ -45,3 +51,138 @@ def test_returns_none_when_group_sanitizes_to_empty(self): |
45 | 51 | """Even if --- somehow ended up in cognito_groups, we shouldn't |
46 | 52 | route a request to an empty tenant_ schema.""" |
47 | 53 | assert find_workspace(["---"], "---") is None |
| 54 | + |
| 55 | + |
| 56 | +class TestComputeDisplayName: |
| 57 | + """Friendly display name resolution (issue #943). |
| 58 | +
|
| 59 | + For a user's own personal tenant the display name is their email |
| 60 | + (verified via the eval-<sub> group / JWT sub match). For any other |
| 61 | + group the display name is just the group name as today. |
| 62 | + """ |
| 63 | + |
| 64 | + def test_personal_tenant_uses_email_when_match(self): |
| 65 | + # Cognito's sub claim is `abc123`; the post-confirmation Lambda |
| 66 | + # created `eval-abc123` for them. The user's email is on |
| 67 | + # principal. Display name should be the email. |
| 68 | + assert ( |
| 69 | + compute_display_name( |
| 70 | + group="eval-abc123", |
| 71 | + cognito_sub="abc123", |
| 72 | + principal="user@example.edu", |
| 73 | + tenant_schema="tenant_eval_abc123", |
| 74 | + ) |
| 75 | + == "user@example.edu" |
| 76 | + ) |
| 77 | + |
| 78 | + def test_shared_group_uses_group_name(self): |
| 79 | + # User is in lif-team (a shared group). Even though we have the |
| 80 | + # email on principal, the group name is the right friendly label |
| 81 | + # for a shared workspace. |
| 82 | + assert ( |
| 83 | + compute_display_name( |
| 84 | + group="lif-team", cognito_sub="abc123", principal="user@example.edu", tenant_schema="tenant_lif_team" |
| 85 | + ) |
| 86 | + == "lif-team" |
| 87 | + ) |
| 88 | + |
| 89 | + def test_eval_group_for_different_sub_does_not_use_email(self): |
| 90 | + # Defense-in-depth: if another user's eval-* group somehow ended |
| 91 | + # up in the caller's group list (shouldn't happen — the post- |
| 92 | + # confirmation Lambda only adds the caller to their own — but |
| 93 | + # belt-and-suspenders), we don't claim someone else's tenant as |
| 94 | + # theirs via the email label. |
| 95 | + assert ( |
| 96 | + compute_display_name( |
| 97 | + group="eval-other_user_sub", |
| 98 | + cognito_sub="abc123", |
| 99 | + principal="user@example.edu", |
| 100 | + tenant_schema="tenant_eval_other_user_sub", |
| 101 | + ) |
| 102 | + == "eval-other_user_sub" |
| 103 | + ) |
| 104 | + |
| 105 | + def test_legacy_principal_without_at_falls_back_to_group(self): |
| 106 | + # Legacy HS256 path or any JWT where `principal` is a sub (no |
| 107 | + # email claim available). We can't surface a friendlier label, |
| 108 | + # so use the group name. The `@` heuristic is what tells us |
| 109 | + # whether principal is an email or a sub. |
| 110 | + assert ( |
| 111 | + compute_display_name( |
| 112 | + group="eval-abc123", |
| 113 | + cognito_sub="abc123", |
| 114 | + principal="abc123", # sub, not email |
| 115 | + tenant_schema="tenant_eval_abc123", |
| 116 | + ) |
| 117 | + == "eval-abc123" |
| 118 | + ) |
| 119 | + |
| 120 | + def test_missing_sub_falls_back_to_group(self): |
| 121 | + # Without a sub we can't verify the eval-* group is the caller's, |
| 122 | + # so play it safe and use the group name. |
| 123 | + assert ( |
| 124 | + compute_display_name( |
| 125 | + group="eval-abc123", cognito_sub=None, principal="user@example.edu", tenant_schema="tenant_eval_abc123" |
| 126 | + ) |
| 127 | + == "eval-abc123" |
| 128 | + ) |
| 129 | + |
| 130 | + def test_missing_principal_falls_back_to_group(self): |
| 131 | + assert ( |
| 132 | + compute_display_name( |
| 133 | + group="eval-abc123", cognito_sub="abc123", principal=None, tenant_schema="tenant_eval_abc123" |
| 134 | + ) |
| 135 | + == "eval-abc123" |
| 136 | + ) |
| 137 | + |
| 138 | + def test_empty_principal_after_strip_falls_back_to_tenant_schema(self): |
| 139 | + # Defense-in-depth per Adam's #947 review: if the resolved |
| 140 | + # candidate is whitespace-only (a principal of " " somehow |
| 141 | + # passed the `@` check, etc.), fall through to tenant_schema |
| 142 | + # rather than emit an empty display_name. The wire contract |
| 143 | + # promises `display_name` is non-empty. |
| 144 | + assert ( |
| 145 | + compute_display_name( |
| 146 | + group="eval-abc123", |
| 147 | + cognito_sub="abc123", |
| 148 | + principal=" @ ", # whitespace + @ would pass the heuristic but strip to nothing useful |
| 149 | + tenant_schema="tenant_eval_abc123", |
| 150 | + ) |
| 151 | + # principal " @ " stripped is "@" — not empty, so it's used as-is. |
| 152 | + # The fallback only kicks in when the candidate is truly empty |
| 153 | + # after strip. The exact behavior here is documented: we don't |
| 154 | + # over-sanitize, just guarantee non-empty. |
| 155 | + == "@" |
| 156 | + ) |
| 157 | + |
| 158 | + def test_empty_group_falls_back_to_tenant_schema(self): |
| 159 | + # Sanity: if a group somehow sanitized to empty string (the |
| 160 | + # listing pipeline filters these out before this point, but |
| 161 | + # belt-and-suspenders), tenant_schema is the ultimate fallback. |
| 162 | + assert ( |
| 163 | + compute_display_name(group="", cognito_sub=None, principal=None, tenant_schema="tenant_lif_team") |
| 164 | + == "tenant_lif_team" |
| 165 | + ) |
| 166 | + |
| 167 | + |
| 168 | +class TestToWorkspaceItem: |
| 169 | + """Projection from internal Workspace into the API-facing WorkspaceItem.""" |
| 170 | + |
| 171 | + def test_includes_friendly_display_name_for_personal_tenant(self): |
| 172 | + ws = Workspace(group="eval-abc123", tenant_schema="tenant_eval_abc123") |
| 173 | + item = to_workspace_item(ws, cognito_sub="abc123", principal="user@example.edu") |
| 174 | + assert item.group == "eval-abc123" |
| 175 | + assert item.tenant_schema == "tenant_eval_abc123" |
| 176 | + assert item.display_name == "user@example.edu" |
| 177 | + |
| 178 | + def test_uses_group_name_for_shared_group(self): |
| 179 | + ws = Workspace(group="lif-team", tenant_schema="tenant_lif_team") |
| 180 | + item = to_workspace_item(ws, cognito_sub="abc123", principal="user@example.edu") |
| 181 | + assert item.display_name == "lif-team" |
| 182 | + |
| 183 | + def test_no_identity_hints_defaults_to_group_name(self): |
| 184 | + # Callers that don't pass identity context get the same shape they |
| 185 | + # got pre-#943 (display_name == group). |
| 186 | + ws = Workspace(group="lif-team", tenant_schema="tenant_lif_team") |
| 187 | + item = to_workspace_item(ws) |
| 188 | + assert item.display_name == "lif-team" |
0 commit comments