From: Benny Baumann <BenBE1987@gmx.net>
Date: Tue, 6 Feb 2018 15:03:52 +0000 (+0100)
Subject: chg: ensure actor, target and support ticket are non-null
X-Git-Url: https://code.wpia.club/?p=gigi.git;a=commitdiff_plain;h=c49333e3b0cbd9973267f927629c739fc3751e59

chg: ensure actor, target and support ticket are non-null

This patch is a defense-in-depth in cases of internal inconsistencies:
If e.g. somehow the session used to authenticate a request gets corrupted or
through a race condition a user gets deleted in the DB between validation
of the password and passing on that user to the actual session login this
will provide a safe-guard. This also centralises the check for acceptable
AuthorisationContexts in the class itself.

Result of this patch is any call to AuthorisationContext.getActor() will
return a non-null User object, as AuthorisationContexts with an null actor,
target or support ticket are rejected as invalid.

Change-Id: I78009fe3385820cd46a31a74c4c68f1cdaa65628
---

diff --git a/src/club/wpia/gigi/pages/LoginPage.java b/src/club/wpia/gigi/pages/LoginPage.java
index b0ed6e69..66412a91 100644
--- a/src/club/wpia/gigi/pages/LoginPage.java
+++ b/src/club/wpia/gigi/pages/LoginPage.java
@@ -120,24 +120,31 @@ public class LoginPage extends Page {
         try (GigiPreparedStatement ps = new GigiPreparedStatement("SELECT `password`, `id` FROM `users` WHERE `email`=? AND verified='1'")) {
             ps.setString(1, un);
             GigiResultSet rs = ps.executeQuery();
-            if (rs.next()) {
-                String dbHash = rs.getString(1);
-                String hash = PasswordHash.verifyHash(pw, dbHash);
-                if (hash != null) {
-                    if ( !hash.equals(dbHash)) {
-                        try (GigiPreparedStatement gps = new GigiPreparedStatement("UPDATE `users` SET `password`=? WHERE `email`=?")) {
-                            gps.setString(1, hash);
-                            gps.setString(2, un);
-                            gps.executeUpdate();
-                        }
+            if ( !rs.next()) {
+                throw new GigiApiException("Username and password didn't match.");
+            }
+
+            User user = User.getById(rs.getInt(2));
+            if (user == null) {
+                throw new GigiApiException("Username and password didn't match.");
+            }
+
+            String dbHash = rs.getString(1);
+            String hash = PasswordHash.verifyHash(pw, dbHash);
+            if (hash != null) {
+                if ( !hash.equals(dbHash)) {
+                    try (GigiPreparedStatement gps = new GigiPreparedStatement("UPDATE `users` SET `password`=? WHERE `email`=?")) {
+                        gps.setString(1, hash);
+                        gps.setString(2, un);
+                        gps.executeUpdate();
                     }
-                    loginSession(req, User.getById(rs.getInt(2)));
-                    req.getSession().setAttribute(LOGIN_METHOD, new TranslateCommand("Password"));
-                    return;
                 }
+
+                loginSession(req, user);
+                req.getSession().setAttribute(LOGIN_METHOD, new TranslateCommand("Password"));
+                return;
             }
         }
-        throw new GigiApiException("Username and password didn't match.");
     }
 
     public static User getUser(HttpServletRequest req) {
diff --git a/src/club/wpia/gigi/util/AuthorizationContext.java b/src/club/wpia/gigi/util/AuthorizationContext.java
index 40a63335..5d70b382 100644
--- a/src/club/wpia/gigi/util/AuthorizationContext.java
+++ b/src/club/wpia/gigi/util/AuthorizationContext.java
@@ -18,24 +18,37 @@ public class AuthorizationContext implements Outputable, Serializable {
 
     private static final long serialVersionUID = -2596733469159940154L;
 
-    private CertificateOwner target;
+    private final CertificateOwner target;
 
-    private User actor;
+    private final User actor;
 
-    private String supporterTicketId;
+    private final String supporterTicketId;
 
     public AuthorizationContext(CertificateOwner target, User actor) {
+        if (actor == null) {
+            throw new Error("Internal Error: The actor of an AuthorizationContext must not be null!");
+        }
+        if (target == null) {
+            throw new Error("Internal Error: The target of an AuthorizationContext must not be null!");
+        }
         this.target = target;
         this.actor = actor;
+        this.supporterTicketId = null;
     }
 
     public AuthorizationContext(User actor, String supporterTicket) throws GigiApiException {
+        if (actor == null) {
+            throw new Error("Internal Error: The actor of an AuthorizationContext must not be null!");
+        }
+        if (supporterTicket == null) {
+            throw new Error("Internal Error: The AuthorizationContext for a Support Engineer requires a valid ticket!");
+        }
         this.target = actor;
         this.actor = actor;
         if ( !isInGroup(Group.SUPPORTER)) {
             throw new GigiApiException("requires a supporter");
         }
-        supporterTicketId = supporterTicket;
+        this.supporterTicketId = supporterTicket;
     }
 
     public CertificateOwner getTarget() {
@@ -50,7 +63,7 @@ public class AuthorizationContext implements Outputable, Serializable {
         return actor.isInGroup(g);
     }
 
-    public User getActor(AuthorizationContext ac) {
+    public static User getActor(AuthorizationContext ac) {
         if (ac == null) {
             return null;
         }