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

REST: add runtime info to root resource response #1570

Merged
merged 1 commit into from Jul 25, 2020

Conversation

ghys
Copy link
Member

@ghys ghys commented Jul 25, 2020

This adds basic information about the runtime in the response
to the root /rest API resource, mostly for display purposes
by UIs: the version, build string and the location of the
configuration and user data folders.

Signed-off-by: Yannick Schaus github@schaus.net

This adds basic information about the runtime in the response
to the root `/rest` API resource, mostly for display purposes
by UIs: the version, build string and the location of the
configuration and user data folders.

Signed-off-by: Yannick Schaus <github@schaus.net>
@kaikreuzer kaikreuzer merged commit 0d635ce into openhab:master Jul 25, 2020
@kaikreuzer
Copy link
Member

Seeing the config folder names now in the root of the rest api, which is unsecured, I somehow feel it isn't really a good idea - it exposes internals that the user might not want to expose this way.
I'd feel better to remove it here again and if you really have a need for this information, it could be moved into a sub-resource that can then be restricted to admin access. Wdyt?

@ghys
Copy link
Member Author

ghys commented Jul 28, 2020

Yes I mentioned before this could be considered sensible information, finally decided it was relatively fine (if someone can use this maliciously, they have access to your filesystem and you probably have bigger problems), the intent in having it displayed is only to help users locate their configuration/userdata folders if they have e.g. a package installation and wonder where they are, so it's not that important.
tl;dr I don't feel too strongly either way - your call!

@cweitkamp cweitkamp added this to the 3.0 milestone Aug 1, 2020
@spacemanspiff2007
Copy link
Contributor

Shouldn't the UUID be part of this, too?

ghys added a commit to ghys/openhab-webui that referenced this pull request Aug 10, 2020
Uses info from openhab/openhab-core#1570 if available.

Signed-off-by: Yannick Schaus <github@schaus.net>
@ghys
Copy link
Member Author

ghys commented Aug 10, 2020

Shouldn't the UUID be part of this, too?

Makes sense.
Edit: there's already rest/uuid for that, are you aware and suggesting it to be moved?

@kaikreuzer
Copy link
Member

there's already rest/uuid for that

Correct. The uuid support is done as a separate bundle in the core framework on purpose as there might be consumers that do not want to include it. We hence cannot and should not include it in the root resource.

@kaikreuzer
Copy link
Member

Yes I mentioned before this could be considered sensible information, finally decided it was relatively fine (if someone can use this maliciously, they have access to your filesystem and you probably have bigger problems), the intent in having it displayed is only to help users locate their configuration/userdata folders if they have e.g. a package installation and wonder where they are, so it's not that important.
tl;dr I don't feel too strongly either way - your call!

I personally would tend to remove it, but I might be simply too cautious here.
@wborn & @cweitkamp Any preferences how to proceed?

kaikreuzer pushed a commit to openhab/openhab-webui that referenced this pull request Aug 11, 2020
* Add version & system info to about page.
Uses info from openhab/openhab-core#1570 if available.

Signed-off-by: Yannick Schaus <github@schaus.net>
@wborn
Copy link
Member

wborn commented Aug 12, 2020

Perhaps the info can be moved to a secured resource? That should prevent user/real names leaking to unauthorized users when OH is ran from a homedir.

The OH version number in the response also makes it easier to attack using known vulnerabilities. But the version can probably also be derived from look/feel, REST resources, OpenAPI spec, Jetty HTTP headers.

@kaikreuzer
Copy link
Member

A secured resource sounds good - for the admin, this is useful information and he is anyhow the only one that should require it.
Is there any existing candidate or would we have to introduce a new resource for that?

@ghys
Copy link
Member Author

ghys commented Aug 12, 2020

In that case, this resource could contain some more information for the admin:

  • system metrics
  • select environment variables (i.e. the Java VM version/vendor, OS name/version...)
  • maybe also various counters (number of things, items, inbox entries, rules...) - the UI Settings page retrieves these counters by querying the whole lists, which is not ideal...
  • ...

@cweitkamp
Copy link
Contributor

Any preferences how to proceed?

I vote for protecting the endpoint. We should not expose sensitive information.

@kaikreuzer
Copy link
Member

Ok, I have created #1608, which introduces a new /systeminfo url, which is only accessible for admins.
The folder information was moved from the root resource to the systeminfo resource, which additionally provides information about Java, OS, processors and memory.

Wrt other system and openHAB metrics, I left this out of scope for now as I think this will be much more complex and we already have #774 for it.

@ghys
Copy link
Member Author

ghys commented Aug 24, 2020

Great, created openhab/openhab-webui#313 to display them, what do you think about those, maybe they would have been interesting to have too?

openhab.logdir  /opt/openhab/userdata/logs (logs are sometimes out of userdata i.e. /var/log/openhab)
java.vendor     Azul Systems, Inc.
java.vendor.version Zulu11.39+61-CA

Also would it be a good idea to add some information from the i18n provider to the unsecured runtimeInfo part of the RootBean; at least the configured locale would enough, not sure about the rest (timezone, measurement system) but certainly not the configured location (latitude/longitude) - that would come in handy when localizing the UI since the locale would be available early and the /rest/services/org.openhab.i18n/config is inaccessible to non-admins anyways. HABPanel uses that in OH2 for its l20n but it would have to be changed as well for this reason.

@wborn
Copy link
Member

wborn commented Aug 24, 2020

Thanks for the Web UI update. I'm fine with adding this info as well. It would also be nice if the UI one day can use this info (incl. installed add-ons) to generate a copy/pastable overview that users can use when creating issues/forum posts. :-)

@kaikreuzer
Copy link
Member

@ghys I've created #1610, so you an directly adapt your PR before it is being merged :-)

@ghys
Copy link
Member Author

ghys commented Aug 25, 2020

Thanks, I added those to the About page.

incl. installed add-ons) to generate a copy/pastable overview that users can use when creating issues/forum posts.

Done that as well. I list the installed bindings (not the other types) from /rest/bindings instead of /rest/addons, IMHO /rest/addons can be misleading & inaccurate because it doesn't show sideloaded add-ons, only those installed from the distro.

splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 11, 2023
This adds basic information about the runtime in the response
to the root `/rest` API resource, mostly for display purposes
by UIs: the version, build string and the location of the
configuration and user data folders.

Signed-off-by: Yannick Schaus <github@schaus.net>
GitOrigin-RevId: 0d635ce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core REST/SSE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants