Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CDRIVER-4689 add mongoc_oidc_callback_t #1945

Merged
merged 50 commits into from
Apr 3, 2025

Conversation

eramongodb
Copy link
Collaborator

@eramongodb eramongodb commented Mar 21, 2025

Implements the abstraction of an OIDC Callback as required for CDRIVER-4689.

This PR does not yet implement usage of the OIDC Callback by mongoc_client_t or mongo_client_pool_t as is documented by the proposed API doc examples (mongoc_client_set_oidc_callback and mongoc_client_pool_set_oidc_callback do not exist).


This PR implements an abstraction for the OIDC Callback feature:

Drivers MUST allow users to provide a callback that returns an OIDC access token. The purpose of the callback is to allow users to integrate with OIDC providers not supported by the Built-in Provider Integrations.

The spec requires the abstraction is capable of extending parameters and return values in a backward compatible manner:

The signature and naming of the callback API is up to the driver's discretion. Drivers MUST ensure that additional optional input parameters and return values can be added to the callback signature in the future without breaking backward compatibility.

Therefore, the parameters (mongoc_oidc_callback_params_t) and the return values (mongoc_oidc_credential_t) are opaque types. All "parameters" and "return values" must be accessed using functions to ensure the addition of new parameters and return types will not break API or ABI compatibility. Therefore, the callback function as implemented by a user must be declared as follows:

mongoc_oidc_credential_t *
example_oidc_callback_fn (mongoc_oidc_callback_params_t *params);

The callback function type is specified as mongoc_oidc_callback_fn_t.


The Authentication spec requires:

Drivers MUST provide a way for the callback to be either automatically canceled, or to cancel itself. This can be as a timeout argument to the callback, a cancellation context passed to the callback, or some other language-appropriate mechanism.

To distinguish between success, cancellation due to a timeout, and cancellation due to an error, the callback function is expected to return with one of three conditions, expressed as pseudocode (this would be implemented within the handshake algorithm):

cred = callback.fn (params) # mongoc_oidc_callback_fn_t

if cred:
    use(cred.access_token, cred.expires_in) # Success.
else if params.cancel_with_timeout:
    handle_timeout() # CSOT Timeout.
else:
    handle_error() # Authentication failure.

Although the current implementation proposes ignoring cancel_with_timeout when cancel_with_error is set (prioritize error over timeout), this can be changed into a Client API usage error if preferable. It would admittedly be unusual for both to ever be set at the same time.

Note

cancel_with_timeout is implemented as an out parameter rather than as return value to avoid requiring users to create and return a mongoc_oidc_credential_t object during cancellation. This should help users better distinguish success vs. cancellation.


The proposed API deliberately avoids allowing the user to specify any details regarding the error or timeout. Note the absence of a bson_error_t parameter or even support for a const char* error message string:

if (...) {
  // No arguments other than `params`.
  return mongoc_oidc_callback_params_cancel_with_timeout (params);
}

if (...) {
  // No arguments at all.
  return NULL;
}

This is to avoid locking the API into supporting bson_error_t codes, error messages, and/or any other details that are irrelevant to satisfying the minimal requirements of the OIDC Callback API. Instead, support for any additional details is completely deferred to the user_data in/out parameter, whose behavior may be defined entirely by the user as they require, as shown in the API example code:

typedef struct {
   const char *error_message;
} user_data_t;

mongoc_oidc_credential_t *
example_callback_fn (mongoc_oidc_callback_params_t *params)
{
   user_data_t *user_data = mongoc_oidc_callback_params_get_user_data (params);

   // This callback function was implemented for OIDC callback API version 1.
   if (mongoc_oidc_callback_params_get_version (params) > 1) {
      user_data->error_message = "OIDC callback API has changed: update example_callback_fn!";
      return NULL;
   }

   // ...
}

Therefore, the mongoc library will not need to handle propagation of user-provided error details. It can focus simply on implementing the MONGODB-OIDC authentication's mechanism algorithmic requirements only.

Note

The cancel_with_* functions always return NULL to support the return syntax above. This is primarily for convenience and to help avoid confusion with return value null vs. not-null requirements. However, users may choose to manually return NULL instead.


The timeout and expires_in parameter types are int64_t. This is to be consistent with bson_get_monotonic_time(), expiration_ms_to_timer(), mongoc_async_cmd_t::timeout_ms, timeout_msec parameters, etc. and to avoid signedness inconsistency due to uint64_t and uint32_t, avoid narrowing conversions due to int32_t.

The timeout parameter (when set) is meant to be directly compared against bson_get_monotonic_time() by the user to determine when to report a timeout, as shown in API example code.

The expires_in parameter is meant to be used after the callback function has (successfully) returned to compute the timestamp when the associated access token will expire (in the cache) and the OIDC callback function must be re-invoked. This will likely be computed as bson_get_monotonic_time() + to_microseconds (*expires_in) (internally represented as an mcd_time_point). Details are TBD and will depend on the implementation of Credential Caching and the overall MONGODB-OIDC authentication algorithm.

@eramongodb eramongodb requested a review from kevinAlbs March 21, 2025 20:42
@eramongodb eramongodb self-assigned this Mar 21, 2025
@kevinAlbs kevinAlbs requested a review from mdbmes March 24, 2025 12:07
@eramongodb eramongodb requested a review from kevinAlbs March 25, 2025 16:37
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

LGTM. I expect the newly added API will not be ready before the upcoming 2.0 release. I think keep this PR open until after the 2.0 release.

@eramongodb
Copy link
Collaborator Author

On further thought, given the removal of the cancel_with_error out parameter, decided to move the access_token and optional expires_in return values into the constructors for mongoc_oidc_credential_t. This is to ensure validity of the provided argument(s) (i.e. access_token must not be NULL) and avoid potential user confusion between a "null return value" (to indicate an error or timeout) vs. "a return value with a null access token" (forbidden).

@eramongodb eramongodb requested a review from mdbmes April 1, 2025 14:48
@eramongodb eramongodb requested a review from mdbmes April 1, 2025 21:19
MONGOC_EXPORT (mongoc_oidc_callback_fn_t)
mongoc_oidc_callback_get_fn (const mongoc_oidc_callback_t *callback);

MONGOC_EXPORT (void *)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be more convenient to return a mongoc_oidc_credential_t* instead of void* here? I think it would matter for users who call the C APIs from a C++ compiler. (They would need to either add a cast, or return their own NULL and ignore this one.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good call. I had not considered C++ code.

@eramongodb eramongodb requested a review from kevinAlbs April 1, 2025 21:34
@eramongodb
Copy link
Collaborator Author

eramongodb commented Apr 1, 2025

@kevinAlbs Re-requesting review due to significant changes, in summary:

  • Required (not-null) arguments are now enforced by constructors (*_new()) which return NULL for invalid arguments.
    • mongoc_oidc_callback_t::fn (removed set_fn()).
    • mongoc_oidc_credential_t::access_token (set_access_token() was already removed).
  • mongoc_oidc_credential_t::expires_in is no longer optional; instead, a value of 0 (infinite expiry duration) is the default. Reverted, see feedback.
  • Doc pages are restructured by "Functions" toctrees in each *_t type doc page. "Lifecycle" sections are also added.
  • The API version check example is adjusted to print a simple message rather than return an error to avoid implying a version bump should be treated as an error condition by the user.

@eramongodb eramongodb merged commit 00c42f8 into mongodb:master Apr 3, 2025
39 of 42 checks passed
@eramongodb eramongodb deleted the cdriver-4689-oidc-callback branch April 3, 2025 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants