Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion src/api/AuthenticateApi.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import util from 'util';

import { State } from '../shared/State';
import { debugMessage } from '../utils/Console';
import { getRealmPath } from '../utils/ForgeRockUtils';
import { generateAmApi } from './BaseApi';
import { generateAmApi, generateIdmApi } from './BaseApi';

const authenticateUrlTemplate = '%s/json%s/authenticate';
const authenticateWithServiceUrlTemplate = `${authenticateUrlTemplate}?authIndexType=service&authIndexValue=%s`;
Expand Down Expand Up @@ -73,3 +74,31 @@ export async function step({
}).post(urlString, body, config);
return data;
}

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

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

I will re-name this to 'authenticateIdm'

body = {},
config = {},
state,
}: {
body?: object;
config?: object;
realm?: string;
service?: string;
state: State;
}): Promise<any> {
debugMessage({
message: `AuthenticateApi.stepIdm: function start `,
state,
});
const urlString = `${state.getHost()}/authentication?_action=login`;
const response = await generateIdmApi({
state,
}).post(urlString, body, config);
return response;
}
6 changes: 5 additions & 1 deletion src/api/BaseApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,14 +312,18 @@ export function generateIdmApi({
...(state.getBearerToken() && {
Authorization: `Bearer ${state.getBearerToken()}`,
}),
...(!state.getBearerToken() && {
'X-OpenIDM-Username': state.getUsername(),
'X-OpenIDM-Password': state.getPassword(),
}),
},
httpAgent: getHttpAgent(),
httpsAgent: getHttpsAgent(state.getAllowInsecureConnection()),
proxy: getProxy(),
},
requestOverride
);

const request = createAxiosInstance(state, requestConfig);

// enable curlirizer output in debug mode
Expand Down
117 changes: 82 additions & 35 deletions src/ops/AuthenticateOps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { createHash, randomBytes } from 'crypto';
import url from 'url';
import { v4 } from 'uuid';

import { step } from '../api/AuthenticateApi';
import { step, stepIdm } from '../api/AuthenticateApi';
import { getServerInfo, getServerVersionInfo } from '../api/ServerInfoApi';
import Constants from '../shared/Constants';
import { State } from '../shared/State';
Expand Down Expand Up @@ -335,6 +335,7 @@ async function determineDeploymentType(state: State): Promise<string> {
return deploymentType;

case Constants.CLASSIC_DEPLOYMENT_TYPE_KEY:
case Constants.IDM_DEPLOYMENT_TYPE_KEY:
debugMessage({
message: `AuthenticateOps.determineDeploymentType: end [type=${deploymentType}]`,
state,
Expand Down Expand Up @@ -377,7 +378,7 @@ async function determineDeploymentType(state: State): Promise<string> {
state,
});
} catch (e) {
// debugMessage(e.response);
// If error is in that condition after sending Authorize
if (
e.response?.status === 302 &&
e.response.headers?.location?.indexOf('code=') > -1
Expand Down Expand Up @@ -409,10 +410,41 @@ async function determineDeploymentType(state: State): Promise<string> {
});
deploymentType = Constants.FORGEOPS_DEPLOYMENT_TYPE_KEY;
} else {
verboseMessage({
message: `Classic deployment`['brightCyan'] + ` detected.`,
state,
});
try {
//I need to check if it is idm here
const idmresponse = await stepIdm({

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

@phalestrivir phalestrivir Sep 17, 2025

Choose a reason for hiding this comment

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

@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.

Choose a reason for hiding this comment

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

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

body: {},
config: {},
state,
});
verboseMessage({
message: `idm response = ${JSON.stringify(idmresponse.status, null, 2)} + ${idmresponse.data.authorization.authLogin}`,
state,
});
if (
idmresponse.status === 200 &&
idmresponse.data?.authorization.authLogin
) {
verboseMessage({
message:
`Ping Identity IDM deployment`['brightCyan'] +
` detected.`,
state,
});
deploymentType = Constants.IDM_DEPLOYMENT_TYPE_KEY;
verboseMessage({
message: 'deployment type in determine =' + deploymentType,
state,
});
} else {
throw new Error('Not IDM');
}
} catch {

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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?

verboseMessage({
message: `Classic deployment`['brightCyan'] + ` detected.`,
state,
});
}
}
}
}
Expand All @@ -421,6 +453,7 @@ async function determineDeploymentType(state: State): Promise<string> {
message: `AuthenticateOps.determineDeploymentType: end [type=${deploymentType}]`,
state,
});

return deploymentType;
}
}
Expand Down Expand Up @@ -1049,7 +1082,6 @@ export type Tokens = {
host?: string;
realm?: string;
};

/**
* Get tokens
* @param {boolean} forceLoginAsUser true to force login as user even if a service account is available (default: false)
Expand Down Expand Up @@ -1098,14 +1130,13 @@ export async function getTokens({
);
}
}

// if host is not a valid URL, try to locate a valid URL and deployment type from connections.json
if (!isValidUrl(state.getHost())) {
const conn = await getConnectionProfile({ state });
state.setHost(conn.tenant);
state.setAllowInsecureConnection(conn.allowInsecureConnection);
state.setDeploymentType(conn.deploymentType);

// fail fast if deployment type not applicable
if (
state.getDeploymentType() &&
Expand All @@ -1116,10 +1147,14 @@ export async function getTokens({
);
}
}

// now that we have the full tenant URL we can lookup the cookie name
state.setCookieName(await determineCookieName(state));

if (state.getHost().endsWith('openidm')) {
state.setDeploymentType(await determineDeploymentType(state));
}
//check if it is idm deployment type, then it will just do some stuff for idm and break
else {
// now that we have the full tenant URL we can lookup the cookie name
state.setCookieName(await determineCookieName(state));
}
// use service account to login?
if (
!forceLoginAsUser &&
Expand Down Expand Up @@ -1160,31 +1195,43 @@ export async function getTokens({
message: `AuthenticateOps.getTokens: Authenticating with user account ${state.getUsername()}`,
state,
});
const token = await getUserSessionToken(callbackHandler, state);
if (token) state.setUserSessionTokenMeta(token);
if (usingConnectionProfile && !token.from_cache) {
// if logging into on prem idm
if (state.getDeploymentType() === Constants.IDM_DEPLOYMENT_TYPE_KEY) {
const token: Tokens = {
subject: state.getUsername(),
host: state.getHost(),
realm: state.getRealm() ? state.getRealm() : 'root',
};
saveConnectionProfile({ host: state.getHost(), state });
}
await determineDeploymentTypeAndDefaultRealmAndVersion(state);
return token;
} else {
const token = await getUserSessionToken(callbackHandler, state);
if (token) state.setUserSessionTokenMeta(token);
if (usingConnectionProfile && !token.from_cache) {
saveConnectionProfile({ host: state.getHost(), state });
}
await determineDeploymentTypeAndDefaultRealmAndVersion(state);

// fail if deployment type not applicable
if (
state.getDeploymentType() &&
!types.includes(state.getDeploymentType())
) {
throw new FrodoError(
`Unsupported deployment type '${state.getDeploymentType()}'`
);
}
// fail if deployment type not applicable
if (
state.getDeploymentType() &&
!types.includes(state.getDeploymentType())
) {
throw new FrodoError(
`Unsupported deployment type '${state.getDeploymentType()}'`
);
}

if (
state.getCookieValue() &&
// !state.getBearerToken() &&
(state.getDeploymentType() === Constants.CLOUD_DEPLOYMENT_TYPE_KEY ||
state.getDeploymentType() === Constants.FORGEOPS_DEPLOYMENT_TYPE_KEY)
) {
const accessToken = await getUserBearerToken(state);
if (accessToken) state.setBearerTokenMeta(accessToken);
if (
state.getCookieValue() &&
// !state.getBearerToken() &&
(state.getDeploymentType() === Constants.CLOUD_DEPLOYMENT_TYPE_KEY ||
state.getDeploymentType() ===
Constants.FORGEOPS_DEPLOYMENT_TYPE_KEY)
) {
const accessToken = await getUserBearerToken(state);
if (accessToken) state.setBearerTokenMeta(accessToken);
}
}
}
// incomplete or no credentials
Expand Down
Loading