telefonicaid / fiware-pep-steelskin

Telefonica's implementation of the FIWARE PEP GE
GNU Affero General Public License v3.0
0 stars 0 forks source link

Possible race condition on variable requestTemplate and roleTemplate #477

Closed Soulike closed 2 years ago

Soulike commented 2 years ago

I think there is a race condition in lib/services/validation.js.

When initializeProxy() is called, it registers middlewares including validationProcess() in validation.js

https://github.com/telefonicaid/fiware-pep-steelskin/blob/7cb31b3f0817813719ae041be0697ca2e18e86a8/lib/fiware-pep-steelskin.js#L253

and validationProcess() asynchronously calls createAccessRequest(), in which reads roleTemplate and requestTemplate

https://github.com/telefonicaid/fiware-pep-steelskin/blob/7cb31b3f0817813719ae041be0697ca2e18e86a8/lib/services/validation.js#L64-L67

It means that roleTemplate and requestTemplate may be read when the proxy server starts listen.

However, the initialization of roleTemplate and requestTemplate is asynchronously done in the listen() callback with validation.loadTemplates()

https://github.com/telefonicaid/fiware-pep-steelskin/blob/7cb31b3f0817813719ae041be0697ca2e18e86a8/lib/fiware-pep-steelskin.js#L280-L296

https://github.com/telefonicaid/fiware-pep-steelskin/blob/7cb31b3f0817813719ae041be0697ca2e18e86a8/lib/services/validation.js#L230-L241

It means that roleTemplate and requestTemplate are still undefined when the proxy server starts listen. If a request comes before loadTemplate() loads roleTemplate and requestTemplate, createAccessRequest() will get undefineds.

I found this but I don't know if the race is harmful. Thank you.

AlvaroVega commented 2 years ago

Thanks for the report, It seems that this issue is not very harmful, but what do you suggest to fix it @Soulike? This is my proposal to fix it: https://github.com/telefonicaid/fiware-pep-steelskin/pull/478

Soulike commented 2 years ago

Thanks for the report, It seems that this issue is not very harmful, but what do you suggest to fix it @Soulike?

Yes, we have exactly the same thought. Instead of calling loadTempate() inside the callback of listen(), we can call listen() inside the callback of loadTempate() to ensure roleTemplate and requestTemplate are loaded before the server starts to listen.

fgalan commented 2 years ago

PR https://github.com/telefonicaid/fiware-pep-steelskin/pull/478 should solve this issue.

@Soulike what do you think?

Soulike commented 2 years ago

PR #478 should solve this issue.

@Soulike what do you think?

👍Good job

fgalan commented 2 years ago

Thanks! :)

Thus, I think the issue can now be closed.