Skip to content

Fix OIDC reauthentication when a session is involved #1719

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stIncMale
Copy link
Member

@stIncMale stIncMale commented May 27, 2025

This PR is waiting for mongodb/specifications#1805.

JAVA-5880

@stIncMale stIncMale requested a review from katcharov May 27, 2025 21:30
@stIncMale stIncMale self-assigned this May 27, 2025
@stIncMale stIncMale marked this pull request as draft May 27, 2025 21:30
Comment on lines 105 to 114
public void reauthenticate(final InternalConnection connection, final OperationContext operationContext) {
authenticate(connection, connection.getDescription(), operationContext);
authenticate(connection, connection.getDescription(), operationContext.withSessionContext(NoOpSessionContext.INSTANCE));
}

public void reauthenticateAsync(final InternalConnection connection, final OperationContext operationContext,
final SingleResultCallback<Void> callback) {
beginAsync().thenRun((c) -> {
authenticateAsync(connection, connection.getDescription(), operationContext, c);
authenticateAsync(connection, connection.getDescription(), operationContext.withSessionContext(NoOpSessionContext.INSTANCE), c);
}).finish(callback);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

The OperationContext for the first authentication when we establish a connection comes from MongoClusterImpl.OperationExecutorImpl.getOperationContext, and does not have a SessionContext.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The one difference is the use of ReadConcernAwareNoOpSessionContext instead of NoOpSessionContext. I doubt it matters, but I don't remember why that class was needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Started using ReadConcernAwareNoOpSessionContext in df84c84 just in case.

@stIncMale stIncMale marked this pull request as ready for review May 28, 2025 16:02
@stIncMale stIncMale requested a review from jyemin May 28, 2025 16:03
Copy link
Collaborator

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

LGTM, and approved for backport as well.

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.

2 participants