From c256866ad0f399530c686380db62b47883e3f63b Mon Sep 17 00:00:00 2001
From: =?utf8?q?Felix=20D=C3=B6rre?= <felix@dogcraft.de>
Date: Fri, 26 Aug 2016 10:08:24 +0200
Subject: [PATCH] add: prevent supporters from modifying their own accounts via
 support

Change-Id: Ie759b769074e5f7c25787cee7f5661fd8b1471a5
---
 src/org/cacert/gigi/dbObjects/SupportedUser.java |  2 +-
 src/org/cacert/gigi/dbObjects/User.java          |  8 +++++++-
 .../admin/support/SupportUserDetailsForm.java    |  4 ++++
 util-testing/org/cacert/gigi/pages/Manager.java  | 16 ++++++++--------
 4 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/src/org/cacert/gigi/dbObjects/SupportedUser.java b/src/org/cacert/gigi/dbObjects/SupportedUser.java
index 940e67fc..67b5e119 100644
--- a/src/org/cacert/gigi/dbObjects/SupportedUser.java
+++ b/src/org/cacert/gigi/dbObjects/SupportedUser.java
@@ -127,7 +127,7 @@ public class SupportedUser {
         }
     }
 
-    public void revoke(Group toMod) {
+    public void revoke(Group toMod) throws GigiApiException {
         target.revokeGroup(supporter, toMod);
         String subject = "Change Group Permissions";
         // send notification to support
diff --git a/src/org/cacert/gigi/dbObjects/User.java b/src/org/cacert/gigi/dbObjects/User.java
index 69b76ad2..b2599688 100644
--- a/src/org/cacert/gigi/dbObjects/User.java
+++ b/src/org/cacert/gigi/dbObjects/User.java
@@ -448,6 +448,9 @@ public class User extends CertificateOwner {
         if (toGrant.isManagedBySupport() && !granter.isInGroup(Group.SUPPORTER)) {
             throw new GigiApiException("Group may only be managed by supporter");
         }
+        if (toGrant.isManagedBySupport() && granter == this) {
+            throw new GigiApiException("Group may only be managed by supporter that is not oneself");
+        }
         groups.add(toGrant);
         try (GigiPreparedStatement ps = new GigiPreparedStatement("INSERT INTO `user_groups` SET `user`=?, `permission`=?::`userGroup`, `grantedby`=?")) {
             ps.setInt(1, getId());
@@ -457,7 +460,10 @@ public class User extends CertificateOwner {
         }
     }
 
-    public void revokeGroup(User revoker, Group toRevoke) {
+    public void revokeGroup(User revoker, Group toRevoke) throws GigiApiException {
+        if (toRevoke.isManagedBySupport() && !revoker.isInGroup(Group.SUPPORTER)) {
+            throw new GigiApiException("Group may only be managed by supporter");
+        }
         groups.remove(toRevoke);
         try (GigiPreparedStatement ps = new GigiPreparedStatement("UPDATE `user_groups` SET `deleted`=CURRENT_TIMESTAMP, `revokedby`=? WHERE `deleted` IS NULL AND `permission`=?::`userGroup` AND `user`=?")) {
             ps.setInt(1, revoker.getId());
diff --git a/src/org/cacert/gigi/pages/admin/support/SupportUserDetailsForm.java b/src/org/cacert/gigi/pages/admin/support/SupportUserDetailsForm.java
index ac7ffd00..d3589c8e 100644
--- a/src/org/cacert/gigi/pages/admin/support/SupportUserDetailsForm.java
+++ b/src/org/cacert/gigi/pages/admin/support/SupportUserDetailsForm.java
@@ -18,6 +18,7 @@ import org.cacert.gigi.output.GroupIterator;
 import org.cacert.gigi.output.GroupSelector;
 import org.cacert.gigi.output.template.Form;
 import org.cacert.gigi.output.template.Template;
+import org.cacert.gigi.pages.LoginPage;
 
 public class SupportUserDetailsForm extends Form {
 
@@ -40,6 +41,9 @@ public class SupportUserDetailsForm extends Form {
         if (user.getTicket() == null) {
             return false;
         }
+        if (user.getTargetUser() == LoginPage.getUser(req)) {
+            throw new GigiApiException("Supporter may not modify himself.");
+        }
         if ((req.getParameter("detailupdate") != null ? 1 : 0) + (req.getParameter("addGroup") != null ? 1 : 0) + (req.getParameter("removeGroup") != null ? 1 : 0) + (req.getParameter("resetPass") != null ? 1 : 0) != 1) {
             throw new GigiApiException("More than one action requested!");
         }
diff --git a/util-testing/org/cacert/gigi/pages/Manager.java b/util-testing/org/cacert/gigi/pages/Manager.java
index 2fd78ba7..ec709a17 100644
--- a/util-testing/org/cacert/gigi/pages/Manager.java
+++ b/util-testing/org/cacert/gigi/pages/Manager.java
@@ -296,16 +296,16 @@ public class Manager extends Page {
                 resp.getWriter().println("User not found.");
                 return;
             }
-            if (req.getParameter("addpriv") != null) {
-                try {
+            try {
+                if (req.getParameter("addpriv") != null) {
                     u.grantGroup(getSupporter(), Group.getByString(req.getParameter("priv")));
-                } catch (GigiApiException e) {
-                    throw new Error(e);
+                    resp.getWriter().println("Privilege granted");
+                } else {
+                    u.revokeGroup(getSupporter(), Group.getByString(req.getParameter("priv")));
+                    resp.getWriter().println("Privilege revoked");
                 }
-                resp.getWriter().println("Privilege granted");
-            } else {
-                u.revokeGroup(u, Group.getByString(req.getParameter("priv")));
-                resp.getWriter().println("Privilege revoked");
+            } catch (GigiApiException e) {
+                throw new Error(e);
             }
         } else if (req.getParameter("fetch") != null) {
             String mail = req.getParameter("femail");
-- 
2.39.2