allura-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From brond...@apache.org
Subject [allura] 01/01: [#8290] move previous_login_details into UserLoginDetails collection
Date Tue, 04 Jun 2019 20:33:52 GMT
This is an automated email from the ASF dual-hosted git repository.

brondsem pushed a commit to branch db/8290
in repository https://gitbox.apache.org/repos/asf/allura.git

commit 00fd56b56b05ab29179a03749f3989b12b6eaafb
Author: Dave Brondsema <dave@brondsema.net>
AuthorDate: Wed May 29 12:25:47 2019 -0400

    [#8290] move previous_login_details into UserLoginDetails collection
---
 Allura/allura/command/show_models.py        |  2 +-
 Allura/allura/lib/plugin.py                 | 15 +++++---
 Allura/allura/model/__init__.py             |  6 ++--
 Allura/allura/model/auth.py                 | 42 ++++++++++++++++------
 Allura/allura/model/session.py              | 28 +++++++++++++++
 Allura/allura/tests/functional/test_auth.py |  2 ++
 Allura/allura/tests/model/test_auth.py      | 10 +++---
 Allura/allura/tests/test_plugin.py          | 54 +++++++++++++++++------------
 8 files changed, 114 insertions(+), 45 deletions(-)

diff --git a/Allura/allura/command/show_models.py b/Allura/allura/command/show_models.py
index ac2ef86..86180d4 100644
--- a/Allura/allura/command/show_models.py
+++ b/Allura/allura/command/show_models.py
@@ -211,7 +211,7 @@ class EnsureIndexCommand(base.Command):
     def command(self):
         from allura import model as M
         main_session_classes = [M.main_orm_session, M.repository_orm_session,
-                                M.task_orm_session]
+                                M.task_orm_session, M.main_explicitflush_orm_session]
         if asbool(self.config.get('activitystream.recording.enabled', False)):
             from activitystream.storage.mingstorage import activity_odm_session
             main_session_classes.append(activity_odm_session)
diff --git a/Allura/allura/lib/plugin.py b/Allura/allura/lib/plugin.py
index 73786a0..3f9769a 100644
--- a/Allura/allura/lib/plugin.py
+++ b/Allura/allura/lib/plugin.py
@@ -187,7 +187,7 @@ class AuthenticationProvider(object):
         else:
             self.session.pop('multifactor-username', None)
 
-        login_details = self.get_login_detail(self.request)
+        login_details = self.get_login_detail(self.request, user)
 
         expire_reason = None
         if self.is_password_expired(user):
@@ -436,6 +436,7 @@ class AuthenticationProvider(object):
         ]
 
     def login_details_from_auditlog(self, auditlog):
+        from allura import model as M
         ip = ua = None
         matches = re.search(r'^IP Address: (.+)\n', auditlog.message, re.MULTILINE)
         if matches:
@@ -444,20 +445,24 @@ class AuthenticationProvider(object):
         if matches:
             ua = matches.group(1)
         if ua or ip:
-            return dict(
+            return M.UserLoginDetails(
+                user_id=auditlog.user_id,
                 ip=ip,
                 ua=ua,
             )
 
-    def get_login_detail(self, request):
-        return dict(
+    def get_login_detail(self, request, user):
+        from allura import model as M
+        return M.UserLoginDetails(
+            user_id=user._id,
             ip=utils.ip_address(request),
             ua=request.headers.get('User-Agent'),
         )
 
     def trusted_login_source(self, user, login_details):
         # TODO: could also factor in User-Agent but hard to know what parts of the UA are
meaningful to check here
-        for prev_login in user.previous_login_details:
+        from allura import model as M
+        for prev_login in M.UserLoginDetails.query.find({'user_id': user._id}):
             if prev_login['ip'] == login_details['ip']:
                 return 'exact ip'
             if asbool(tg.config.get('auth.trust_ip_3_octets_match', False)) and \
diff --git a/Allura/allura/model/__init__.py b/Allura/allura/model/__init__.py
index 0b1bea2..b5bbe1b 100644
--- a/Allura/allura/model/__init__.py
+++ b/Allura/allura/model/__init__.py
@@ -27,7 +27,7 @@ from .artifact import VotableArtifact
 from .discuss import Discussion, Thread, PostHistory, Post, DiscussionAttachment
 from .attachments import BaseAttachment
 from .auth import AuthGlobals, User, ProjectRole, EmailAddress, OldProjectRole
-from .auth import AuditLog, audit_log, AlluraUserProperty
+from .auth import AuditLog, audit_log, AlluraUserProperty, UserLoginDetails
 from .filesystem import File
 from .notification import Notification, Mailbox, SiteNotification
 from .repository import Repository, RepositoryImplementation
@@ -39,7 +39,7 @@ from .webhook import Webhook
 from .multifactor import TotpKey
 
 from .types import ACE, ACL, EVERYONE, ALL_PERMISSIONS, DENY_ALL, MarkdownCache
-from .session import main_doc_session, main_orm_session
+from .session import main_doc_session, main_orm_session, main_explicitflush_orm_session
 from .session import project_doc_session, project_orm_session
 from .session import artifact_orm_session, repository_orm_session
 from .session import task_orm_session
@@ -61,4 +61,4 @@ __all__ = [
     'OAuthRequestToken', 'OAuthAccessToken', 'MonQTask', 'Webhook', 'ACE', 'ACL', 'EVERYONE',
'ALL_PERMISSIONS',
     'DENY_ALL', 'MarkdownCache', 'main_doc_session', 'main_orm_session', 'project_doc_session',
'project_orm_session',
     'artifact_orm_session', 'repository_orm_session', 'task_orm_session', 'ArtifactSessionExtension',
'repository',
-    'repo_refresh', 'SiteNotification', 'TotpKey']
+    'repo_refresh', 'SiteNotification', 'TotpKey', 'UserLoginDetails', 'main_explicitflush_orm_session']
diff --git a/Allura/allura/model/auth.py b/Allura/allura/model/auth.py
index 3520dcd..ce429b7 100644
--- a/Allura/allura/model/auth.py
+++ b/Allura/allura/model/auth.py
@@ -16,17 +16,17 @@
 #       under the License.
 
 import logging
-import urllib
 import calendar
 from urlparse import urlparse
 from email import header
 from hashlib import sha256
 from datetime import timedelta, datetime, time
-
 import os
 import re
+
 from pytz import timezone
 import pymongo
+from pymongo.errors import DuplicateKeyError
 from tg import config
 from tg import tmpl_context as c, app_globals as g
 from tg import request
@@ -44,7 +44,7 @@ from allura.lib import plugin
 from allura.lib import utils
 from allura.lib.decorators import memoize
 from allura.lib.search import SearchIndexable
-from .session import main_orm_session, main_doc_session
+from .session import main_orm_session, main_doc_session, main_explicitflush_orm_session
 from .session import project_orm_session
 from .timeline import ActivityNode, ActivityObject
 
@@ -297,11 +297,6 @@ class User(MappedClass, ActivityNode, ActivityObject, SearchIndexable):
         session_date=S.DateTime,
         session_ip=str,
         session_ua=str))
-    previous_login_details = FieldProperty([{
-        str: S.Anything
-        # "ip" and "ua" are standard fields
-        # but allow for anything to be stored, like other headers, geo info, frequency of
use, etc
-    }])
 
     def __repr__(self):
         return (u'<User username={s.username!r} display_name={s.display_name!r} _id={s._id!r}
'
@@ -370,8 +365,10 @@ class User(MappedClass, ActivityNode, ActivityObject, SearchIndexable):
             session(self).flush(self)
 
     def add_login_detail(self, detail):
-        if detail not in self.previous_login_details:
-            self.previous_login_details.append(detail)
+        try:
+            session(detail).flush(detail)
+        except DuplicateKeyError:
+            session(detail).expunge(detail)
 
     def backfill_login_details(self, auth_provider):
         # ".*" at start of regex and the DOTALL flag is needed only for the test, which uses
mim
@@ -998,3 +995,28 @@ main_orm_session.mapper(AuditLog, audit_log, properties=dict(
     project=RelationProperty('Project'),
     user_id=AlluraUserProperty(),
     user=RelationProperty('User')))
+
+
+class UserLoginDetails(MappedClass):
+    """
+    Store unique entries for users' previous login details.
+
+    Used to help determine if new logins are suspicious or not
+    """
+
+    class __mongometa__:
+        name = 'user_login_details'
+        session = main_explicitflush_orm_session
+        indexes = ['user_id']
+        unique_indexes = [('user_id', 'ip', 'ua'),  # DuplicateKeyError checked in add_login_detail
+                          ]
+
+    _id = FieldProperty(S.ObjectId)
+    user_id = AlluraUserProperty(required=True)
+    ip = FieldProperty(str)
+    ua = FieldProperty(str)
+    extra = FieldProperty({
+        str: S.Anything
+    })
+
+    user = RelationProperty('User')
diff --git a/Allura/allura/model/session.py b/Allura/allura/model/session.py
index 2ab5961..bdd89ac 100644
--- a/Allura/allura/model/session.py
+++ b/Allura/allura/model/session.py
@@ -20,6 +20,7 @@ import pymongo
 from collections import defaultdict
 
 from ming import Session
+from ming.odm.base import ObjectState
 from ming.orm.base import state
 from ming.orm.ormsession import ThreadLocalORMSession, SessionExtension
 from contextlib import contextmanager
@@ -220,6 +221,29 @@ class BatchIndexer(ArtifactSessionExtension):
                 raise
 
 
+class ExplicitFlushOnlySessionExtension(SessionExtension):
+    """
+    Used to avoid auto-flushing objects after merely creating them.
+
+    Only save them when we really want to by calling flush(obj) or setting obj.explicit_flush
= True
+    """
+
+    def before_flush(self, obj=None):
+        """Before the session is flushed for ``obj``
+
+        If ``obj`` is ``None`` it means all the objects in
+        the UnitOfWork which can be retrieved by iterating
+        over ``ODMSession.uow``
+        """
+        if obj is not None:
+            # this was an explicit flush() call, so let it go through
+            return
+
+        for o in self.session.uow:
+            if not getattr(o, 'explicit_flush', False):
+                state(o).status = ObjectState.clean
+
+
 @contextmanager
 def substitute_extensions(session, extensions=None):
     """
@@ -251,6 +275,10 @@ main_orm_session = ThreadLocalORMSession(
     doc_session=main_doc_session,
     extensions=[IndexerSessionExtension]
     )
+main_explicitflush_orm_session = ThreadLocalORMSession(
+    doc_session=main_doc_session,
+    extensions=[IndexerSessionExtension, ExplicitFlushOnlySessionExtension]
+)
 project_orm_session = ThreadLocalORMSession(
     doc_session=project_doc_session,
     extensions=[IndexerSessionExtension]
diff --git a/Allura/allura/tests/functional/test_auth.py b/Allura/allura/tests/functional/test_auth.py
index 05bfaa4..3578231 100644
--- a/Allura/allura/tests/functional/test_auth.py
+++ b/Allura/allura/tests/functional/test_auth.py
@@ -150,6 +150,8 @@ class TestAuth(TestController):
         assert_equal(kwargs['subject'], u'Update your %s password' % config['site_name'])
         assert_in('/auth/forgotten_password/', kwargs['text'])
 
+        assert_equal([], M.UserLoginDetails.query.find().all())  # no records created
+
     @patch('allura.tasks.mail_tasks.sendsimplemail')
     def test_login_hibp_compromised_password_trusted_client(self, sendsimplemail):
         self.app.extra_environ = {'disable_auth_magic': 'True'}
diff --git a/Allura/allura/tests/model/test_auth.py b/Allura/allura/tests/model/test_auth.py
index 0f03acb..47dc8b6 100644
--- a/Allura/allura/tests/model/test_auth.py
+++ b/Allura/allura/tests/model/test_auth.py
@@ -436,7 +436,9 @@ def test_user_backfill_login_details():
     auth_provider = plugin.AuthenticationProvider.get(None)
     c.user.backfill_login_details(auth_provider)
 
-    assert_equal(sorted(c.user.previous_login_details), [
-        {'ip': '127.0.0.1', 'ua': 'TestBrowser/56'},
-        {'ip': '127.0.0.1', 'ua': 'TestBrowser/57'},
-    ])
+    details = M.UserLoginDetails.query.find({'user_id': c.user._id}).sort('ua').all()
+    assert_equal(len(details), 2, details)
+    assert_equal(details[0].ip, '127.0.0.1')
+    assert_equal(details[0].ua, 'TestBrowser/56')
+    assert_equal(details[1].ip, '127.0.0.1')
+    assert_equal(details[1].ua, 'TestBrowser/57')
diff --git a/Allura/allura/tests/test_plugin.py b/Allura/allura/tests/test_plugin.py
index c7bd22b..f02f731 100644
--- a/Allura/allura/tests/test_plugin.py
+++ b/Allura/allura/tests/test_plugin.py
@@ -600,38 +600,48 @@ class TestLocalAuthenticationProvider(object):
         assert_equal(user.disabled, True)
 
     def test_login_details_from_auditlog(self):
-        assert_equal(self.provider.login_details_from_auditlog(M.AuditLog(
-                        message='')),
+        user = M.User(username='asfdasdf')
+
+        assert_equal(self.provider.login_details_from_auditlog(M.AuditLog(message='')),
                      None)
 
-        assert_equal(self.provider.login_details_from_auditlog(M.AuditLog(
-                        message='IP Address: 1.2.3.4\nFoo')),
-                     dict(ip='1.2.3.4', ua=None))
+        detail = self.provider.login_details_from_auditlog(M.AuditLog(message='IP Address:
1.2.3.4\nFoo', user=user))
+        assert_equal(detail.user_id, user._id)
+        assert_equal(detail.ip, '1.2.3.4')
+        assert_equal(detail.ua, None)
 
-        assert_equal(self.provider.login_details_from_auditlog(M.AuditLog(
-                        message='Foo\nIP Address: 1.2.3.4\nFoo')),
-                     dict(ip='1.2.3.4', ua=None))
+        detail = self.provider.login_details_from_auditlog(M.AuditLog(message='Foo\nIP Address:
1.2.3.4\nFoo', user=user))
+        assert_equal(detail.ip, '1.2.3.4')
+        assert_equal(detail.ua, None)
 
         assert_equal(self.provider.login_details_from_auditlog(M.AuditLog(
-                        message='blah blah IP Address: 1.2.3.4\nFoo')),
+                        message='blah blah IP Address: 1.2.3.4\nFoo', user=user)),
                      None)
 
-        assert_equal(self.provider.login_details_from_auditlog(M.AuditLog(
-                        message='User-Agent: Mozilla/Firefox\nFoo')),
-                     dict(ip=None, ua='Mozilla/Firefox'))
+        detail = self.provider.login_details_from_auditlog(M.AuditLog(
+                        message='User-Agent: Mozilla/Firefox\nFoo', user=user))
+        assert_equal(detail.ip, None)
+        assert_equal(detail.ua, 'Mozilla/Firefox')
 
-        assert_equal(self.provider.login_details_from_auditlog(M.AuditLog(
-                        message='IP Address: 1.2.3.4\nUser-Agent: Mozilla/Firefox\nFoo')),
-                     dict(ip='1.2.3.4', ua='Mozilla/Firefox'))
+        detail = self.provider.login_details_from_auditlog(M.AuditLog(
+                        message='IP Address: 1.2.3.4\nUser-Agent: Mozilla/Firefox\nFoo',
user=user))
+        assert_equal(detail.ip, '1.2.3.4')
+        assert_equal(detail.ua, 'Mozilla/Firefox')
 
     def test_get_login_detail(self):
-        assert_equal(self.provider.get_login_detail(Request.blank('/')),
-                     dict(ip=None, ua=None))
-
-        assert_equal(self.provider.get_login_detail(Request.blank('/',
-                                                                  headers={'User-Agent':
'mybrowser'},
-                                                                  environ={'REMOTE_ADDR':
'3.3.3.3'})),
-                     dict(ip='3.3.3.3', ua='mybrowser'))
+        user = M.User(username='foobarbaz')
+        detail = self.provider.get_login_detail(Request.blank('/'), user)
+        assert_equal(detail.user_id, user._id)
+        assert_equal(detail.ip, None)
+        assert_equal(detail.ua, None)
+
+        detail = self.provider.get_login_detail(Request.blank('/',
+                                                              headers={'User-Agent': 'mybrowser'},
+                                                              environ={'REMOTE_ADDR': '3.3.3.3'}),
+                                                user)
+        assert_equal(detail.user_id, user._id)
+        assert_equal(detail.ip, '3.3.3.3')
+        assert_equal(detail.ua, 'mybrowser')
 
 
 class TestAuthenticationProvider(object):


Mime
View raw message