MDSAuthCaps: print a special error message for wrong permissions

Permissions mentioned in MDS caps flags can either begin with "r" or
"rw", or can be "*" and "all". But it can't start with or be just "w" or
something else. This is confusing for some CephFS users since MON caps
can be just "w".

Command "ceph fs authorize" complains about this to the user. But other
commands (specifically, "ceph auth add", "ceph auth caps",
"ceph auth get-or-create" and "ceph auth get-or-create-key") don't. Make
these commands too print a helpful message, the way "ceph fs authorize"
command does.

Fixes: https://tracker.ceph.com/issues/61666
Signed-off-by: Rishabh Dave <ridave@redhat.com>
(cherry picked from commit f163dd3ef1fd9f05f05fa50eda9993225770d524)

Conflicts:
	qa/tasks/cephfs/test_admin.py
	Lines surrounding the patch to be applied were absent in quincy
	branch.

	src/mds/MDSAuthCaps.cc
	Conflict was due to the fact that "std:string" was replaced by
	"string".

diff --git a/qa/tasks/cephfs/test_admin.py b/qa/tasks/cephfs/test_admin.py
index 280044d9cb1881e39ea17c26790d5b0dfaadfb92..ae04a4ca3154088ff97e3aec0f60afce2a0d9e08 100644 (file)
--- a/qa/tasks/cephfs/test_admin.py
+++ b/qa/tasks/cephfs/test_admin.py
@@ -1122,3 +1122,61 @@ class TestAdminCommandDumpTree(CephFSTestCase):
         self.fs.mds_asok(['dump', 'tree', '/'])
         log.info("  subtree: '~mdsdir'")
         self.fs.mds_asok(['dump', 'tree', '~mdsdir'])
+
+
+class TestPermErrMsg(CephFSTestCase):
+
+    CLIENT_NAME = 'client.testuser'
+    FS1_NAME, FS2_NAME, FS3_NAME = 'abcd', 'efgh', 'ijkl'
+
+    EXPECTED_ERRNO = 22
+    EXPECTED_ERRMSG = ("Permission flags in MDS caps must start with 'r' or "
+                       "'rw' or be '*' or 'all'")
+
+    MONCAP = f'allow r fsname={FS1_NAME}'
+    OSDCAP = f'allow rw tag cephfs data={FS1_NAME}'
+    MDSCAPS = [
+        'allow w',
+        f'allow w fsname={FS1_NAME}',
+
+        f'allow rw fsname={FS1_NAME}, allow w fsname={FS2_NAME}',
+        f'allow w fsname={FS1_NAME}, allow rw fsname={FS2_NAME}',
+        f'allow w fsname={FS1_NAME}, allow w fsname={FS2_NAME}',
+
+        (f'allow rw fsname={FS1_NAME}, allow rw fsname={FS2_NAME}, allow '
+         f'w fsname={FS3_NAME}'),
+
+        # without space after comma
+        f'allow rw fsname={FS1_NAME},allow w fsname={FS2_NAME}',
+
+
+        'allow wr',
+        f'allow wr fsname={FS1_NAME}',
+
+        f'allow rw fsname={FS1_NAME}, allow wr fsname={FS2_NAME}',
+        f'allow wr fsname={FS1_NAME}, allow rw fsname={FS2_NAME}',
+        f'allow wr fsname={FS1_NAME}, allow wr fsname={FS2_NAME}',
+
+        (f'allow rw fsname={FS1_NAME}, allow rw fsname={FS2_NAME}, allow '
+         f'wr fsname={FS3_NAME}'),
+
+        # without space after comma
+        f'allow rw fsname={FS1_NAME},allow wr fsname={FS2_NAME}']
+
+    def _negtestcmd(self, SUBCMD, MDSCAP):
+        return self.negtest_ceph_cmd(
+            args=(f'{SUBCMD} {self.CLIENT_NAME} '
+                  f'mon "{self.MONCAP}" osd "{self.OSDCAP}" mds "{MDSCAP}"'),
+            retval=self.EXPECTED_ERRNO, errmsgs=self.EXPECTED_ERRMSG)
+
+    def test_auth_add(self):
+        for mdscap in self.MDSCAPS:
+            self._negtestcmd('auth add', mdscap)
+
+    def test_auth_get_or_create(self):
+        for mdscap in self.MDSCAPS:
+            self._negtestcmd('auth get-or-create', mdscap)
+
+    def test_auth_get_or_create_key(self):
+        for mdscap in self.MDSCAPS:
+            self._negtestcmd('auth get-or-create-key', mdscap)

diff --git a/src/mds/MDSAuthCaps.cc b/src/mds/MDSAuthCaps.cc
index d6bc6abe429bd98ed74196ebf64f7f7b66d23418..50bbe0fcfb02ddd88c228eb9f6a5ac8f8696d63c 100644 (file)
--- a/src/mds/MDSAuthCaps.cc
+++ b/src/mds/MDSAuthCaps.cc
@@ -363,10 +363,15 @@ bool MDSAuthCaps::parse(std::string_view str, ostream *err)
     // Make sure no grants are kept after parsing failed!
     grants.clear();
 
-    if (err)
-      *err << "mds capability parse failed, stopped at '"
-          << std::string(iter, end)
-           << "' of '" << str << "'";
+    if (err) {
+      if (string(iter, end).find("allow") != string::npos) {
+       *err << "Permission flags in MDS caps must start with 'r' or " <<
+              "'rw' or be '*' or 'all'";
+      } else {
+       *err << "mds capability parse failed, stopped at '"
+            << string(iter, end) << "' of '" << str << "'";
+      }
+    }
     return false; 
   }
 }