Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/responder/pam/pamsrv_json.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ obtain_oauth2_data(TALLOC_CTX *mem_ctx, struct pam_data *pd,

_auth_data->oauth2->uri = talloc_steal(mem_ctx, uri);
_auth_data->oauth2->code = talloc_steal(mem_ctx, code);
_auth_data->oauth2->complete_uri = talloc_steal(mem_ctx, uri_complete);
ret = EOK;

done:
Expand Down Expand Up @@ -759,11 +760,12 @@ json_format_mechanisms(struct auth_data *auth_data, json_t **_list_mech)
}

if (auth_data->oauth2->enabled) {
json_oauth2 = json_pack("{s:s,s:s,s:s,s:s,s:s,s:s,s:i}",
json_oauth2 = json_pack("{s:s,s:s,s:s,s:s,s:s,s:s,s:s,s:i}",
"name", "Web Login",
"role", "eidp",
"initPrompt", auth_data->oauth2->init_prompt,
"linkPrompt", auth_data->oauth2->link_prompt,
"completeUri", auth_data->oauth2->complete_uri,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The pull request description mentions that completeUri should ideally be used "if non-empty". To better align with this, consider omitting the completeUri field from the JSON object when its value is NULL or an empty string. This would simplify parsing on the consumer side, as it would only need to check for the key's existence.

This would require refactoring the json_pack() call to build the object by adding fields conditionally, for example:

json_oauth2 = json_pack("{s:s,s:s,s:s,s:s,s:s,s:i}",
                        "name", "Web Login",
                        "role", "eidp",
                        "initPrompt", auth_data->oauth2->init_prompt,
                        "linkPrompt", auth_data->oauth2->link_prompt,
                        "uri", auth_data->oauth2->uri,
                        "code", auth_data->oauth2->code,
                        "timeout", 300);
if (json_oauth2 == NULL) { ... }

if (auth_data->oauth2->complete_uri && *auth_data->oauth2->complete_uri) {
    json_object_set_new(json_oauth2, "completeUri",
                      json_string(auth_data->oauth2->complete_uri));
    // ... error handling ...
}

"uri", auth_data->oauth2->uri,
"code", auth_data->oauth2->code,
"timeout", 300);
Expand Down
1 change: 1 addition & 0 deletions src/responder/pam/pamsrv_json.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ struct oauth2_data {
char *code;
char *init_prompt;
char *link_prompt;
char *complete_uri;
};

struct sc_data {
Expand Down
7 changes: 6 additions & 1 deletion src/tests/cmocka/test_pamsrv_json.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
#include "src/responder/pam/pamsrv_json.h"

#define OAUTH2_URI "short.url.com/tmp\0"
#define OAUTH2_URI_COMP "\0"
#define OAUTH2_URI_COMP "short.url.com/tmp?code=1234-5678\0"
#define OAUTH2_CODE "1234-5678"
#define OAUTH2_STR OAUTH2_URI OAUTH2_URI_COMP OAUTH2_CODE
#define PASSKEY_CRYPTO_CHAL "6uDMvRKj3W5xJV3HaQjZrtXMNmUUAjRGklFG2MIhN5s="
Expand Down Expand Up @@ -62,6 +62,7 @@
"\"name\": \"Web Login\", \"role\": \"eidp\", " \
"\"initPrompt\": \"" OAUTH2_INIT_PROMPT "\", " \
"\"linkPrompt\": \"" OAUTH2_LINK_PROMPT "\", " \
"\"completeUri\": \"short.url.com/tmp?code=1234-5678\", " \
"\"uri\": \"short.url.com/tmp\", \"code\": \"1234-5678\", " \
"\"timeout\": 300}"
#define BASIC_SC "\"smartcard\": {" \
Expand Down Expand Up @@ -353,6 +354,7 @@ void test_json_format_mechanisms_oauth2(void **state)
auth_data->oauth2->code = discard_const(OAUTH2_CODE);
auth_data->oauth2->init_prompt = discard_const(OAUTH2_INIT_PROMPT);
auth_data->oauth2->link_prompt = discard_const(OAUTH2_LINK_PROMPT);
auth_data->oauth2->complete_uri = discard_const(OAUTH2_URI_COMP);

ret = json_format_mechanisms(auth_data, &mechs);
assert_int_equal(ret, EOK);
Expand Down Expand Up @@ -547,6 +549,7 @@ void test_json_format_auth_selection_oauth2(void **state)
auth_data->oauth2->code = discard_const(OAUTH2_CODE);
auth_data->oauth2->init_prompt = discard_const(OAUTH2_INIT_PROMPT);
auth_data->oauth2->link_prompt = discard_const(OAUTH2_LINK_PROMPT);
auth_data->oauth2->complete_uri = discard_const(OAUTH2_URI_COMP);

will_return(__wrap_json_array_append_new, false);
ret = json_format_auth_selection(test_ctx, auth_data, &json_msg);
Expand Down Expand Up @@ -652,6 +655,7 @@ void test_json_format_auth_selection_all(void **state)
auth_data->oauth2->code = discard_const(OAUTH2_CODE);
auth_data->oauth2->init_prompt = discard_const(OAUTH2_INIT_PROMPT);
auth_data->oauth2->link_prompt = discard_const(OAUTH2_LINK_PROMPT);
auth_data->oauth2->complete_uri = discard_const(OAUTH2_URI_COMP);
auth_data->sc->enabled = true;
auth_data->sc->names[0] = talloc_strdup(auth_data->sc->names, SC1_TOKEN_NAME);
assert_non_null(auth_data->sc->names[0]);
Expand Down Expand Up @@ -719,6 +723,7 @@ void test_json_format_auth_selection_failure(void **state)
auth_data->oauth2->code = discard_const(OAUTH2_CODE);
auth_data->oauth2->init_prompt = discard_const(OAUTH2_INIT_PROMPT);
auth_data->oauth2->link_prompt = discard_const(OAUTH2_LINK_PROMPT);
auth_data->oauth2->complete_uri = discard_const(OAUTH2_URI_COMP);
auth_data->sc->enabled = true;
auth_data->sc->names[0] = talloc_strdup(auth_data->sc->names, SC1_TOKEN_NAME);
assert_non_null(auth_data->sc->names[0]);
Expand Down
Loading