Merge ~kajiya/+git/google-compute-engine-oslogin:update-master-to-20240701.00 into ~ubuntu-core-dev/+git/google-compute-engine-oslogin:ubuntu/master

[Chloé Smith]

New upstream version 20240701.00

Related MPs...

upstream: https://code.launchpad.net/~kajiya/+git/google-compute-engine-oslogin/+merge/469489
pristine-tar: https://code.launchpad.net/~kajiya/+git/google-compute-engine-oslogin/+merge/469488

[Utkarsh Gupta (utkarsh)]

Hey,

Thank you for the MP, it looks good but it doesn't do the right .so replacement.

I am wondering how'd it work for you? Do any of the tests test the .so?

review: Needs Fixing

[Utkarsh Gupta (utkarsh)]

Ah, I debugged and that's what you need:

commit 421ec6034aebb2f0346d973e4e33a95259583c54 (HEAD -> ubuntu/master, tag: ubuntu/20240701.00-0ubuntu1)
Author: Utkarsh Gupta <email address hidden>
Date: Wed Jul 24 17:19:21 2024 +0900

    d/links: Update links w/ respective .so shipped

diff --git a/debian/changelog b/debian/changelog
index 82fd2b8..f16cf4f 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,5 +1,9 @@
 google-compute-engine-oslogin (20240701.00-0ubuntu1) oracular; urgency=medium

+ [ Utkarsh Gupta ]
+ * d/links: Update links w/ respective .so shipped
+
+ [ Chloé 'kajiya' Smith ]
   * New upstream version for upstream tag 20240320.00. (LP: #2073166)

  -- Chloé 'kajiya' Smith <email address hidden> Mon, 15 Jul 2024 17:56:01 +0100
diff --git a/debian/links b/debian/links
index 12e449f..f5a4762 100644
--- a/debian/links
+++ b/debian/links
@@ -1,2 +1,2 @@
-lib/libnss_cache_oslogin-20231004.00.so lib/libnss_cache_oslogin.so.2
-lib/libnss_oslogin-20231004.00.so lib/libnss_oslogin.so.2
+lib/libnss_cache_oslogin-20240701.00.so lib/libnss_cache_oslogin.so.2
+lib/libnss_oslogin-20240701.00.so lib/libnss_oslogin.so.2

Sponsoring your upload with this now.

review: Approve

diff --git a/OWNERS b/OWNERS
index edde10e..59fca61 100644
--- a/OWNERS
+++ b/OWNERS
@@ -3,14 +3,14 @@
 
 approvers:
   - a-crate
+  - ajorg
   - bkatyl
   - chaitanyakulkarni28
   - dorileo
   - drewhli
   - elicriffield
+  - gaughen
   - jjerger
   - karnvadaliya
   - koln67
-  - quintonamore
-  - vorakl
   - zmarano
diff --git a/debian/changelog b/debian/changelog
index 2c83f42..82fd2b8 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,9 @@
+google-compute-engine-oslogin (20240701.00-0ubuntu1) oracular; urgency=medium
+
+  * New upstream version for upstream tag 20240320.00. (LP: #2073166)
+
+ -- Chloé 'kajiya' Smith <[e-mail]>  Mon, 15 Jul 2024 17:56:01 +0100
+
 google-compute-engine-oslogin (20231004.00-0ubuntu5) oracular; urgency=medium
 
   * d/p/0002-inherit-cflags.patch: inherit environment's build flags
diff --git a/packaging/google-compute-engine-oslogin.spec b/packaging/google-compute-engine-oslogin.spec
index 8f5eac2..5cf6bc6 100644
--- a/packaging/google-compute-engine-oslogin.spec
+++ b/packaging/google-compute-engine-oslogin.spec
@@ -67,6 +67,7 @@ make install DESTDIR=%{buildroot} LIBDIR=/%{_lib} VERSION=%{version} INSTALL_SEL
 /%{_lib}/libnss_cache_oslogin-%{version}.so
 /%{_lib}/libnss_oslogin.so.2
 /%{_lib}/libnss_cache_oslogin.so.2
+/%{_lib}/security/pam_oslogin_admin.so
 /%{_lib}/security/pam_oslogin_login.so
 /usr/bin/google_authorized_keys
 /usr/bin/google_authorized_keys_sk
diff --git a/src/Makefile b/src/Makefile
index a633c7c..4f21719 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -21,12 +21,33 @@ CRONDIR = /etc/cron.d
 SYSTEMDDIR = /lib/systemd/system
 PRESETDIR = /lib/systemd/system-preset
 
+DEST_LIBDIR = $(LIBDIR)
+DEST_BINDIR = $(BINDIR)
+DEST_PAMDIR = $(PAMDIR)
+DEST_MANDIR = $(MANDIR)
+DEST_SELINUX = /usr/share/selinux/packages
+DEST_CRONDIR = $(CRONDIR)
+DEST_SYSTEMDDIR = $(SYSTEMDDIR)
+DEST_PRESETDIR = $(PRESETDIR)
+
+ifneq ($(DESTDIR),)
+DEST_LIBDIR = $(DESTDIR)/$(LIBDIR)
+DEST_BINDIR = $(DESTDIR)/$(BINDIR)
+DEST_PAMDIR = $(DESTDIR)/$(PAMDIR)
+DEST_MANDIR = $(DESTDIR)/$(MANDIR)
+DEST_SELINUX = $(DESTDIR)/usr/share/selinux/packages
+DEST_CRONDIR = $(DESTDIR)/$(CRONDIR)
+DEST_SYSTEMDDIR = $(DESTDIR)/$(SYSTEMDDIR)
+DEST_PRESETDIR = $(DESTDIR)/$(PRESETDIR)
+endif
+
 NSS_OSLOGIN_SONAME       = libnss_oslogin.so.2
 NSS_CACHE_OSLOGIN_SONAME = libnss_cache_oslogin.so.2
 
 NSS_OSLOGIN              = libnss_oslogin-$(VERSION).so
 NSS_CACHE_OSLOGIN        = libnss_cache_oslogin-$(VERSION).so
 
+PAM_ADMIN                = pam_oslogin_admin.so
 PAM_LOGIN                = pam_oslogin_login.so
 
 BINARIES = google_oslogin_nss_cache google_authorized_keys google_authorized_keys_sk google_authorized_principals
@@ -34,7 +55,7 @@ BINARIES = google_oslogin_nss_cache google_authorized_keys google_authorized_key
 .PHONY: all clean install
 .DEFAULT_GOAL := all
 
-all: $(NSS_OSLOGIN) $(NSS_CACHE_OSLOGIN) $(PAM_LOGIN) $(BINARIES)
+all: $(NSS_OSLOGIN) $(NSS_CACHE_OSLOGIN) $(PAM_LOGIN) $(PAM_ADMIN) $(BINARIES)
 
 clean:
 	rm -f $(BINARIES)
@@ -52,12 +73,15 @@ $(NSS_CACHE_OSLOGIN): nss/nss_cache_oslogin.o nss/compat/getpwent_r.o oslogin_ut
 
 # PAM modules
 
-$(PAM_LOGIN): pam/pam_oslogin_login.o oslogin_sshca.o oslogin_utils.o include/oslogin_sshca.h
+$(PAM_LOGIN): pam/pam_oslogin_login.o oslogin_sshca.o oslogin_utils.o
+	$(CXX) $(CXXFLAGS) $(CPPFLAGS) -shared $^ -o $@ $(PAMLIBS)
+
+$(PAM_ADMIN): pam/pam_oslogin_admin.o oslogin_sshca.o oslogin_utils.o
 	$(CXX) $(CXXFLAGS) $(CPPFLAGS) -shared $^ -o $@ $(PAMLIBS)
 
 # Utilities.
 
-google_authorized_principals: authorized_principals/authorized_principals.o oslogin_utils.o oslogin_sshca.o include/oslogin_sshca.h
+google_authorized_principals: authorized_principals/authorized_principals.o oslogin_utils.o oslogin_sshca.o
 	$(CXX) $(CXXFLAGS) $(CPPFLAGS) $^ -o $@ $(LDLIBS)
 
 google_authorized_keys: authorized_keys/authorized_keys.o oslogin_utils.o
@@ -71,37 +95,37 @@ google_oslogin_nss_cache: cache_refresh/cache_refresh.o oslogin_utils.o
 
 install: all
 	# Make dirs
-	install -d $(DESTDIR)$(LIBDIR)
-	install -d $(DESTDIR)$(PAMDIR)
-	install -d $(DESTDIR)$(BINDIR)
-	install -d $(DESTDIR)$(MANDIR)/man8
+	install -d $(DEST_LIBDIR)
+	install -d $(DEST_PAMDIR)
+	install -d $(DEST_BINDIR)
+	install -d $(DEST_MANDIR)/man8
 	# NSS modules
-	install -m 0644 -t $(DESTDIR)$(LIBDIR) $(NSS_OSLOGIN) $(NSS_CACHE_OSLOGIN)
-	ln -sf $(NSS_OSLOGIN)         $(DESTDIR)$(LIBDIR)/$(NSS_OSLOGIN_SONAME)
-	ln -sf $(NSS_CACHE_OSLOGIN)   $(DESTDIR)$(LIBDIR)/$(NSS_CACHE_OSLOGIN_SONAME)
+	install -m 0644 -t $(DEST_LIBDIR) $(NSS_OSLOGIN) $(NSS_CACHE_OSLOGIN)
+	ln -sf $(NSS_OSLOGIN)         $(DEST_LIBDIR)/$(NSS_OSLOGIN_SONAME)
+	ln -sf $(NSS_CACHE_OSLOGIN)   $(DEST_LIBDIR)/$(NSS_CACHE_OSLOGIN_SONAME)
 	# PAM modules
-	install -m 0644 -t $(DESTDIR)$(PAMDIR) $(PAM_LOGIN)
+	install -m 0644 -t $(DEST_PAMDIR) $(PAM_LOGIN) $(PAM_ADMIN)
 	# Binaries
-	install -m 0755 -t $(DESTDIR)$(BINDIR) $(BINARIES)
+	install -m 0755 -t $(DEST_BINDIR) $(BINARIES)
 	# Manpages
-	install -m 0644 -t $(DESTDIR)$(MANDIR)/man8 $(TOPDIR)/man/nss-oslogin.8 $(TOPDIR)/man/nss-cache-oslogin.8
-	gzip -9f $(DESTDIR)$(MANDIR)/man8/nss-oslogin.8
-	gzip -9f $(DESTDIR)$(MANDIR)/man8/nss-cache-oslogin.8
-	ln -sf nss-oslogin.8.gz       $(DESTDIR)$(MANDIR)/man8/$(NSS_OSLOGIN_SONAME).8.gz
-	ln -sf nss-cache-oslogin.8.gz $(DESTDIR)$(MANDIR)/man8/$(NSS_CACHE_OSLOGIN_SONAME).8.gz
+	install -m 0644 -t $(DEST_MANDIR)/man8 $(TOPDIR)/man/nss-oslogin.8 $(TOPDIR)/man/nss-cache-oslogin.8
+	gzip -9f $(DEST_MANDIR)/man8/nss-oslogin.8
+	gzip -9f $(DEST_MANDIR)/man8/nss-cache-oslogin.8
+	ln -sf nss-oslogin.8.gz       $(DEST_MANDIR)/man8/$(NSS_OSLOGIN_SONAME).8.gz
+	ln -sf nss-cache-oslogin.8.gz $(DEST_MANDIR)/man8/$(NSS_CACHE_OSLOGIN_SONAME).8.gz
 ifdef INSTALL_SELINUX
 	# SELinux policy package
-	install -d $(DESTDIR)/usr/share/selinux/packages
-	install -m 0644 -t $(DESTDIR)/usr/share/selinux/packages $(TOPDIR)/selinux/oslogin.pp
+	install -d $(DEST_SELINUX)
+	install -m 0644 -t $(DEST_SELINUX) $(TOPDIR)/selinux/oslogin.pp
 endif
 ifdef INSTALL_CRON
 	# Cache refresh cron
-	install -d $(DESTDIR)$(CRONDIR)
-	install -m 0644 $(TOPDIR)/cron.d $(DESTDIR)$(CRONDIR)/google-compute-engine-oslogin
+	install -d $(DEST_CRONDIR)
+	install -m 0644 $(TOPDIR)/cron.d $(DEST_CRONDIR)/google-compute-engine-oslogin
 else
 	# Cache refresh systemd timer
-	install -d $(DESTDIR)$(SYSTEMDDIR)
-	install -m 0644 -t $(DESTDIR)$(SYSTEMDDIR) $(TOPDIR)/google-oslogin-cache.timer $(TOPDIR)/google-oslogin-cache.service
-	install -d $(DESTDIR)$(PRESETDIR)
-	install -m 0644 -t $(DESTDIR)$(PRESETDIR) $(TOPDIR)/90-google-compute-engine-oslogin.preset
+	install -d $(DEST_SYSTEMDDIR)
+	install -m 0644 -t $(DEST_SYSTEMDDIR) $(TOPDIR)/google-oslogin-cache.timer $(TOPDIR)/google-oslogin-cache.service
+	install -d $(DEST_PRESETDIR)
+	install -m 0644 -t $(DEST_PRESETDIR) $(TOPDIR)/90-google-compute-engine-oslogin.preset
 endif
diff --git a/src/include/oslogin_utils.h b/src/include/oslogin_utils.h
index 368790e..e7c41fe 100644
--- a/src/include/oslogin_utils.h
+++ b/src/include/oslogin_utils.h
@@ -186,6 +186,9 @@ size_t OnCurlWrite(void* buf, size_t size, size_t nmemb, void* userp);
 bool HttpGet(const string& url, string* response, long* http_code);
 bool HttpPost(const string& url, const string& data, string* response,
               long* http_code);
+// Based on known MDS status codes returns whether the HTTP request
+// should be retried or not.
+bool ShouldRetry(long http_code);
 
 // Returns whether user_name is a valid OsLogin user name.
 bool ValidateUserName(const string& user_name);
@@ -294,6 +297,11 @@ extern void SysLogErr(const char *fmt, ...);
 
 // AuthoOptions wraps authorization options.
 struct AuthOptions {
+  // admin_policy_required determines if a user is only authorized if admin
+  // policy is available for such a user. i.e. AuthorizeUser() should return
+  // false if adminLogin is not available.
+  bool admin_policy_required;
+
   // security_key determines if the MDS "/users?..." should use
   // the view=securityKey parameter.
   bool security_key;
diff --git a/src/oslogin_utils.cc b/src/oslogin_utils.cc
index f8bbf9c..aad560f 100644
--- a/src/oslogin_utils.cc
+++ b/src/oslogin_utils.cc
@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Requires libcurl4-openssl-dev libjson0 and libjson0-dev
+// Requires libcurl4-openssl-dev, libjson-c5, and libjson-c-dev
 #include <curl/curl.h>
 #include <errno.h>
 #include <grp.h>
@@ -48,7 +48,10 @@
 using std::string;
 
 // Maximum number of retries for HTTP requests.
-const int kMaxRetries = 1;
+const int kMaxRetries = 3;
+
+// Backoff duration 1 sec between retries.
+const int kBackoffDuration = 1;
 
 // Regex for validating user names.
 static const char kUserNameRegex[] = "^[a-zA-Z0-9._][a-zA-Z0-9._-]{0,31}$";
@@ -189,11 +192,29 @@ bool NssCache::GetNextGroup(BufferManager* buf, struct group* result, int* errno
   return ParseJsonToGroup(cached_passwd, result, buf, errnop);
 }
 
+// ParseJsonRoot is declared early here, away from the other parsing functions
+// found later (in the "JSON Parsing" section), so LoadJsonUsersToCache can
+// take advantage of the improved error handling ParseJsonRoot offers.
+json_object* ParseJsonRoot(const string& json) {
+  json_object* root = NULL;
+  struct json_tokener* tok = json_tokener_new();
+
+  root = json_tokener_parse_ex(tok, json.c_str(), -1);
+  if (root == NULL) {
+    enum json_tokener_error jerr = json_tokener_get_error(tok);
+    string error_message = json_tokener_error_desc(jerr);
+    SysLogErr("Failed to parse root JSON element: \"%s\", from input \"%s\"",
+              error_message, json);
+  }
+
+  json_tokener_free(tok);
+  return root;
+}
+
 bool NssCache::LoadJsonUsersToCache(string response) {
   Reset();
 
-  json_object* root = NULL;
-  root = json_tokener_parse(response.c_str());
+  json_object* root = ParseJsonRoot(response);
   if (root == NULL) {
     return false;
   }
@@ -392,6 +413,22 @@ size_t OnCurlWrite(void* buf, size_t size, size_t nmemb, void* userp) {
   return 0;
 }
 
+bool ShouldRetry(long http_code) {
+  if (http_code == 200) {
+    // Request returned successfully, no need to retry.
+    return false;
+  }
+  if (http_code == 404) {
+    // Metadata key does not exist, no point of retrying.
+    return false;
+  }
+  if (http_code == 400) {
+    // Request parameters are bad, no point of retrying.
+    return false;
+  }
+  return true;
+}
+
 bool HttpDo(const string& url, const string& data, string* response, long* http_code) {
   if (response == NULL || http_code == NULL) {
     return false;
@@ -410,6 +447,10 @@ bool HttpDo(const string& url, const string& data, string* response, long* http_
       return false;
     }
     do {
+      // Apply backoff strategy before retrying.
+      if (retry_count > 0) {
+        sleep(kBackoffDuration);
+      }
       response_stream.str("");
       response_stream.clear();
       curl_easy_setopt(curl, CURLOPT_HTTPHEADER, header_list);
@@ -428,7 +469,7 @@ bool HttpDo(const string& url, const string& data, string* response, long* http_
         return false;
       }
       curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, http_code);
-    } while (retry_count++ < kMaxRetries && *http_code == 500);
+    } while (retry_count++ < kMaxRetries && ShouldRetry(*http_code));
     curl_slist_free_all(header_list);
   }
   *response = response_stream.str();
@@ -506,14 +547,12 @@ bool ValidatePasswd(struct passwd* result, BufferManager* buf, int* errnop) {
 // ----------------- JSON Parsing -----------------
 
 bool ParseJsonToUsers(const string& json, std::vector<string>* result) {
-  json_object* root = NULL;
-  root = json_tokener_parse(json.c_str());
-  if (root == NULL) {
-    return false;
-  }
-
   bool ret = false;
 
+  json_object* root = ParseJsonRoot(json);
+  if (root == NULL) {
+    return ret;
+  }
   json_object* users = NULL;
   if (!json_object_object_get_ex(root, "usernames", &users)) {
     ret = true; // means no users, not invalid.
@@ -535,19 +574,22 @@ cleanup:
 }
 
 bool ParseJsonToGroups(const string& json, std::vector<Group>* result) {
-  json_object* root = NULL;
-  root = json_tokener_parse(json.c_str());
-  if (root == NULL) {
-    return false;
-  }
-
   bool ret = false;
 
-  json_object* groups = NULL;
+  json_object* root = ParseJsonRoot(json);
+  if (root == NULL) {
+    return ret;
+  }
+  json_object* groups;
+  json_type groupType;
   if (!json_object_object_get_ex(root, "posixGroups", &groups)) {
+    SysLogErr("failed to parse POSIX groups from \"%s\"", json);
     goto cleanup;
   }
-  if (json_object_get_type(groups) != json_type_array) {
+  groupType = json_object_get_type(groups);
+  if (groupType != json_type_array) {
+    SysLogErr("parsed unexpected type for field \"posixGroups\"; "
+              "want a list, got %s", groupType);
     goto cleanup;
   }
   for (int idx = 0; idx < (int)json_object_array_length(groups); idx++) {
@@ -555,11 +597,12 @@ bool ParseJsonToGroups(const string& json, std::vector<Group>* result) {
 
     json_object* gid;
     if (!json_object_object_get_ex(group, "gid", &gid)) {
+      SysLogErr("failed to parse gid from group %s", json_object_get_string(group));
       goto cleanup;
     }
-
     json_object* name;
     if (!json_object_object_get_ex(group, "name", &name)) {
+      SysLogErr("failed to parse name from group %s", json_object_get_string(group));
       goto cleanup;
     }
 
@@ -589,22 +632,19 @@ cleanup:
 
 bool ParseJsonToGroup(const string& json, struct group* result, BufferManager*
                       buf, int* errnop) {
+  bool ret = false;
   *errnop = EINVAL;
   int gr_gid = 65535;
 
-  json_object* group = NULL;
-  group = json_tokener_parse(json.c_str());
+  json_object* group = ParseJsonRoot(json);
   if (group == NULL) {
     return false;
   }
 
-  bool ret = false;
-
   json_object* gid;
   if (!json_object_object_get_ex(group, "gid", &gid)) {
     goto cleanup;
   }
-
   json_object* name;
   if (!json_object_object_get_ex(group, "name", &name)) {
     goto cleanup;
@@ -631,16 +671,13 @@ cleanup:
 
 std::vector<string> ParseJsonToSshKeys(const string& json) {
   std::vector<string> result;
-  json_object* ssh_public_keys = NULL;
-
-  json_object* root = NULL;
-  root = json_tokener_parse(json.c_str());
+  json_object* root = ParseJsonRoot(json);
   if (root == NULL) {
     return result;
   }
 
   // Locate the sshPublicKeys object.
-  json_object* login_profiles = NULL;
+  json_object* login_profiles;
   if (!json_object_object_get_ex(root, "loginProfiles", &login_profiles)) {
     goto cleanup;
   }
@@ -649,6 +686,7 @@ std::vector<string> ParseJsonToSshKeys(const string& json) {
   }
   login_profiles = json_object_array_get_idx(login_profiles, 0);
 
+  json_object* ssh_public_keys;
   if (!json_object_object_get_ex(login_profiles, "sshPublicKeys", &ssh_public_keys)) {
     goto cleanup;
   }
@@ -701,16 +739,14 @@ cleanup:
 
 std::vector<string> ParseJsonToSshKeysSk(const string& json) {
   std::vector<string> result;
-  json_object* security_keys = NULL;
 
-  json_object* root = NULL;
-  root = json_tokener_parse(json.c_str());
+  json_object* root = ParseJsonRoot(json);
   if (root == NULL) {
     return result;
   }
 
   // Locate the securityKeys array.
-  json_object* login_profiles = NULL;
+  json_object* login_profiles;
   if (!json_object_object_get_ex(root, "loginProfiles", &login_profiles)) {
     goto cleanup;
   }
@@ -720,9 +756,11 @@ std::vector<string> ParseJsonToSshKeysSk(const string& json) {
 
   login_profiles = json_object_array_get_idx(login_profiles, 0);
 
+  json_object* security_keys;
   if (!json_object_object_get_ex(login_profiles, "securityKeys", &security_keys)) {
     goto cleanup;
   }
+
   if (json_object_get_type(security_keys) != json_type_array) {
     goto cleanup;
   }
@@ -757,19 +795,18 @@ cleanup:
 
 bool ParseJsonToPasswd(const string& json, struct passwd* result, BufferManager*
                        buf, int* errnop) {
+  bool ret = false;
   *errnop = EINVAL;
   json_object* root = NULL;
   json_object* origroot = NULL;
 
-  origroot = root = json_tokener_parse(json.c_str());
+  origroot = root = ParseJsonRoot(json);
   if (root == NULL) {
     return false;
   }
 
-  bool ret = false;
-  json_object* posix_accounts = NULL;
-
-  json_object* login_profiles = NULL;
+  json_object* posix_accounts;
+  json_object* login_profiles;
   // If this is called from getpwent_r, loginProfiles won't be in the response.
   if (json_object_object_get_ex(root, "loginProfiles", &login_profiles)) {
     if (json_object_get_type(login_profiles) != json_type_array) {
@@ -888,17 +925,16 @@ bool AddUsersToGroup(std::vector<string> users, struct group* result,
 }
 
 bool ParseJsonToEmail(const string& json, string* email) {
-  json_object* root = NULL;
-  root = json_tokener_parse(json.c_str());
+  bool ret = false;
+
+  json_object* root = ParseJsonRoot(json);
   if (root == NULL) {
-    return false;
+    return ret;
   }
 
-  bool ret = false;
-  json_object* json_email = NULL;
-
   // Locate the email object.
-  json_object* login_profiles = NULL;
+  json_object* login_profiles;
+  json_object* json_email;
   if (!json_object_object_get_ex(root, "loginProfiles", &login_profiles)) {
     goto cleanup;
   }
@@ -918,8 +954,7 @@ cleanup:
 }
 
 bool ParseJsonToSuccess(const string& json) {
-  json_object* root = NULL;
-  root = json_tokener_parse(json.c_str());
+  json_object* root = ParseJsonRoot(json);
   if (root == NULL) {
     return false;
   }
@@ -934,17 +969,15 @@ bool ParseJsonToSuccess(const string& json) {
 }
 
 bool ParseJsonToKey(const string& json, const string& key, string* response) {
-  json_object* root = NULL;
-  root = json_tokener_parse(json.c_str());
+  bool ret = false;
+
+  json_object* root = ParseJsonRoot(json);
   if (root == NULL) {
-    return false;
+    return ret;
   }
 
-  bool ret = false;
   json_object* json_response = NULL;
   const char* c_response = NULL;
-
-
   if (!json_object_object_get_ex(root, key.c_str(), &json_response)) {
     goto cleanup;
   }
@@ -962,13 +995,13 @@ cleanup:
 }
 
 bool ParseJsonToChallenges(const string& json, std::vector<Challenge>* challenges) {
-  json_object* root = NULL;
-  root = json_tokener_parse(json.c_str());
+  bool ret = false;
+
+  json_object* root = ParseJsonRoot(json);
   if (root == NULL) {
-    return false;
+    return ret;
   }
 
-  bool ret = false;
   json_object* challengeId = NULL;
   json_object* challengeType = NULL;
   json_object* challengeStatus = NULL;
@@ -1260,18 +1293,18 @@ static bool ApplyPolicy(const char *user_name, string email, const char *policy,
   long http_code = 0;
   // Invalid user, just leave from here - the principal will not be allowed/authorized.
   if (!HttpGet(url.str(), &response, &http_code)) {
-    SysLogErr("Failed to validate organization user %s has login permission.", user_name);
+    SysLogErr("Failed to validate that OS Login user %s has %s permission.", user_name, policy);
     return false;
   }
 
   if (http_code != 200) {
-    SysLogErr("Failed to validate organization user %s has login permission, "
-              "got HTTP response code: %lu", user_name, http_code);
+    SysLogErr("Failed to validate that OS Login user %s has %s permission; "
+              "got HTTP response code: %lu", user_name, policy, http_code);
     return false;
   }
 
   if (!ParseJsonToSuccess(response)) {
-    SysLogErr("Organization user %s does not have login permission.", user_name);
+    SysLogErr("OS Login user %s does not have %s permission.", user_name, policy);
     return false;
   }
 
@@ -1369,6 +1402,9 @@ bool AuthorizeUser(const char *user_name, struct AuthOptions opts, string *user_
     }
   } else {
     remove(sudoers_filename.c_str());
+    if (opts.admin_policy_required) {
+      return false;
+    }
   }
 
   return true;
diff --git a/src/pam/pam_oslogin_admin.cc b/src/pam/pam_oslogin_admin.cc
new file mode 100644
index 0000000..06bb10b
--- /dev/null
+++ b/src/pam/pam_oslogin_admin.cc
@@ -0,0 +1,53 @@
+// Copyright 2024 Google Inc. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <security/pam_modules.h>
+
+#include <compat.h>
+#include <oslogin_utils.h>
+
+using std::string;
+
+using oslogin_utils::AuthOptions;
+
+extern "C" {
+
+// pm_sm_acct_mgmt is the account management PAM implementation for admin users (or users
+// with the proper loginAdmin policy). This account management module is intended for custom
+// configuration handling only, where users need a way to in their stack configurations to
+// differentiate a OS Login user. The Google Guest Agent will not manage the lifecycle of
+// this module, it will not add this to the stack as part of the standard/default configuration
+// set.
+PAM_EXTERN int
+pam_sm_acct_mgmt(pam_handle_t* pamh, int flags, int argc, const char** argv) {
+  struct AuthOptions opts;
+  const char *user_name;
+  string user_response;
+
+  if (pam_get_user(pamh, &user_name, NULL) != PAM_SUCCESS) {
+    PAM_SYSLOG(pamh, LOG_INFO, "Could not get pam user.");
+    return PAM_PERM_DENIED;
+  }
+
+  opts = { 0 };
+  opts.admin_policy_required = true;
+
+  if (!AuthorizeUser(user_name, opts, &user_response)) {
+    return PAM_PERM_DENIED;
+  }
+
+  return PAM_SUCCESS;
+}
+
+}
diff --git a/src/pam/pam_oslogin_login.cc b/src/pam/pam_oslogin_login.cc
index ca41ed3..5d863d2 100644
--- a/src/pam/pam_oslogin_login.cc
+++ b/src/pam/pam_oslogin_login.cc
@@ -22,6 +22,7 @@
 #include <compat.h>
 #include <oslogin_utils.h>
 
+using oslogin_utils::AuthOptions;
 using oslogin_utils::ContinueSession;
 using oslogin_utils::GetUser;
 using oslogin_utils::ParseJsonToChallenges;
@@ -32,6 +33,32 @@ using oslogin_utils::ValidateUserName;
 
 extern "C" {
 
+// pm_sm_acct_mgmt is the account management PAM implementation for non-admin users (or users
+// without the proper loginAdmin policy). This account management module is intended for custom
+// configuration handling only, where users need a way to in their stack configurations to
+// differentiate a OS Login user. The Google Guest Agent will not manage the lifecycle of
+// this module, it will not add this to the stack as part of the standard/default configuration
+// set.
+PAM_EXTERN int
+pam_sm_acct_mgmt(pam_handle_t* pamh, int flags, int argc, const char** argv) {
+  struct AuthOptions opts;
+  const char *user_name;
+  string user_response;
+
+  if (pam_get_user(pamh, &user_name, NULL) != PAM_SUCCESS) {
+    PAM_SYSLOG(pamh, LOG_INFO, "Could not get pam user.");
+    return PAM_PERM_DENIED;
+  }
+
+  opts = { 0 };
+
+  if (!AuthorizeUser(user_name, opts, &user_response)) {
+    return PAM_PERM_DENIED;
+  }
+
+  return PAM_SUCCESS;
+}
+
 PAM_EXTERN int
 pam_sm_setcred(pam_handle_t* pamh, int flags, int argc, const char** argv) {
   return PAM_SUCCESS;
diff --git a/test/oslogin_utils_test.cc b/test/oslogin_utils_test.cc
index 3451f61..0646a34 100644
--- a/test/oslogin_utils_test.cc
+++ b/test/oslogin_utils_test.cc
@@ -460,6 +460,13 @@ TEST(GetGroupByTest, GetGroupByGIDSucceeds) {
   ASSERT_EQ(errnop, 0);
 }
 
+TEST(CurlClient, RetryLogic) {
+  ASSERT_FALSE(ShouldRetry(200));
+  ASSERT_FALSE(ShouldRetry(404));
+  ASSERT_FALSE(ShouldRetry(400));
+  ASSERT_TRUE(ShouldRetry(429));
+}
+
 TEST(ParseJsonEmailTest, SuccessfullyParsesEmail) {
   string test_user =
       "{\"loginProfiles\":[{\"name\":\"[e-mail]\",\"posixAccounts\":["