HTTP caller IP address as logger variable

[hendrik.fuss@…]

When trac produces log messages like HTTPForbidden, it would be very helpful to see which IP address the HTTP request was made from, e.g. to determine whether this was an external attack or a legal user who forgot their login data.

According to TracIni, the IP address is currently not available as formatter variable.

Along with this, probably other HTTP variables would be nice to include in the log file.

[Remy Blank]

That's probably relatively easy to implement. PatchWelcome.

[Thijs Triemstra <lists@…>]

This patch prints the following in the log:

22:17:32 Trac[perm] DEBUG: No policy allowed anonymous performing TIMELINE_VIEW on None
22:17:32 Trac[main] WARNING: Forbidden request from IP: 127.0.0.1
22:17:32 Trac[main] WARNING: HTTPForbidden: 403 Forbidden (TIMELINE_VIEW privileges are required to perform this operation)

trac/web/main.py

@@ -276 +276 @@
                                    exception_to_unicode(e, traceback=True))
                 raise err[0], err[1], err[2]
         except PermissionError, e:
+            self.log.warn("Forbidden request from IP: %s" % req.remote_addr)
             raise HTTPForbidden(to_unicode(e))
         except ResourceNotFound, e:
             raise HTTPNotFound(e)

[Christian Boos]

That's probably good enough for satisfying the use case explained in the description.

As for making the "IP address (available as a) logger variable" I think that's not realistic, as the IP address is stored in the Request which bears no direct connection with the logging system.

So, thanks for the patch!

[Remy Blank]

Would it be possible to include the IP address in the same logging statement as the exception itself, instead of using a separate logging statement?

[Thijs Triemstra <lists@…>]

The attached patch includes the IP in the exception logging statement:

02:27:51 Trac[main] WARNING: HTTPForbidden: 403 Forbidden (TIMELINE_VIEW privileges are required to perform this operation - IP: 127.0.0.1)

[Thijs Triemstra <lists@…>]

Just noticed that including it in the exception causes Trac to print it in the web view as well, and I'm not sure if web users need to see their IP printed on the screen.

[Remy Blank]

Replying to Thijs Triemstra <lists@…>:

Just noticed that including it in the exception causes Trac to print it in the web view as well, and I'm not sure if web users need to see their IP printed on the screen.

I would prefer not having the address in the error itself, only in the log statement.

[Thijs Triemstra <lists@…>]

Attached patch produces:

23:52:17 Trac[main] WARNING: HTTPForbidden: 403 Forbidden (TIMELINE_VIEW privileges are required to perform this operation)
23:52:17 Trac[main] WARNING: IP: 127.0.0.1

Or including the user-agent?

trac/web/main.py

@@ -541 +541 @@
         data = {'title': title, 'type': 'TracError', 'message': message,
                 'frames': [], 'traceback': None}
         if e.code == 403 and req.authname == 'anonymous':
+            env.log.warn("IP: %s - %s" % (req.remote_addr, req.get_header('User-Agent')))
             # TRANSLATOR: ... not logged in, you may want to 'do so' now (link)
             do_so = tag.a(_("do so"), href=req.href.login())
             req.chrome['notices'].append(

23:55:02 Trac[main] WARNING: HTTPForbidden: 403 Forbidden (TIMELINE_VIEW privileges are required to perform this operation)
23:55:02 Trac[main] WARNING: IP: 127.0.0.1 - Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_8; en-us) AppleWebKit/534.10+ (KHTML, like Gecko) Version/5.0.2 Safari/533.18.5

[Remy Blank]

I was rather thinking something like (for permission errors only):

trac/web/main.py

@@ -517 +517 @@
         # This part is a bit more complex than it should be.
         # See trac/web/api.py for the definition of HTTPException subclasses.
         if env:
-            env.log.warn(exception_to_unicode(e))
+            message = exception_to_unicode(e)
+            if e.code == 403:
+                message = '[%s] %s' % (req.remote_addr, message)
+            env.log.warn(message)
         try:
             # We try to get localized error messages here, 
             # but we should ignore secondary errors

Or even simpler, for all HTTPExceptions:

trac/web/main.py

@@ -517 +517 @@
         # This part is a bit more complex than it should be.
         # See trac/web/api.py for the definition of HTTPException subclasses.
         if env:
-            env.log.warn(exception_to_unicode(e))
+            env.log.warn('[%s] %s' % (req.remote_addr,
+                                      exception_to_unicode(e)))
         try:
             # We try to get localized error messages here, 
             # but we should ignore secondary errors

(No enhancements on 0.12-stable)

[Remy Blank]

Second patch in comment:10 applied in [10260].

[Ryan J Ollos]

Normalizing keywords.

[Ryan J Ollos]

Normalizing keywords.