Skip to content

Commit

Permalink
Capture OTEL metrics in Identity Federation (#3594)
Browse files Browse the repository at this point in the history 
* SAML Fed authorize requests

* Use counter attributes and better naming for metrics

* Capture OIDC Fed metrics

* Track metrics in the saml response path

* Tweaks and fixes

* Fix casing

* Push metrics for oidc initiated login
  • Loading branch information
@niwsa
niwsa authored Feb 18, 2025
1 parent 447af0b commit a0b5e23
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 38 deletions.
41 changes: 31 additions & 10 deletions npm/src/controller/oauth.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,8 @@ export class OAuthController implements IOAuthController {
if (client_id.startsWith(`${clientIDFederatedPrefix}${clientIDOIDCPrefix}`)) {
isOIDCFederated = true;
protocol = 'oidc-federation';
fedApp = await this.idFedApp.get({
id: client_id.replace(clientIDFederatedPrefix, ''),
});
metrics.increment('idfedAuthorize', { protocol, login_type });
fedApp = await this.idFedApp.get({ id: client_id.replace(clientIDFederatedPrefix, '') });

const response = await this.ssoHandler.resolveConnection({
tenant: fedApp.tenant,
Expand Down Expand Up @@ -275,7 +274,7 @@ export class OAuthController implements IOAuthController {
}
} catch (err: unknown) {
const error_description = getErrorMessage(err);
metrics.increment('oauthAuthorizeError', {
metrics.increment(isOIDCFederated ? 'idfedAuthorizeError' : 'oauthAuthorizeError', {
protocol,
login_type,
});
Expand Down Expand Up @@ -684,6 +683,8 @@ export class OAuthController implements IOAuthController {

try {
isIdPFlow = !RelayState.startsWith(relayStatePrefix);
login_type = isIdPFlow ? 'idp-initiated' : 'sp-initiated';
metrics.increment('oauthResponse', { protocol: 'saml', login_type });
rawResponse = Buffer.from(SAMLResponse, 'base64').toString();
issuer = saml.parseIssuer(rawResponse);

Expand All @@ -696,7 +697,6 @@ export class OAuthController implements IOAuthController {
);
}

login_type = isIdPFlow ? 'idp-initiated' : 'sp-initiated';
if (isIdPFlow) {
protocol = 'saml';
}
Expand Down Expand Up @@ -727,6 +727,12 @@ export class OAuthController implements IOAuthController {
isOIDCFederated = session && 'oidcFederated' in session;
const isSPFlow = !isIdPFlow && !isSAMLFederated;
protocol = isOIDCFederated ? 'oidc-federation' : isSAMLFederated ? 'saml-federation' : 'saml';
if (protocol !== 'saml') {
metrics.increment('idfedResponse', {
protocol,
login_type,
});
}
// IdP initiated SSO flow
if (isIdPFlow) {
const response = await this.ssoHandler.resolveConnection({
Expand Down Expand Up @@ -807,7 +813,10 @@ export class OAuthController implements IOAuthController {

redirect_uri = ((session && session.redirect_uri) as string) || connection.defaultRedirectUrl;
} catch (err: unknown) {
metrics.increment('oAuthResponseError', { protocol, login_type });
metrics.increment(isOIDCFederated || isSAMLFederated ? 'idfedResponseError' : 'oauthResponseError', {
protocol,
login_type,
});
// Save the error trace
await this.ssoTraces.saveTrace({
error: getErrorMessage(err),
Expand Down Expand Up @@ -858,7 +867,10 @@ export class OAuthController implements IOAuthController {

return { redirect_url: redirect.success(redirect_uri, params) };
} catch (err: unknown) {
metrics.increment('oAuthResponseError', { protocol, login_type });
metrics.increment(isOIDCFederated || isSAMLFederated ? 'idfedResponseError' : 'oauthResponseError', {
protocol,
login_type,
});
const error_description = getErrorMessage(err);
this.opts.logger.error(`SAMLResponse error: ${error_description}`);
// Trace the error
Expand Down Expand Up @@ -915,6 +927,7 @@ export class OAuthController implements IOAuthController {

let RelayState = callbackParams.state || '';
try {
metrics.increment('oauthResponse', { protocol: 'oidc', login_type });
if (!RelayState) {
throw new JacksonError('State from original request is missing.', 403);
}
Expand All @@ -929,7 +942,9 @@ export class OAuthController implements IOAuthController {
isOIDCFederated = session && 'oidcFederated' in session;

protocol = isOIDCFederated ? 'oidc-federation' : isSAMLFederated ? 'saml-federation' : 'oidc';

if (protocol !== 'oidc') {
metrics.increment('idfedResponse', { protocol, login_type });
}
oidcConnection = await this.connectionStore.get(session.id);

if (!oidcConnection) {
Expand All @@ -953,7 +968,10 @@ export class OAuthController implements IOAuthController {
}
}
} catch (err) {
metrics.increment('oAuthResponseError', { protocol, login_type });
metrics.increment(protocol === 'oidc' ? 'oauthResponseError' : 'idfedResponseError', {
protocol,
login_type,
});
await this.ssoTraces.saveTrace({
error: getErrorMessage(err),
context: {
Expand Down Expand Up @@ -1037,10 +1055,13 @@ export class OAuthController implements IOAuthController {

return { redirect_url: redirect.success(redirect_uri!, params) };
} catch (err: any) {
metrics.increment(protocol === 'oidc' ? 'oauthResponseError' : 'idfedResponseError', {
protocol,
login_type,
});
const { error, error_description, error_uri, session_state, scope, stack } = err;
const error_message = error_description || getErrorMessage(err);
this.opts.logger.error(`OIDCResponse error: ${error_message}`);
metrics.increment('oAuthResponseError', { protocol, login_type });
const traceId = await this.ssoTraces.saveTrace({
error: error_message,
context: {
Expand Down
7 changes: 6 additions & 1 deletion npm/src/ee/identity-federation/idp-login.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
SSOTrace,
SSOTracesInstance,
} from '../../typings';
import * as metrics from '../../opentelemetry/metrics';
import { throwIfInvalidLicense } from '../common/checkLicense';
import { App } from './app';

Expand All @@ -30,6 +31,10 @@ export class IdPLogin {
body: OIDCIdPInitiatedReq & { fedAppId: string }
): Promise<{ redirect_url: string }> {
await throwIfInvalidLicense(this.opts.boxyhqLicenseKey);
const protocol = 'saml-federation';
const login_type = 'idp-initiated';

metrics.increment('idfedAuthorize', { protocol, login_type });

let connection: OIDCSSORecord | undefined;
let fedApp: IdentityFederationApp | undefined;
Expand Down Expand Up @@ -110,7 +115,7 @@ export class IdPLogin {
});
} catch (err: unknown) {
const error_description = getErrorMessage(err);

metrics.increment('idfedAuthorizeError', { protocol, login_type });
this.ssoTraces.saveTrace({
error: error_description,
context,
Expand Down
39 changes: 13 additions & 26 deletions npm/src/ee/identity-federation/sso.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type {
SSOTrace,
JacksonOptionWithRequiredLogger,
} from '../../typings';
import * as metrics from '../../opentelemetry/metrics';
import { GENERIC_ERR_STRING, getErrorMessage, isConnectionActive } from '../../controller/utils';
import { throwIfInvalidLicense } from '../common/checkLicense';

Expand Down Expand Up @@ -55,14 +56,16 @@ export class SSO {
}) => {
await throwIfInvalidLicense(this.opts.boxyhqLicenseKey);

const protocol = 'saml-federation';
const login_type = 'sp-initiated';

metrics.increment('idfedAuthorize', { protocol, login_type });

const isPostBinding = samlBinding === 'HTTP-POST';
let connection: SAMLSSORecord | OIDCSSORecord | undefined;
let app: IdentityFederationApp | undefined;
let id, acsUrl, entityId, publicKey, providerName, decodedRequest;
const context = {
isSAMLFederated: true,
relayState,
} as unknown as SSOTrace['context'];
const context = { isSAMLFederated: true, relayState } as unknown as SSOTrace['context'];

try {
decodedRequest = await saml.decodeBase64(request, !isPostBinding);
Expand Down Expand Up @@ -98,20 +101,13 @@ export class SSO {
idp_hint,
authFlow: 'saml',
idFedAppId: app.id,
originalParams: {
RelayState: relayState,
SAMLRequest: request,
samlBinding,
},
originalParams: { RelayState: relayState, SAMLRequest: request, samlBinding },
tenants: app.tenants,
});

// If there is a redirect URL, then we need to redirect to that URL
if ('redirectUrl' in response) {
return {
redirect_url: response.redirectUrl,
authorize_form: null,
};
return { redirect_url: response.redirectUrl, authorize_form: null };
}

// If there is a connection, use that connection
Expand Down Expand Up @@ -141,27 +137,18 @@ export class SSO {
};

return isSAMLConnection(connection)
? await this.ssoHandler.createSAMLRequest({
connection,
requestParams,
mappings: app.mappings,
})
? await this.ssoHandler.createSAMLRequest({ connection, requestParams, mappings: app.mappings })
: await this.ssoHandler.createOIDCRequest({
connection,
requestParams,
mappings: app.mappings,
ssoTraces: {
instance: this.ssoTraces,
context,
},
ssoTraces: { instance: this.ssoTraces, context },
});
} catch (err: unknown) {
const error_description = getErrorMessage(err);
metrics.increment('idfedAuthorizeError', { protocol, login_type });

this.ssoTraces.saveTrace({
error: error_description,
context,
});
this.ssoTraces.saveTrace({ error: error_description, context });

throw err;
}
Expand Down
41 changes: 40 additions & 1 deletion npm/src/opentelemetry/metrics.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,14 @@ const counters = {
counterOptions: { description: 'Number of errors in oauth authorize requests' },
counterAttributes,
}),
oAuthResponseError: (counterAttributes: CounterOperationParams['counterAttributes']) =>
oauthResponse: (counterAttributes: CounterOperationParams['counterAttributes']) =>
incrementCounter({
meter: METER,
name: 'jackson.oauth.response',
counterOptions: { description: 'Number of oauth response requests' },
counterAttributes,
}),
oauthResponseError: (counterAttributes: CounterOperationParams['counterAttributes']) =>
incrementCounter({
meter: METER,
name: 'jackson.oauth.response.error',
Expand Down Expand Up @@ -96,6 +103,38 @@ const counters = {
counterOptions: { description: 'Indicate that a batch of dsync events failed' },
});
},
idfedAuthorize: (counterAttributes: CounterOperationParams['counterAttributes']) => {
incrementCounter({
meter: METER,
name: 'jackson.idfed.authorize',
counterOptions: { description: 'Number of identity federation authorize requests' },
counterAttributes,
});
},
idfedAuthorizeError: (counterAttributes: CounterOperationParams['counterAttributes']) => {
incrementCounter({
meter: METER,
name: 'jackson.idfed.authorize.error',
counterOptions: { description: 'Number of errors in identity federation authorize requests' },
counterAttributes,
});
},
idfedResponse: (counterAttributes: CounterOperationParams['counterAttributes']) => {
incrementCounter({
meter: METER,
name: 'jackson.idfed.response',
counterOptions: { description: 'Number of identity federation response requests' },
counterAttributes,
});
},
idfedResponseError: (counterAttributes: CounterOperationParams['counterAttributes']) => {
incrementCounter({
meter: METER,
name: 'jackson.idfed.response.error',
counterOptions: { description: 'Number of errors in identity federation response requests' },
counterAttributes,
});
},
};

const increment = (
Expand Down

0 comments on commit a0b5e23

Please # to comment.