Listing users still gives output when the SRE does not exist within the current context

craddm commented 1 month ago

:white_check_mark: Checklist

[x] I have searched open and closed issues for duplicates.
[x] This is a problem observed when managing a Data Safe Haven.
[x] I can reproduce this with the latest version.
[x] I have read through the documentation.
[x] This isn't an open-ended question (open a discussion if it is).

:computer: System information

Operating System: macOs
Data Safe Haven version: develop

:package: Packages

List of packages

```none Paste list of packages here ```

:no_entry_sign: Describe the problem

dsh users list <sre> still prints out a table when the SRE doesn't exist in the current context. It should give clearer error messages telling the user why this hasn't worked.

(data-safe-haven) deploydsh@0b11d89ae8c5:/workspaces/data-safe-haven$ dsh users list oda
You are logged into the Azure CLI as:                                                                                                                                                                 
        user: Matthew Craddock (1b2a59f2-f3ea-42e7-b8b8-aeea30572764)                                                                                                                                 
        tenant: turing.ac.uk (4395f4a7-e455-4f95-8a9f-1fbaef6384f9)                                                                                                                                   
Are these details correct? [y/n] (y): y
You are logged into the Microsoft Graph API as:                                                                                                                                                       
        user: aad.admin.matt.craddock@green.develop.turingsafehaven.ac.uk (3b3f1d49-9980-4a09-99e4-784038a740fa)                                                                                      
        tenant: green.develop.turingsafehaven.ac.uk (cb94a6f6-ef7a-42ab-bcad-4f0b887cfd3e)                                                                                                            
Are these details correct? [y/n] (y): y
Blob file 'sre-oda.yaml' could not be downloaded from 'shmgreen'.                                                                                                                                     
Could not load file 'sre-oda.yaml' from Azure storage.                                                                                                                                                
Could not load users for SRE 'oda'.                                                                                                                                                                   
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━┳━━━━━━━━━┓
┃ username                     ┃ Entra ID ┃ SRE oda ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━╇━━━━━━━━━┩
│ aad.admin.jim.madge          │ x        │         │
│ aad.admin.martin.oreilly     │ x        │         │
│ aad.admin.matt.craddock      │ x        │         │
│ ada.lovelace                 │ x        │         │
│ entra.admin.emergency.access │ x        │         │
│ entra.admin.james.robinson   │ x        │         │
│ grace.hopper                 │ x        │         │
│ james.robinson               │ x        │         │
│ jim.madge                    │ x        │         │
│ matt.craddock                │ x        │         │
│ sherlock.holmes              │ x        │         │
└──────────────────────────────┴──────────┴─────────┘

:steam_locomotive: Workarounds or solutions

jemrobinson commented 1 month ago

Can we tell the difference between "we couldn't load the YAML file because it doesn't exist" and "we couldn't load the YAML file because there is no SRE with that name"?

craddm commented 1 month ago

To be fair, my report is slightly misleading - I've made it sound a bit like it only happens when the SRE does exist somewhere, and I don't think that matters. I think I meant more that saying the SRE is not found in the current context catches both cases. If you're inattentive, you might not realise there's no SRE up because the table was printed.

jemrobinson commented 1 month ago

Ah OK - can you make the fix to not show the table in this situation then @craddm? Should be in administration > users > user_handler.py in the list() method.

JimMadge commented 2 weeks ago

@craddm Are you working on this?

alan-turing-institute / data-safe-haven