]> git.ipfire.org Git - people/jschlag/pbs.git/commitdiff
Refactor session handling
authorMichael Tremer <michael.tremer@ipfire.org>
Sat, 7 Oct 2017 10:30:11 +0000 (11:30 +0100)
committerMichael Tremer <michael.tremer@ipfire.org>
Sat, 7 Oct 2017 10:35:30 +0000 (11:35 +0100)
Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>
src/buildservice/sessions.py
src/buildservice/users.py
src/database.sql
src/templates/sessions/index.html
src/web/handlers_auth.py
src/web/handlers_base.py

index 7b71f0f1dabee378ef858baa7fbdec1c08196617..f9b9ca6a8fadf628cb06ef245ff5b8b77c3f7d66 100644 (file)
 #!/usr/bin/python
 
-import uuid
-
 from . import base
 from . import users
 
-class Sessions(base.Object):
-       def get(self, session_id):
-               try:
-                       session = Session(self.pakfire, session_id)
-               except:
-                       return
-
-               return session
+from .decorators import *
 
-       def get_all(self):
-               query = "SELECT session_id FROM sessions WHERE valid_until >= NOW() \
+class Sessions(base.Object):
+       def __iter__(self):
+               query = "SELECT * FROM sessions WHERE valid_until >= NOW() \
                        ORDER BY valid_until DESC"
 
                sessions = []
-               for s in self.db.query(query):
-                       s = Session(self.pakfire, s.session_id)
-                       sessions.append(s)
+               for row in self.db.query(query):
+                       session = Session(self.backend, row.id, data=row)
+                       sessions.append(session)
 
-               return sessions
+               # Sort
+               sessions.sort()
 
-       def cleanup(self):
-               # Delete all sessions that are not valid any more.
-               self.db.execute("DELETE FROM sessions WHERE valid_until < NOW()")
+               return iter(sessions)
 
+       def create(self, user, address, user_agent=None):
+               """
+                       Creates a new session in the data.
 
-class Session(base.Object):
-       def __init__(self, pakfire, session_id):
-               base.Object.__init__(self, pakfire)
+                       The user is not checked and it is assumed that the user exists
+                       and has the right to log in.
+               """
+               session_id = users.generate_random_string(48)
+
+               res = self.db.get("INSERT INTO session(session_id, user_id, address, user_agent) \
+                       VALUES(%s, %s, %s, %s) RETURNING *", session_id, user.id, address, user_agent)
+
+               return Session(self.backend, res.id, data=res)
 
-               # Save the session_id.
-               self.id = session_id
+       def get_by_session_id(self, session_id):
+               res = self.db.get("SELECT * FROM sessions \
+                       WHERE session_id = %s AND valid_until >= NOW()", session_id)
 
-               self.data = self.db.get("SELECT * FROM sessions WHERE session_id = %s \
-                       AND valid_until >= NOW()", self.id)
+               if res:
+                       return Session(self.backend, res.id, data=res)
 
-               if not self.data:
-                       raise Exception, "No such session: %s" % self.id
+       # Alias function
+       get = get_by_session_id
 
-               # Cache.
-               self._user = None
-               self._impersonated_user = None
+       def cleanup(self):
+               # Delete all sessions that are not valid any more.
+               self.db.execute("DELETE FROM sessions WHERE valid_until < NOW()")
 
-       @staticmethod
-       def has_session(pakfire, session_id):
-               if self.db.get("SELECT session_id FROM sessions WHERE session_id = %s \
-                               AND valid_until >= NOW()", session_id):
-                       return True
 
-               return False
+class Session(base.DataObject):
+       def __eq__(self, other):
+               if isinstance(other, self.__class__):
+                       return self.id == other.id
 
-       def refresh(self, address=None):
-               self.db.execute("UPDATE sessions \
-                       SET valid_until = DATE_ADD(NOW(), INTERVAL 3 DAY), from_address = %s \
-                       WHERE session_id = %s", address, self.id)
+       def __lt__(self, other):
+               if isinstance(other, self.__class__):
+                       return self.user < other.user
 
        def destroy(self):
-               self.db.execute("DELETE FROM sessions WHERE session_id = %s", self.id)
+               self.db.execute("DELETE FROM sessions WHERE id = %s", self.id)
 
-       @property
+       @lazy_property
        def user(self):
-               if self._user is None:
-                       self._user = users.User(self.pakfire, self.data.user_id)
-                       self._user.session = self
+               return self.backend.users.get_by_id(self.data.user_id)
 
-               return self._user
+       @lazy_property
+       def impersonated_user(self):
+               if self.data.impersonated_user_id:
+                       return self.backend.users.get_by_id(self.data.impersonated_user_id)
 
        @property
-       def creation_time(self):
-               return self.data.creation_time
+       def created_at(self):
+               return self.data.created_at
 
        @property
        def valid_until(self):
                return self.data.valid_until
 
        @property
-       def from_address(self):
-               return self.data.from_address
+       def address(self):
+               return self.data.address
 
        @property
-       def impersonated_user(self):
-               if not self.data.impersonated_user_id:
-                       return
-
-               if self._impersonated_user is None:
-                       self._impersonated_user = \
-                               users.User(self.pakfire, self.data.impersonated_user_id)
-                       self._impersonated_user.session = self
-
-               return self._impersonated_user
+       def user_agent(self):
+               return self.data.user_agent
 
        def start_impersonation(self, user):
-               assert self.user.is_admin(), "Only admins can impersonate other users."
+               if not self.user.is_admin():
+                       raise RuntimeError("Only admins can impersonate other users")
 
-               # You cannot impersonate yourself.
                if self.user == user:
-                       return
+                       raise RuntimeError("You cannot impersonate yourself")
 
-               self.db.execute("UPDATE sessions SET impersonated_user_id = %s \
-                       WHERE session_id = %s", user.id, self.id)
+               self._set_attribute("impersonated_user_id", user.id)
 
        def stop_impersonation(self):
-               self.db.execute("UPDATE sessions SET impersonated_user_id = NULL \
-                       WHERE session_id = %s", self.id)
-
-       @classmethod
-       def create(cls, pakfire, user):
-               """
-                       Creates a new session in the data.
-
-                       The user is not checked and it is assumed that the user exsists
-                       and has the right to log in.
-               """
-               # Check if user has too much open sessions.
-               sessions = pakfire.db.get("SELECT COUNT(*) as count FROM sessions \
-                       WHERE user_id = %s AND valid_until >= NOW()", user.id)
-
-               sessions_max = pakfire.settings.get_int("sessions_max", 0)
-
-               if sessions.count >= sessions_max:
-                       raise Exception, "User exceeded maximum number of allowed sessions"
-
-               # Create a new session in the database.
-               session_id = "%s" % uuid.uuid4()
-
-               pakfire.db.execute("""
-                       INSERT INTO sessions(session_id, user_id, creation_time, valid_until)
-                       VALUES(%s, %s, NOW(), DATE_ADD(NOW(), INTERVAL 3 DAY))
-               """, session_id, user.id)
-
-               return cls(pakfire, session_id)
-
+               self._set_attribute("impersonated_user_id", None)
index 2157010e4451b110b7d3cf672bb20f89415ad424..98ccf51337924edeed62fbe9887b9e0a3f2b5501 100644 (file)
@@ -217,9 +217,6 @@ class User(base.Object):
                base.Object.__init__(self, pakfire)
                self.id = id
 
-               # A valid session of the user.
-               self.session = None
-
                # Cache.
                self._data = None
                self._emails = None
index 50cde782501eb6b665de6aa9a1b8b0b46c9eb5df..d03fcb14046fb8eec48471e68a92cb53ffe28c4f 100644 (file)
@@ -1894,17 +1894,41 @@ ALTER SEQUENCE repositories_id_seq OWNED BY repositories.id;
 --
 
 CREATE TABLE sessions (
+    id integer NOT NULL,
     session_id text NOT NULL,
+    created_at timestamp without time zone DEFAULT now() NOT NULL,
+    valid_until timestamp without time zone DEFAULT (now() + '7 days'::interval) NOT NULL,
     user_id integer NOT NULL,
     impersonated_user_id integer,
-    creation_time timestamp without time zone,
-    valid_until timestamp without time zone,
-    from_address inet
+    address inet,
+    user_agent text,
+    CONSTRAINT sessions_impersonation_check CHECK (((impersonated_user_id IS NULL) OR (user_id <> impersonated_user_id)))
 );
 
 
 ALTER TABLE sessions OWNER TO pakfire;
 
+--
+-- Name: sessions_id_seq; Type: SEQUENCE; Schema: public; Owner: pakfire
+--
+
+CREATE SEQUENCE sessions_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+
+ALTER TABLE sessions_id_seq OWNER TO pakfire;
+
+--
+-- Name: sessions_id_seq; Type: SEQUENCE OWNED BY; Schema: public; Owner: pakfire
+--
+
+ALTER SEQUENCE sessions_id_seq OWNED BY sessions.id;
+
+
 --
 -- Name: settings; Type: TABLE; Schema: public; Owner: pakfire; Tablespace: 
 --
@@ -2406,6 +2430,13 @@ ALTER TABLE ONLY repositories_aux ALTER COLUMN id SET DEFAULT nextval('repositor
 ALTER TABLE ONLY repositories_builds ALTER COLUMN id SET DEFAULT nextval('repositories_builds_id_seq'::regclass);
 
 
+--
+-- Name: id; Type: DEFAULT; Schema: public; Owner: pakfire
+--
+
+ALTER TABLE ONLY sessions ALTER COLUMN id SET DEFAULT nextval('sessions_id_seq'::regclass);
+
+
 --
 -- Name: id; Type: DEFAULT; Schema: public; Owner: pakfire
 --
@@ -2742,6 +2773,22 @@ ALTER TABLE ONLY jobs_packages
     ADD CONSTRAINT jobs_packages_unique UNIQUE (job_id, pkg_id);
 
 
+--
+-- Name: sessions_pkey; Type: CONSTRAINT; Schema: public; Owner: pakfire; Tablespace: 
+--
+
+ALTER TABLE ONLY sessions
+    ADD CONSTRAINT sessions_pkey PRIMARY KEY (id);
+
+
+--
+-- Name: sessions_session_id_key; Type: CONSTRAINT; Schema: public; Owner: pakfire; Tablespace: 
+--
+
+ALTER TABLE ONLY sessions
+    ADD CONSTRAINT sessions_session_id_key UNIQUE (session_id);
+
+
 --
 -- Name: builders_arches_builder_id; Type: INDEX; Schema: public; Owner: pakfire; Tablespace: 
 --
@@ -2973,13 +3020,6 @@ CREATE UNIQUE INDEX idx_2198189_build_id ON repositories_builds USING btree (bui
 CREATE INDEX idx_2198193_build_id ON repositories_history USING btree (build_id);
 
 
---
--- Name: idx_2198196_session_id; Type: INDEX; Schema: public; Owner: pakfire; Tablespace: 
---
-
-CREATE UNIQUE INDEX idx_2198196_session_id ON sessions USING btree (session_id);
-
-
 --
 -- Name: idx_2198199_k; Type: INDEX; Schema: public; Owner: pakfire; Tablespace: 
 --
index ec388ec88826e54114666d4ae1cb5f9bc024dab1..784766b7cce05bcbddba3cb5ccac355982c0c079 100644 (file)
                                                                <tr>
                                                                        <th>{{ _("Started") }}</th>
                                                                        <th>{{ _("Valid until") }}</th>
-                                                                       <th>{{ _("Last seen at") }}</th>
+                                                                       <th>{{ _("Address") }}/{{ _("User Agent") }}</th>
                                                                </tr>
                                                        </thead>
                                                        <tbody>
                                                                {% for s in user_sessions %}
                                                                        <tr>
                                                                                <td>
-                                                                                       {{ format_date(s.creation_time) }}
+                                                                                       {{ format_date(s.created_at) }}
                                                                                </td>
                                                                                <td>
                                                                                        {{ format_date(s.valid_until) }}
                                                                                </td>
                                                                                <td>
-                                                                                       {{ s.from_address or _("N/A") }}
+                                                                                       {{ s.address or _("N/A") }}
+
+                                                                                       {% if s.user_agent %}
+                                                                                               <br>
+                                                                                               <span class="muted">{{ s.user_agent }}</span>
+                                                                                       {% end %}
                                                                                </td>
                                                                        </tr>
                                                                {% end %}
index 60d04a660d132b0f7c9a9e182a6d2d2499280376..451a40434db18acc06da7bc60d75aea94c672fd8 100644 (file)
@@ -2,8 +2,6 @@
 
 import tornado.web
 
-from .. import sessions
-
 from .handlers_base import *
 
 class LoginHandler(BaseHandler):
@@ -20,24 +18,27 @@ class LoginHandler(BaseHandler):
                name = self.get_argument("name", None)
                passphrase = self.get_argument("pass", None)
 
+               # Log in the user
                user = self.pakfire.users.auth(name, passphrase)
 
-               if user:
-                       # Create a new session for the user.
-                       session = sessions.Session.create(self.pakfire, user)
+               # If the login was unsuccessful
+               if not user:
+                       self.set_status(403, "Login failed")
+                       return self.render("login.html", failed=True)
 
-                       # Set a cookie and update the current user.
-                       self.set_cookie("session_id", session.id, expires=session.valid_until)
-                       self._current_user = user
+               # Create a new session for the user.
+               with self.db.transaction():
+                       self.session = self.backend.sessions.create(user,
+                               self.current_address, user_agent=self.user_agent)
 
-                       # If there is "next" given, we redirect the user accordingly.
-                       # Otherwise we redirect to the front page.
-                       next = self.get_argument("next", "/")
-                       self.redirect(next)
+               # Set a cookie and update the current user.
+               self.set_cookie("session_id", self.session.session_id,
+                       expires=self.session.valid_until)
 
-               else:
-                       # If the login failed we return an error message.
-                       self.render("login.html", failed=True)
+               # If there is "next" given, we redirect the user accordingly.
+               # Otherwise we redirect to the front page.
+               next = self.get_argument("next", "/")
+               self.redirect(next)
 
 
 class RegisterHandler(BaseHandler):
@@ -142,11 +143,12 @@ class PasswordRecoveryHandler(BaseHandler):
 class LogoutHandler(BaseHandler):
        @tornado.web.authenticated
        def get(self):
+               # Destroy the user's session.
+               with self.db.transaction():
+                       self.session.destroy()
+
                # Remove the cookie, that identifies the user.
                self.clear_cookie("session_id")
 
-               # Destroy the user's session.
-               self.session.destroy()
-
                # Redirect the user to the front page.
                self.redirect("/")
index 9f615e67ec510cf79abae301e72e5aa7f6297a85..19d5c2a561ee9bfa2dffa0e54f81749818343a15 100644 (file)
@@ -11,39 +11,29 @@ import tornado.web
 import traceback
 
 from .. import misc
+from ..decorators import *
 
 class BaseHandler(tornado.web.RequestHandler):
        @property
        def cache(self):
                return self.pakfire.cache
 
-       def get_current_user(self):
-               session_id = self.get_cookie("session_id")
-               if not session_id:
-                       return
-
-               # Get the session from the database.
-               session = self.pakfire.sessions.get(session_id)
-
-               # Return nothing, if no session was found.
-               if not session:
-                       return
-
-               # Update the session lifetime.
-               session.refresh(self.request.remote_ip)
-               self.set_cookie("session_id", session.id, expires=session.valid_until)
+       @lazy_property
+       def session(self):
+               # Get the session from the cookie
+               session_id = self.get_cookie("session_id", None)
 
-               # If the session impersonated a user, we return that one.
-               if session.impersonated_user:
-                       return session.impersonated_user
+               # Search for a valid database session
+               if session_id:
+                       session = self.backend.sessions.get(session_id)
 
-               # By default, we return the user of this session.
-               return session.user
+                       # Found a valid session
+                       if session:
+                               return session
 
-       @property
-       def session(self):
-               if self.current_user:
-                       return self.current_user.session
+       def get_current_user(self):
+               if self.session:
+                       return self.session.impersonated_user or self.session.user
 
        def get_user_locale(self):
                # Get the locale from the user settings.
@@ -81,11 +71,7 @@ class BaseHandler(tornado.web.RequestHandler):
 
        @property
        def render_args(self):
-               session = None
-               if self.current_user:
-                       session = self.current_user.session
-
-               ret = {
+               return {
                        "bugtracker"      : self.pakfire.bugzilla,
                        "hostname"        : self.request.host,
                        "format_date"     : self.format_date,
@@ -95,12 +81,10 @@ class BaseHandler(tornado.web.RequestHandler):
                        "format_filemode" : misc.format_filemode,
                        "lang"            : self.locale.code[:2],
                        "pakfire_version" : pakfire.__version__,
-                       "session"         : session,
+                       "session"         : self.session,
                        "year"            : time.strftime("%Y"),
                }
 
-               return ret
-
        def render(self, *args, **kwargs):
                kwargs.update(self.render_args)
                tornado.web.RequestHandler.render(self, *args, **kwargs)