Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bypass for InResponseTo #87

Merged
merged 39 commits into from
Jun 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
71483ba
Path to fix issue #339 with multiple subject confirmations
Mar 8, 2022
232f1c9
Missing test
Mar 8, 2022
1493f34
Missing package lock.
Mar 14, 2022
187ecc1
Audit fix & package binary
Mar 16, 2022
ce9734b
Merge branch 'master' into pr/43
cjbarth Apr 3, 2022
38428ac
Lint
cjbarth Apr 3, 2022
18df38d
Fix to type declaration in code
afurlane Apr 3, 2022
02e645f
Test support for multiple subjectconfirmations in one assertion
afurlane Apr 3, 2022
1fbe535
Merge branch 'master' into pr/43
cjbarth Apr 3, 2022
1ac4719
Lint
cjbarth Apr 3, 2022
0d2c959
missing package lock.
Apr 4, 2022
8acccd6
Correct test for multiple subject confirmation
Apr 4, 2022
19543f1
Reverted package json
Apr 4, 2022
2791b37
prettify and test run
Apr 4, 2022
d8f5965
Removed wrong import from test
afurlane Apr 5, 2022
381515d
The correct workflow suggested *is*. When you have multiple SC do sea…
afurlane Apr 5, 2022
cf286a9
Merge remote-tracking branch 'origin/master' into pr/43
cjbarth Apr 21, 2022
2c6071b
Fix some test issues
cjbarth Apr 21, 2022
0a688d3
Moved test in correct section
afurlane Apr 22, 2022
06a003b
Code optimization and removal of wrong condition. If any subjectconfi…
afurlane Apr 27, 2022
a6bf700
Handle arrays correctly
cjbarth Apr 27, 2022
771ed84
Removed useless check on subjectconfirmation extracting confirmdata. …
afurlane Apr 27, 2022
e033e87
Merge branch 'master' into pr/43
cjbarth Apr 27, 2022
97a519f
Fix test for second SubjectConfirmationData
cjbarth Apr 29, 2022
c3d1c18
Simplify tests data
cjbarth Apr 29, 2022
4c03f94
make pretty
cjbarth Apr 29, 2022
b3c2d7c
Cleanup unneeded branches
cjbarth Apr 29, 2022
fd48f81
Add test for no SubjectConfirmation
cjbarth Apr 29, 2022
7ff315f
Attempt to cover empty array case
cjbarth Apr 29, 2022
ba65a9c
Remove pointless branch
cjbarth Apr 29, 2022
70c734c
First working version; need more test cases
afurlane May 12, 2022
d020fbb
Useless comment removed; test need a deep examination since there is …
afurlane May 12, 2022
2b81e19
Fix to tests and code error on find.
afurlane May 12, 2022
6d12d0a
Fix for correct message
afurlane May 13, 2022
7a84e78
Fix to test specs
afurlane May 13, 2022
6a0df34
Code cleanup before pull request
afurlane May 13, 2022
d63234d
Merge branch 'master' into bypass_for_inresponseto
afurlane May 18, 2022
62ebedc
Fix to wrong merge operation
afurlane May 18, 2022
b2c55ee
Make tests for existence more explicit. Lint.
cjbarth Jun 16, 2022
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
60 changes: 32 additions & 28 deletions src/saml.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1074,8 +1074,9 @@ class SAML {
}

const subject = assertion.Subject;
let subjectConfirmation: XMLOutput | null = null;
let subjectConfirmation: XMLOutput | null | undefined;
let confirmData: XMLOutput | null = null;
let subjectConfirmations: XMLOutput[] | null = null;
if (subject) {
const nameID = subject[0].NameID;
if (nameID && nameID[0]._) {
Expand All @@ -1087,33 +1088,31 @@ class SAML {
profile.spNameQualifier = nameID[0].$.SPNameQualifier;
}
}

subjectConfirmation = subject[0].SubjectConfirmation?.find(
(_subjectConfirmation: XMLOutput) => {
const _confirmData = _subjectConfirmation.SubjectConfirmationData?.[0];
if (_confirmData?.$) {
const subjectNotBefore = _confirmData.$.NotBefore;
const subjectNotOnOrAfter = _confirmData.$.NotOnOrAfter;
const maxTimeLimitMs = this.processMaxAgeAssertionTime(
this.options.maxAssertionAgeMs,
subjectNotOnOrAfter,
assertion.$.IssueInstant
);

const subjErr = this.checkTimestampsValidityError(
nowMs,
subjectNotBefore,
subjectNotOnOrAfter,
maxTimeLimitMs
);
if (subjErr === null) return true;
}

return false;
subjectConfirmations = subject[0].SubjectConfirmation;
subjectConfirmation = subjectConfirmations?.find((_subjectConfirmation: XMLOutput) => {
const _confirmData = _subjectConfirmation.SubjectConfirmationData?.[0];
if (_confirmData?.$) {
const subjectNotBefore = _confirmData.$.NotBefore;
const subjectNotOnOrAfter = _confirmData.$.NotOnOrAfter;
const maxTimeLimitMs = this.processMaxAgeAssertionTime(
this.options.maxAssertionAgeMs,
subjectNotOnOrAfter,
assertion.$.IssueInstant
);

const subjErr = this.checkTimestampsValidityError(
nowMs,
subjectNotBefore,
subjectNotOnOrAfter,
maxTimeLimitMs
);
if (subjErr === null) return true;
}
);

if (subjectConfirmation) {
return false;
});

if (subjectConfirmation != null) {
confirmData = subjectConfirmation.SubjectConfirmationData[0];
}
}
Expand Down Expand Up @@ -1144,8 +1143,13 @@ class SAML {
}
}
} else {
await this.cacheProvider.removeAsync(inResponseTo);
break getInResponseTo;
if (subjectConfirmations != null && subjectConfirmation == null) {
msg = "No valid subject confirmation found among those available in the SAML assertion";
throw new Error(msg);
} else {
await this.cacheProvider.removeAsync(inResponseTo);
break getInResponseTo;
}
}
} else {
break getInResponseTo;
Expand Down
65 changes: 59 additions & 6 deletions test/tests.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,33 @@ describe("node-saml /", function () {
expect(profile.nameID).to.equal("vincent.vega@evil-corp.com");
});

it("valid xml document with multiple SubjectConfirmation should fail if no one is valid", async () => {
fakeClock = sinon.useFakeTimers(Date.parse("2020-09-25T19:00:00+00:00"));
const base64xml = fs.readFileSync(
__dirname + "/static/response.root-signed.message-signed-double-subjectconfirmation.xml",
"base64"
);
const container = { SAMLResponse: base64xml };
const signingCert = fs.readFileSync(__dirname + "/static/cert.pem", "utf-8");
const privateKey = fs.readFileSync(__dirname + "/static/key.pem", "utf-8");

const samlObj = new SAML({
cert: signingCert,
privateKey: privateKey,
issuer: "onesaml_login",
audience: false,
validateInResponseTo: ValidateInResponseTo.always,
});

// Prime cache so we can validate InResponseTo
await samlObj.cacheProvider.saveAsync("_e8df3fe5f04237d25670", new Date().toISOString());
// The second `SubjectConfirmationData` is invalid, the first one could not be used so we should get
await assert.rejects(samlObj.validatePostResponseAsync(container), {
message:
"No valid subject confirmation found among those available in the SAML assertion",
});
});

it("valid xml document with multiple SubjectConfirmation should validate, first is expired so it should take the second one", async () => {
fakeClock = sinon.useFakeTimers(Date.parse("2020-09-25T16:00:00+00:00"));
const base64xml = fs.readFileSync(
Expand All @@ -719,15 +746,15 @@ describe("node-saml /", function () {
// Prime cache so we can validate InResponseTo
await samlObj.cacheProvider.saveAsync("_e8df3fe5f04237d25670", new Date().toISOString());
// The second `SubjectConfirmationData` purposefully has the wrong InResponseTo so we can check for it
assert.rejects(samlObj.validatePostResponseAsync(container), {
await assert.rejects(samlObj.validatePostResponseAsync(container), {
message: "InResponseTo does not match subjectInResponseTo",
});
});

it("valid xml document with no SubjectConfirmation should not validate", async () => {
it("valid xml document with multiple SubjectConfirmations should fail if InResponseTo does not match a valid SubjectConfirmation", async () => {
fakeClock = sinon.useFakeTimers(Date.parse("2020-09-25T16:00:00+00:00"));
const base64xml = fs.readFileSync(
__dirname + "/static/response.root-signed.message-signed-no-subjectconfirmation.xml",
__dirname + "/static/response.root-signed.message-signed-double-subjectconfirmation.xml",
"base64"
);
const container = { SAMLResponse: base64xml };
Expand All @@ -744,11 +771,36 @@ describe("node-saml /", function () {

// Prime cache so we can validate InResponseTo
await samlObj.cacheProvider.saveAsync("_e8df3fe5f04237d25670", new Date().toISOString());
assert.rejects(samlObj.validatePostResponseAsync(container), {
// The second `SubjectConfirmationData` purposefully has the wrong InResponseTo so we can check for it
await assert.rejects(samlObj.validatePostResponseAsync(container), {
message: "InResponseTo does not match subjectInResponseTo",
});
});

it("valid xml document with no SubjectConfirmation should validate", async () => {
fakeClock = sinon.useFakeTimers(Date.parse("2020-09-25T16:00:00+00:00"));
const base64xml = fs.readFileSync(
__dirname + "/static/response.root-signed.message-signed-no-subjectconfirmation.xml",
"base64"
);
const container = { SAMLResponse: base64xml };
const signingCert = fs.readFileSync(__dirname + "/static/cert.pem", "utf-8");
const privateKey = fs.readFileSync(__dirname + "/static/key.pem", "utf-8");

const samlObj = new SAML({
cert: signingCert,
privateKey: privateKey,
issuer: "onesaml_login",
audience: false,
validateInResponseTo: ValidateInResponseTo.always,
});

// Prime cache so we can validate InResponseTo
await samlObj.cacheProvider.saveAsync("_e8df3fe5f04237d25670", new Date().toISOString());
const { profile } = await samlObj.validatePostResponseAsync(container);
assertRequired(profile, "profile must exist");
});

it("valid xml document with only empty SubjectConfirmation should not validate", async () => {
fakeClock = sinon.useFakeTimers(Date.parse("2020-09-25T16:00:00+00:00"));
const base64xml = fs.readFileSync(
Expand All @@ -769,8 +821,9 @@ describe("node-saml /", function () {

// Prime cache so we can validate InResponseTo
await samlObj.cacheProvider.saveAsync("_e8df3fe5f04237d25670", new Date().toISOString());
assert.rejects(samlObj.validatePostResponseAsync(container), {
message: "InResponseTo does not match subjectInResponseTo",
await assert.rejects(samlObj.validatePostResponseAsync(container), {
message:
"No valid subject confirmation found among those available in the SAML assertion",
});
});

Expand Down