Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔮 Admin overview / Security Guard #40160

Closed
6 of 8 tasks
jancborchardt opened this issue Aug 30, 2023 · 37 comments
Closed
6 of 8 tasks

🔮 Admin overview / Security Guard #40160

jancborchardt opened this issue Aug 30, 2023 · 37 comments
Assignees
Labels
1. to develop Accepted and waiting to be taken care of design Design, UI, UX, etc. feature: settings overview ❄️ 2023-Winter
Milestone

Comments

@jancborchardt
Copy link
Member

jancborchardt commented Aug 30, 2023

In the settings it would be great to have a sort of a "Admin dashboard" / overview where you can quickly see some health info regarding your cloud. Like the following:

Admin dashboard

  • Are there any sync problems
  • Does someone use outdated clients
  • Where is the quota full
  • Is there anyone not syncing since a long time

Further ideas:

  • Does everyone have 2FA?
  • Does everyone have secure passwords?
  • LDAP server connection
  • Overview who accesses Nextcloud from where (Device, location)
  • Show all outgoing share links
  • Who accesses share links from where
  • What are guest users doing?

We have to watch out to not go into the field of too much tracking though.

Task breakdown

@jancborchardt jancborchardt added design Design, UI, UX, etc. 1. to develop Accepted and waiting to be taken care of feature: settings labels Aug 30, 2023
@jancborchardt jancborchardt added this to the Nextcloud 28 milestone Aug 30, 2023
@jancborchardt jancborchardt changed the title Admin & account dashboard 🔮 Admin & account dashboard Aug 30, 2023
@marcoambrosini
Copy link
Member

@jancborchardt @AndyScherzinger and @szaimen can we have a kickoff quick meeting to better define what's needed?

We have to watch out to not go into the field of too much tracking though.

Wouldn't this be too much already?

Overview who accesses Nextcloud from where (Device, location)
What are guest users doing?

@marcoambrosini marcoambrosini self-assigned this Aug 31, 2023
@joshtrichards
Copy link
Member

Related: #29439

@marcoambrosini
Copy link
Member

Other updates from the design call:

  • Account dashboard is probably not needed, focusing on admin for now (updating title)
  • In this iteration, we'll add the new info to the admin overview page as a new section

@marcoambrosini marcoambrosini changed the title 🔮 Admin & account dashboard 🔮 Admin dashboard Sep 13, 2023
@quentinDupont
Copy link

Very much interested ! Since NC 26, we had a lot of performances issues, due to config problems but also outdated sync clients. I don't know how big companies handles this but in our cooperative company, we had to (and still does) call every person and update sync clients after solving false conflicts etc. Hugh amount of work that could be really be easier with such a feature that permits to identify quickly which accounts use old sync clients that causes problems.

Thanks for all the work !

@marcoambrosini
Copy link
Member

marcoambrosini commented Sep 15, 2023

Hi all, here are a couple of wireframes for 2 approaches, a card dashboard-like layout and a list that one can filter with toggle tags. I personally think that the list solves the problem in a much better way than the dashboard-like approach, and there's more space to show a preview of the warning in the subline of the list item component
What do you think @nextcloud/designers ?

MacBook Pro 16_ - 2

MacBook Pro 16_ - 1

@nimishavijay
Copy link
Member

Very cool! I would vote for the dashboard though, right off the bat gives you an overview without clicking anywhere. With the list view, the switcher can get kind of lost because that kind of interaction isn't common within the settings, what do you think?

@juliushaertl
Copy link
Member

From an admin perspective it might be useful to show the count of total entries

Like 30 of 300 total clients have sync issues.

@marcoambrosini
Copy link
Member

@nimishavijay I forgot to include the "all" category in the previous mockup. That would be the overview. Either of these patterns would be new in the settings and I think that the list view just works better for admins.

Screenshot 2023-09-19 at 09 48 18

@marcoambrosini
Copy link
Member

design review comments:

  • OK list with tabbed navigation

@sorbaugh
Copy link
Contributor

Hi! A few questions to help us decide how to best proceed :)

What are we expecting to see in the list of Sync Problem? Is it something like "LDAP connectivity connecting correctly" or more about showing information that we store before?

Do we want to have custom widgets for the admin dashboard like in the dashboard app?

A good starting point might be to start with the info that we are already storing. Maybe we can work out a list. For example, are some sync problems already being recorded? (cc @icewind1991 )

@jancborchardt
Copy link
Member Author

Would agree with @marcoambrosini that the tabbed view with number indicator (proposal 2) is better for admins, rather than the "Dashboard"-like view which works mostly well for few entries. Also since the default tab will be "All" there will be an overview anyway. :)

@jancborchardt jancborchardt changed the title 🔮 Admin dashboard 🔮 Admin overview Sep 27, 2023
@sorbaugh
Copy link
Contributor

(big) update on the requirements:

The goal is to merge the code from the Nextcloud Guard app into core and use its capabilities from there. The infos and checks that are provided via guard should be displayed in the Admin overview, for the ones possible.

Anything else not initially provided in the Guard app we should discuss to either be drop, delay, offer them for cli only, or as cron since it requires root permissions. (because of the limited time)

Frontend should reflect the tabbed-view provided by @marcoambrosini instead of the Dashboard/Widget one.

For the first steps, it would be good to have a first executable mockup for the admin overview and the first, already available server infos displayed.

@sorbaugh
Copy link
Contributor

Seeing that the checks from Guard were meant to be triggered on cli or per-cron basis, is the expectation that all these checks are triggered everytime we open the Admin overview?

What would be the performance impact as we add more and more?

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Sep 28, 2023

Outdated clients: this info can be retrieved on server as all clients send an user agent with their version number.
Quota: is also available purely on server
Sync conflict:

  • send sync conflicts with timestamp
  • show registration date / first sync
  • show last sync, with threshold of 7 days to highlight non-syncable clients. Compare with apptoken, to rule out deleted clients

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Oct 2, 2023

API:
Clients send

  • bucket, type, count, oldest timestamp
  • only when there is a problem

@Chartman123
Copy link

  • Because this list is possibly endless, it might make sense to have it be the last section in the Admin overview

An admin would then have to scroll down to see if there are some problems worth investigating... In this case I think that it would be nice to have some kind of indicator on top to directly gain the attention of an admin. Just my two cents... 😉

@jancborchardt
Copy link
Member Author

Had another meeting with @karlitschek @AndyScherzinger @schiessle and this is the outcome:

  • We want to simplify this to a list/dashboard with 1 entry per topic. Either it has a nice green checkmark, or not and shows why it does not.
    • Depending on what is easier, each of these entries could either open a modal with details, or link to the specific section we already have. E.g. for things regarding accounts, there could be new sections in account management for each.
  • In the future this can be extended to also send emails regarding specific things.
  • According to @AndyScherzinger, all via dashboard available data should be exposed via API so it is accessible to external monitoring apps
  • On the "Privacy" page we should show to people whether this app is active and what it shows to admins
  • The app is not shipped and thus not enabled by default

Server status

  • Performance: Is the server slow?
  • Storage availability: Is there enough storage available on disk or not?
  • SSL working: Is the SSL certificate valid? Can be ported over from Guard
  • Errors in logfile: Any clusters of specific issues, or crucial errors?
  • Connections to other services: database, external storages, LDAP, Redis, HPB Files/Talk, Office, push proxy, etc.

Security

  • 2FA not enabled: Does everyone have it enabled or how many people don’t?
  • Insecure passwords: Does everyone have secure passwords or is anything in password leak databases?
  • Suspicious logins: Any suspicious logins? If it is difficult to to automatically, there could be an overview "Overview who accesses Nextcloud from where (Device, location)"
  • Brute force protection: Any suspicious attempts recently?
  • File access control: Are there recurring things being blocked by this?
  • Suspicious shares: Probably best to check specifically if there is an unusual pattern of access recently, similar to suspicious logins. Possibly it is acceptable to only show the filename/path, but definitely not allow the admin to view the content. This is possibly the biggest difficult topic as we don’t have existing logic for this.

Accounts

  • Quota almost full: Is someone very close to the quota? (90–100%)
  • Sync problems: Any sync issues, specific outages?
  • Inactive accounts: Are there any accounts not being active for a long amount of time?
  • Outdated apps/clients: Does anyone use outdated versions of the apps or clients? Here is the question where we display the full list. Could be a modal, or somehow in the user management.

Can we have a judgement about the complexity of each of the topics here, so we can look for quick wins? @come-nc @tobiasKaminsky @susnux, cc @sorbaugh

@tobiasKaminsky
Copy link
Member

For "me" with "sync problems" it stays the same, I think.
We will still send the same info as discussed, but overview condenses it to ✔ or ✘, and then details can be shown.
Otherwise it would be pointless, when 1 out of 800 users has a problem and it is then ✘ ;-)

@come-nc
Copy link
Contributor

come-nc commented Oct 9, 2023

I’m gonna continue on #32550 which accounts for having an API to register and display setup checks in a nice way.
I can move it to an application if wanted, but preferably a shipped one as there is a tight coupling with core anyway (applications have to register their checks using an OCP interface).

I am not sure why this would not replace existing setup checks, but we can keep both for the transition period if wanted?

Server status

  • Performance: Is the server slow?

For me this should go through its own feature ticket and specifications, benchmarks are a complicated beast and cannot (or should not?) be done live.

  • Storage availability: Is there enough storage available on disk or not?

I expect adding a check for this should be straightforward, only tricky thing is to avoid false-positive in some setups (like readonly full local drive and all data in s3 or something)

  • SSL working: Is the SSL certificate valid? Can be ported over from Guard

As mentionned in #40659 (comment) , this is already checked by the setup checks, maybe the check is not good enough? Does this point come from a specific usecase?
What is special about this setup check is that it is done frontend side, because it makes more sense for the client to check the server certificate than for the server to check itself. For those frontend side setup checks I do not know how to integrate them to the new API. Maybe we’ll have to do it server side instead, I’ll have to gather opinions on feasability (ie does the server always has access to the public URI?).

  • Errors in logfile: Any clusters of specific issues, or crucial errors?

What does crucial means, can we have more detailed specifications for this one?

  • Connections to other services: database, external storages, LDAP, Redis, HPB Files/Talk, Office, push proxy, etc.

For database I think it’s not possible to load admin settings without it. For the rest it can be added.

Security

  • 2FA not enabled: Does everyone have it enabled or how many people don’t?
  • Insecure passwords: Does everyone have secure passwords or is anything in password leak databases?

Is that a new feature or do we already have some integrations of password leak databases?
This looks like quite a heavy feature because we’ll need to send password hashes to an external service I think? And also we have to do this at login I think as we may not have the password in other code paths. This should have its own ticket and specifications.

  • Suspicious logins: Any suspicious logins? If it is difficult to to automatically, there could be an overview "Overview who accesses Nextcloud from where (Device, location)"

This would be an integration of https://github.com/nextcloud/suspicious_login , right?
I will have to dig into the app to see if it is easy or not to extract some global statistic on the whole user base for a setup check.

  • Brute force protection: Any suspicious attempts recently?
  • File access control: Are there recurring things being blocked by this?
  • Suspicious shares: Probably best to check specifically if there is an unusual pattern of access recently, similar to suspicious logins. Possibly it is acceptable to only show the filename/path, but definitely not allow the admin to view the content. This is possibly the biggest difficult topic as we don’t have existing logic for this.

Accounts

  • Quota almost full: Is someone very close to the quota? (90–100%)
  • Sync problems: Any sync issues, specific outages?
  • Inactive accounts: Are there any accounts not being active for a long amount of time?
  • Outdated apps/clients: Does anyone use outdated versions of the apps or clients? Here is the question where we display the full list. Could be a modal, or somehow in the user management.

For accounts it looks like we would still need #40744 only without the UI.
But at the same time some kind of UI will be needed at some point since knowing there are problems without knowing which accounts is useless.

I would love a bit more specifications about the UI elements expected for each check result. Currently in #32550 there is a description (plain text), a title, a status (success/info/warning/error), and a link to related documentation (sometimes nextcloud documentation, sometimes something else like PHP or apache documentation). From your description we would need to add a link to related settings page in Nextcloud, and in some cases a link to detailed UI (userlist and things like that?).

@come-nc
Copy link
Contributor

come-nc commented Oct 9, 2023

Small update:

  • UI should be in a separate application called admin_overview (unless someone has a better naming in mind?)
  • Passwords are already compared against a password leak database, at login, and the information is stored in configuration database, so it will be easy and cheap to extract the information (pointer Allow to check against haveibeenpwned.com password list password_policy#60 )

@jancborchardt
Copy link
Member Author

UI should be in a separate application called admin_overview (unless someone has a better naming in mind?)

Should it not be in core? Especially with the new mockup by @marcoambrosini (please post it here :) the security & setup checks could eventually be replaced by that much nicer UI?

@sorbaugh
Copy link
Contributor

UI should be in a separate application called admin_overview (unless someone has a better naming in mind?)

Should it not be in core? Especially with the new mockup by @marcoambrosini (please post it here :) the security & setup checks could eventually be replaced by that much nicer UI?

That was the original plan, but there were a few requirement changes. The most significant being that this feature should be a separate app. However as I understand it it will still be possible to display everything where we want it.

@marcoambrosini
Copy link
Member

Here's the updated dashboard mockup

Screenshot 2023-10-23 at 12 43 26

If there are problems, like in the case of sync problems in the mockup, it would be possible to click on the item and open a list of problems in a modal window.

If, like in this case there are further nested "problem categories" we can add those in the form of a tabbed navigation in the modal, like in the mockup below

Screenshot 2023-10-23 at 12 59 52

@susnux
Copy link
Contributor

susnux commented Oct 24, 2023

I think for this we would need an other way of grouping, the current categorization is too loose.
Maybe we should add a new attribute to ISetupCheck like type which can only be server status, security, accounts.

Otherwise it looks like this:

vokoscreenNG-2023-10-25_00-08-54.mp4

(yes I now not perfect, it is just a testing version, and especially the subtitles need to be shrunk, but you see the Problem)

@szaimen
Copy link
Contributor

szaimen commented Oct 24, 2023

I think would be good to show this section below security & setup warnings right @nextcloud/designers ?

@marcoambrosini
Copy link
Member

@szaimen Those warnings will be present in the dashboard too if the app is available so I think it's fine to display it above everything else

@jancborchardt
Copy link
Member Author

@marcoambrosini can you update your mockup to include some more specific examples with wording, and what to do when there is more text like in @susnux’s video? Could also be succinct text in the overview and the ability to expand or having details in a modal.

And yes @szaimen @marcoambrosini ideally this section replaces the "Security & setup warnings" and all of the contents within are present inside here. Question is whether that is possible in the first version.

@susnux
Copy link
Contributor

susnux commented Oct 25, 2023

Maybe we should add a new attribute to ISetupCheck like type which can only be server status, security, accounts.

Or adjust the existing checks to only include the name and not a full description within the name.
E.g. from the video:
Checking DAV system address book should be rewritten to DAV system address book
Check PHP version -> PHP version

And then use the description for explanation. Otherwise the name it self will take all space where the subtitle / description should be.

and what to do when there is more text like in @susnux’s video?

Maybe just show like x chars / words (summarize it) and show the full description in the modal? Like some websites do it with "read more" or something like this.

@marcoambrosini
Copy link
Member

@jancborchardt @susnux In the mock-up I thought about showing only the number of problems in the dashboard, while the full text of the warning would appear only once the warning itself is clicked.
@susnux @come-nc could you please give me an example of the data structure for the checks so that I can think about it a bit more?

@come-nc
Copy link
Contributor

come-nc commented Oct 26, 2023

Hey, so some informations:

  1. The current naming is bad and is being worked on in Improve setup checks naming and improve database version check #41083 , especially to remove all the «Checking for» prefixes.
  2. Categories were put kind of random and should be adapted. That said I find it a bit restrictive to only use system/security/accounts, because that would put almost all of the checks in system.
  3. I think we should keep more categories in the data that in the UI, I like that apps are allowed to put their own category in category and their check will show up in «System» or a new «Apps» category. So the idea would be that category would be either one of the predefined ones (system/security/accounts), or a custom one which would appear in system in the UI.
  4. The current UI and mockups do not include the link to documentation which is in the data

@marcoambrosini Data structure as returned to the frontend (pardon my french):

{
    "dav": {
      "Vérification du carnet d'adresses système DAV": {
        "severity": "success",
        "description": "No outstanding DAV system address book sync.",
        "linkToDoc": null
      }
    },
    "security": {
      "Vérification de l'ancien certificat utilisateur importé": {
        "severity": "success",
        "description": null,
        "linkToDoc": null
      },
      "Vérification de la désactivation de l'ancien chiffrement côté serveur": {
        "severity": "success",
        "description": null,
        "linkToDoc": null
      },
      "Vérification de la version de PHP": {
        "severity": "warning",
        "description": "Vous exécutez actuellement PHP 8.0.30. PHP 8.0 est maintenant obsolète pour Nextcloud 27. Nextcloud 28 nécessite au moins PHP 8.1. Veuillez mettre à jour vers l'une des versions PHP officiellement compatibles fournies par le PHP Group dès que possible.",
        "linkToDoc": "https://secure.php.net/supported-versions.php"
      }
    },
    "config": {
      "Vérification du préfixe de région par défaut": {
        "severity": "info",
        "description": "Votre installation n’a pas de préfixe de région par défaut. C’est nécessaire pour valider les numéros de téléphone dans les paramètres du profil sans code pays. Pour autoriser les numéros sans code pays, veuillez ajouter \"default_phone_region\" avec le code ISO 3166-1 respectif de la région dans votre fichier de configuration.",
        "linkToDoc": "https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2#Officially_assigned_code_elements"
      },
      "Vérification des droits d'accès au fichier de configuration": {
        "severity": "success",
        "description": "Le fichier de configuration Nextcloud est modifiable",
        "linkToDoc": null
      }
    },
    "network": {
      "Connexion Internet": {
        "severity": "success",
        "description": null,
        "linkToDoc": null
      }
    },
    "php": {
      "Vérification du jeu de caractères par défaut de PHP": {
        "severity": "success",
        "description": null,
        "linkToDoc": null
      },
      "Vérification de l'option « output_buffering » de PHP": {
        "severity": "success",
        "description": null,
        "linkToDoc": null
      }
    },
    "database": {
      "Vérification de la version de la base de données": {
        "severity": "success",
        "description": null,
        "linkToDoc": null
      }
    },
    "ldap": {
      "Vérification des UUID LDAP invalides": {
        "severity": "success",
        "description": null,
        "linkToDoc": null
      }
    }
}

@susnux
Copy link
Contributor

susnux commented Oct 26, 2023

One problem I see here with the data structure is that categories are not fixed but random strings.

Meaning:

  1. As seen in the video that would be a lot of (single entry) categories
  2. If grouped under "Server status" nearly everything will be there
  3. ⚠️ We can not use the category in the UI as it is not translated -> will confuse users.

Especially 3 is bad, thats why fixed categories are better -> we can provide translations for them.

But we could also group them like:
image

But this would still require something like ISetupCheckCategory that provides a translation for that category or some other way of injecting it.
Then we can show all checks inside the category on the dialog.

For the UI this would also quite easy to implement without changing the current data structure,
as we only need a second object containing the category -> translation mapping { category: categoryL10N }.

For grouping we can define the categories to be grouped in the admin_overview app, this does not need to go into core.

@come-nc
Copy link
Contributor

come-nc commented Oct 26, 2023

But this would still require something like ISetupCheckCategory that provides a translation for that category or some other way of injecting it. Then we can show all checks inside the category on the dialog.

This is out of scope for current feature, let’s keep it simple, there is enough to do.

I propose that for now we stay with fixed list of categories in admin_overview system/security/accounts, and any unknown category is merged into system. Once all checks are migrated or added we can see how many checks we have in each category and whether we need to change this.

@susnux
Copy link
Contributor

susnux commented Oct 26, 2023

This is out of scope for current feature, let’s keep it simple, there is enough to do.

Yes but adding this API afterwards seems to be more complicated?

@marcoambrosini
Copy link
Member

marcoambrosini commented Oct 27, 2023

Okay, given the complexity of this, I would introduce a horizontal navigation between 2 modal screens:

  • problems list
  • problems details

In case there's an additional level of nesting I would show the tabbed navigation we introduced in previous designs. So in total, we have 3 levels where to organize all this data:

Dashboard buckets (system/security/accounts)
Tabbed navigation (No more than a few categories)
Problems list

Section 1

@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@jancborchardt jancborchardt changed the title 🔮 Admin overview 🔮 Admin overview / Security Guard Dec 13, 2023
@sorbaugh
Copy link
Contributor

De-scoping #40661 to be worked on as it's own feature for the future

@come-nc come-nc closed this as completed Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of design Design, UI, UX, etc. feature: settings overview ❄️ 2023-Winter
Projects
Archived in project
Archived in project
Development

Successfully merging a pull request may close this issue.