Proposal: easier tenant configuration based on application.properties

[Aleksandar Vidakovic <al...@apache.org>]

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

Re: Proposal: easier tenant configuration based on application.properties

[Manoj VM <ma...@fynarfin.io>]

..Thanks Aleks.


Thanks,
Manoj

On Mon, 19 Dec, 2022, 21:28 Aleksandar Vidakovic, <ch...@monkeysintown.com>
wrote:

> @Manoj VM <ma...@fynarfin.io> ... good point... there's a little bit of a
> caveat there... adding a tenant is usually not a task on the same level
> like adding a user... with that in mind, I think adding a tenant is more of
> a devops task than it is something that let's say a manager should be able
> to do. And if it's a devops task then a quick redeploy with changed startup
> parameters, new tenant configuration file and a consistent state of your
> Spring context shouldn't be too bad... if you have just one container
> instance running with Docker Compose then this might be less seamless...
> but on Kubernetes the redeployment should be ok (in terms of outages).
> Noted the suggestion... let's see if we can come with a better approach
> until the PR is wrapped up.
>
> @Bharath Gowda <bg...@mifos.org> as soon as I have something to show for
> I'll let you know... any help appreciated. Docs will be important to get
> everyone on board with this.
>
> On Mon, Dec 19, 2022 at 8:53 AM Bharath Gowda <bg...@mifos.org> wrote:
>
>> +1 for this Idea Aleks.
>>
>> This will help a lot of organizations and make work easier to spin out
>> new tenants when required.
>> Let me know how I can help you the same, maybe with documentation I could
>> be useful.
>>
>>
>> Regards,
>> Bharath
>> Lead Implementation Analyst | Mifos Initiative
>> Skype: live:cbharath4| Mobile: +91.7019635592
>> http://mifos.org  <http://facebook.com/mifos>
>> <http://www.twitter.com/mifos>
>>
>>
>> On Mon, Dec 19, 2022 at 11:27 AM Manoj VM <ma...@fynarfin.io> wrote:
>>
>>> 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 <
>>> cheetah@monkeysintown.com> 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 <ja...@gmail.com>
>>>> 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 <al...@apache.org>
>>>>> 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
>>>>>
>>>>

Re: Proposal: easier tenant configuration based on application.properties

[Aleksandar Vidakovic <ch...@monkeysintown.com>]

@Manoj VM <ma...@fynarfin.io> ... good point... there's a little bit of a
caveat there... adding a tenant is usually not a task on the same level
like adding a user... with that in mind, I think adding a tenant is more of
a devops task than it is something that let's say a manager should be able
to do. And if it's a devops task then a quick redeploy with changed startup
parameters, new tenant configuration file and a consistent state of your
Spring context shouldn't be too bad... if you have just one container
instance running with Docker Compose then this might be less seamless...
but on Kubernetes the redeployment should be ok (in terms of outages).
Noted the suggestion... let's see if we can come with a better approach
until the PR is wrapped up.

@Bharath Gowda <bg...@mifos.org> as soon as I have something to show for
I'll let you know... any help appreciated. Docs will be important to get
everyone on board with this.

On Mon, Dec 19, 2022 at 8:53 AM Bharath Gowda <bg...@mifos.org> wrote:

> +1 for this Idea Aleks.
>
> This will help a lot of organizations and make work easier to spin out new
> tenants when required.
> Let me know how I can help you the same, maybe with documentation I could
> be useful.
>
>
> Regards,
> Bharath
> Lead Implementation Analyst | Mifos Initiative
> Skype: live:cbharath4| Mobile: +91.7019635592
> http://mifos.org  <http://facebook.com/mifos>
> <http://www.twitter.com/mifos>
>
>
> On Mon, Dec 19, 2022 at 11:27 AM Manoj VM <ma...@fynarfin.io> wrote:
>
>> 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 <
>> cheetah@monkeysintown.com> 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 <ja...@gmail.com>
>>> 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 <al...@apache.org>
>>>> 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
>>>>
>>>

Re: Proposal: easier tenant configuration based on application.properties

[Bharath Gowda <bg...@mifos.org>]

+1 for this Idea Aleks.

This will help a lot of organizations and make work easier to spin out new
tenants when required.
Let me know how I can help you the same, maybe with documentation I could
be useful.


Regards,
Bharath
Lead Implementation Analyst | Mifos Initiative
Skype: live:cbharath4| Mobile: +91.7019635592
http://mifos.org  <http://facebook.com/mifos>
<http://www.twitter.com/mifos>


On Mon, Dec 19, 2022 at 11:27 AM Manoj VM <ma...@fynarfin.io> wrote:

> 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 <
> cheetah@monkeysintown.com> 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 <ja...@gmail.com>
>> 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 <al...@apache.org>
>>> 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
>>>
>>

Re: Proposal: easier tenant configuration based on application.properties

[Manoj VM <ma...@fynarfin.io>]

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 <
cheetah@monkeysintown.com> 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 <ja...@gmail.com>
> 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 <al...@apache.org>
>> 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
>>
>

Re: Proposal: easier tenant configuration based on application.properties

[Aleksandar Vidakovic <ch...@monkeysintown.com>]

... 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 <ja...@gmail.com> 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 <al...@apache.org>
> 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
>

Re: Proposal: easier tenant configuration based on application.properties

[James Dailey <ja...@gmail.com>]

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 <al...@apache.org>
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