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

#13 Add support for NC-Token #15

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

shalak
Copy link

@shalak shalak commented Feb 18, 2023

Proposed fix for #13

@shalak
Copy link
Author

shalak commented Feb 18, 2023

@arnowelzel

From what I see in the UrlService.py, the following PR should add the possibility to use NC-Token.

However, for some reasons, it doesn't work on my installation:

netdata@localhost:/usr/libexec/netdata/python.d$ /usr/libexec/netdata/plugins.d/python.d.plugin nextcloud debug trace
2023-02-18 22:13:50: python.d INFO: plugin[main] : using python v3
2023-02-18 22:13:50: python.d DEBUG: plugin[main] : looking for '/etc/netdata/python.d.conf'
2023-02-18 22:13:50: python.d INFO: plugin[main] : '/etc/netdata/python.d.conf' was not found
2023-02-18 22:13:50: python.d DEBUG: plugin[main] : looking for '/usr/lib/netdata/conf.d/python.d.conf'
2023-02-18 22:13:50: python.d DEBUG: plugin[main] : '/usr/lib/netdata/conf.d/python.d.conf' is loaded
2023-02-18 22:13:50: python.d DEBUG: plugin[main] : looking for 'pythond-jobs-statuses.json' in /var/lib/netdata
2023-02-18 22:13:50: python.d DEBUG: plugin[main] : loading '/var/lib/netdata/pythond-jobs-statuses.json'
2023-02-18 22:13:50: python.d DEBUG: plugin[main] : '/var/lib/netdata/pythond-jobs-statuses.json' is loaded
2023-02-18 22:13:50: python.d DEBUG: plugin[main] : [nextcloud] loaded module source : '/usr/libexec/netdata/python.d/nextcloud.chart.py'
2023-02-18 22:13:50: python.d DEBUG: plugin[main] : [nextcloud] looking for 'nextcloud.conf' in ['/etc/netdata/python.d', '/usr/lib/netdata/conf.d/python.d']
2023-02-18 22:13:50: python.d DEBUG: plugin[main] : [nextcloud] loading '/etc/netdata/python.d/nextcloud.conf'
2023-02-18 22:13:50: python.d DEBUG: plugin[main] : [nextcloud] '/etc/netdata/python.d/nextcloud.conf' is loaded
2023-02-18 22:13:50: python.d INFO: plugin[main] : [nextcloud] built 1 job(s) configs
2023-02-18 22:13:50: python.d DEBUG: nextcloud[local] : urllib3 version: 1.26.5
2023-02-18 22:13:52: python.d ERROR: nextcloud[local] : Url: https://localhost/ocs/v2.php/apps/serverinfo/api/v1/info?format=json. Error: HTTPSConnectionPool(host='localhost', port=443): Max retries exceeded with url: /ocs/v2.php/apps/serverinfo/api/v1/info?format=json (Caused by ReadTimeoutError("HTTPSConnectionPool(host='localhost', port=443): Read timed out. (read timeout=1)"))
2023-02-18 22:13:52: python.d ERROR: nextcloud[local] : Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 445, in _make_request
    six.raise_from(e, None)
  File "<string>", line 3, in raise_from
  File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 440, in _make_request
    httplib_response = conn.getresponse()
  File "/usr/lib/python3.10/http/client.py", line 1374, in getresponse
    response.begin()
  File "/usr/lib/python3.10/http/client.py", line 318, in begin
    version, status, reason = self._read_status()
  File "/usr/lib/python3.10/http/client.py", line 279, in _read_status
    line = str(self.fp.readline(_MAXLINE + 1), "iso-8859-1")
  File "/usr/lib/python3.10/socket.py", line 705, in readinto
    return self._sock.recv_into(b)
  File "/usr/lib/python3.10/ssl.py", line 1274, in recv_into
    return self.read(nbytes, buffer)
  File "/usr/lib/python3.10/ssl.py", line 1130, in read
    return self._sslobj.read(len, buffer)
TimeoutError: The read operation timed out

However, the direct curl call works correctly:

netdata@localhost:/usr/libexec/netdata/python.d$ curl https://localhost/ocs/v2.php/apps/serverinfo/api/v1/info?format=json -H 'NC-Token: yourtoken'
{"ocs":{"meta":{"status":"ok","statuscode":200,"message":"OK"},"data":{"nextcloud":{"sys...

I was hoping you may test on your machine, maybe my configuration is somehow broken.

@shalak shalak changed the title PR#13 Add support for NC-Token #13 Add support for NC-Token Feb 18, 2023
@arnowelzel
Copy link
Owner

arnowelzel commented Feb 18, 2023

About your errror - most likely you must not use HTTPS for "localhost" since this will not provide a valid certificate:

return self._sslobj.read(len, buffer) TimeoutError: The read operation timed out

In addition: please first fix the things I commented above - if someone does not use a token at all, self.header = {"NC-Token": self.configuration.get('token')} will fail and passing an emtpy token will also fail.

@shalak
Copy link
Author

shalak commented Feb 18, 2023

About your errror - most likely you must not use HTTPS for "localhost" since this will not provide a valid certificate:

return self._sslobj.read(len, buffer) TimeoutError: The read operation timed out

I redacted my original hostname in this log and replaced it with localhost. I'm using my real domain to test. And with it, the curl call works OK, but somehow urllib3 doesn't.

In addition: please first fix the things I commented above - if someone does not use a token at all, self.header = {"NC-Token": self.configuration.get('token')} will fail and passing an emtpy token will also fail.

From what I tested, even providing an empty header in config, doesn't break the config, but you're right - makes no sense to include it if it's empty or None. PR updated.

But the issue I mentioned above is something we need to fix before merging this. I'm open for suggestions what might be wrong.

@arnowelzel
Copy link
Owner

About your errror - most likely you must not use HTTPS for "localhost" since this will not provide a valid certificate:
return self._sslobj.read(len, buffer) TimeoutError: The read operation timed out

I redacted my original hostname in this log and replaced it with localhost. I'm using my real domain to test. And with it, the curl call works OK, but somehow urllib3 doesn't.

Hmm - but the error message is typical for a non valid certificate. Will it disappear if you do not use a token?

In addition: please first fix the things I commented above - if someone does not use a token at all, self.header = {"NC-Token": self.configuration.get('token')} will fail and passing an emtpy token will also fail.

From what I tested, even providing an empty header in config, doesn't break the config, but you're right - makes no sense to include it if it's empty or None. PR updated.

What happens, if one does not provide username/password?

@shalak
Copy link
Author

shalak commented Feb 18, 2023

Hmm - but the error message is typical for a non valid certificate.

WIth invalid cert, it would return SSLError(SSLError(1, '[SSL: TLSV1_ALERT_INTERNAL_ERROR] tlsv1 alert internal error (_ssl.c:997)'))). The read operation timed out looks like a wonky server, that doesn't reply in time.

Will it disappear if you do not use a token?
What happens, if one does not provide username/password?

Without token, it falls-back to username/password. And without any of them, we get 401:

2023-02-18 23:14:56: python.d DEBUG: nextcloud[local] : Url: https://MY_DOMAIN/ocs/v2.php/apps/serverinfo/api/v1/info?format=json. Http response status code: 401

@shalak
Copy link
Author

shalak commented Feb 18, 2023

Ah, ok - this was indeed the "not-fast-enough" server issue. The timeout was set to 1, and one second wasn't enough for my nextcloud instance. With bigger timeout, I get successful connection, however with the 401 error code. Apparently the NC-Token is not being properly detected when injected in this way.

So the solution doesn't work, I'm open to ideas how to fix this.

@arnowelzel
Copy link
Owner

Ah, ok - this was indeed the "not-fast-enough" server issue. The timeout was set to 1, and one second wasn't enough for my nextcloud instance. With bigger timeout, I get successful connection, however with the 401 error code. Apparently the NC-Token is not being properly detected when injected in this way.

So the solution doesn't work, I'm open to ideas how to fix this.

I think there is no way to fix this.

The tokens are intended to be used by applications inside the Web-UI of Nextcloud to be able to communicate with the server backend and not for the API.

As I already mentioned in the the documentation the idea is to create an "app password" so you don't have to use your real user password for the Netdata plugin.

@shalak
Copy link
Author

shalak commented Feb 18, 2023

It works :) The NC-Token is explicitly intended to be used by external monitoring applications. No need for dedicated user :)

However if you provide user+pass, the token is being ignored. I was getting 401s because I did not use real values for user and pass and hoping that UrlService will use both user/pass & the token. It didnt't, it must explicitly omit user/pass in order for the self.header to be set properly from upstream configuration dict.

PR is ready for review. I added a exception in case someone sets both. I can change it to self.error("user/pass provided, ignoring token value"), if you prefer.

@shalak
Copy link
Author

shalak commented Feb 18, 2023

I updated the docs to reflect new functionality, and I removed the mention of metadata, as it's not needed to run this plugin (I don't have it).

Docs suggested that setting hostname is enough, where in reality,
a user must set base url (i.e. protocol + hostname)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants