Skip to content

Conversation

@rustagir
Copy link
Contributor

@rustagir rustagir commented Jun 10, 2024

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-40133
Staging - https://preview-mongodbrustagir.gatsbyjs.io/kafka-connector/DOCSP-40133-custom-authprovider/security-and-authentication/custom-auth/

Self-Review Checklist

Comment on lines 105 to 107
The following code specifies the configuration properties to use the
``MONGODB-AWS`` authentication method and add a custom authentication
provider:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I don't think "code" is the right way to describe config variables.

Suggested change
The following code specifies the configuration properties to use the
``MONGODB-AWS`` authentication method and add a custom authentication
provider:
The following configuration properties specify the use of the
``MONGODB-AWS`` authentication method and add a custom authentication
provider:

Comment on lines 94 to 96
Depending on the design of your implementation class, you might also
set the ``mongodbaws.auth.mechanism.roleArn`` property, which
provides the Amazon Resource Name (ARN).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this be a part of the bulleted list above and marked as (optional)?

Comment on lines 116 to 117
The ``AwsAssumeRoleCredentialProvider`` class defines ``init()`` and
``validate()`` methods that are called when the connector initializes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It could be helpful for developers reading this to give a quick rundown of these methods and what devs would have to do to implement them – maybe a few comments above those methods in the code sample to give a brief overview of what they do?

@RWaltersMA
Copy link
Contributor

Its a custom authentication provider, not specific to AWS IAM. We are providing a sample to do AWS IAM Auth though, the way the docs are written sounds like it is just AWS IAM support.

@rustagir rustagir requested review from RWaltersMA and mcmorisi June 13, 2024 15:10
Copy link
Collaborator

@mcmorisi mcmorisi left a comment

Choose a reason for hiding this comment

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

LGTM. Really great job on this!

Atlas Group IAM Role.
access to MongoDB Atlas. In the AWS IAM console, the IAM account that is
running {+kafka-connect+} has ``AssumeRole`` permissions to this Atlas
Group IAM Role.
Copy link
Contributor

Choose a reason for hiding this comment

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

Atlas User Group

"mongodbaws.auth.mechanism.roleArn":"arn:aws:iam::99999999:role/KafkaAtlasRole"
"mongo.custom.auth.mechanism.enable": "true",
"mongo.custom.auth.mechanism.providerClass": "com.mongodb.SampleAssumeRoleCredential",
"mongodbaws.auth.mechanism.roleArn": "arn:aws:iam::99999999:role/KafkaAtlasRole"
Copy link
Contributor

Choose a reason for hiding this comment

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

replace arn:aws:iam with

Copy link
Contributor

Choose a reason for hiding this comment

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

aws iam ARN something like that

@rustagir rustagir requested a review from RWaltersMA June 24, 2024 18:24
@RWaltersMA
Copy link
Contributor

LGTM

@rustagir rustagir merged commit e21986b into mongodb:master Jun 24, 2024
rustagir added a commit that referenced this pull request Jun 24, 2024
* DOCSP-40133: custom auth provider

* wip

* vale

* fixes

* updates

* add to wn

* wip RW tech comments

* RW comments

* fix

* fix

* RW comments

(cherry picked from commit e21986b)
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