Page MenuHomeFreeBSD

www/tt-rss: Fix permissions, apache sample, add php dep.
ClosedPublic

Authored by dereks_lifeofadishwasher.com on Aug 13 2020, 9:58 PM.

Details

Summary
  • Default permissions for cache/images broken on new installations
  • Permissions on misc dirs not set to www:www
  • apache sample updated with 2.4 syntax
  • Add missing PHP extension iconv

Reported by Ulrich Spörlein #248601

Test Plan
  • bulk, reinstall
  • confirm WWWDIR/tt-rss/ has zero root:wheel files
  • confirm installer doesn't complain about missing iconv ext.
  • test apache sample config
  • Tested this with uninstall/reinstalls confirming the permissions were what we expect.
  • Ran thru the web installer without errors.
  • Switched options to confirm the proper depends were being added.
  • Reviewed ttrssd was be commented properly with the new OPT_VARS= method.

Diff Detail

Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 33018
Build 30406: arc lint + arc unit

Event Timeline

As for (2), I think we should fix it, especially as:

  • the fix is trivial, just don't rename the file, and
  • the web-based setup seems more documented online and apparently is what the developers of tt-rss support, and
  • that web-based setup correctly flagged the missing iconv module, so it's a good thing :) It was also easier to setup and init the database. I rarely need to use the psql or mysql CLIs so not having to deal with them is a plus :)

Is there a reason the file is being renamed in the first place? That seems a bit gratuitous. If the changed name is really required, I would suggest to copy/link instead of renaming it, so both dist or sample files are present (and updated).

The file was moved by 197045. It was suitable as a use case for the @sample plist documented here.

If I had to guess why the config.php was crated it prevents someone from entering the web based installer on first install that you otherwise didn't want to make changes and to enforce the file permissions of 0400. This is only a guess having a config.php has existed since the ports creation. I'd rather keep it this way.

However, supporting the web installer might still be possible by an addition to the pkg-message mentioning:

  • if you want to use the web installer:
    • rm config.php
    • ln -s config.php.sample config.php-dist (maybe do this by default in the port as you mentioned).
    • Goto http://.../install/

However, depending on the config of the web server this might leave config.php readable by group/others. Maybe add a patch for touch/chmod the file before writing it?

I'm not sure the rename is worth it, just to make the @sample usage simpler. There's even support for -dist, so let's use that.

Don't worry too much about permissions either. The web UI helpfully prints out the full config (except it wasn't able to print anything in my case) telling folks to copy-paste that into their config. There's a button that one can use to _try_ and have the webserver write it out, but of course that _should_ fail most of the time for strictly configured webservers.

Oh wait, if we use @sample, then config.php-dist will be copied to config.php (but it has bogus values) so the whole web config will fall on its nose. So I think the best way forward is to not use @sample at all and instead ship config.php-dist as a regular file (that get's updated with a new version).

Then the users can copy that to config.php or let the web UI spit out a config.php that they can write down using sudo or whatever.

I still believe having a config.php is the right thing to do here. Looking at other packaged tt-rss they all include a config.php so I think the same should still appear here.
config.php-dist is now there so opting in to the web installer is simple enough (rm config.php) and the web installer just works with writing the config.

Cleaned the pkg-plist removing useless files and to only be www user writable in the places its required. root:www g+r every where else.

A couple nitpicks with the Makefile and if conditionals moving to portish options.

If you have a portable version of nginx config you want to add here I think that would be a good addition.

  • patch web isntaller to make config.php 0400
  • Use modern port settings mechinisms
  • Don't copy .empty files
  • Clean up pkg-plist to be root:www and www:www where write is needed
  • Don't try to edit config.php-dist on the fly. Leave that to admin post install or use the web installer.
  • Keep config.php-dist around for the web installer if desired
  • Rewrite inscrutions to support the web installer by opt-in by deleting config.php created by @sample

Tested this with uninstall/reinstalls confirming the permissions were what we expect.
Ran thru the web installer without errors.
Switched options to confirm the proper depends were being added.
Reviewed ttrssd was be commented properly with the new OPT_VARS= method.

Make the base directory root:www owned 755

Making the base directory www owned means it can delete and write to the directory undoing the main purpose of only allowing www write to only the places it needs to.

Since the web installer can't write to the base directory remove the patch as well.

Looks good, do you want me to go ahead and commit this on your behalf?

www/tt-rss/files/pkg-message.in
21–23

This can be mistaken as being obsolete when using the web installer. Maybe it should be rephrased?

"In both cases, enable the daemon ..."

This revision is now accepted and ready to land.Aug 18 2020, 12:51 PM

Reword ttrssd instructions required for both web and manual installs

This revision now requires review to proceed.Aug 18 2020, 1:32 PM
In D26061#579331, @uqs wrote:

Looks good, do you want me to go ahead and commit this on your behalf?

Yes, thanks!

This revision is now accepted and ready to land.Aug 18 2020, 2:36 PM