Thanks Alekx, .... One feature that currently we do not have is to set up a new tenant dynamically on a running system. The configurations usually are loaded in the startup and that may also inherit the same issue. While most of the implementations do not require dynamic loading of a new tenant, I feel this is a good-to-have and usefull feature.
you have already addressed the "sharing credentials in the property files" concern. Thanks and Regards, Manoj Mohanan On Mon, Dec 19, 2022 at 11:02 AM Aleksandar Vidakovic < [email protected]> wrote: > ... definitely James... just left a comment on the Jira ticket in response > to Victor. Another important point is to introduce this feature in a > non-disruptive way. Those that have systems already running will get a hint > (in the boot log for example) to move to the new way of tenant > configuration and if you start Fineract fresh (first time) properties > approach could be default (pending vote of community). > In the comment to Victor I've mentioned also that we can't just dump > credentials in files or logs... made a proposal with a secured REST > endpoint that admins can use to download any migration properties (in > production you can turn this feature off). Pretty sure there will be more > input from the community on that point. > In any case: documentation will be included. > > On Mon, Dec 19, 2022 at 6:12 AM James Dailey <[email protected]> > wrote: > >> Thanks Aleks >> >> Back in the day, indeed before this use of a database table for tenants, >> we used a property file for all sorts of useful run time configuration. >> >> My only caution would be to - through clear documentation- make it >> obvious what belongs in the application.properties and what does not. >> >> >> >> On Sun, Dec 18, 2022 at 8:59 PM Aleksandar Vidakovic <[email protected]> >> wrote: >> >>> Hi everyone, >>> >>> ... just wanted to run an idea by you that I think would be a great >>> improvement both in terms of security and usability. I've created a Jira >>> ticket with all the details (as much as I have them currently on my radar) >>> at https://issues.apache.org/jira/browse/FINERACT-1833 and would really >>> appreciate your thoughts and additional feedback, things that I might have >>> missed, things you miss in the proposal. I'll post the description here for >>> convenience. >>> >>> At the moment Fineract's multi-tenancy feature is based on a separate >>> tenant database with a single table; each row contains the database >>> connection details and timezone settings for each tenant. I am proposing to >>> make this feature an official Fineract extension point and to provide an >>> alternative implementation based on application.properties instead of the >>> database approach. >>> >>> Using application.properties is - for a Spring Boot - the best place to >>> put any kind of application configuration. Since Fineract's inception a lot >>> has happened in the Spring/Boot eco-system. Reloadable configurations are >>> nothing strange anymore and a solved problem. In fact, they are especially >>> useful when applications are deployed in a Kubernetes environment and >>> ensure that Fineract's application context is always in a correct state. A >>> recent example where this was applied was in the fix for a file traversal >>> vulnerability related to ContentRepository (see FINERACT-1794). Instead of >>> using the JDBC based ConfigurationService we moved the configuration for >>> file system and S3 based file storage to application.properties. This makes >>> life immediately easier for everyone, we removed another point of failure >>> (the database) and we laid the ground to make this a true extensible >>> feature (watch this space, we have a proposal to add Azure Storage) without >>> having to deal with database schemas and Liquibase migrations. It is so >>> much easier just to deal with the properties files and to adapt them if >>> needed. >>> >>> To ensure that configuration information is separated by tenant we would >>> not store everything in the default application.properties file. Obviously >>> we don't know yet which tenants users want to add; and if we stored the >>> tenant information in that file we would need to constantly (well, every >>> time we add a tenant) overwrite that file. This would be at the least very >>> annoying, because this file is under Git control, means: when the next >>> release upgrade needs to be applied then there is a potential for dropping >>> the ball and someone overwrites your tenant configuration. Instead, each >>> tenant's configuration would be provided in a separate Spring Boot profile >>> configuration, e. g. the default tenant's configuration would be provided >>> in a file named "application-tenant-default.properties". The prefix >>> "application-tenant-" is a convention that everyone should follow. We might >>> have later other features that could use profile configurations and/or >>> custom modules might use these (profile) mechanics too. Just to avoid any >>> collisions. This file based tenant configuration approach would allow you >>> to easily add and remove (e. g. via Docker volume mount for the files and a >>> simple command line parameter for Fineract's startup command via >>> "-Dspring.profiles.active=default,tenant-default,tenant-abc,...") >>> additional tenants in a way that is very likely more "natural" for your >>> DevOps people, having to deal with configuration in a database is a bit of >>> a distracting context switch. >>> >>> And finally: the current approach has also some security related issues. >>> The credentials for the tenants' database connections are stored in plain >>> text which is pretty much a no-go (and we've already received requests from >>> community members to address this issue). If we move this to the properties >>> files then you can use pretty much any sensible strategy that is available >>> today to safely store credentials like vaults (Hashicorp Vault, Kubernetes >>> Secrets) or environment variables for example. These approaches are also >>> first class citizens supported by Spring Cloud (Kubernetes Secrets, vaults >>> etc.). The current database configuration also doesn't allow to properly >>> separate concerns between DevOps and developers (the database migration is >>> maintained in Git which means under the developers' control). Usually you >>> would want to keep this apart from each other. >>> >>> And no worries, there are no plans to remove the current way how tenants >>> are configured, but I think it would be a good idea to default to the >>> easier and more secure approach (properties) and still leave an option for >>> those who can't or don't want to switch. The idea is also to provide some >>> help with migrating your existing tenants to properties files. There could >>> be some migration component that can help creating the necessary files >>> (details to be defined), e. g. when the application boots up in the log >>> messages (similar how Spring Boot does it if there are e. g. configuration >>> properties changes etc.). >>> >>> Please let me know what you think. >>> >>> Cheers, >>> >>> Aleks >>> >> -- >> Sent from Gmail Mobile >> >
