Skip to content

Commit

Permalink
use separate scope lists for auth- and refresh- scopes; #568 #553
Browse files Browse the repository at this point in the history
  • Loading branch information
zachmann committed Feb 13, 2024
1 parent e46f924 commit 3528528
Show file tree
Hide file tree
Showing 15 changed files with 106 additions and 66 deletions.
41 changes: 30 additions & 11 deletions src/account/account.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/account/account.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
36 changes: 24 additions & 12 deletions src/account/setandget.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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));
}
}

Expand All @@ -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) {
Expand Down
8 changes: 5 additions & 3 deletions src/account/setandget.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions src/defines/agent_values.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 4 additions & 3 deletions src/oidc-agent/oidc/flows/code.c
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion src/oidc-agent/oidc/flows/device.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion src/oidc-agent/oidc/flows/oidc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions src/oidc-agent/oidc/flows/password.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))) {
Expand Down
19 changes: 9 additions & 10 deletions src/oidc-agent/oidc/flows/refresh.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand All @@ -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)) {
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/oidc-agent/oidc/flows/registration.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 5 additions & 4 deletions src/oidc-agent/oidcd/oidcd_handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions src/oidc-agent/oidcp/oidcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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'",
Expand Down
2 changes: 1 addition & 1 deletion src/oidc-gen/gen_handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
23 changes: 12 additions & 11 deletions src/oidc-gen/promptAndSet/scope.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,38 +40,39 @@ void askOrNeedScope(struct oidc_account* account,
void _askOrNeedScope(char* supportedScope, struct oidc_account* account,
const struct arguments* arguments, int optional) {
if (readScope(account, arguments)) {
if (strequal(account_getScope(account), AGENT_SCOPE_ALL)) {
account_setScope(account, supportedScope);
if (strequal(account_getAuthScope(account), AGENT_SCOPE_ALL)) {
account_setAuthScope(account, supportedScope);
}
return;
}
ERROR_IF_NO_PROMPT(optional, ERROR_MESSAGE("scope", OPT_LONG_SCOPE));
printNormal("The following scopes are supported: %s\n", supportedScope);
if (!strValid(account_getScope(account))) {
account_setScope(account, oidc_strcopy(DEFAULT_SCOPE));
if (!strValid(account_getAuthScope(account))) {
account_setAuthScope(account, oidc_strcopy(DEFAULT_SCOPE));
}
char* res = _gen_promptMultipleSpaceSeparated(
"Scopes or '" AGENT_SCOPE_ALL "'", account_getScope(account), optional);
"Scopes or '" AGENT_SCOPE_ALL "'", account_getAuthScope(account),
optional);
if (res) {
account_setScopeExact(account, res);
account_setAuthScopeExact(account, res);
}
if (strequal(account_getScope(account), AGENT_SCOPE_ALL)) {
account_setScope(account, supportedScope);
if (strequal(account_getAuthScope(account), AGENT_SCOPE_ALL)) {
account_setAuthScope(account, supportedScope);
} else {
secFree(supportedScope);
}
}

int readScope(struct oidc_account* account, const struct arguments* arguments) {
if (arguments->scope) {
void (*setter)(struct oidc_account*, char*) = account_setScope;
void (*setter)(struct oidc_account*, char*) = account_setAuthScope;
if (strequal(arguments->scope, AGENT_SCOPE_ALL)) {
setter = account_setScopeExact;
setter = account_setAuthScopeExact;
}
setter(account, oidc_strcopy(arguments->scope));
return 1;
}
if (prompt_mode() == 0 && strValid(account_getScope(account))) {
if (prompt_mode() == 0 && strValid(account_getAuthScope(account))) {
return 1;
}
return 0;
Expand Down

0 comments on commit 3528528

Please sign in to comment.