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

[improve][client] Adding support for token supplier for the AuthenticationToken #395

Merged
merged 6 commits into from
Oct 20, 2024

Conversation

Bouk250
Copy link
Contributor

@Bouk250 Bouk250 commented Sep 17, 2024

Master Issue: #396

Motivation

The AuthenticationToken only supports static token, limiting us to use token without expiration.
So the integration with existing authentication returns tokens with expiration becomes not possible.

Modifications

Added the necessary cpp code for support token supplier as a js callback returns string tokens.

Modified file:

  • index.d.ts - Update constructor signature for support TS.
  • src/Authentication.cc - Implement the logic of communication between js callback and cpp token supplier.

New file

-src/TokenSupplier.h - Needed for the logic implementation.
-examples/consummer_token.js - Example

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Added tow new test in the e2e file to cover token supplier (sync/async)

Documentation

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    The changes is internally only, the feature is already documented.

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@Bouk250
Copy link
Contributor Author

Bouk250 commented Sep 19, 2024

@shibd Hi :),
I fixed the issue with the test, I forget to release the callback. Now it will work.

Copy link
Member

@shibd shibd left a comment

Choose a reason for hiding this comment

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

Great work! Left some small comments.

tstest.ts Outdated Show resolved Hide resolved
examples/consummer_token_auth.js Outdated Show resolved Hide resolved
@shibd shibd merged commit 530420d into apache:master Oct 20, 2024
12 checks passed
@shibd shibd added this to the 1.13.0 milestone Oct 20, 2024
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