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

Updating client libraries to add auth token authentication support #3219

Open
wants to merge 21 commits into
base: dev
Choose a base branch
from

Conversation

silva-fj
Copy link
Contributor

@silva-fj silva-fj commented Dec 24, 2024

Updated packages:

parachain-api: https://www.npmjs.com/package/@litentry/parachain-api/v/0.9.21-next.3
client-sdk: https://www.npmjs.com/package/@litentry/client-sdk/v/1.0.0-next.10

Also the client-sdk workspace have been fixed. (The build process was broken due to a mismatch in the nx dependencies version`

Copy link

linear bot commented Dec 24, 2024

@0xverin
Copy link
Contributor

0xverin commented Dec 30, 2024

Still in WIP?Or did you forget to change the state? Just to confirm if a review is needed

@silva-fj
Copy link
Contributor Author

Still in WIP?Or did you forget to change the state? Just to confirm if a review is needed

Yep, this is still WIP. I'm currently working on the client-sdk part. Thank you for checking 👍🏼

@silva-fj silva-fj marked this pull request as ready for review December 30, 2024 11:50
@silva-fj silva-fj requested review from a team and 0xverin December 30, 2024 11:50
@silva-fj
Copy link
Contributor Author

I've realised that more changes are needed.

@silva-fj silva-fj marked this pull request as draft December 30, 2024 16:47
@silva-fj silva-fj marked this pull request as ready for review December 31, 2024 10:24
@Kailai-Wang
Copy link
Collaborator

@0xverin could you please take a look?

Copy link
Contributor

@0xverin 0xverin left a comment

Choose a reason for hiding this comment

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

Overall looks fine, but are all the links in the README incorrect? All go to 404 error page.

@@ -21,9 +25,11 @@ export function createTCAuthenticationType(
[data.type]:
data.type === 'Email'
? data.verificationCode
: createLitentryMultiSignature(registry, {
: data.type === 'Web3'
Copy link
Contributor

Choose a reason for hiding this comment

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

How about changing it to map or switch case? It might be more intuitive. I think there might be additional types added here in the future.

for example:

const getValue = () => {
  switch (data.type) {
    case 'Email':
      return data.verificationCode;
    case 'Web3':
      return createLitentryMultiSignature(registry, {
        who: data.signer,
        signature: data.signature,
      });
    default:
      return data.token;
  }
};

[data.type]: getValue()

@@ -104,13 +107,13 @@ is the signed payload.

#### Defined in

[lib/requests/call-ethereum.request.ts:31](https://github.com/litentry/client-sdk/blob/develop/lib/requests/call-ethereum.request.ts#L31)
[lib/requests/call-ethereum.request.ts:32](https://github.com/litentry/client-sdk/blob/develop/lib/requests/call-ethereum.request.ts#L32)
Copy link
Contributor

Choose a reason for hiding this comment

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

404 not found?

@@ -5,7 +5,7 @@
"main": "dist/src/index.js",
"module": "dist/src/index.js",
"sideEffects": false,
"version": "0.9.21-next.0",
"version": "0.9.21-next.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the update content in the changelog file.Thank you.

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