Doug, I agree with your points and I see where you are going with the attribute based configuration. I will have a think about the best way to add in an Identity factory, but apart from that I think we know what we are doing. I will have a go at implementing this and see how I get on.
Cheers, Nicko > -----Original Message----- > From: de la Torre, Douglas [mailto:[EMAIL PROTECTED] > Sent: 28 September 2004 02:28 > To: Log4NET Dev > Subject: RE: Allow log4net to impersonate a user via > app-specific mechanism. > > Hi Nicko, > > Thanks for the feedback. Lots of good info, as usual. > > > Having a global hook for supplying the credentials may be the way to > go but I would first like to explore using the config file to > attach an object to a > property on a specified appender. > > I like the clarity of your the approach for specifying config > file entries. > > However, for our application, it would be difficult to support. Why? > Because it requires updates to the config file. We have lots > of customers that have deployed our application which uses > log4net and associated config files. Asking them to edit the > config file entries is scary - the often muck it up with > typos, which means all our logging stops. When that happens, > support becomes very difficult. > > We'd like the next version of our application to be able to > use the impersonation, without requiring them to change their > config files. > This would let us simplify the permissions needed to enable > logging, and not introduce a risk they will break something. > > It might be a stretch to impose our deployment needs on the > broader log4net group, but I'm hoping there is a way to get > the feature and retain backward compatibility of the config > files. So far, the only way I can think of for this is via > the attribute. However, it might make sense to add support > for both the attribute and the config file entries as you > outline below. Potentially we could live with just config > file support, but this would require us to write an upgrade > utility that could find config files and insert the right > impersonation statements into the config files (ugh :) > > So, these are the options > 1) Config file support for impersonation > 2) DLL-attribute support for impersonation > 3) Both 1 and 2 > > Seems like it would be easy enough to go with option 3, since > the underlying impersonation logic works the same, and the > only difference is how it gets wired into an appender. > > Do you see any problem going with option 3? > > > > > Another question is where in the Appender inheritance > hierarchy do we > put the credential? > > What would you think of putting a property on Appenders that > expose credential management? I would probably make an > interface like: > > public interface IImpersonationConsumer > { > // See below...IImpersonation replaces > IExternalCredential > IImpersonation Impersonation > { > get; > set; > } > } > > Then, the appenders that support impersonation would > implement IImpersonationConsumer, exposing the property to > set the impersonation object to be used. During initial > configuration, this could be wired up like this: > > 1) Check for a <credential> node in the configuration XML > for the appender. > - If found, > - create an instance of the specified appender > - attach it to the appender (assuming the > appender implements IImpersonationConsumer) > > 2) If credentials not set in 1, then check for the > ImpersonationFactoryAttribute. > - If found, > - instantiate the impersonation factory > - this factory can create the impersonation object > - attach it to the appender (assuming the > appender implements IImpersonationConsumer) > > > > > I have thought long and hard about the API syntax for the > IExternalCredential. I think that I prefer the C# using syntax. i.e. > > where the Impersonate() method returns an IDisposable > instance. In the > default or null case this work well because if Impersonate() > returns null to the > > I love it. IDisposable makes sense, and is very clean. Plus > it is null-friendly. Sounds like a no-brainer > > > > > I'm not sure that we have got the terminology quite right. While I > don't necessarily want to just use the MS Windows terminology > it seems slightly odd to > > Perhaps more emphasis on the impersonation and less on the > 'credential'? > Instead of IExternalCredential, how about IImpersonation? > > public interface IImpersonation : IDisposable > { > void Impersonate(); > void Undo(); > } > > Note that an Undo should be implicitly called via the > IDispoable if it hasn't already. > > This seems like it would be less tied to what it is in some > cases (credential), and more tied to what it does > (impersonation). That way it always works regardless of what > the underlying code has going on. > > For example, this way there is less need to associate it with > a specific identity, in cases where some external app wants > to impersonate something other than a single user (maybe from > a user pool?), or some other scenario where there is not a > tight association with a specific fixed identity. > > > > So where are we? > > This solution is sounding pretty good. For the most part I > think we agree on the following > 1) Impersonation via objects > - objects are customer-extensible, allowing custom impersonation > > 2) Wiring up the impersonation objects at config time via > config and/or DLL attribute > 3) Using a property (IImpersonationConsumer) on Appenders > that support impersonation > 4) Appenders wrap code that accesses system resources with > calls to the Impersonation. > > The only points still to resolve (or confirm) are the naming > and maybe the configuration (item 2). Can you think of any others? > > -Doug > > > -----Original Message----- > From: Nicko Cadell [mailto:[EMAIL PROTECTED] > Sent: Sunday, September 26, 2004 5:28 PM > To: Log4NET Dev > Subject: RE: Allow log4net to impersonate a user via > app-specific mechanism. > > Doug, > > Having a global hook for supplying the credentials may be the > way to go but I would first like to explore using the config > file to attach an object to a property on a specified appender. > > This would allow the credential object to be specified for > each appender that supported credentials. > A user could write something like this in the config file: > > <appender type="log4net.Appender.FileAppender"> > > <credential type="log4net.Util.WindowsCredential"> > <username value="DOMAIN\User"/> > <password value="****"/> > </credential> > > ... > </appender> > > Obviously we would have other implementations of > IExternalCredential that would not encourage users to embed > system passwords in plain text files, but you see how it > could instead specify registry keys to read. > Alternatively it could be a type specified by the application > developer which retrieves the credentials from the calling > application. > > <appender type="log4net.Appender.FileAppender"> > > <credential type="MyApp.MyCredential, MyAssembly" /> > > ... > </appender> > > What do you think? > > Another question is where in the Appender inheritance > hierarchy do we put the credential? We could put it only onto > classes that require it or we could put it on to the > AppenderSkeleton and just let the appenders decide to use it > or not. Putting it on the actual appenders that use it will > mean that users won't get confused as to why the property > exists on the appenders that don't use it. > > I have thought long and hard about the API syntax for the > IExternalCredential. I think that I prefer the C# using syntax. i.e. > where the Impersonate() method returns an IDisposable > instance. In the default or null case this work well because > if Impersonate() returns null to the using block this does > not cause a problem. I generally like the syntax: > > protected void RollFile(string fromFile, string toFile) { > ... > using(credential.Impersonate()) > { > target.Delete(); > } > } > > In the default case this should be fast because Impersonate > would return null. By default the credential object would be > a NullCredential singleton instance. This is probably > slightly slower than checking against null, but you don't > really know how good a job the JIT will do. > > Returning an object to do the revert will be slower than > calling a Revert method directly, however it should not be > significant relative to the cost of doing some sort of impersonation. > > I'm not sure that we have got the terminology quite right. > While I don't necessarily want to just use the MS Windows > terminology it seems slightly odd to impersonate a credential > as in windows you would impersonate an identity. We either need to use > ExternalIdentity.Impersonate() or Identity.Impersonate() or > ExternalCredential.Enable() / Activate() something like that maybe. > > What do you think? > > Nicko > > > > -----Original Message----- > > From: de la Torre, Douglas [mailto:[EMAIL PROTECTED] > > Sent: 25 September 2004 00:43 > > To: Log4NET Dev > > Subject: RE: Allow log4net to impersonate a user via app-specific > > mechanism. > > > > Hi All, > > > > Here's some sample code to illustrate how impersonation > could easily > > be added to the appender classes. The goal would be to let the > > application that consumes log4net be able to handle the > impersonation > > when needed. > > The sample's I'm including use an attribute called > > ImpersonationFactoryAttribute that log4net can check for on > the user's > > > DLL to let that DLL override the factory that supplies > objects which > > do the impersonation. > > > > Here's how this would work. > > > > 1) User adds this attribute to their DLL in the > AssemblyInfo.cs file, > > > and specify the type of factory to read from their own DLL > > > > [assembly: > > log4net.Appender.Impersonation.ImpersonationFactoryAttribute(I > > mpersonati > > onFactoryType=typeof(LoggingTestApp.MyImpersonationFactory))] > > > > 2) In the user DLL, the factory class would look something > like this > > > > public class MyImpersonationFactory : IImpersonationFactory > > { > > public IExternalCredential > > Create(log4net.Appender.IAppender appender) > > { > > if( appender is > > log4net.Appender.RollingFileAppender || appender is > > log4net.Appender.FileAppender ) > > { > > return new MyImpersonator(); > > } > > return null; > > } > > } > > > > 3) The appender would add code to instantiate the > IExternalCredential > > > object to use for impersonation. For example, in > RollingFileAppender > > > > override public void ActivateOptions() > > { > > this.credential = > > Impersonation.ImpersonationFactory.Instance.Create( this ); > > > > ... > > } > > > > 4) The appender would impersonate around operations that access > > system resources. For example, when the > RollingFileAppender tries to > > delete a > > file: > > > > protected void RollFile(string fromFile, string toFile) > > { > > ... > > > > LogLog.Debug("RollingFileAppender: > > Deleting existing target file ["+target+"]"); > > try > > { > > this.credential.Impersonate(); > > target.Delete(); > > } > > finally > > { > > this.credential.Revert(); > > } > > > > ... > > } > > > > This is the basic mechanism, and you can see how easy this is both > > from the external DLL that consumes log4net to extend, as > well as how > > simple it is for log4net to support. > > > > Note the above code is using a NullImpersonation class, which has > > empty Impersonate and Revert methods, allowing us to eliminate null > > checks for better readability. Alternatively, we could > make the code > > add the null check guard clauses, in which case the appender code > > above would change to this: > > > > protected void RollFile(string fromFile, string toFile) > > { > > ... > > > > LogLog.Debug("RollingFileAppender: > > Deleting existing target file ["+target+"]"); > > try > > { > > if( null != this.credential ) > > this.credential.Impersonate(); > > target.Delete(); > > } > > finally > > { > > if( null != this.credential ) > > this.credential.Revert(); > > } > > > > ... > > } > > > > What do you all think of this approach? > > > > This seems like a clean way to let external code handle the > > impersonation. For example, we would like this capability > so we can > > impersonate as a single user for file I/O operations, thus > allowing us > > > to simplify our deployments for an ASP.NET application. Currently, > > any site user must have full permissions in the log directory. A > > change like this to the appenders would allow us to > increase security > > and simplify management of the logging folder by impersonating as a > > single user. > > > > Can we go ahead with adding this into the project? If this > approach > > seems sound, the files I've attached can be added to CVS > first, then > > once added, the next stage of work in the appenders could begin, > > changing the resource access to account for the impersonation calls. > > > > I appreciate your feedback and consideration! > > > > -Doug > > >
