Re: [PATCH v3] Move AUR_OVERWRITE privilege check from git/auth to git/update

[Lukas Fleischer]

On Sun, 21 Jan 2018 at 17:51:02, Johannes Löthberg wrote:
> git/auth is run as an AutherizedKeysCommand which does not get the
> environment variables passed to it, so AUR_OVERWRITE always got
> hard-set to '0' by it.  Instead we need to perform the actual privilege
> check in git/update instead.
> 
> Signed-off-by: Johannes Löthberg 
> ---
> Removed unrelated garbage README change
> 
>  aurweb/git/auth.py   |  1 -
>  aurweb/git/update.py |  2 +-
>  test/t1100-git-auth.sh   | 17 -
>  test/t1300-git-update.sh | 14 +-
>  4 files changed, 14 insertions(+), 20 deletions(-)

Merged, thanks!

[PATCH v3] Move AUR_OVERWRITE privilege check from git/auth to git/update

[Johannes Löthberg]

git/auth is run as an AutherizedKeysCommand which does not get the
environment variables passed to it, so AUR_OVERWRITE always got
hard-set to '0' by it.  Instead we need to perform the actual privilege
check in git/update instead.

Signed-off-by: Johannes Löthberg 
---
Removed unrelated garbage README change

 aurweb/git/auth.py   |  1 -
 aurweb/git/update.py |  2 +-
 test/t1100-git-auth.sh   | 17 -
 test/t1300-git-update.sh | 14 +-
 4 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/aurweb/git/auth.py b/aurweb/git/auth.py
index b7819a9..828ed4e 100755
--- a/aurweb/git/auth.py
+++ b/aurweb/git/auth.py
@@ -53,7 +53,6 @@ def main():
 env_vars = {
 'AUR_USER': user,
 'AUR_PRIVILEGED': '1' if account_type > 1 else '0',
-'AUR_OVERWRITE' : os.environ.get('AUR_OVERWRITE', '0') if account_type 
> 1 else '0',
 }
 key = keytype + ' ' + keytext
 
diff --git a/aurweb/git/update.py b/aurweb/git/update.py
index f681ddb..da48eb3 100755
--- a/aurweb/git/update.py
+++ b/aurweb/git/update.py
@@ -238,7 +238,7 @@ def main():
 user = os.environ.get("AUR_USER")
 pkgbase = os.environ.get("AUR_PKGBASE")
 privileged = (os.environ.get("AUR_PRIVILEGED", '0') == '1')
-allow_overwrite = (os.environ.get("AUR_OVERWRITE", '0') == '1')
+allow_overwrite = (os.environ.get("AUR_OVERWRITE", '0') == '1') and 
privileged
 warn_or_die = warn if privileged else die
 
 if len(sys.argv) == 2 and sys.argv[1] == "restore":
diff --git a/test/t1100-git-auth.sh b/test/t1100-git-auth.sh
index dd20bea..71d526f 100755
--- a/test/t1100-git-auth.sh
+++ b/test/t1100-git-auth.sh
@@ -25,21 +25,4 @@ test_expect_success 'Test authentication with a wrong key.' '
test_must_be_empty out
 '
 
-test_expect_success 'Test AUR_OVERWRITE passthrough.' '
-   AUR_OVERWRITE=1 \
-   "$GIT_AUTH" "$AUTH_KEYTYPE_TU" "$AUTH_KEYTEXT_TU" >out &&
-   grep -q AUR_OVERWRITE=1 out
-'
-
-test_expect_success 'Make sure that AUR_OVERWRITE is unset by default.' '
-   "$GIT_AUTH" "$AUTH_KEYTYPE_TU" "$AUTH_KEYTEXT_TU" >out &&
-   grep -q AUR_OVERWRITE=0 out
-'
-
-test_expect_success 'Make sure regular users cannot set AUR_OVERWRITE.' '
-   AUR_OVERWRITE=1 \
-   "$GIT_AUTH" "$AUTH_KEYTYPE_USER" "$AUTH_KEYTEXT_USER" >out &&
-   grep -q AUR_OVERWRITE=0 out
-'
-
 test_done
diff --git a/test/t1300-git-update.sh b/test/t1300-git-update.sh
index b9c4f53..06d1498 100755
--- a/test/t1300-git-update.sh
+++ b/test/t1300-git-update.sh
@@ -137,7 +137,19 @@ test_expect_success 'Performing a non-fast-forward ref 
update as Trusted User.'
test_cmp expected actual
 '
 
-test_expect_success 'Performing a non-fast-forward ref update with 
AUR_OVERWRITE=1.' '
+test_expect_success 'Performing a non-fast-forward ref update as normal user 
with AUR_OVERWRITE=1.' '
+   old=$(git -C aur.git rev-parse HEAD) &&
+   new=$(git -C aur.git rev-parse HEAD^) &&
+   cat >expected <<-EOD &&
+   error: denying non-fast-forward (you should pull first)
+   EOD
+   test_must_fail \
+   env AUR_USER=user AUR_PKGBASE=foobar AUR_PRIVILEGED=0 AUR_OVERWRITE=1 \
+   "$GIT_UPDATE" refs/heads/master "$old" "$new" 2>&1 &&
+   test_cmp expected actual
+'
+
+test_expect_success 'Performing a non-fast-forward ref update as Trusted User 
with AUR_OVERWRITE=1.' '
old=$(git -C aur.git rev-parse HEAD) &&
new=$(git -C aur.git rev-parse HEAD^) &&
AUR_USER=tu AUR_PKGBASE=foobar AUR_PRIVILEGED=1 AUR_OVERWRITE=1 \
-- 
2.16.0