Merge branch 'prerel' into enc/oidc-add-issuer

CHANGELOG.md

@@ -14,6 +14,20 @@
 
 ## oidc-agent 5.1.1
 
+### Change / Enhancement / Bugfix
+
+The previous release stated that:
+
+    When an account configuration is generated and the OP returns scopes in the initial token flow, the account
+    configuration is updated with those scopes.
+
+This did not work as intended. We made the following changes:
+
+- Fixed a bug, so that the agent now actually behaves as described.
+- Implemented separate scope lists for the initial token flow and the refreshing of tokens. Only the refresh-scope-list
+  is updated. This way access tokens can be obtained with the correct (updated) scope, but re-authentication flows can
+  still use the original scope list.
+
 ### Enhancements
 
 - `oidc-add` can now also take an issuer url to load the default account for this issuer, i.e. `oidc-add <issuer_url>`

src/account/account.c

@@ -135,15 +135,17 @@ struct oidc_account* getAccountFromJSON(const char* json) {
                  AGENT_KEY_CONFIG_ENDPOINT, AGENT_KEY_SHORTNAME,
                  OIDC_KEY_CLIENTID, OIDC_KEY_CLIENTSECRET, OIDC_KEY_USERNAME,
                  OIDC_KEY_PASSWORD, OIDC_KEY_REFRESHTOKEN, AGENT_KEY_CERTPATH,
-                 OIDC_KEY_REDIRECTURIS, OIDC_KEY_SCOPE,
-                 OIDC_KEY_DEVICE_AUTHORIZATION_ENDPOINT, OIDC_KEY_CLIENTNAME,
-                 AGENT_KEY_DAESETBYUSER, OIDC_KEY_AUDIENCE, AGENT_KEY_OAUTH,
-                 AGENT_KEY_USESPUBCLIENT, AGENT_KEY_MYTOKENPROFILE);
+                 OIDC_KEY_REDIRECTURIS, OIDC_KEY_SCOPE, AGENT_KEY_AUTHSCOPE,
+                 AGENT_KEY_REFRESHSCOPE, OIDC_KEY_DEVICE_AUTHORIZATION_ENDPOINT,
+                 OIDC_KEY_CLIENTNAME, AGENT_KEY_DAESETBYUSER, OIDC_KEY_AUDIENCE,
+                 AGENT_KEY_OAUTH, AGENT_KEY_USESPUBCLIENT,
+                 AGENT_KEY_MYTOKENPROFILE);
   GET_JSON_VALUES_RETURN_NULL_ONERROR(json);
   KEY_VALUE_VARS(issuer_url, issuer, mytoken_url, config_endpoint, shortname,
                  client_id, client_secret, username, password, refresh_token,
-                 cert_path, redirect_uris, scope, device_authorization_endpoint,
-                 clientname, daeSetByUser, audience, oauth, pub, profile);
+                 cert_path, redirect_uris, scope, auth_scope, refresh_scope,
+                 device_authorization_endpoint, clientname, daeSetByUser,
+                 audience, oauth, pub, profile);
   struct oidc_account* p   = secAlloc(sizeof(struct oidc_account));
   struct oidc_issuer*  iss = secAlloc(sizeof(struct oidc_issuer));
   if (_issuer_url) {
@@ -174,7 +176,21 @@ struct oidc_account* getAccountFromJSON(const char* json) {
   account_setPassword(p, _password);
   account_setRefreshToken(p, _refresh_token);
   account_setCertPath(p, _cert_path);
-  account_setScopeExact(p, _scope);
+  if (strValid(_auth_scope)) {
+    // set auth scope
+    account_setAuthScopeExact(p, _auth_scope);
+    if (strValid(_refresh_scope)) {
+      // set refresh scope
+      account_setRefreshScope(p, _refresh_scope);
+    } else {
+      secFree(_refresh_scope);
+    }
+    secFree(_scope);
+  } else {
+    secFree(_refresh_scope);
+    secFree(_auth_scope);
+    account_setAuthScopeExact(p, _scope);
+  }
   account_setAudience(p, _audience);
   account_setUsedMytokenProfile(p, _profile);
   list_t* redirect_uris = JSONArrayStringToList(_redirect_uris);
@@ -228,8 +244,10 @@ cJSON* _accountToJSON(const struct oidc_account* p, int useCredentials) {
       strValid(account_getRefreshToken(p)) ? account_getRefreshToken(p) : "",
       AGENT_KEY_CERTPATH, cJSON_String,
       strValid(account_getCertPath(p)) ? account_getCertPath(p) : "",
-      OIDC_KEY_SCOPE, cJSON_String,
-      strValid(account_getScope(p)) ? account_getScope(p) : "",
+      AGENT_KEY_AUTHSCOPE, cJSON_String,
+      strValid(account_getAuthScope(p)) ? account_getAuthScope(p) : "",
+      AGENT_KEY_REFRESHSCOPE, cJSON_String,
+      strValid(account_getRefreshScope(p)) ? account_getRefreshScope(p) : "",
       OIDC_KEY_AUDIENCE, cJSON_String,
       strValid(account_getAudience(p)) ? account_getAudience(p) : "",
       AGENT_KEY_OAUTH, cJSON_Number, account_getIsOAuth2(p),
@@ -287,7 +305,8 @@ void secFreeAccountContent(struct oidc_account* p) {
   account_setIssuer(p, NULL);
   account_setClientId(p, NULL);
   account_setClientSecret(p, NULL);
-  account_setScopeExact(p, NULL);
+  account_setAuthScopeExact(p, NULL);
+  account_setRefreshScope(p, NULL);
   account_setAudience(p, NULL);
   account_setUsername(p, NULL);
   account_setPassword(p, NULL);
@@ -337,7 +356,7 @@ int hasRedirectUris(const struct oidc_account* account) {
 }
 
 list_t* defineUsableScopeList(const struct oidc_account* account) {
-  char*   wanted_str = account_getScope(account);
+  char*   wanted_str = account_getAuthScope(account);
   list_t* wanted     = delimitedStringToList(wanted_str, ' ');
   if (wanted == NULL) {
     wanted = createList(1, NULL);

src/account/account.h

@@ -20,7 +20,8 @@ struct oidc_account {
   char*               clientname;
   char*               client_id;
   char*               client_secret;
-  char*               scope;
+  char*               refresh_scope;
+  char*               auth_scope;
   char*               audience;
   char*               used_mytoken_profile;
   char*               username;

src/account/setandget.c

@@ -76,8 +76,12 @@ char* account_getClientSecret(const struct oidc_account* p) {
   return p ? p->client_secret : NULL;
 }
 
-char* account_getScope(const struct oidc_account* p) {
-  return p ? p->scope : NULL;
+char* account_getAuthScope(const struct oidc_account* p) {
+  return p ? p->auth_scope : NULL;
+}
+
+char* account_getRefreshScope(const struct oidc_account* p) {
+  return p ? p->refresh_scope : NULL;
 }
 
 char* account_getAudience(const struct oidc_account* p) {
@@ -225,19 +229,27 @@ void account_setClientSecret(struct oidc_account* p, char* client_secret) {
   p->client_secret = client_secret;
 }
 
-void account_setScopeExact(struct oidc_account* p, char* scope) {
-  if (p->scope == scope) {
+void account_setRefreshScope(struct oidc_account* p, char* scope) {
+  if (p->refresh_scope == scope) {
+    return;
+  }
+  secFree(p->refresh_scope);
+  p->refresh_scope = scope;
+}
+
+void account_setAuthScopeExact(struct oidc_account* p, char* scope) {
+  if (p->auth_scope == scope) {
     return;
   }
-  secFree(p->scope);
-  p->scope = scope;
+  secFree(p->auth_scope);
+  p->auth_scope = scope;
 }
 
-void account_setScope(struct oidc_account* p, char* scope) {
-  account_setScopeExact(p, scope);
+void account_setAuthScope(struct oidc_account* p, char* scope) {
+  account_setAuthScopeExact(p, scope);
   if (strValid(scope)) {
     char* usable = defineUsableScopes(p);
-    account_setScopeExact(p, usable);
+    account_setAuthScopeExact(p, usable);
   }
 }
 
@@ -247,8 +259,8 @@ void account_setIssuer(struct oidc_account* p, struct oidc_issuer* issuer) {
   }
   secFreeIssuer(p->issuer);
   p->issuer = issuer;
-  if (issuer && strValid(account_getScope(p))) {
-    account_setScopeExact(p, defineUsableScopes(p));
+  if (issuer && strValid(account_getAuthScope(p))) {
+    account_setAuthScopeExact(p, defineUsableScopes(p));
   }
 }
 
@@ -262,7 +274,7 @@ void account_setScopesSupported(struct oidc_account* p,
   }
   issuer_setScopesSupported(p->issuer, scopes_supported);
   char* usable = defineUsableScopes(p);
-  account_setScopeExact(p, usable);
+  account_setAuthScopeExact(p, usable);
 }
 
 void account_setAudience(struct oidc_account* p, char* audience) {

src/account/setandget.h

@@ -20,7 +20,8 @@ char* account_getName(const struct oidc_account* p);
 char* account_getClientName(const struct oidc_account* p);
 char* account_getClientId(const struct oidc_account* p);
 char* account_getClientSecret(const struct oidc_account* p);
-char* account_getScope(const struct oidc_account* p);
+char* account_getRefreshScope(const struct oidc_account* p);
+char* account_getAuthScope(const struct oidc_account* p);
 char* account_getAudience(const struct oidc_account* p);
 char* account_getUsedMytokenProfile(const struct oidc_account* p);
 char* account_getUsername(const struct oidc_account* p);
@@ -50,8 +51,9 @@ void account_setName(struct oidc_account* p, char* shortname,
                      const char* client_identifier);
 void account_setClientId(struct oidc_account* p, char* client_id);
 void account_setClientSecret(struct oidc_account* p, char* client_secret);
-void account_setScopeExact(struct oidc_account* p, char* scope);
-void account_setScope(struct oidc_account* p, char* scope);
+void account_setAuthScopeExact(struct oidc_account* p, char* scope);
+void account_setAuthScope(struct oidc_account* p, char* scope);
+void account_setRefreshScope(struct oidc_account* p, char* scope);
 void account_setIssuer(struct oidc_account* p, struct oidc_issuer* issuer);
 void account_setScopesSupported(struct oidc_account* p, char* scopes_supported);
 void account_setAudience(struct oidc_account* p, char* audience);

src/defines/agent_values.h

@@ -2,6 +2,8 @@
 #define AGENT_MAGIC_VALUES_H
 
 #define AGENT_SCOPE_ALL "max"
+#define AGENT_KEY_AUTHSCOPE "auth_scope"
+#define AGENT_KEY_REFRESHSCOPE "refresh_scope"
 #define AGENT_KEY_ISSUERURL "issuer_url"
 #define AGENT_KEY_DAESETBYUSER "daeSetByUser"
 #define AGENT_KEY_CONFIG_ENDPOINT "config_endpoint"

src/oidc-agent/oidc/flows/code.c

@@ -109,9 +109,10 @@ char* buildCodeFlowUri(const struct oidc_account* account, char** state_ptr,
     secFree(*state_ptr);
     *state_ptr = tmp;
   }
-  char*   scope = only_at ? removeScope(oidc_strcopy(account_getScope(account)),
-                                        OIDC_SCOPE_OFFLINE_ACCESS)
-                          : oidc_strcopy(account_getScope(account));
+  char*   scope    = only_at
+                         ? removeScope(oidc_strcopy(account_getAuthScope(account)),
+                                       OIDC_SCOPE_OFFLINE_ACCESS)
+                         : oidc_strcopy(account_getAuthScope(account));
   list_t* postData = createList(
       LIST_CREATE_DONT_COPY_VALUES, OIDC_KEY_RESPONSETYPE,
       OIDC_RESPONSETYPE_CODE, OIDC_KEY_CLIENTID, account_getClientId(account),

src/oidc-agent/oidc/flows/device.c

@@ -13,7 +13,7 @@
 
 char* generateDeviceCodePostData(const struct oidc_account* a) {
   return generatePostData(OIDC_KEY_CLIENTID, account_getClientId(a),
-                          OIDC_KEY_SCOPE, account_getScope(a), NULL);
+                          OIDC_KEY_SCOPE, account_getAuthScope(a), NULL);
 }
 
 char* generateDeviceCodeLookupPostData(const struct oidc_account* a,

src/oidc-agent/oidc/flows/oidc.c

@@ -109,7 +109,9 @@ char* parseTokenResponseCallbacks(
     // if we get a scope value back from the OP when the initial AT is obtained,
     // we update the config, because it might be possible that the OP made
     // changes to the scopes.
-    account_setScopeExact(a, _scope);
+    // We use this updated scope value in future refresh request. For a
+    // reauthenticate we will use the initial authorization scopes.
+    account_setRefreshScope(a, _scope);
   } else {
     secFree(_scope);
   }

src/oidc-agent/oidc/flows/password.c

@@ -24,10 +24,10 @@ char* generatePasswordPostData(const struct oidc_account* a,
   list_rpush(postDataList, list_node_new(account_getUsername(a)));
   list_rpush(postDataList, list_node_new(OIDC_KEY_PASSWORD));
   list_rpush(postDataList, list_node_new(account_getPassword(a)));
-  if (scope || strValid(account_getScope(a))) {
+  if (scope || strValid(account_getAuthScope(a))) {
     list_rpush(postDataList, list_node_new(OIDC_KEY_SCOPE));
     list_rpush(postDataList,
-               list_node_new((char*)scope ?: account_getScope(a)));
+               list_node_new((char*)scope ?: account_getAuthScope(a)));
   }
   char* aud_tmp = NULL;
   if (strValid(account_getAudience(a))) {

src/oidc-agent/oidc/flows/refresh.c

@@ -13,13 +13,13 @@
 char* generateRefreshPostData(const struct oidc_account* a, const char* scope,
                               const char* audience) {
   char* refresh_token = account_getRefreshToken(a);
-  char* scope_tmp     = oidc_strcopy(
-      strValid(scope) ? scope
-                      : account_getScope(
-                            a));  // if scopes are explicitly set use these, if
-                                  // not we use the same as for the used refresh
-                                  // token. Usually this parameter can be
-                                  // omitted. For unity we have to include this.
+  if (NULL == scope) {
+    scope = strValid(account_getRefreshScope(a)) ? account_getRefreshScope(a)
+                                                 : account_getAuthScope(a);
+    // if scopes are explicitly set use these, if not we use the same as for the
+    // used refresh token. Usually this parameter can be omitted. For unity we
+    // have to include this.
+  }
   list_t* postDataList = list_new();
   // list_rpush(postDataList, list_node_new(OIDC_KEY_CLIENTID));
   // list_rpush(postDataList, list_node_new(account_getClientId(a)));
@@ -42,9 +42,9 @@ char* generateRefreshPostData(const struct oidc_account* a, const char* scope,
     }
   }
 
-  if (strValid(scope_tmp)) {
+  if (strValid(scope)) {
     list_rpush(postDataList, list_node_new(OIDC_KEY_SCOPE));
-    list_rpush(postDataList, list_node_new(scope_tmp));
+    list_rpush(postDataList, list_node_new((void*)scope));
   }
   char* aud_tmp = NULL;
   if (strValid(audience)) {
@@ -59,7 +59,6 @@ char* generateRefreshPostData(const struct oidc_account* a, const char* scope,
   }
   char* str = generatePostDataFromList(postDataList);
   list_destroy(postDataList);
-  secFree(scope_tmp);
   secFree(aud_tmp);
   return str;
 }

src/oidc-agent/oidc/flows/registration.c

@@ -42,7 +42,7 @@ char* getRegistrationPostData(const struct oidc_account* account, list_t* flows,
       OIDC_KEY_APPLICATIONTYPE, cJSON_String, application_type,
       OIDC_KEY_CLIENTNAME, cJSON_String, client_name, OIDC_KEY_RESPONSETYPES,
       cJSON_Array, response_types, OIDC_KEY_GRANTTYPES, cJSON_Array,
-      grant_types, OIDC_KEY_SCOPE, cJSON_String, account_getScope(account),
+      grant_types, OIDC_KEY_SCOPE, cJSON_String, account_getAuthScope(account),
       OIDC_KEY_REDIRECTURIS, cJSON_Array, redirect_uris_json, NULL);
   secFree(response_types);
   secFree(grant_types);

src/oidc-agent/oidcd/oidcd_handler.c

@@ -154,7 +154,7 @@ void _handleGenFlows(struct ipcPipe pipes, struct oidc_account* account,
     } else if (strcaseequal(current_flow->val, FLOW_VALUE_DEVICE) ||
                strcaseequal(current_flow->val, FLOW_VALUE_MT_OIDC)) {
       if (scope) {
-        account_setScopeExact(account, oidc_strcopy(scope));
+        account_setAuthScopeExact(account, oidc_strcopy(scope));
       }
       struct oidc_device_code* dc =
           strcaseequal(current_flow->val, FLOW_VALUE_MT_OIDC)
@@ -248,9 +248,10 @@ void oidcd_handleGen(struct ipcPipe pipes, const char* account_json,
   }
 
   const int only_at = strToInt(only_at_str);
-  char* scope = only_at ? removeScope(oidc_strcopy(account_getScope(account)),
-                                      OIDC_SCOPE_OFFLINE_ACCESS)
-                        : NULL;
+  char*     scope   = only_at
+                          ? removeScope(oidc_strcopy(account_getAuthScope(account)),
+                                        OIDC_SCOPE_OFFLINE_ACCESS)
+                          : NULL;
 
   _handleGenFlows(pipes, account, flow, scope, only_at, nowebserver_str,
                   noscheme_str, arguments);

src/oidc-agent/oidcp/oidcp.c

@@ -556,14 +556,14 @@ void handleAutoGen(struct ipcPipe pipes, int sock,
   }
   if (strequal(scopes, AGENT_SCOPE_ALL)) {
     if (account_getMytokenUrl(account)) {
-      account_setScopeExact(account, oidc_strcopy(scopes));
+      account_setAuthScopeExact(account, oidc_strcopy(scopes));
     } else {
-      account_setScope(account, usedUserClient
-                                    ? getScopesForUserClient(account)
-                                    : getScopesForPublicClient(account));
+      account_setAuthScope(account, usedUserClient
+                                        ? getScopesForUserClient(account)
+                                        : getScopesForPublicClient(account));
     }
   } else {
-    account_setScope(account, oidc_strcopy(scopes));
+    account_setAuthScope(account, oidc_strcopy(scopes));
   }
 
   agent_log(DEBUG, "Prompting user for confirmation for autogen for '%s'",

src/oidc-gen/gen_handler.c

@@ -779,7 +779,7 @@ struct oidc_account* registerClient(struct arguments* arguments) {
       secFreeJson(merged_json);
       exit(EXIT_FAILURE);
     }
-    const char* requested_scope = account_getScope(account);
+    const char* requested_scope = account_getAuthScope(account);
     if (strequal(requested_scope, "max") && _max_scopes) {
       requested_scope = _max_scopes;
     }