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

Support extended tokens + expose token feature for library use #15

Merged
merged 6 commits into from
Nov 5, 2024

Conversation

beesaferoot
Copy link
Collaborator

@beesaferoot beesaferoot commented Aug 25, 2024

I updated some sample tokens as they did not match the expected tokens which I verified using the original python library.

@dmohns You can confirm if the changes are accurate. Closes #10

@beesaferoot beesaferoot requested a review from dmohns August 25, 2024 21:35
@dmohns
Copy link
Member

dmohns commented Nov 4, 2024

Hey @beesaferoot

following up on my earlier comments. I think the issue was that we never actually had Extended Tokens in the test JSON 😅 I generated some using the legacy Library.

Could you add the JSON as per this PR: #34 to your PR? (Probably you can just cherry pick my commit) And see if this resolves this issue?

test/encoder.test.js Outdated Show resolved Hide resolved
@beesaferoot
Copy link
Collaborator Author

Hello @dmohns I got around this and updated the sample tokens from your PR.

Everything seems to be working as expected now (for standard and extended token generation).

@dmohns
Copy link
Member

dmohns commented Nov 5, 2024

Hey @beesaferoot

Thank you for your patience with this one 🫣 I got some more information about the issue I mentioned earlier.

The Test Tokens I generated originally, where created using the Legacy Python library. It looks like the library is actually not generating extended Tokens correctly, see EnAccess/OpenPAYGO-python#21

Let's merge your PR as it is :shipit:

@dmohns dmohns merged commit 32c2387 into main Nov 5, 2024
8 checks passed
@dmohns dmohns deleted the js-port branch November 5, 2024 16:02
@beesaferoot
Copy link
Collaborator Author

Using #35 We can cross check the implementation, will look into that next.

Thanks for merging 👍🏾

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.

Create usage documentation guide
2 participants