Roundup Tracker - Issues

Issue 2551048

classification
Config: secret_key in [web] section: Default not constant for multi-process
Type: Severity: normal
Components: API Versions: devel, 2.0.0alpha
process
Status: fixed fixed
:
: rouilj : rouilj, schlatterbeck
Priority: : rest

Created on 2019-06-23 12:52 by schlatterbeck, last changed 2019-06-25 21:05 by rouilj.

Messages
msg6558 Author: [hidden] (schlatterbeck) Date: 2019-06-23 12:52
When running roundup as a multi-process application like CGI or WSGI,
the default value for the variable 'secret_key' in secion [web] is not
constant and will be different for each process. The reason is that it
uses a random value for initialisation if the item is unconfigured. This
random value is different in each process.

The implication is that with the (empty) default value for secret_key
REST-API updates will not work because the etag verification will fail
(at least if the first request to retrieve the etag and the second
request to do the update are serviced by different processes which is
very likely).

We should either come up with a fix or document this in the REST-API
documentation.
msg6560 Author: [hidden] (rouilj) Date: 2019-06-23 17:12
Hi Ralf:

In message <1561294337.71.0.360196406483.issue2551048@roundup.psfhosted.org>,
Ralf Schlatterbeck writes:
>When running roundup as a multi-process application like CGI or WSGI,
>the default value for the variable 'secret_key' in secion [web] is not
>constant and will be different for each process. The reason is that it
>uses a random value for initialisation if the item is unconfigured. This
>random value is different in each process.
> [...]
>We should either come up with a fix or document this in the REST-API
>documentation.

One of two things should be happening here:

  1) If that option is not set in config.ini, there should be a
     failure on startup. Similar to not setting TRACKER_WEB.
     Is secret_key in your config.ini set to nothing:
       [web]
       secret_key =
     or is secret_key not present in your config.ini? If not present,
     I can see how this could happen, but I am not sure how to fix it. 

  2) That key should be set to a constant value when running:

      roundup-admin -i . updateconfig config.ini

     as specified in doc/upgrading.txt as:

       Migrating from 1.6.0 to x.y.0
       =============================

       Upgrade tracker's config.ini file
       --------------------------------------
       Once you have installed the new roundup, use:

	 roundup-admin -i /path/to/tracker updateconfig new_init_file.ini

       to generate a new ini file preserving all your settings. You can then
       ...

     so that value shouldn't be empty.

The secret_key is defined using configuration.py's MandatoryOption
class. So using:

  secret_key =

should be an error. If secret_key is not in the config.ini, then
configuration.py doesn't try to set secret_key to an unset value and
the exception for a null value is not raised. Could this be the
failure mode you are seeing?
msg6563 Author: [hidden] (rouilj) Date: 2019-06-23 23:04
Hi Ralf:

In message <20190623171201.362E14C0411@itserver6.localdomain>,
"John P. Rouillard" writes:
>In message <1561294337.71.0.360196406483.issue2551048@roundup.psfhosted.org>,
>Ralf Schlatterbeck writes:
>>We should either come up with a fix or document this in the REST-API
>>documentation.

I have a changed to the code so that an error is thrown if you try to
access WEB_SECRET_KEY. Since the rest interface is all that does that
at the moment, this trips when using the rest interface.

The rest client sees the error:

    {
	"error": {
	    "status": 400,
	    "msg": "Sun Jun 23 18:02:03 2019: An error occurred. Please check the server log for more information."
	}
    }

The traceback error in the server log/emailed to the admin is:

    ...
    File "/home/rouilj/local/lib/python3.4/site-packages/roundup/configuration.py", line 197, in get
      raise OptionUnsetError(self)
  roundup.configuration.OptionUnsetError: WEB_SECRET_KEY is not set and has no default

That at least makes the misconfig obvious.

I don't like it though. What I really want is the ability to get a
secret key automatically generated when running roundup-admin with:

   install
   genconfig or
   updateconfig

if secret_key is not set. This key is not something that the
user/admin should care about.  But I can't figure out how to make that
work at the configuration class level.

If secret_key is not present in the config file and I supply a default
value in the definition of secret_key, we end up without an error and
a continually changing secret_key. I.E. your bug.

If I set the default value to NODEFAULT I don't get an automatic
secret_key but we do get the error above.

>One of two things should be happening here:
>
>  1) If that option is not set in config.ini, there should be a
>     failure on startup. Similar to not setting TRACKER_WEB.
>     Is secret_key in your config.ini set to nothing:
>       [web]
>       secret_key =
>     or is secret_key not present in your config.ini? If not present,
>     I can see how this could happen, but I am not sure how to fix it. 

The failure is triggered if the config has secret_key set to nothing in
config.ini.

>  2) That key should be set to a constant value when running:
>
>      roundup-admin -i . updateconfig config.ini
>
>     as specified in doc/upgrading.txt ...

My guess is you didn't run this and as a result have a hidden problem.

You suggested documenting this in the rest doc. Would adding something
like this at the end of the "Enabling the REST API" section work:

  Make sure that config.ini has a secret_key option defined in the web
  section of config.ini. If you followed the upgrading directions, it
  is done automatically when running "roundup-admin ... updateconfig
  config.ini". If you are installing a new tracker with "roundup-admin
  ... install" the secret_key value is automatically set to some
  random value. If this is not set, you will see different etag values
  for the same unchanged item on each REST call.

I would prefer to document this and leve the existing code as is as I
think it is more user friendly provided the update instructions to run
"roundup-admin .... updateconfig" works.

Thoughts?
msg6564 Author: [hidden] (schlatterbeck) Date: 2019-06-24 09:28
On Sun, Jun 23, 2019 at 11:04:23PM +0000, John Rouillard wrote:
> The traceback error in the server log/emailed to the admin is:
...
> That at least makes the misconfig obvious.
> 
> I don't like it though. What I really want is the ability to get a
> secret key automatically generated when running roundup-admin with:

Yes, the value should somehow be set.

> >  2) That key should be set to a constant value when running:
> >
> >      roundup-admin -i . updateconfig config.ini
> >
> >     as specified in doc/upgrading.txt ...
> 
> My guess is you didn't run this and as a result have a hidden problem.

No, I didn't. And when grepping for secret_key in doc it didn't turn up
anything .-)

> 
> You suggested documenting this in the rest doc. Would adding something
> like this at the end of the "Enabling the REST API" section work:
> 
>   Make sure that config.ini has a secret_key option defined in the web
>   section of config.ini. If you followed the upgrading directions, it
>   is done automatically when running "roundup-admin ... updateconfig
>   config.ini". If you are installing a new tracker with "roundup-admin
>   ... install" the secret_key value is automatically set to some
>   random value. If this is not set, you will see different etag values
>   for the same unchanged item on each REST call.
> 
> I would prefer to document this and leve the existing code as is as I
> think it is more user friendly provided the update instructions to run
> "roundup-admin .... updateconfig" works.

Yes, I like the variant with the documentation. This proposal also takes
care about the grep for secret_key turning up something .-)

Thanks for looking into this

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   http://www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
msg6565 Author: [hidden] (rouilj) Date: 2019-06-25 00:03
Hi Ralf:

In message <20190624092826.ims6dm6nlscxwjlj@runtux.com>,
Ralf Schlatterbeck writes:
>On Sun, Jun 23, 2019 at 11:04:23PM +0000, John Rouillard wrote:
>> >  2) That key should be set to a constant value when running:
>> >
>> >      roundup-admin -i . updateconfig config.ini
>> >
>> >     as specified in doc/upgrading.txt ...
>> 
>> My guess is you didn't run this and as a result have a hidden problem.
>
>No, I didn't. And when grepping for secret_key in doc it didn't turn up
>anything .-)

Can you try this to make sure the proper initialized entry is added to
config.ini. The updateconfig should make a config.ini.bak with your
original config.ini. However I would make a separate backup first.

>> You suggested documenting this in the rest doc. Would adding something
>> like this at the end of the "Enabling the REST API" section work:
>>     [removed]
>> I would prefer to document this and leve the existing code as is as I
>> think it is more user friendly provided the update instructions to run
>> "roundup-admin .... updateconfig" works.
>
>Yes, I like the variant with the documentation. This proposal also takes
>care about the grep for secret_key turning up something .-)

Sounds good. If you verify the upgrade instructions, I'll work on
adding the docs.

>Thanks for looking into this

Sure, I wrote the code so I am responsible.

Have a great week.
msg6566 Author: [hidden] (schlatterbeck) Date: 2019-06-25 11:38
On Tue, Jun 25, 2019 at 12:03:43AM +0000, John Rouillard wrote:
> >No, I didn't. And when grepping for secret_key in doc it didn't turn up
> >anything .-)
> 
> Can you try this to make sure the proper initialized entry is added to
> config.ini. The updateconfig should make a config.ini.bak with your
> original config.ini. However I would make a separate backup first.

Once I discovered the reason (changing key) I made sure I had a config
with that value configured. It started working then, sorry when I was
unclear about that. The mechanism (updating the config will set a random
value in config.ini) does work.

Thanks
Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   http://www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
msg6567 Author: [hidden] (rouilj) Date: 2019-06-25 21:05
Per Ralf's last email this seems to work as intended.
Updated rest docs and pushed as part of rev5826:8e17c34a5cf0.
History
Date User Action Args
2019-06-25 21:05:06rouiljsetstatus: new -> fixed
assignee: rouilj
resolution: fixed
messages: + msg6567
2019-06-25 11:38:01schlatterbecksetmessages: + msg6566
2019-06-25 00:03:43rouiljsetmessages: + msg6565
2019-06-24 09:28:32schlatterbecksetmessages: + msg6564
2019-06-24 00:39:54rouiljsetcomponents: + API
2019-06-24 00:39:38rouiljsetkeywords: + rest
versions: + devel, 2.0.0alpha
2019-06-23 23:04:23rouiljsetmessages: + msg6563
2019-06-23 17:12:18rouiljsetmessages: + msg6560
2019-06-23 12:52:17schlatterbeckcreate