CarlosRuiz: full meeting

2014-03-20T02:44:50Z

full meeting

New page

⇐
[[IDempiere|Table of Contents]] |
[[IDempiere/Full Meeting Minutes|Full Meeting Minutes]] |
Full Meeting 2014-03-19


'''''CarlosRuiz''''': Good morning 
'''''nmicoud''''': Bonjour 
'''''tbayen''''': Daarestiet! 
'''''red1_''''': Chow Ang 
'''''red1_''''': CarlosRuiz: I moved 2 trackers for your review. 
'''''red1_''''': About BOM Drop to Production reversal 
'''''CarlosRuiz''''': good 
'''''JanThielemann''''': hi carlos, can you take a look at https://idempiere.atlassian.net/browse/IDEMPIERE-1831 and https://idempiere.atlassian.net/browse/IDEMPIERE-1675 and tell me your opinion about the patches? 
'''''CarlosRuiz''''': sure JanThielemann 
'''''Deepak''''': Good Morning 
'''''CarlosRuiz''''': Morning Deepak 
'''''red1_''''': Morning Deepak 
'''''Deepak''''': I created 2 tickets and will submit patch.. Adding some security major 
'''''Deepak''''': https://idempiere.atlassian.net/browse/IDEMPIERE-1833 and https://idempiere.atlassian.net/browse/IDEMPIERE-1832 
'''''Deepak''''': Monday, I had discussion with Hengsin and discussion was majorly went on Security.. and out come of discussion is that if we can add one util method to check sql for read only 
'''''CarlosRuiz''''': JanThielemann, on the webservice security ticket that you opened - I added a comment and a patch 
'''''CarlosRuiz''''': can you also please peer review it and give us feedback? 
'''''Deepak''''': This util method must be called from places wherever user can enter SQL and sql expected to be read only 
'''''ocurieles_DCS''''': Hi For all.. 
'''''CarlosRuiz''''': Hi Orlando 
'''''Deepak''''': CarlosRuiz, did we have done on SQL injection checking? 
'''''tbayen''''': We work on a documentation about the chart of accounts and how to maintain it. I have a question about it: If the deleopers introduce a new default account (this happened last year and broke compatibility with old csv files) - how can I be sure that old installations are updated? Is the "new" default account null after running the migration scripts? 
'''''CarlosRuiz''''': Deepak, you comment sounds related to IDEMPIERE-1784 too 
'''''CarlosRuiz''''': yes tbayen - it's set to null 
'''''tbayen''''': thanks 
'''''CarlosRuiz''''': probably migration scripts must take care of assigning a value just for GardenWorld - but we can't for the rest of tenants 
'''''CarlosRuiz''''': yep Deepak - a util method to check that sounds useful 
'''''Deepak''''': CarlosRuiz, yes sql injection verification needed at many places 
'''''JanThielemann''''': CarlosRuiz: as far as i understand your patch, you simply prohibit "free" for filter but how would i achieve a queryData where i want all entries in a range of ids (e. g. ad_org_id in (1000000, 1000001) )? 
'''''JanThielemann''''': can this be done via multiple datarow entries for the same column? 
'''''CarlosRuiz''''': if the IDs are variable it can't be done with actual tools - draft idea -> we would need some way to set up a context variable and define the constant filter to use those context variables 
'''''CarlosRuiz''''': JanThielemann, the util SQL read-only checker method proposed by Deepak also can help to solve the IDEMPIERE-1784 ticket 
'''''tbayen''''': CarlosRuiz, +1 for the idea to set a context vaiable. I have another scenario for that. 
'''''JanThielemann''''': that would be a better solution i think 
'''''JanThielemann''''': deepak did you already do some work on it? 
'''''tbayen''''': If you use the master roles then it can be that a user has a master role (near other roles) that makes him e.g. an accountant. You can not test this role in a field's evaluation functions (show some fields only to accountants). I would like to have a context variable set by a role. 
'''''tbayen''''': That means we can use context variables per role and per client. Sorry, if I make it more complicated. ;-) 
'''''ocurieles_DCS''''': @CarlosRuiz have you tested the accounting for Payroll ? 
'''''JanThielemann''''': tbayen you could do this via a session model validator 
'''''JanThielemann''''': you can check the role id and set your own context variable 
'''''ocurieles_DCS''''': by the way of integrate the Payroll Concept 
'''''CarlosRuiz''''': ah yes - the latest version of LCO do that - set context variables on login 
'''''CarlosRuiz''''': ocurieles_DCS, I did some checks about that some time ago and fixed a couple of things - is tricky to configure it properly 
'''''CarlosRuiz''''': but my usage of payroll in the end when I used it (I'm not using that nowadays) was to avoid accounting on the payroll - generate "employee invoices" with charges to pay the payroll the normal way as vendors are paid 
'''''CarlosRuiz''''': so, the invoices are posted - not the payroll 
'''''ocurieles_DCS''''': mmmm... We are working to resolve 
'''''ocurieles_DCS''''': for normal way :D 
'''''ocurieles_DCS''''': without invoice 
'''''tbayen''''': JanThielemann, thanks! The idea with the session validator is great. :-) 
'''''JanThielemann''''': tbayen you are welcome :D 
'''''tbayen''''': Hi adnan_ :-) 
'''''CarlosRuiz''''': JanThielemann, I was not able to reproduce it in postgresql - did you test it in oracle? 
'''''JanThielemann''''': no postgres 
'''''JanThielemann''''': i'll check again 
'''''CarlosRuiz''''': I receive this exception 
'''''CarlosRuiz''''': org.postgresql.util.PSQLException: Multiple ResultSets were returned by the query. 
'''''CarlosRuiz''''': don't understand what's different on your tests than here 
'''''CarlosRuiz''''': the exception is raised at org.postgresql.jdbc2.AbstractJdbc2Statement.executeQuery line 306 
'''''CarlosRuiz''''': my jdbc is postgresql-9.2-1004.jdbc4.jar 
'''''JanThielemann''''': CarlosRuiz: https://www.dropbox.com/s/ybfgstx6j65ug0r/psqlexcept.jpg 
'''''CarlosRuiz''''': ah yes 
'''''CarlosRuiz''''': you're right 
'''''CarlosRuiz''''': in another test case the exception was thrown 
'''''Deepak''''': JanThielemann, yes we did and commit soon 
'''''JanThielemann''''': CarlosRuiz: the good news is that i was not able to delete something via sql injection in the filter 
'''''CarlosRuiz''''': delete not allowed? 
'''''JanThielemann''''': i wasn't able to delete 
'''''JanThielemann''''': however, update and insert is possible 
'''''JanThielemann''''': got to go now, bye @ all 
'''''CarlosRuiz''''': thanks JanThielemann 
'''''nmicoud''''': Hi CarlosRuiz : if you have time, could you review https://idempiere.atlassian.net/browse/IDEMPIERE-1829 please ? In fact, the attached patch fixes 2 things (and i think is harmless) : ability to send the ResetPassword email in the current language and allow to use translation of mail template (actually, it's overwritten with the 'super' content). 
'''''CarlosRuiz''''': yep nmicoud - let me check that one 
'''''CarlosRuiz''''': nmicoud, which language it uses to notify the user? the language of first tenant for the user? 
'''''nmicoud''''': actually, it's english 
'''''CarlosRuiz''''': after your patch? 
'''''nmicoud''''': no before 
'''''CarlosRuiz''''': yep - that's the ticket 
'''''nmicoud''''': after, it takes the language defined in the combo box 
'''''nmicoud''''': of the login panel 
'''''CarlosRuiz''''': I mean - a user can have accounts in two tenants - and different languages potentially 
'''''CarlosRuiz''''': ah - I see 
'''''nmicoud''''': yes, but he can choose the language on the 1st screen 
'''''nmicoud''''': that this one which is used 
'''''CarlosRuiz''''': easier that way 
'''''nmicoud''''': yes :) 
'''''nmicoud''''': and seems logical 
'''''nmicoud''''': CarlosRuiz : i've added another patch to https://idempiere.atlassian.net/browse/IDEMPIERE-1829. It tests the email in upper case. 
'''''CarlosRuiz''''': nmicoud, it overwrites first patch - or complement it? 
'''''nmicoud''''': complement it 
'''''CarlosRuiz''''': ok 
'''''nmicoud''''': getting late here... gtg, bye bye 
'''''CarlosRuiz''''': bye thanks

IDempiere/FullMeeting20140319 - Revision history

CarlosRuiz: full meeting