From: Michael Tremer Date: Sat, 7 Oct 2017 10:30:11 +0000 (+0100) Subject: Refactor session handling X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d27380572cb5ea9abc3a806464c09562d0343ab5;p=pbs.git Refactor session handling Signed-off-by: Michael Tremer --- diff --git a/src/buildservice/sessions.py b/src/buildservice/sessions.py index 7b71f0f1..f9b9ca6a 100644 --- a/src/buildservice/sessions.py +++ b/src/buildservice/sessions.py @@ -1,138 +1,99 @@ #!/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) diff --git a/src/buildservice/users.py b/src/buildservice/users.py index 2157010e..98ccf513 100644 --- a/src/buildservice/users.py +++ b/src/buildservice/users.py @@ -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 diff --git a/src/database.sql b/src/database.sql index 50cde782..d03fcb14 100644 --- a/src/database.sql +++ b/src/database.sql @@ -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: -- diff --git a/src/templates/sessions/index.html b/src/templates/sessions/index.html index ec388ec8..784766b7 100644 --- a/src/templates/sessions/index.html +++ b/src/templates/sessions/index.html @@ -34,20 +34,25 @@ {{ _("Started") }} {{ _("Valid until") }} - {{ _("Last seen at") }} + {{ _("Address") }}/{{ _("User Agent") }} {% for s in user_sessions %} - {{ format_date(s.creation_time) }} + {{ format_date(s.created_at) }} {{ format_date(s.valid_until) }} - {{ s.from_address or _("N/A") }} + {{ s.address or _("N/A") }} + + {% if s.user_agent %} +
+ {{ s.user_agent }} + {% end %} {% end %} diff --git a/src/web/handlers_auth.py b/src/web/handlers_auth.py index 60d04a66..451a4043 100644 --- a/src/web/handlers_auth.py +++ b/src/web/handlers_auth.py @@ -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("/") diff --git a/src/web/handlers_base.py b/src/web/handlers_base.py index 9f615e67..19d5c2a5 100644 --- a/src/web/handlers_base.py +++ b/src/web/handlers_base.py @@ -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)