[#8290] move previous_login_details into UserLoginDetails collection
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 @@
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 @@
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 @@
]
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 @@
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 .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 .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 @@
'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 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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)
+
+ 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='Foo\nIP Address: 1.2.3.4\nFoo')),
- dict(ip='1.2.3.4', 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))
+ 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)
- 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'))
+ 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):
