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.

Sign up for GitHub

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

Jump to bottom

Add account warnings to admin settings #40744

Closed
wants to merge 11 commits into from
Closed

Add account warnings to admin settings #40744

wants to merge 11 commits into from

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Oct 2, 2023 •
edited
Loading

Summary

TODO

  • Add internal API to register providers for account warnings
  • Add rest API for the frontend to get the list of warnings
  • Implement provider for quota
  • Implement provider for outdated client version
  • Implement provider for client reported issues
  • Add capability for client diagnostics

Checklist

@come-nc come-nc added enhancement 2. developing Work in progress labels Oct 2, 2023
@come-nc come-nc added this to the Nextcloud 28 milestone Oct 2, 2023
@come-nc come-nc self-assigned this Oct 2, 2023
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 3, 2023
View reviewed changes
apps/settings/lib/AccountWarnings/AccountWarningsManager.php
}

/**
* @return array<class-string, array{name:string,warnings:array<string,string>}>

Check failure

Code scanning / Psalm

InvalidReturnType

The declared return type 'array<class-string, array{name: string, warnings: array<string, string>}>' for OCA\Settings\AccountWarnings\AccountWarningsManager::getAll is incorrect, got 'array<class-string<OCP\Settings\IAccountWarningsProvider>, array{name: string, warnings: array<'error'|'info'|'warning', non-empty-list<string>>}>'
apps/settings/lib/AccountWarnings/AccountWarningsManager.php
$results[$provider::class]['warnings'][$category][] = $warning->getText();
}
}
return $results;

Check failure

Code scanning / Psalm

InvalidReturnStatement

The inferred type 'array<class-string<OCP\Settings\IAccountWarningsProvider>, array{name: string, warnings: array<'error'|'info'|'warning', non-empty-list<string>>}>' does not match the declared return type 'array<class-string, array{name: string, warnings: array<string, string>}>' for OCA\Settings\AccountWarnings\AccountWarningsManager::getAll
apps/settings/lib/AccountWarnings/AccountWarningsManager.php
*/
public function getAll(): array {
$results = [];
$providerRegistrations = $this->coordinator->getRegistrationContext()->getAccountWarningsProviders();

Check notice

Code scanning / Psalm

PossiblyNullReference

Cannot call method getAccountWarningsProviders on possibly null value
come-nc added 10 commits October 3, 2023 17:23
@come-nc
Add ClientDiagnostic mapper and controller
88d6c8e 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Add migration to create DB table for client diagnostics
86fd999 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
json_encode client diagnostic
4687603 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Add OCP API to register account warnings providers
e67969e 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Add a provider of account warnings for outdated clients
f398b31 
Stub for now

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Implement account provider registration in RegistrationContext
a24a3d8 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Add route to get account warnings from server
b0528d3 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Add QuotaWarningsProvider to warn about account close to full quota
d01ad74 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Add user id information to quota warning
2f96c57 
This is a temporary solution, we’ll need to add the list in array format
 so that the UI can do something nice with it.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Add ClientDiagnosticWarningsProvider and fix diagnostic PUT route
3298503 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc force-pushed the enh/add-account-warnings-to-admin branch from b55394a to 3298503 Compare October 3, 2023 15:23
github-advanced-security[bot]
github-advanced-security bot found potential problems Oct 3, 2023
View reviewed changes
apps/settings/lib/Db/ClientDiagnostic.php
* @method \DateTime getTimestamp()
* @method void setTimestamp(\DateTime $timestamp)
*/
class ClientDiagnostic extends Entity {

Check notice

Code scanning / Psalm

PropertyNotSetInConstructor

Property OCA\Settings\Db\ClientDiagnostic::$id is not defined in constructor of OCA\Settings\Db\ClientDiagnostic or in any methods called in the constructor
apps/settings/lib/Db/ClientDiagnostic.php
];

/** @var int */
protected $authtokenid;

Check notice

Code scanning / Psalm

PropertyNotSetInConstructor

Property OCA\Settings\Db\ClientDiagnostic::$authtokenid is not defined in constructor of OCA\Settings\Db\ClientDiagnostic or in any methods called in the constructor
apps/settings/lib/Db/ClientDiagnostic.php
protected $authtokenid;

/** @var string json-encoded*/
protected $diagnostic;

Check notice

Code scanning / Psalm

PropertyNotSetInConstructor

Property OCA\Settings\Db\ClientDiagnostic::$diagnostic is not defined in constructor of OCA\Settings\Db\ClientDiagnostic or in any methods called in the constructor
apps/settings/lib/Db/ClientDiagnostic.php
protected $diagnostic;

/** @var \DateTime */
public $timestamp;

Check notice

Code scanning / Psalm

PropertyNotSetInConstructor

Property OCA\Settings\Db\ClientDiagnostic::$timestamp is not defined in constructor of OCA\Settings\Db\ClientDiagnostic or in any methods called in the constructor
@come-nc
Copy link
Contributor Author

come-nc commented Oct 3, 2023

To test current version:

  1. Run migration to create DB table: occ migrations:execute core 28000Date20231002171502
  2. Send diagnostic using an app password: curl -u admin:TDpBt-o8po8-4Tszg-FgNSL-AB4k3 --compressed -H 'User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/118.0' -H 'Accept: application/json, text/plain, */*' -H 'Content-Type: application/json' https://nextcloud.local/index.php/settings/diagnostics -X PUT -d '{"problems":{"conflict":{"count":3,"oldest":1695592800},"failed-upload":{"count":1,"oldest":1695592900}}}'
  3. Get account warning list, using cookies from browser:
$ curl 'https://nextcloud.local/index.php/settings/accountwarnings' --compressed -H 'User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/118.0' -H 'Accept: application/json, text/plain, */*' -H 'Accept-Language: fr,fr-FR;q=0.8,en-US;q=0.5,en;q=0.3' -H 'Accept-Encoding: gzip, deflate, br' -H 'requesttoken: RP+Iy80p7CpIAe9nm7AvBPVDUImk1zVapcKvKc6iodE=:I8/QmIhGmhkjc7cq0oVmfrIVZdzwok0T8KqYGqLu97s=' -H 'Connection: keep-alive' -H 'Cookie: nc_sameSiteCookielax=true; nc_sameSiteCookiestrict=true; __Host-nc_sameSiteCookielax=true; __Host-nc_sameSiteCookiestrict=true; oc0jgqj12wso=cd292f3de00c5dc05b53fe76014218cb; oc_sessionPassphrase=80lIfOj9C4k2O1Ez50FgVRE8DK195jqBsGppHfHxmaOYl4eLLkFC29%2Fh%2F6OuwSnezNTMkjghKAXw2HFdfLDxzuC0Em0C7UgnQ6Sv7ULtg0TPNHbJutf10TuFgJOVFft1; oc7sh4belq6y=a9453df81f6cf17f754d487d30ee8e45; ocmm5cagww7k=0243cb2330f4ece1638ff797bc712d8d; ocb483ivlvw3=7067c45e40e464e1efb1a37c1772d678; ocd6z2qt3ed4=d42ca7a29d04e0cb6b4e9a6730b75377; ocno0q30p5zb=4cd77e8ba87bbb36afaedcbd95566798; oc9vsucyughg=1013a59f75cfd29da19c5bf38875d400; oc7m24rvn3az=414572e055a44be404db457f5b17ff82; oca71g0c9utv=75d6a35ca8043847a5633029124c28ed; nc_username=admin; nc_token=vOoO4v0tTd1N15avMtv%2FcV6PXJDoMVbH; nc_session_id=75d6a35ca8043847a5633029124c28ed' -H 'Sec-Fetch-Dest: empty' -H 'Sec-Fetch-Mode: cors' -H 'Sec-Fetch-Site: same-origin' -H 'DNT: 1' -H 'Sec-GPC: 1' -H 'TE: trailers'|jq .
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   326  100   326    0     0    440      0 --:--:-- --:--:-- --:--:--   440
{
  "OCA\\Settings\\AccountWarnings\\ClientDiagnosticWarningsProvider": {
    "name": "Client errors",
    "warnings": {
      "warning": [
        "Account \"admin\" had 3 conflicts on client testdiag, oldest one on 2023-09-24 22:00:00",
        "Account admin had 1 failed upload on client testdiag on 2023-09-24 22:01:40"
      ]
    }
  },
  "OCA\\Settings\\AccountWarnings\\OutdatedClientWarningsProvider": {
    "name": "Outdated clients",
    "warnings": {
      "warning": [
        "2 users are using outdated client version \"Example Desktop client 3.5.0\""
      ]
    }
  },
  "OCA\\Settings\\AccountWarnings\\QuotaWarningsProvider": {
    "name": "Quota",
    "warnings": {
      "warning": [
        "2 accounts are using more than 90% of their quota: admin,user1"
      ],
      "error": [
        "1 account is using more than 98% of their quota: alice"
      ]
    }
  }
}

@come-nc
Move the route for pushing diagnostics to ocs endpoint
0cde8b9 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Copy link
Contributor Author

come-nc commented Oct 5, 2023

Last commit moves https://nextcloud.local/index.php/settings/diagnostics to https://nextcloud.local/ocs/v2.php/apps/settings/diagnostics

@come-nc come-nc mentioned this pull request Oct 9, 2023
Closed
8 tasks
@come-nc come-nc mentioned this pull request Oct 19, 2023
Closed
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
Merged
@blizzz blizzz mentioned this pull request Nov 6, 2023
Merged
@come-nc come-nc removed this from the Nextcloud 28 milestone Nov 9, 2023
@solracsf solracsf added this to the Nextcloud 28 milestone Nov 21, 2023
@blizzz blizzz mentioned this pull request Nov 22, 2023
Merged
5 tasks
@blizzz blizzz removed this from the Nextcloud 28 milestone Nov 23, 2023
@blizzz blizzz added this to the Nextcloud 29 milestone Nov 23, 2023
@come-nc come-nc closed this Feb 1, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Feb 23, 2024
@come-nc come-nc mentioned this pull request Feb 29, 2024
Closed
@skjnldsv skjnldsv deleted the enh/add-account-warnings-to-admin branch March 14, 2024 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@come-nc come-nc

Labels
2. developing Work in progress enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

🔮 Admin overview / Security Guard
4 participants
@come-nc @blizzz @solracsf @skjnldsv