RGW/Roles: use the target/new max-session-duration value when validating it

If we validate before updating the role's max-session-duration, the
validator function wrongly uses the on-disk (existing/old) value for
validation. Note that the "role" object being updated is in-memory and
only after validation passes, it's persisted on-disk. So, calling role
object's update_max_session_duration API function is OK before the
role->validate_max_session_duration call.

validate_max_session_duration is used by both "role creation" and "role
update". The latter wrongly uses existing role's max_session_duration
value for validation instead of the new/target duration:

$ radosgw-admin ... role create --role-name=myrole ...
$ radosgw-admin ... role get --role-name=myrole | jq
'.MaxSessionDuration'
3600

where 3600 seconds is the default value.

$ radosgw-admin ... role update --role-name=myrole
--max_session_duration=100000
Max session duration updated successfully for role: myrole

Although above update call should have failed since 100K is higher than
43200 (the default max), it succeeded.

$ radosgw-admin ... role get --role-name=myrole | jq
'.MaxSessionDuration'
100000

Fixes: https://tracker.ceph.com/issues/63109
Signed-off-by: Oguzhan Ozmen <oozmen@bloomberg.net>

diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc
index cc7f5811c9efeee2a3341cbddb5e9a8c8637e34d..25955a52213c632ba34d51f6dc019910886eacd2 100644 (file)
--- a/src/rgw/rgw_admin.cc
+++ b/src/rgw/rgw_admin.cc
@@ -6922,11 +6922,11 @@ int main(int argc, const char **argv)
       if (ret < 0) {
         return -ret;
       }
+      role->update_max_session_duration(max_session_duration);
       if (!role->validate_max_session_duration(dpp())) {
         ret = -EINVAL;
         return ret;
       }
-      role->update_max_session_duration(max_session_duration);
       ret = role->update(dpp(), null_yield);
       if (ret < 0) {
         return -ret;

diff --git a/src/rgw/rgw_rest_role.cc b/src/rgw/rgw_rest_role.cc
index e71dff5708ff4300b9aba4b8eae0995d3fe72d4d..14e164553665a2fc7ab5dce26a6ac9042affb35e 100644 (file)
--- a/src/rgw/rgw_rest_role.cc
+++ b/src/rgw/rgw_rest_role.cc
@@ -1005,12 +1005,12 @@ void RGWUpdateRole::execute(optional_yield y)
     }
   }
 
+  _role->update_max_session_duration(max_session_duration);
   if (!_role->validate_max_session_duration(this)) {
     op_ret = -EINVAL;
     return;
   }
 
-  _role->update_max_session_duration(max_session_duration);
   op_ret = _role->update(this, y);
 
   s->formatter->open_object_section("UpdateRoleResponse");