#4872 Add a dedicated logger for everything that is auth related
Merged 3 years ago by pingou. Opened 3 years ago by pingou.

file modified
+83 -56
@@ -811,73 +811,84 @@ 

  

  ::

  

-     LOGGING = {

-         'version': 1,

-         'disable_existing_loggers': False,

-         'formatters': {

-             'standard': {

-                 'format': '%(asctime)s [%(levelname)s] %(name)s: %(message)s'

+    LOGGING = {

+         "version": 1,

+         "disable_existing_loggers": False,

+         "formatters": {

+             "standard": {

+                 "format": "%(asctime)s [%(levelname)s] %(name)s: %(message)s"

              },

-             'email_format': {

-                 'format': MSG_FORMAT

-             }

+             "email_format": {"format": MSG_FORMAT},

          },

-         'filters': {

-             'myfilter': {

-                 '()': ContextInjector,

-             }

-         },

-         'handlers': {

-             'console': {

-                 'level': 'INFO',

-                 'formatter': 'standard',

-                 'class': 'logging.StreamHandler',

-                 'stream': 'ext://sys.stdout',

+         "filters": {"myfilter": {"()": ContextInjector}},

+         "handlers": {

+             "console": {

+                 "formatter": "standard",

+                 "class": "logging.StreamHandler",

+                 "stream": "ext://sys.stdout",

+             },

+             "auth_handler": {

+                 "formatter": "standard",

+                 "class": "logging.StreamHandler",

+                 "stream": "ext://sys.stdout",

              },

-             'email': {

-                 'level': 'ERROR',

-                 'formatter': 'email_format',

-                 'class': 'logging.handlers.SMTPHandler',

-                 'mailhost': 'localhost',

-                 'fromaddr': 'pagure@localhost',

-                 'toaddrs': 'root@localhost',

-                 'subject': 'ERROR on pagure',

-                 'filters': ['myfilter'],

+             "email": {

+                 "level": "ERROR",

+                 "formatter": "email_format",

+                 "class": "logging.handlers.SMTPHandler",

+                 "mailhost": "localhost",

+                 "fromaddr": "pagure@localhost",

+                 "toaddrs": "root@localhost",

+                 "subject": "ERROR on pagure",

+                 "filters": ["myfilter"],

              },

          },

          # The root logger configuration; this is a catch-all configuration

          # that applies to all log messages not handled by a different logger

-         'root': {

-             'level': 'INFO',

-             'handlers': ['console'],

-         },

-         'loggers': {

-             'pagure': {

-                 'handlers': ['console'],

-                 'level': 'DEBUG',

-                 'propagate': True

+         "root": {"level": "INFO", "handlers": ["console"]},

+         "loggers": {

+             "pagure": {

+                 "handlers": ["console"],

+                 "level": "DEBUG",

+                 "propagate": True,

              },

-             'flask': {

-                 'handlers': ['console'],

-                 'level': 'INFO',

-                 'propagate': False

+             "pagure_auth": {

+                 "handlers": ["auth_handler"],

+                 "level": "DEBUG",

+                 "propagate": False,

              },

-             'sqlalchemy': {

-                 'handlers': ['console'],

-                 'level': 'WARN',

-                 'propagate': False

+             "flask": {

+                 "handlers": ["console"],

+                 "level": "INFO",

+                 "propagate": False,

              },

-             'binaryornot': {

-                 'handlers': ['console'],

-                 'level': 'WARN',

-                 'propagate': True

+             "sqlalchemy": {

+                 "handlers": ["console"],

+                 "level": "WARN",

+                 "propagate": False,

              },

-             'pagure.lib.encoding_utils': {

-                 'handlers': ['console'],

-                 'level': 'WARN',

-                 'propagate': False

+             "binaryornot": {

+                 "handlers": ["console"],

+                 "level": "WARN",

+                 "propagate": True,

              },

-         }

+             "MARKDOWN": {

+                 "handlers": ["console"],

+                 "level": "WARN",

+                 "propagate": True,

+             },

+             "PIL": {"handlers": ["console"], "level": "WARN", "propagate": True},

+             "chardet": {

+                 "handlers": ["console"],

+                 "level": "WARN",

+                 "propagate": True,

+             },

+             "pagure.lib.encoding_utils": {

+                 "handlers": ["console"],

+                 "level": "WARN",

+                 "propagate": False,

+             },

+         },

      }

  

  .. note:: as you can see there is an ``email`` handler defined. It's not used
@@ -888,6 +899,22 @@ 

  

          'handlers': ['console', 'email'],

  

+ .. note:: The ``pagure_auth`` logger is a special one logging all activities

+     regarding read/write access to git repositories. It will be a pretty

+     important log for auditing if needed.

+     You can separate this log into its own file if you like by using the

+     following handler:

+     ::

+ 

+         "auth_handler": {

+             "formatter": "standard",

+             "class": "logging.FileHandler",

+             "filename": "/var/log/pagure/pagure_auth.log",

+         }

+ 

+     Beware if you do this that you will also likely want to enable logrotate

+     on the system.

+ 

  

  ITEM_PER_PAGE

  ~~~~~~~~~~~~~
@@ -989,7 +1016,7 @@ 

  

  

  SMTP_CERTFILE

- ^^^^^^^^^^^^

+ ^^^^^^^^^^^^^

  

  This configuration key allows to specify a certificate file to be used in

  the `starttls` command when connecting to the smtp server.

file modified
+10
@@ -460,6 +460,11 @@ 

              "class": "logging.StreamHandler",

              "stream": "ext://sys.stdout",

          },

+         "auth_handler": {

+             "formatter": "standard",

+             "class": "logging.StreamHandler",

+             "stream": "ext://sys.stdout",

+         },

          "email": {

              "level": "ERROR",

              "formatter": "email_format",
@@ -480,6 +485,11 @@ 

              "level": "DEBUG",

              "propagate": True,

          },

+         "pagure_auth": {

+             "handlers": ["auth_handler"],

+             "level": "DEBUG",

+             "propagate": False,

+         },

          "flask": {

              "handlers": ["console"],

              "level": "INFO",

@@ -37,6 +37,7 @@ 

  

  

  _log = logging.getLogger(__name__)

+ _auth_log = logging.getLogger("pagure.auth")

  

  

  MERGE_OPTIONS = {
@@ -97,6 +98,11 @@ 

      """ Looks up an SSH key by search_key for keyhelper.py """

      search_key = flask.request.form["search_key"]

      username = flask.request.form.get("username")

+     _auth_log.info(

+         "User is trying to access pagure using the ssh key: %s -- "

+         "|user: %s|IP: %s|method: N/A|repo: N/A|query: N/A"

+         % (search_key, username, flask.request.remote_addr)

+     )

      key = pagure.lib.query.find_ssh_key(flask.g.session, search_key, username)

  

      if not key:
@@ -123,6 +129,11 @@ 

      """ Determines whether a user has any access to the requested repo. """

      gitdir = flask.request.form["gitdir"]

      remoteuser = flask.request.form["username"]

+     _auth_log.info(

+         "User is asking to access a project via ssh -- "

+         "|user: %s|IP: %s|method: N/A|repo: %s|query: N/A"

+         % (remoteuser, flask.request.remote_addr, gitdir)

+     )

  

      # Build a fake path so we can use get_repo_info_from_path

      path = os.path.join(pagure_config["GIT_FOLDER"], gitdir)
@@ -142,6 +153,12 @@ 

  

      if repo is None:

          _log.info("Project name could not be extracted from path")

+         _auth_log.info(

+             "The path specified by the user could not be matched with a "

+             "project -- "

+             "|user: %s|IP: %s|method: N/A|repo: %s|query: N/A"

+             % (remoteuser, flask.request.remote_addr, gitdir)

+         )

          return flask.jsonify({"access": False})

  

      project = pagure.lib.query.get_authorized_project(
@@ -153,6 +170,11 @@ 

      )

  

      if not project:

+         _auth_log.info(

+             "User tried to access a private project they don't have access "

+             "to -- |user: %s|IP: %s|method: N/A|repo: %s|query: N/A"

+             % (remoteuser, flask.request.remote_addr, gitdir)

+         )

          _log.info("Project not found with this path")

          return flask.jsonify({"access": False})

      _log.info("Checking ACLs on project: %s" % project.fullname)
@@ -163,6 +185,11 @@ 

          # Deploy keys are not allowed on ticket and PR repos but they are

          # allowed for main and docs repos.

          _log.info("%s is not a contributor to this project" % remoteuser)

+         _auth_log.info(

+             "User tried to access a projec they do not have access to -- "

+             "|user: %s|IP: %s|method: N/A|repo: %s|query: N/A"

+             % (remoteuser, flask.request.remote_addr, gitdir)

+         )

          return flask.jsonify({"access": False})

  

      _log.info("Access granted to %s on: %s" % (remoteuser, project.fullname))

file modified
+85 -5
@@ -32,6 +32,7 @@ 

  from pagure.ui import UI_NS

  

  _log = logging.getLogger(__name__)

+ _auth_log = logging.getLogger("pagure.auth")

  

  

  def _get_remote_user(project):
@@ -121,6 +122,17 @@ 

          "HTTP_CONTENT_ENCODING": flask.request.content_encoding,

      }

  

+     _auth_log.info(

+         "Serving git to |user: %s|IP: %s|method: %s|repo: %s|query: %s"

+         % (

+             remote_user,

+             flask.request.remote_addr,

+             flask.request.method,

+             project.path,

+             flask.request.query_string,

+         )

+     )

+ 

      gitolite = pagure_config["HTTP_REPO_ACCESS_GITOLITE"]

      if gitolite:

          gitenv.update(
@@ -278,6 +290,16 @@ 

      access to the attempted repository.

      """

      if not pagure_config["ALLOW_HTTP_PULL_PUSH"]:

+         _auth_log.info(

+             "User tried to access the git repo via http but this is not "

+             "enabled -- |user: N/A|IP: %s|method: %s|repo: %s|query: %s"

+             % (

+                 flask.request.remote_addr,

+                 flask.request.method,

+                 project,

+                 flask.request.query_string,

+             )

+         )

          flask.abort(403, description="HTTP pull/push is not allowed")

  

      service = None
@@ -287,6 +309,9 @@ 

      p1 = pagure.lib.query.get_authorized_project(

          flask.g.session, project, user=username, namespace=namespace

      )

+     p1_path = "invalid repo"

+     if p1:

+         p1_path = p1.path

      remote_user = _get_remote_user(p1)

  

      if flask.request.path.endswith("/info/refs"):
@@ -294,12 +319,45 @@ 

          if not service:

              # This is a Git client older than 1.6.6, and it doesn't work with

              # the smart protocol. We do not support the old protocol via HTTP.

+             _auth_log.info(

+                 "User is using a git client to old (pre-1.6.6) -- "

+                 "|user: %s|IP: %s|method: %s|repo: %s|query: %s"

+                 % (

+                     remote_user,

+                     flask.request.remote_addr,

+                     flask.request.method,

+                     p1_path,

+                     flask.request.query_string,

+                 )

+             )

              flask.abort(400, description="Please switch to newer Git client")

          if service not in ("git-upload-pack", "git-receive-pack"):

+             _auth_log.info(

+                 "User asked for an unknown service "

+                 "|user: %s|IP: %s|method: %s|repo: %s|query: %s"

+                 % (

+                     remote_user,

+                     flask.request.remote_addr,

+                     flask.request.method,

+                     p1_path,

+                     flask.request.query_string,

+                 )

+             )

              flask.abort(400, description="Unknown service requested")

  

      if "git-receive-pack" in flask.request.full_path:

          if not pagure_config["ALLOW_HTTP_PUSH"]:

+             _auth_log.info(

+                 "User tried a git push over http while this is not enabled -- "

+                 "|user: %s|IP: %s|method: %s|repo: %s|query: %s"

+                 % (

+                     remote_user,

+                     flask.request.remote_addr,

+                     flask.request.method,

+                     p1_path,

+                     flask.request.query_string,

+                 )

+             )

              # Pushing (git-receive-pack) over HTTP is not allowed

              flask.abort(403, description="HTTP pushing disabled")

  
@@ -312,6 +370,17 @@ 

                  "WWW-Authenticate": 'Basic realm="%s"' % realm,

                  "X-Frame-Options": "DENY",

              }

+             _auth_log.info(

+                 "User tried a git push over http but was not authenticated -- "

+                 "|user: %s|IP: %s|method: %s|repo: %s|query: %s"

+                 % (

+                     remote_user,

+                     flask.request.remote_addr,

+                     flask.request.method,

+                     p1_path,

+                     flask.request.query_string,

+                 )

+             )

              response = flask.Response(

                  response="Authorization Required",

                  status=401,
@@ -320,14 +389,25 @@ 

              )

              flask.abort(response)

  

-     project = pagure.lib.query.get_authorized_project(

+     project_obj = pagure.lib.query.get_authorized_project(

          flask.g.session,

          project,

          user=username,

          namespace=namespace,

          asuser=remote_user,

      )

-     if not project:

+     if not project_obj:

+         _auth_log.info(

+             "User asked to access a git repo that they are not allowed to "

+             "access -- |user: %s|IP: %s|method: %s|repo: %s|query: %s"

+             % (

+                 remote_user,

+                 flask.request.remote_addr,

+                 flask.request.method,

+                 p1_path,

+                 flask.request.query_string,

+             )

+         )

          _log.info(

              "%s could not find project: %s for user %s and namespace %s",

              remote_user,
@@ -337,10 +417,10 @@ 

          )

          flask.abort(404, description="Project not found")

  

-     if project.is_on_repospanner:

-         return proxy_repospanner(project, service)

+     if project_obj.is_on_repospanner:

+         return proxy_repospanner(project_obj, service)

      else:

-         return proxy_raw_git(project)

+         return proxy_raw_git(project_obj)

  

  

  def add_clone_proxy_cmds():

Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr

This isn't yet ready to be merged, I want to add a default handler for it in the configuration and document it

rebased onto b4117d258eb0f67abc912cc31d5c07af8c2c2e7d

3 years ago

We have /var/log/pagure now as a directory since 5.10.0, we should put this there...

rebased onto 6068939aea61555f9c66d7310488e16237c73f70

3 years ago

2 new commits added

  • Fix warning when compiling the doc, title had an underline too short
  • Update the default LOGGING configuration
3 years ago

rebased onto 48664c3

3 years ago

This is ready to be reviewed :)

Pull-Request has been merged by pingou

3 years ago