Skip to content

Add ability to authenticate with Amster credentials#526

Merged
vscheuber merged 2 commits intorockcarver:mainfrom
trivir:feature/amster-authentication
Jan 28, 2026
Merged

Add ability to authenticate with Amster credentials#526
vscheuber merged 2 commits intorockcarver:mainfrom
trivir:feature/amster-authentication

Conversation

@phalestrivir
Copy link
Copy Markdown
Contributor

@phalestrivir phalestrivir commented Jan 28, 2026

This PR adds the ability to authenticate to an AM classic deployment using Amster credentials (i.e. a public/private key pair). The private key can be in a variety of formats such as PKCS, JWK, and OpenSSH, but is ultimately stored in PKCS#8 format. You can also use encrypted private keys by providing the passphrase when creating the connection profile.

I also refactored AuthenticateOps a bit since some of the code for doing authentication is re-used since it involves getting and writing callbacks in a journey similar to how the authentication is done with username/password in AIC deployments.

Another PR in the CLI makes the corresponding changes there to the frodo conn add/save command.

@vscheuber vscheuber merged commit 2157635 into rockcarver:main Jan 28, 2026
8 checks passed
@vscheuber
Copy link
Copy Markdown
Contributor

@phalestrivir I believe there are pre-requisites for your new classic tests in AuthenticateOps.test.js that are not available in the rockcarver org and frodo-lib repo. So the tests passed before I merged the PR but the tests failed after merging:

FAIL src/ops/AuthenticateOps.test.ts
  ● AuthenticateOps › Classic Tests › getTokens() › 0: Authenticate successfully as user
    FrodoError: Error getting tokens
      1316 |     }
      1317 |   } catch (error) {
    > 1318 |     throw new FrodoError(`Error getting tokens`, error);
           |           ^
      1319 |   }
      1320 | }
      1321 |
      at Module.getTokens (src/ops/AuthenticateOps.ts:1318:11)
      at Object.<anonymous> (src/ops/AuthenticateOps.test.ts:119:26)
  ● AuthenticateOps › Classic Tests › getTokens() › 1: Authenticate successfully using Amster credentials
    FrodoError: Error getting tokens
      1316 |     }
      1317 |   } catch (error) {
    > 1318 |     throw new FrodoError(`Error getting tokens`, error);
           |           ^
      1319 |   }
      1320 | }
      1321 |
      at Module.getTokens (src/ops/AuthenticateOps.ts:1318:11)
      at Object.<anonymous> (src/ops/AuthenticateOps.test.ts:144:26)
  ● AuthenticateOps › Classic Tests › getTokens() › 2: Authenticate successfully using alternative Amster subject and service
    FrodoError: Error getting tokens
      1316 |     }
      1317 |   } catch (error) {
    > 1318 |     throw new FrodoError(`Error getting tokens`, error);
           |           ^
      1319 |   }
      1320 | }
      1321 |
      at Module.getTokens (src/ops/AuthenticateOps.ts:1318:11)
      at Object.<anonymous> (src/ops/AuthenticateOps.test.ts:176:26)

The tests that run before merge run in the source repo, while of course after the merge they run in the rockcarver/frodo-lib repo. Could you maybe look into that and send me instructions on what to setup so the tests pass?

@phalestrivir
Copy link
Copy Markdown
Contributor Author

phalestrivir commented Jan 28, 2026

@vscheuber I looked at the error when I try to run it locally, the error I got was on a line that was running import sshpk from 'sshpk'. This dependency is included in the package.json. I had to run npm i to get the test to pass. I noticed this updates the package-lock.json, which I didn't commit those changes in the PR since I didn't think they would be needed, although I think they are needed to ensure that the tests have the dependencies they need to run (although I'm not sure how the tests were passing in the PR without those package-lock changes, usually they fail if they package-lock needs to be updated in my experience).

Would you like me to create another PR with those package-lock.json changes or do you want to do that?

Edit: Nevermind, I double checked and the package-lock does not get updated from doing npm i, so I think you just need to run npm i locally to get the test passing.

Edit 2: I noticed the actions are already using npm ci, so I don't think the issue is dependency related. I'll look into it in some more detail and see if I can find what the real error is.

@vscheuber
Copy link
Copy Markdown
Contributor

@phalestrivir it's really odd: the tests pass for me on MacOS running node 24, which is the nodes version failing on GitHub. Granted, we don't know if other node versions would fail, too, because I configured it to fail fast. But I am really flabbergasted by this.

@phalestrivir
Copy link
Copy Markdown
Contributor Author

phalestrivir commented Jan 29, 2026

@vscheuber I tried a few different things, such as running the tests in different linux VM's that I have (just to make sure the environment wasn't affecting it) and also creating/merging a PR in our TriVir repo to see if the GitHub actions would fail there, but so far I haven't been able to reproduce the errors. I've already talked with @hfranklin about it and he's looking into it as well, so hopefully we'll be able to find out what's causing it here sometime soon.

@phalestrivir phalestrivir deleted the feature/amster-authentication branch February 4, 2026 16:32
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