Skip to content

Conversation

@phalestrivir
Copy link
Collaborator

Add's ability to authenticate with Frodo using Amster private key credentials.

@phalestrivir phalestrivir requested a review from brain-hol July 30, 2025 23:55
@phalestrivir phalestrivir force-pushed the feature/amster-authentication branch from ec986d7 to ce1ba69 Compare July 31, 2025 15:07

if (
state.getCookieValue() &&
// !state.getBearerToken() &&

Choose a reason for hiding this comment

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

Did you mean to leave this commented out?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it was commented out previously so I left it in, although I could go ahead and delete it since it's not being used. The only reason it says that I added it in is because I did some refactoring and moved some code around

);
});

test(`frodo.createInstanceWithAmsterAccount(): FrodoLib is instantiable using factory helper`, async () => {

Choose a reason for hiding this comment

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

I'm confused by this test. Why are we testing both PKCS#1 and PKCS#8 in the same test and with the assertions interleaved? If we are testing more than one private key format, should we be testing all of the formats like in CryptoUtils.test.ts?

Copy link
Collaborator Author

@phalestrivir phalestrivir Sep 19, 2025

Choose a reason for hiding this comment

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

The reason I tested both was because I needed a case where I create a connection profile using the default Amster journey and one using a custom Amster journey. Plus, if you look at the other tests, each of them create two instances per test as well, so that's why I didn't separate them into separate tests. I tested with different private keys because why not? I guess I could've used the same private key for both of them, I could change it to be like that if you like? We don't need to test all the formats I don't think because we are already testing all the formats in CryptoUtils.test.ts, this test is just testing creating a new connection profile instance.

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.

4 participants