From 853ea5ab59e5d7845d389e46527038575c3c170c Mon Sep 17 00:00:00 2001
From: Colleen Murphy <comurphy@suse.com>
Date: Tue, 14 Mar 2017 01:24:31 +0100
Subject: [PATCH] Narrow expected responses for CheckUserInGroup

When checking whether a given user is in a given group, keystone will
return a 404 Not Found if all went well but the user was not in the
group. It may also return a 403 if the user and the group are in
different backends, which would also mean that the user was not in the
group[1]. Any other 400 response is a client error and any 500 response
is a server error to which the user should be alerted.

Without this patch, openstackclient treats any exception as a valid "not
found" and may end up hiding server errors. This patch reduces the
caught exceptions to 403 and 404 responses and treats everything else as
an error.

[1] https://developer.openstack.org/api-ref/identity/v3/?expanded=check-whether-user-belongs-to-group-detail#check-whether-user-belongs-to-group

Closes-bug: #1672634

Change-Id: Id3f3b2409b7cee480ee3c19b6d6c3070599ffe8f
---
 openstackclient/identity/v3/group.py            | 15 +++++++++------
 .../tests/unit/identity/v3/test_group.py        | 17 +++++++++++++++++
 .../notes/bug-1672634-ef754cb5109dd0f2.yaml     |  5 +++++
 3 files changed, 31 insertions(+), 6 deletions(-)
 create mode 100644 releasenotes/notes/bug-1672634-ef754cb5109dd0f2.yaml

diff --git a/openstackclient/identity/v3/group.py b/openstackclient/identity/v3/group.py
index 2afdabc171..b5f5d8ad88 100644
--- a/openstackclient/identity/v3/group.py
+++ b/openstackclient/identity/v3/group.py
@@ -102,12 +102,15 @@ class CheckUserInGroup(command.Command):
 
         try:
             identity_client.users.check_in_group(user_id, group_id)
-        except Exception:
-            msg = _("%(user)s not in group %(group)s\n") % {
-                'user': parsed_args.user,
-                'group': parsed_args.group,
-            }
-            sys.stderr.write(msg)
+        except ks_exc.http.HTTPClientError as e:
+            if e.http_status == 403 or e.http_status == 404:
+                msg = _("%(user)s not in group %(group)s\n") % {
+                    'user': parsed_args.user,
+                    'group': parsed_args.group,
+                }
+                sys.stderr.write(msg)
+            else:
+                raise e
         else:
             msg = _("%(user)s in group %(group)s\n") % {
                 'user': parsed_args.user,
diff --git a/openstackclient/tests/unit/identity/v3/test_group.py b/openstackclient/tests/unit/identity/v3/test_group.py
index 00bd217dad..5870e1dbc3 100644
--- a/openstackclient/tests/unit/identity/v3/test_group.py
+++ b/openstackclient/tests/unit/identity/v3/test_group.py
@@ -115,6 +115,23 @@ class TestGroupCheckUser(TestGroup):
             self.user.id, self.group.id)
         self.assertIsNone(result)
 
+    def test_group_check_user_server_error(self):
+        def server_error(*args):
+            raise ks_exc.http.InternalServerError
+        self.users_mock.check_in_group.side_effect = server_error
+        arglist = [
+            self.group.name,
+            self.user.name,
+        ]
+        verifylist = [
+            ('group', self.group.name),
+            ('user', self.user.name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(ks_exc.http.InternalServerError,
+                          self.cmd.take_action, parsed_args)
+
 
 class TestGroupCreate(TestGroup):
 
diff --git a/releasenotes/notes/bug-1672634-ef754cb5109dd0f2.yaml b/releasenotes/notes/bug-1672634-ef754cb5109dd0f2.yaml
new file mode 100644
index 0000000000..be874ab616
--- /dev/null
+++ b/releasenotes/notes/bug-1672634-ef754cb5109dd0f2.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Narrow acceptable negative response codes for ``group contains user``
+    [Bug `1672634 <https://bugs.launchpad.net/python-openstackclient/+bug/1672634>`_]