Skip to content

Commit

Permalink
Allow to validate InReponseTo only if provided, to support IDP-initia…
Browse files Browse the repository at this point in the history
…ted login (#40)

`validateInResponseTo` no longer takes a boolean, it now takes "always", "never", or "ifPresent".
  • Loading branch information
ghusse committed Apr 2, 2022
1 parent 6f73350 commit 496c54e
Show file tree
Hide file tree
Showing 4 changed files with 197 additions and 110 deletions.
7 changes: 5 additions & 2 deletions README.md
Expand Up @@ -88,7 +88,10 @@ const saml = new SAML(options);
```

- **InResponseTo Validation**
- `validateInResponseTo`: if truthy, then InResponseTo will be validated from incoming SAML responses
- `validateInResponseTo`:
- if `"always"`, then InResponseTo will be validated from incoming SAML responses
- if `"never"`, then InResponseTo won't be validated
- if `"ifPresent"`, then InResponseTo will only be validated if present in the incoming SAML response
- `requestIdExpirationPeriodMs`: Defines the expiration time when a Request ID generated for a SAML request will not be valid if seen in a SAML response in the `InResponseTo` field. Default is 8 hours.
- `cacheProvider`: Defines the implementation for a cache provider used to store request Ids generated in SAML requests as part of `InResponseTo` validation. Default is a built-in in-memory cache provider. For details see the 'Cache Provider' section.
- **Issuer Validation**
Expand Down Expand Up @@ -249,7 +252,7 @@ in the SAML response.

## Subject confirmation validation

When configured (turn `validateInResponseTo` to `true` in the Passport-SAML config), the `InResponseTo` attribute will be validated.
When configured (turn `validateInResponseTo` to `always` in the Passport-SAML config), the `InResponseTo` attribute will be validated.
Validation will succeed if Passport-SAML previously generated a SAML request with an id that matches the value of `InResponseTo`.

Also note that `InResponseTo` is validated as an attribute of the top level `Response` element in the SAML response, as well
Expand Down
23 changes: 15 additions & 8 deletions src/saml.ts
Expand Up @@ -26,6 +26,7 @@ import {
XMLInput,
XMLObject,
XMLOutput,
ValidateInResponseTo,
} from "./types";
import { AuthenticateOptions, AuthorizeOptions } from "./passport-saml-types";
import { assertRequired, signXmlMetadata } from "./utility";
Expand Down Expand Up @@ -153,7 +154,7 @@ class SAML {
authnContext: ctorOptions.authnContext ?? [
"urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport",
],
validateInResponseTo: ctorOptions.validateInResponseTo ?? false,
validateInResponseTo: ctorOptions.validateInResponseTo ?? ValidateInResponseTo.never,
cert: assertRequired(ctorOptions.cert, "cert is required"),
requestIdExpirationPeriodMs: ctorOptions.requestIdExpirationPeriodMs ?? 28800000, // 8 hours
cacheProvider:
Expand Down Expand Up @@ -234,7 +235,7 @@ class SAML {
const id = this.options.generateUniqueId();
const instant = generateInstant();

if (this.options.validateInResponseTo) {
if (this.mustValidateInResponseTo(true)) {
await this.cacheProvider.saveAsync(id, instant);
}
const request: AuthorizeRequestXML = {
Expand Down Expand Up @@ -883,24 +884,22 @@ class SAML {
}
} catch (err) {
debug("validatePostResponse resulted in an error: %s", err);
if (this.options.validateInResponseTo != null) {
if (this.mustValidateInResponseTo(Boolean(inResponseTo!))) {
await this.cacheProvider.removeAsync(inResponseTo!);
}
throw err;
}
}

private async validateInResponseTo(inResponseTo: string | null): Promise<undefined> {
if (this.options.validateInResponseTo) {
private async validateInResponseTo(inResponseTo: string | null): Promise<void> {
if (this.mustValidateInResponseTo(Boolean(inResponseTo))) {
if (inResponseTo) {
const result = await this.cacheProvider.getAsync(inResponseTo);
if (!result) throw new Error("InResponseTo is not valid");
return;
} else {
throw new Error("InResponseTo is missing from response");
}
} else {
return;
}
}

Expand Down Expand Up @@ -1108,10 +1107,11 @@ class SAML {

// Test to see that if we have a SubjectConfirmation InResponseTo that it matches
// the 'InResponseTo' attribute set in the Response
if (this.options.validateInResponseTo) {
if (this.mustValidateInResponseTo(Boolean(inResponseTo))) {
if (subjectConfirmation) {
if (confirmData && confirmData.$) {
const subjectInResponseTo = confirmData.$.InResponseTo;

if (inResponseTo && subjectInResponseTo && subjectInResponseTo != inResponseTo) {
await this.cacheProvider.removeAsync(inResponseTo);
throw new Error("InResponseTo is not valid");
Expand Down Expand Up @@ -1458,6 +1458,13 @@ class SAML {
const maxAssertionTimeMs = issueInstantMs + maxAssertionAgeMs;
return maxAssertionTimeMs < notOnOrAfterMs ? maxAssertionTimeMs : notOnOrAfterMs;
}

private mustValidateInResponseTo(hasInResponseTo: boolean): boolean {
return (
this.options.validateInResponseTo === ValidateInResponseTo.always ||
(this.options.validateInResponseTo === ValidateInResponseTo.ifPresent && hasInResponseTo)
);
}
}

export { SAML };
8 changes: 7 additions & 1 deletion src/types.ts
Expand Up @@ -85,6 +85,12 @@ interface SamlScopingConfig {
requesterId?: string[] | string;
}

export enum ValidateInResponseTo {
never = "never",
ifPresent = "ifPresent",
always = "always",
}

/**
* The options required to use a SAML strategy
* These may be provided by means of defaults specified in the constructor
Expand Down Expand Up @@ -122,7 +128,7 @@ export interface SamlOptions extends Partial<SamlSigningOptions>, MandatorySamlO
signMetadata?: boolean;

// InResponseTo Validation
validateInResponseTo: boolean;
validateInResponseTo: ValidateInResponseTo;
requestIdExpirationPeriodMs: number;
cacheProvider: CacheProvider;

Expand Down

0 comments on commit 496c54e

Please sign in to comment.