Thanks Daan. Thanks, Mandar
On Fri, Nov 29, 2013 at 5:16 PM, Daan Hoogland <daan.hoogl...@gmail.com>wrote: > Mandar, > > I agree, go ahead. Let me know if you need any help. > > On Fri, Nov 29, 2013 at 12:40 PM, Mandar Barve <mandar.ba...@sungard.com> > wrote: > > Hi Daan, > > I see your point in not bothering API implementers when it comes to > > checking or setting the flag. (In my implementation I am proposing flag > > check using base class ref and set in API child class. Flags and get/set > > methods are members of the base class.) > > > > If the base class has to do the check and set at load time as you suggest > > here are some things to consider: > > > > 1. API command class is loaded by its name at run time. > > 2. At this time code doesn't know if the API command is carrying > sensitive > > data or not. > > 3. I did not find any annotation in the API command classes for return > > types that indicate password/secret key that could be used as a simple > > match string to help decide sensitivity of the class at load time, which > > means class needs to be modified to set such flag perhaps in its > > constructor or when it executes the command as suggested earlier. In a > > nutshell API command class modification is needed. > > > > I also agree with your second point that with introduction of another > > member variable and expectation of it being set by the child class, if > > there is a developer oversight purpose can get lost. I think by making a > > base class wrapper method e.g. InformCommandSensitivity abstract we > should > > be able to enforce this at compile time. All API command classes will > have > > to implement this method leading to setting/resetting of the flag as > needed. > > > > Like you said I don't know the future security implications if any for > such > > code. > > > > Considering all these I would like to go ahead with the approach I > > suggested. > > > > Thanks, > > Mandar > > > > > > On Thu, Nov 28, 2013 at 8:07 PM, Daan Hoogland <daan.hoogl...@gmail.com > >wrote: > > > >> Almost, I would like to see some construct that would allow for child > >> classes not to bother and let the baseclass do the checking and set > >> the flag at load time so api implemeters don't have to worry about > >> this. It is kind of a challenge but using reflection it should be > >> possible. > >> > >> As I understand your proposal every implementer of an api has to make > >> the consideration considering every field of the class. It will work > >> but automated processing of the api class could prevent oversights by > >> developers. > >> > >> On the other hand we will run into privacy and security related > >> situations not foreseeable right now. I am not sure if what I am > >> saying will pay off. > >> > >> > >> > >> On Thu, Nov 28, 2013 at 3:24 PM, Mandar Barve <mandar.ba...@sungard.com > > > >> wrote: > >> > Daan, > >> > The child classes will "know" (since the API request/response > >> > parameters are known) at load time if they are to carry sensitive data > >> and > >> > they can set the flags at class load time/construction like you > >> mentioned. > >> > Flag check will be at the time of logging. > >> > > >> > Did you mean to suggest the same or am I missing something? > >> > > >> > Thanks, > >> > Mandar > >> > > >> > > >> > On Thu, Nov 28, 2013 at 5:22 PM, Daan Hoogland < > daan.hoogl...@gmail.com > >> >wrote: > >> > > >> >> On Thu, Nov 28, 2013 at 10:38 AM, Mandar Barve < > >> mandar.ba...@sungard.com> > >> >> wrote: > >> >> > > >> >> > In my opinion we should implement approach #2. I have tested this > >> >> approach > >> >> > for couple of sensitive commands list Users and list Accounts. It > >> looks > >> >> to > >> >> > work fine. > >> >> > >> >> > >> >> H Mandar, I agree but can some automation be done i.e. recognize > >> >> sensitive names of fields [pP]assword or varieties thereof? This can > >> >> be done at construction or even classloadtime and shouldn't impact > >> >> performance very much. > >> >> > >> >> otherwise #2 is more elegant and i mean this as an extension of #2 > >> >> > >> >> > >> > >> > >