Added on-prem IDM Deployment type #21

skootrivir · 2025-05-07T20:03:10Z

Added code to support on prem deployment type.
IDM will authenticate REST API calls using a username and password.

src/ops/AuthenticateOps-backup.ts

src/api/BaseApi.ts

src/api/AuthenticateApi.ts

src/ops/AuthenticateOps.ts

src/ops/ConfigOps.ts

src/ops/AuthenticateOps.ts

src/ops/ConfigOps.ts

src/ops/AuthenticateOps.ts

phalestrivir

Looks good

phalestrivir

One more change. I discovered a bug in the ConfigExport Ops where it will try to export realms or script types if the includeReadOnly flag is true. You'll want to update that code with the following:

realm: (
  await exportWithErrorHandling(
    exportRealms,
    stateObj,
    errors,
    (includeReadOnly && isPlatformDeployment) || isClassicDeployment
  )
)?.realm,
scripttype: (
  await exportWithErrorHandling(
    exportScriptTypes,
    stateObj,
    errors,
    (includeReadOnly && isPlatformDeployment) || isClassicDeployment
  )
)?.scripttype,

hfranklin

@skootrivir What would be the LOE to add unit tests for these changes?

hfranklin · 2025-09-17T01:34:19Z

src/api/AuthenticateApi.ts

+ * @param {any} config request config
+ * @returns Promise resolving to the authentication service response
+ */
+export async function stepIdm({


Generally I prefer consistency, but in this case I don't think the function name stepIdm makes sense. I think something like loginIdm or authenticateIdm would make more sense since IDM authentication doesn't have multiple steps like AM. Did I miss something here?

I believe the reason he named it that is because there is a step function that does the usual AM authentication calls, so he called this one stepIdm since it is for IDM authentication which is why I didn't think too much of it. However, I agree that stepIdm is not the best name for it, since the reason the AM one is called step is because it is stepping through a journey to authenticate the Admin user when we create connection profiles. For IDM, we don't have steps to authenticate, it's just part of any request that is made to IDM to include the username/password to authenticate, so this would be a good change for you to make @skootrivir, assuming we can't find a better alternative to determine if the deployment is an IDM deployment as mentioned in the next comment.

I will re-name this to 'authenticateIdm'

hfranklin · 2025-09-17T01:38:06Z

src/ops/AuthenticateOps.ts

-                state,
-              });
+              try {
+                const idmresponse = await stepIdm({


If you are just trying to determine if the server is a stand alone IDM deployment, there should be a better way to do this than attempting an authentication.

@hfranklin There probably is another way to do it, but I'm thinking that this might be the best way to do it, the reason being that typically when you are creating a new connection profile you want it to validate the credentials you are providing so that you aren't saving invalid credentials. This function should only be called when creating a connection profile when it tries to determine the deployment type for the first time (unless we are attempting to use Frodo without an existing connection profile, which is possible, you would just need to provide the username/password for every command), and when it is called using the authenticate endpoint like we're doing it helps validate that the credentials work against the deployment. For AM, Cloud, and ForgeOps deployments they do this by exchanging the username/password for a cookie or by exchanging the service account id/jwk for a token sometime after determining the deployment type. However, since we can't do this for IDM deployments, we would need to authenticate at some point, so we do it in this function when it is trying to determine deployment type.

Another way we could do it is instead have some other way to determine it's an IDM deployment (such as attempting to hit the serverinfo endpoint, which will only work for AM, Cloud, and ForgeOps), but we would still have to call this function later anyways just to validate the connection, so I think using this authenticate function hits two birds with one stone by determining the deployment type while also validating that the credentials are valid.

As a side note, creating a connection profile, you can include the --no-validate to not validate credentials when creating the connection profile, but if you do this you'll notice it doesn't save the deployment type to the connection profile because it never validated that the credentials you gave it actually work.

However, I do think the way this is currently implemented (referring to how Frodo determines deployment type and validates credentials) is very confusing and not obvious. I think that possibly refactoring and cleaning it all up like we were talking about today would be very good. I did do some refactoring changes to this file already when I was working on this PR for the Amster authentication changes, but I didn't modify or refactor the code that determines the deployment type, only the code that validates the credentials, so I think waiting until this PR and that one are merged in might be a good idea before we try to refactor this file.

My main complaint is it isn't obvious what the intent of the code is from the way it is structured right now. I'm fine refactoring it after merging this and PR #33

hfranklin · 2025-09-17T01:39:48Z

src/ops/AuthenticateOps.ts

+                    state,
+                  });
+                }
+              } catch {


It seems like this may not be a good assumption that all exceptions indicate that this is not a stand alone IDM deployment

This is true, I think we should be at least checking for the 401 case (if the credentials are invalid), that's the only case I can think of that this would throw which wouldn't result in the deployment being something other than an IDM deployment. @skootrivir Would you be able to add this check in the catch statement?

hfranklin · 2025-09-17T01:41:38Z

src/ops/AuthenticateOps.ts

      otpCallbackHandler: otpCallback,
      state,
    });
+    if (!token) return token;


This logic is either incorrect or it is unclear what condition you are handling. If the token is falsey, why are you returning it?

The reason for the if statement is that it is possible for the getFreshUserSessionToken to return null, and I believe it does return null if it's an IDM deployment (since the method is intended only for deployments with AM where it can receive a token), and so we are returning null to indicate that it wasn't able to get any tokens. However, it's been a while since I looked at it, so it could very well be unnecessary. @skootrivir Would you be able to confirm this is necessary?

I get that getFreshUserSessionToken could return null. My point is it is confusing to return token if token is falsey. It would be better to return null, undefined, false, or nothing depending on what the API contract is with the calling functions.

I see what you mean. I think we just return null then in this case if the token is falsey since that's one of the possible expected return values for that function. @skootrivir, would you be able to change this line return null instead?

src/ops/ConfigOps.ts

hfranklin · 2025-09-17T15:53:51Z

src/ops/AuthenticateOps.ts

+    cookieName = data.cookieName;
+  } catch (e) {
+    if (
+      e.response?.status !== 401 ||


In what scenarios would we get a 401 or Access Denied?

Good catch, I don't think we should get a 401 here since the endpoint doesn't require any credentials, so we can remove that. We do need to keep the try-catch though since there is no server info endpoint for IDM, and we don't want the program to fail because of it. @skootrivir Could you remove this if check?

Don't we already know the deployment type by the time this call is being made? If so, it seems like it would be better to check the state to see if this is an IDM deployment.

If it's the first time the connection is created, we don't know the deployment type, so this is necessary. I remember we tried to find a way to re-order things so that we try and get the cookie name after determining the deployment type, but it's not possible because we need the cookie name for determining the deployment type (at least for AM, Cloud, and ForgeOps deployments) since it is used in the requests that it makes.

Changed stepIdm to authenticateIdm other fixes based on the PR review.

hfranklin · 2025-11-03T20:10:27Z

src/api/AuthenticateApi.test.ts

+      state.setPassword(process.env.FRODO_PASSWORD || 'openidm-admin');
+      const config = {
+        headers: {
+          'X-OpenAM-Username': state.getUsername(),


Why are the AM authentication hearders being included?

skootrivir added 2 commits May 12, 2025 13:38

modified for idm

10d8ec8

Changes for on prem IDM

842addf

skootrivir force-pushed the add-idm branch from d27a5e8 to 842addf Compare May 12, 2025 19:38

skootrivir changed the base branch from main to trivir May 12, 2025 19:54

ran lint

c756f58

phalestrivir requested changes May 12, 2025

View reviewed changes

Made changes based on the PR feedback

706ffc6

phalestrivir requested changes May 14, 2025

View reviewed changes

src/ops/AuthenticateOps.ts Show resolved Hide resolved

src/ops/ConfigOps.ts Outdated Show resolved Hide resolved

src/ops/ConfigOps.ts Outdated Show resolved Hide resolved

src/ops/ConfigOps.ts Outdated Show resolved Hide resolved

src/ops/AuthenticateOps.ts Outdated Show resolved Hide resolved

skootrivir added 2 commits May 15, 2025 11:03

Made changes on parenthesis

de6b8e5

changed authenticateOps.ts

b0ff7e5

phalestrivir approved these changes Jun 4, 2025

View reviewed changes

phalestrivir changed the base branch from trivir to main June 4, 2025 18:04

phalestrivir requested a review from hfranklin June 4, 2025 18:05

phalestrivir requested changes Jun 4, 2025

View reviewed changes

Changed includeReadOnly

f214452

phalestrivir approved these changes Jun 17, 2025

View reviewed changes

hfranklin requested changes Sep 17, 2025

View reviewed changes

Added AuthenticationApi test for on-prem idm

c6dda67

Changed stepIdm to authenticateIdm other fixes based on the PR review.

hfranklin reviewed Nov 3, 2025

View reviewed changes

skootrivir mentioned this pull request Nov 5, 2025

Feature/extract scripts trivir/frodo-cli#42

Open

Modified AuthenticateApi.test made a new record for the test

cb11e18

Added on-prem IDM Deployment type #21

Are you sure you want to change the base?

Added on-prem IDM Deployment type #21

Uh oh!

Conversation

skootrivir commented May 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

phalestrivir left a comment

Choose a reason for hiding this comment

Uh oh!

phalestrivir left a comment

Choose a reason for hiding this comment

Uh oh!

hfranklin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phalestrivir Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hfranklin Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

phalestrivir Sep 17, 2025 •

edited

Loading

hfranklin Nov 3, 2025 •

edited

Loading